fix: strip feature dep when dep is dev dep

Author: baby230211

src/cargo/ops/registry/publish.rs

@@ -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;
@@ -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;
@@ -411,14 +413,31 @@ fn transmit(
         gctx.shell().warn("aborting upload due to dry run")?;
         return Ok(());
     }
+    let deps_map = deps
+        .iter()
+        .map(|dep| (InternedString::new(dep.name.as_str()), dep))
+        .collect::<BTreeMap<InternedString, &NewCrateDependency>>();
 
     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_map.contains_key(&dep_name)
+                                }
+                                _ => true,
+                            }
+                        })
+                        .map(|fv| fv.to_string())
+                        .collect(),
                 )
             })
             .collect::<BTreeMap<String, Vec<String>>>(),

src/cargo/util/toml/mod.rs

@@ -22,7 +22,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,6 +316,12 @@ pub fn prepare_for_publish(
         }
     }
     let all = |_d: &manifest::TomlDependency| true;
+    let dependencies = map_deps(gctx, me.dependencies.as_ref(), all)?;
+    let dev_dependencies = map_deps(
+        gctx,
+        me.dev_dependencies(),
+        manifest::TomlDependency::is_version_specified,
+    )?;
     return Ok(manifest::TomlManifest {
         package: Some(package),
         project: None,
@@ -325,16 +331,16 @@ pub fn prepare_for_publish(
         example: me.example.clone(),
         test: me.test.clone(),
         bench: me.bench.clone(),
-        dependencies: map_deps(gctx, me.dependencies.as_ref(), all)?,
-        dev_dependencies: map_deps(
-            gctx,
-            me.dev_dependencies(),
-            manifest::TomlDependency::is_version_specified,
-        )?,
+        dependencies: dependencies.clone(),
+        dev_dependencies: dev_dependencies.clone(),
         dev_dependencies2: None,
         build_dependencies: map_deps(gctx, me.build_dependencies(), all)?,
         build_dependencies2: None,
-        features: me.features.clone(),
+        features: map_feature(
+            me.features.as_ref(),
+            dependencies.as_ref(),
+            dev_dependencies.as_ref(),
+        )?,
         target: match me.target.as_ref().map(|target_map| {
             target_map
                 .iter()
@@ -368,6 +374,46 @@ pub fn prepare_for_publish(
         lints: me.lints.clone(),
     });
 
+    fn map_feature(
+        features: Option<&BTreeMap<manifest::FeatureName, Vec<String>>>,
+        deps: Option<&BTreeMap<manifest::PackageName, manifest::InheritableDependency>>,
+        dev_deps: Option<&BTreeMap<manifest::PackageName, manifest::InheritableDependency>>,
+    ) -> CargoResult<Option<BTreeMap<manifest::FeatureName, Vec<String>>>> {
+        let Some(features) = features else {
+            return Ok(None);
+        };
+        let features = features
+            .iter()
+            .map(|(name, feature_deps)| {
+                let feature_deps = feature_deps
+                    .iter()
+                    .filter(|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();
+
+                                if let Some(deps_map) = deps {
+                                    return deps_map.contains_key(k);
+                                }
+                                if let Some(dev_deps_map) = dev_deps {
+                                    return dev_deps_map.contains_key(k);
+                                }
+                                false
+                            }
+                            _ => true,
+                        }
+                    })
+                    .map(|feature_dep| feature_dep.to_string())
+                    .collect::<Vec<String>>();
+                (name, feature_deps)
+            })
+            .map(|(name, feature_deps)| Ok((name.clone(), feature_deps)))
+            .collect::<CargoResult<BTreeMap<_, _>>>()?;
+        Ok(Some(features))
+    }
+
     fn map_deps(
         gctx: &GlobalContext,
         deps: Option<&BTreeMap<manifest::PackageName, manifest::InheritableDependency>>,
@@ -936,17 +982,34 @@ pub fn to_real_manifest(
         .unwrap_or_default();
     let empty_features = BTreeMap::new();
 
+    let deps_map = deps
+        .iter()
+        .filter(|dep| dep.is_transitive() || dep.specified_req())
+        .map(|dep| (dep.name_in_toml(), dep))
+        .collect::<BTreeMap<InternedString, &Dependency>>();
     let summary = Summary::new(
         pkgid,
-        deps,
+        deps.clone(),
         &me.features
             .as_ref()
             .unwrap_or(&empty_features)
             .iter()
             .map(|(k, v)| {
                 (
                     InternedString::new(k),
-                    v.iter().map(InternedString::from).collect(),
+                    v.iter()
+                        .filter(|feature_dep| {
+                            let feature_value = FeatureValue::new(InternedString::new(feature_dep));
+                            match feature_value {
+                                FeatureValue::Dep { dep_name }
+                                | FeatureValue::DepFeature { dep_name, .. } => {
+                                    deps_map.contains_key(&dep_name)
+                                }
+                                _ => true,
+                            }
+                        })
+                        .map(InternedString::from)
+                        .collect(),
                 )
             })
             .collect(),

tests/testsuite/publish.rs

@@ -1687,7 +1687,7 @@ repository = "foo"
 }
 
 #[cargo_test]
-fn invalid_publish_feature_point_to_stripped_dep() {
+fn publish_feature_point_to_stripped_dep() {
     let registry = RegistryBuilder::new().http_api().http_index().build();
 
     let p = project()
@@ -1739,18 +1739,71 @@ fn invalid_publish_feature_point_to_stripped_dep() {
 
     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 `bar/cat`, but `bar` is not a dependency
+[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": [],
+          "description": "foo",
+          "documentation": "foo",
+          "features": {
+            "foo_feature": []
+          },
+          "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"
+
+[dev-dependencies]
+
+[features]
+foo_feature = []
+"#,
+                cargo::core::package::MANIFEST_PREAMBLE
+            ),
+        )],
+    );
 }
 #[cargo_test]
 fn credentials_ambiguous_filename() {