Skip to content

Commit 25ecfce

Browse files
committed
fix(publish): Don't strip non-dev features
First, we added support for stripping of local-only dev-dependencies. This was dual-implemented for `Cargo.toml` and `Summary`. This left off stripping of `dep/feature` that reference dev-dependencies (enabling features within dev-dependencies). When we fixed this, we again dual-implemented it. The `Cargo.toml` version was correct but the `Summary` version was instead stripping too many features, particularly features that reference renamed dependencies. We didn't have tests for this case and it wasn't caught earlier because crates.io re-generates the `Summary` from `Cargo.toml`, ignoring what we post. That makes this only show up with custom registries that trust what Cargo posts. Rather than fixing the `Summary` generation, I remove the dual-implementation and instead generate the `Summary` from the published `Cargo.toml`. Unfortunately, we don't have access directly to the packaged `Cargo.toml`. It could be passed around and I originally did so, hoping to remove use of the local `Package`. However, the local `Package` is needed for things like reading the `README`. So I scaled back and isolate the change to only what needs it. This also makes it easier for `prepare_transmit` callers. Fully open to someone exploring removing this extra `prepare_for_publish` in the future. Fixes #14321
1 parent aa68ea7 commit 25ecfce

File tree

3 files changed

+18
-33
lines changed

3 files changed

+18
-33
lines changed

src/cargo/ops/registry/publish.rs

Lines changed: 14 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
//! [1]: https://doc.rust-lang.org/nightly/cargo/reference/registry-web-api.html#publish
44
55
use std::collections::BTreeMap;
6-
use std::collections::BTreeSet;
76
use std::collections::HashSet;
87
use std::fs::File;
98
use std::time::Duration;
@@ -21,7 +20,6 @@ use crate::core::dependency::DepKind;
2120
use crate::core::manifest::ManifestMetadata;
2221
use crate::core::resolver::CliFeatures;
2322
use crate::core::Dependency;
24-
use crate::core::FeatureValue;
2523
use crate::core::Package;
2624
use crate::core::PackageIdSpecQuery;
2725
use crate::core::SourceId;
@@ -35,7 +33,7 @@ use crate::sources::CRATES_IO_REGISTRY;
3533
use crate::util::auth;
3634
use crate::util::cache_lock::CacheLockMode;
3735
use crate::util::context::JobsConfig;
38-
use crate::util::interning::InternedString;
36+
use crate::util::toml::prepare_for_publish;
3937
use crate::util::Progress;
4038
use crate::util::ProgressStyle;
4139
use crate::CargoResult;
@@ -184,6 +182,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
184182
.status("Uploading", pkg.package_id().to_string())?;
185183
transmit(
186184
opts.gctx,
185+
ws,
187186
pkg,
188187
tarball.file(),
189188
&mut registry,
@@ -323,19 +322,19 @@ fn verify_dependencies(
323322

324323
fn transmit(
325324
gctx: &GlobalContext,
325+
ws: &Workspace<'_>,
326326
local_pkg: &Package,
327327
tarball: &File,
328328
registry: &mut Registry,
329329
registry_id: SourceId,
330330
dry_run: bool,
331331
) -> CargoResult<()> {
332-
let deps = local_pkg
332+
let included = None; // don't filter build-targets
333+
let publish_pkg = prepare_for_publish(local_pkg, ws, included)?;
334+
335+
let deps = publish_pkg
333336
.dependencies()
334337
.iter()
335-
.filter(|dep| {
336-
// Skip dev-dependency without version.
337-
dep.is_transitive() || dep.specified_req()
338-
})
339338
.map(|dep| {
340339
// If the dependency is from a different registry, then include the
341340
// registry in the dependency.
@@ -380,7 +379,7 @@ fn transmit(
380379
})
381380
})
382381
.collect::<CargoResult<Vec<NewCrateDependency>>>()?;
383-
let manifest = local_pkg.manifest();
382+
let manifest = publish_pkg.manifest();
384383
let ManifestMetadata {
385384
ref authors,
386385
ref description,
@@ -397,15 +396,18 @@ fn transmit(
397396
ref rust_version,
398397
} = *manifest.metadata();
399398
let rust_version = rust_version.as_ref().map(ToString::to_string);
400-
let readme_content = readme
399+
let readme_content = local_pkg
400+
.manifest()
401+
.metadata()
402+
.readme
401403
.as_ref()
402404
.map(|readme| {
403405
paths::read(&local_pkg.root().join(readme)).with_context(|| {
404406
format!("failed to read `readme` file for package `{}`", local_pkg)
405407
})
406408
})
407409
.transpose()?;
408-
if let Some(ref file) = *license_file {
410+
if let Some(ref file) = local_pkg.manifest().metadata().license_file {
409411
if !local_pkg.root().join(file).exists() {
410412
bail!("the license file `{}` does not exist", file)
411413
}
@@ -417,31 +419,13 @@ fn transmit(
417419
return Ok(());
418420
}
419421

420-
let deps_set = deps
421-
.iter()
422-
.map(|dep| dep.name.clone())
423-
.collect::<BTreeSet<String>>();
424-
425422
let string_features = match manifest.resolved_toml().features() {
426423
Some(features) => features
427424
.iter()
428425
.map(|(feat, values)| {
429426
(
430427
feat.to_string(),
431-
values
432-
.iter()
433-
.filter(|fv| {
434-
let feature_value = FeatureValue::new(InternedString::new(fv));
435-
match feature_value {
436-
FeatureValue::Dep { dep_name }
437-
| FeatureValue::DepFeature { dep_name, .. } => {
438-
deps_set.contains(&dep_name.to_string())
439-
}
440-
_ => true,
441-
}
442-
})
443-
.map(|fv| fv.to_string())
444-
.collect(),
428+
values.iter().map(|fv| fv.to_string()).collect(),
445429
)
446430
})
447431
.collect::<BTreeMap<String, Vec<String>>>(),

tests/testsuite/inheritable_workspace_fields.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -752,11 +752,11 @@ You may press ctrl-c to skip waiting; the crate should be available shortly.
752752
"homepage": "https://www.rust-lang.org",
753753
"keywords": ["cli"],
754754
"license": "MIT",
755-
"license_file": "../LICENSE",
755+
"license_file": "LICENSE",
756756
"links": null,
757757
"name": "bar",
758758
"readme": "README.md",
759-
"readme_file": "../README.md",
759+
"readme_file": "README.md",
760760
"repository": "https://github.com/example/example",
761761
"rust_version": "1.60",
762762
"vers": "1.2.3"

tests/testsuite/publish.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1865,7 +1865,8 @@ You may press ctrl-c to skip waiting; the crate should be available shortly.
18651865
"target-build-only/cat",
18661866
"target-normal-and-dev/cat",
18671867
"optional-dep-feature/cat",
1868-
"dep:optional-namespaced"
1868+
"dep:optional-namespaced",
1869+
"dep:optional-renamed-namespaced10"
18691870
]
18701871
},
18711872
"homepage": "foo",

0 commit comments

Comments
 (0)