From 465e36150c5e7f44c2b7f01d539d0e3bbc05984e Mon Sep 17 00:00:00 2001 From: SeanHsieh Date: Tue, 5 Mar 2024 20:23:46 +0800 Subject: [PATCH 1/2] test(add): add publish feature contains different kinds dep --- tests/testsuite/publish.rs | 110 +++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 33baaa6a41d..b38e3fa0b22 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -1686,6 +1686,116 @@ repository = "foo" ); } +#[cargo_test] +fn publish_with_feature_point_diff_kinds_dep() { + let registry = RegistryBuilder::new().http_api().http_index().build(); + Package::new("normal-only", "1.0.0") + .feature("cat", &[]) + .publish(); + Package::new("build-only", "1.0.0") + .feature("cat", &[]) + .publish(); + Package::new("normal-and-dev", "1.0.0") + .feature("cat", &[]) + .publish(); + Package::new("target-normal-only", "1.0.0") + .feature("cat", &[]) + .publish(); + Package::new("target-build-only", "1.0.0") + .feature("cat", &[]) + .publish(); + Package::new("target-normal-and-dev", "1.0.0") + .feature("cat", &[]) + .publish(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + authors = [] + license = "MIT" + description = "foo" + documentation = "foo" + homepage = "foo" + repository = "foo" + + + [features] + foo_feature = [ + "normal-only/cat", + "build-only/cat", + "dev-only/cat", + "normal-and-dev/cat", + "target-normal-only/cat", + "target-build-only/cat", + "target-dev-only/cat", + "target-normal-and-dev/cat", + ] + + [dependencies] + normal-only = { version = "1.0", features = ["cat"] } + normal-and-dev = { version = "1.0", features = ["cat"] } + + [build-dependencies] + build-only = { version = "1.0", features = ["cat"] } + + [dev-dependencies] + dev-only = { path = "../dev-only", features = ["cat"] } + normal-and-dev = { version = "1.0", features = ["cat"] } + + [target.'cfg(unix)'.dependencies] + target-normal-only = { version = "1.0", features = ["cat"] } + target-normal-and-dev = { version = "1.0", features = ["cat"] } + + [target.'cfg(unix)'.build-dependencies] + target-build-only = { version = "1.0", features = ["cat"] } + + [target.'cfg(unix)'.dev-dependencies] + target-dev-only = { path = "../dev-only", features = ["cat"] } + target-normal-and-dev = { version = "1.0", features = ["cat"] } + "#, + ) + .file("src/main.rs", "") + .file( + "dev-only/Cargo.toml", + r#" + [package] + name = "dev-only" + version = "0.1.0" + edition = "2015" + authors = [] + + [features] + cat = [] + "#, + ) + .file( + "dev-only/src/lib.rs", + r#" + #[cfg(feature = "cat")] + pub fn cat() {} + "#, + ) + .build(); + + p.cargo("publish --no-verify") + .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 +", + ) + .run(); +} #[cargo_test] fn credentials_ambiguous_filename() { // `publish` generally requires a remote registry From 75130eb6e3a83a30146be3219a721cb787d2a1a3 Mon Sep 17 00:00:00 2001 From: SeanHsieh Date: Tue, 12 Mar 2024 20:54:46 +0800 Subject: [PATCH 2/2] fix(add): strip feature dep when dep is dev dep or target dev dep --- src/cargo/ops/registry/publish.rs | 23 +++- src/cargo/util/toml/mod.rs | 53 +++++++- tests/testsuite/publish.rs | 198 +++++++++++++++++++++++++++++- 3 files changed, 264 insertions(+), 10 deletions(-) diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index 5ec2da64ee1..65b9a960730 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -3,6 +3,7 @@ //! [1]: https://doc.rust-lang.org/nightly/cargo/reference/registry-web-api.html#publish use std::collections::BTreeMap; +use std::collections::BTreeSet; use std::collections::HashSet; use std::fs::File; use std::time::Duration; @@ -20,6 +21,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; @@ -33,6 +35,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; @@ -412,13 +415,31 @@ fn transmit( return Ok(()); } + let deps_set = deps + .iter() + .map(|dep| dep.name.clone()) + .collect::>(); + 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, .. } => { + deps_set.contains(&dep_name.to_string()) + } + _ => true, + } + }) + .map(|fv| fv.to_string()) + .collect(), ) }) .collect::>>(), diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 80910aecd36..7470c333db5 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -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; @@ -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}; @@ -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(), @@ -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, + deps: Option<&BTreeMap>, + ) { + 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; + }; + + features.values_mut().for_each(|feature_deps| { + 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, diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index b38e3fa0b22..745a6a6893d 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -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() {