Skip to content

Commit

Permalink
fix(add): strip feature dep when dep is dev dep or target dev dep
Browse files Browse the repository at this point in the history
  • Loading branch information
@baby230211
baby230211 committed Mar 13, 2024
1 parent 465e361 commit 119477c
Show file tree
Hide file tree
Showing 3 changed files with 268 additions and 10 deletions.
27 changes: 26 additions & 1 deletion src/cargo/ops/registry/publish.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use crate::core::dependency::DepKind;
use crate::core::manifest::ManifestMetadata;
use crate::core::resolver::CliFeatures;
use crate::core::Dependency;
use crate::core::FeatureValue;
use crate::core::Package;
use crate::core::PackageIdSpecQuery;
use crate::core::SourceId;
Expand All @@ -33,6 +34,7 @@ use crate::sources::CRATES_IO_REGISTRY;
use crate::util::auth;
use crate::util::cache_lock::CacheLockMode;
use crate::util::context::JobsConfig;
use crate::util::interning::InternedString;
use crate::util::Progress;
use crate::util::ProgressStyle;
use crate::CargoResult;
Expand Down Expand Up @@ -412,13 +414,36 @@ fn transmit(
return Ok(());
}

let deps_map = pkg
.dependencies()
.iter()
.map(|dep| (InternedString::new(dep.name_in_toml().as_str()), dep))
.collect::<BTreeMap<InternedString, &Dependency>>();

let string_features = match manifest.original().features() {
Some(features) => features
.iter()
.map(|(feat, values)| {
(
feat.to_string(),
values.iter().map(|fv| fv.to_string()).collect(),
values
.iter()
.filter(|fv| {
let feature_value = FeatureValue::new(InternedString::new(fv));
match feature_value {
FeatureValue::Dep { dep_name }
| FeatureValue::DepFeature { dep_name, .. } => {
if let Some(dep) = deps_map.get(&dep_name) {
dep.is_transitive() || dep.specified_req()
} else {
true
}
}
_ => true,
}
})
.map(|fv| fv.to_string())
.collect(),
)
})
.collect::<BTreeMap<String, Vec<String>>>(),
Expand Down
53 changes: 49 additions & 4 deletions src/cargo/util/toml/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use crate::AlreadyPrintedError;
use anyhow::{anyhow, bail, Context as _};
use cargo_platform::Platform;
use cargo_util::paths;
use cargo_util_schemas::manifest;
use cargo_util_schemas::manifest::RustVersion;
use cargo_util_schemas::manifest::{self, TomlManifest};
use itertools::Itertools;
use lazycell::LazyCell;
use pathdiff::diff_paths;
Expand All @@ -21,7 +21,7 @@ use crate::core::compiler::{CompileKind, CompileTarget};
use crate::core::dependency::{Artifact, ArtifactTarget, DepKind};
use crate::core::manifest::{ManifestMetadata, TargetSourcePath, Warnings};
use crate::core::resolver::ResolveBehavior;
use crate::core::{find_workspace_root, resolve_relative_path, CliUnstable};
use crate::core::{find_workspace_root, resolve_relative_path, CliUnstable, FeatureValue};
use crate::core::{Dependency, Manifest, PackageId, Summary, Target};
use crate::core::{Edition, EitherManifest, Feature, Features, VirtualManifest, Workspace};
use crate::core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, WorkspaceRootConfig};
Expand Down Expand Up @@ -316,7 +316,7 @@ pub fn prepare_for_publish(
}
}
let all = |_d: &manifest::TomlDependency| true;
return Ok(manifest::TomlManifest {
let mut manifest = manifest::TomlManifest {
package: Some(package),
project: None,
profile: me.profile.clone(),
Expand Down Expand Up @@ -366,7 +366,52 @@ pub fn prepare_for_publish(
badges: me.badges.clone(),
cargo_features: me.cargo_features.clone(),
lints: me.lints.clone(),
});
};
strip_features(&mut manifest);
return Ok(manifest);

fn strip_features(manifest: &mut TomlManifest) {
fn insert_dep_name(
dep_name_set: &mut BTreeSet<manifest::PackageName>,
deps: Option<&BTreeMap<manifest::PackageName, manifest::InheritableDependency>>,
) {
let Some(deps) = deps else {
return;
};
deps.iter().for_each(|(k, _v)| {
dep_name_set.insert(k.clone());
});
}
let mut dep_name_set = BTreeSet::new();
insert_dep_name(&mut dep_name_set, manifest.dependencies.as_ref());
insert_dep_name(&mut dep_name_set, manifest.dev_dependencies());
insert_dep_name(&mut dep_name_set, manifest.build_dependencies());
if let Some(target_map) = manifest.target.as_ref() {
target_map.iter().for_each(|(_k, v)| {
insert_dep_name(&mut dep_name_set, v.dependencies.as_ref());
insert_dep_name(&mut dep_name_set, v.dev_dependencies());
insert_dep_name(&mut dep_name_set, v.build_dependencies());
});
}

let features = manifest.features.as_mut();
let Some(features) = features else {
return;
};

for (_name, feature_deps) in features.iter_mut() {
feature_deps.retain(|feature_dep| {
let feature_value = FeatureValue::new(InternedString::new(feature_dep));
match feature_value {
FeatureValue::Dep { dep_name } | FeatureValue::DepFeature { dep_name, .. } => {
let k = &manifest::PackageName::new(dep_name.to_string()).unwrap();
dep_name_set.contains(k)
}
_ => true,
}
});
}
}

fn map_deps(
gctx: &GlobalContext,
Expand Down
198 changes: 193 additions & 5 deletions tests/testsuite/publish.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1782,19 +1782,207 @@ fn publish_with_feature_point_diff_kinds_dep() {
.build();

p.cargo("publish --no-verify")
.env("RUSTFLAGS", "--cfg unix")
.replace_crates_io(registry.index_url())
.with_status(101)
.with_stderr(
"\
[UPDATING] [..]
[PACKAGING] foo v0.1.0 [..]
[ERROR] failed to prepare local package for uploading
Caused by:
feature `foo_feature` includes `dev-only/cat`, but `dev-only` is not a dependency
[UPDATING] [..]
[PACKAGED] [..] files, [..] ([..] compressed)
[UPLOADING] foo v0.1.0 [..]
[UPLOADED] foo v0.1.0 [..]
[NOTE] waiting [..]
You may press ctrl-c [..]
[PUBLISHED] foo v0.1.0 [..]
",
)
.run();

publish::validate_upload_with_contents(
r#"
{
"authors": [],
"badges": {},
"categories": [],
"deps": [
{
"default_features": true,
"features": [
"cat"
],
"kind": "normal",
"name": "normal-and-dev",
"optional": false,
"target": null,
"version_req": "^1.0"
},
{
"default_features": true,
"features": [
"cat"
],
"kind": "normal",
"name": "normal-only",
"optional": false,
"target": null,
"version_req": "^1.0"
},
{
"default_features": true,
"features": [
"cat"
],
"kind": "dev",
"name": "normal-and-dev",
"optional": false,
"target": null,
"version_req": "^1.0"
},
{
"default_features": true,
"features": [
"cat"
],
"kind": "build",
"name": "build-only",
"optional": false,
"target": null,
"version_req": "^1.0"
},
{
"default_features": true,
"features": [
"cat"
],
"kind": "normal",
"name": "target-normal-and-dev",
"optional": false,
"target": "cfg(unix)",
"version_req": "^1.0"
},
{
"default_features": true,
"features": [
"cat"
],
"kind": "normal",
"name": "target-normal-only",
"optional": false,
"target": "cfg(unix)",
"version_req": "^1.0"
},
{
"default_features": true,
"features": [
"cat"
],
"kind": "build",
"name": "target-build-only",
"optional": false,
"target": "cfg(unix)",
"version_req": "^1.0"
},
{
"default_features": true,
"features": [
"cat"
],
"kind": "dev",
"name": "target-normal-and-dev",
"optional": false,
"target": "cfg(unix)",
"version_req": "^1.0"
}
],
"description": "foo",
"documentation": "foo",
"features": {
"foo_feature": [
"normal-only/cat",
"build-only/cat",
"normal-and-dev/cat",
"target-normal-only/cat",
"target-build-only/cat",
"target-normal-and-dev/cat"
]
},
"homepage": "foo",
"keywords": [],
"license": "MIT",
"license_file": null,
"links": null,
"name": "foo",
"readme": null,
"readme_file": null,
"repository": "foo",
"rust_version": null,
"vers": "0.1.0"
}
"#,
"foo-0.1.0.crate",
&["Cargo.lock", "Cargo.toml", "Cargo.toml.orig", "src/main.rs"],
&[(
"Cargo.toml",
&format!(
r#"{}
[package]
edition = "2015"
name = "foo"
version = "0.1.0"
authors = []
description = "foo"
homepage = "foo"
documentation = "foo"
license = "MIT"
repository = "foo"
[dependencies.normal-and-dev]
version = "1.0"
features = ["cat"]
[dependencies.normal-only]
version = "1.0"
features = ["cat"]
[dev-dependencies.normal-and-dev]
version = "1.0"
features = ["cat"]
[build-dependencies.build-only]
version = "1.0"
features = ["cat"]
[features]
foo_feature = [
"normal-only/cat",
"build-only/cat",
"normal-and-dev/cat",
"target-normal-only/cat",
"target-build-only/cat",
"target-normal-and-dev/cat",
]
[target."cfg(unix)".dependencies.target-normal-and-dev]
version = "1.0"
features = ["cat"]
[target."cfg(unix)".dependencies.target-normal-only]
version = "1.0"
features = ["cat"]
[target."cfg(unix)".build-dependencies.target-build-only]
version = "1.0"
features = ["cat"]
[target."cfg(unix)".dev-dependencies.target-normal-and-dev]
version = "1.0"
features = ["cat"]
"#,
cargo::core::package::MANIFEST_PREAMBLE
),
)],
);
}
#[cargo_test]
fn credentials_ambiguous_filename() {
Expand Down

0 comments on commit 119477c

Please sign in to comment.