Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(publish): Don't strip non-dev features #14325

Merged
merged 8 commits into from
Jul 31, 2024
11 changes: 8 additions & 3 deletions src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Vec<Fi
} else {
let tarball = create_package(ws, &pkg, ar_files, local_reg.as_ref())?;
if let Some(local_reg) = local_reg.as_mut() {
local_reg.add_package(&pkg, &tarball)?;
local_reg.add_package(ws, &pkg, &tarball)?;
}
outputs.push((pkg, opts, tarball));
}
Expand Down Expand Up @@ -1299,7 +1299,12 @@ impl<'a> TmpRegistry<'a> {
self.root.join("index")
}

fn add_package(&mut self, package: &Package, tar: &FileLock) -> CargoResult<()> {
fn add_package(
&mut self,
ws: &Workspace<'_>,
package: &Package,
tar: &FileLock,
) -> CargoResult<()> {
debug!(
"adding package {}@{} to local overlay at {}",
package.name(),
Expand All @@ -1317,7 +1322,7 @@ impl<'a> TmpRegistry<'a> {
tar_copy.flush()?;
}

let new_crate = super::registry::prepare_transmit(self.gctx, package, self.upstream)?;
let new_crate = super::registry::prepare_transmit(self.gctx, ws, package, self.upstream)?;

tar.file().seek(SeekFrom::Start(0))?;
let cksum = cargo_util::Sha256::new()
Expand Down
51 changes: 18 additions & 33 deletions src/cargo/ops/registry/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
//! [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;
Expand All @@ -21,7 +20,6 @@ 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 @@ -35,7 +33,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::toml::prepare_for_publish;
use crate::util::Progress;
use crate::util::ProgressStyle;
use crate::CargoResult;
Expand Down Expand Up @@ -185,6 +183,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
.status("Uploading", pkg.package_id().to_string())?;
transmit(
opts.gctx,
ws,
pkg,
tarball.file(),
&mut registry,
Expand Down Expand Up @@ -324,16 +323,16 @@ fn verify_dependencies(

pub(crate) fn prepare_transmit(
gctx: &GlobalContext,
ws: &Workspace<'_>,
local_pkg: &Package,
registry_id: SourceId,
) -> CargoResult<NewCrate> {
let deps = local_pkg
let included = None; // don't filter build-targets
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is unclear why the argument affects build target filtering.

let publish_pkg = prepare_for_publish(local_pkg, ws, included)?;

let deps = publish_pkg
.dependencies()
.iter()
.filter(|dep| {
// Skip dev-dependency without version.
dep.is_transitive() || dep.specified_req()
})
.map(|dep| {
// If the dependency is from a different registry, then include the
// registry in the dependency.
Expand Down Expand Up @@ -378,7 +377,7 @@ pub(crate) fn prepare_transmit(
})
})
.collect::<CargoResult<Vec<NewCrateDependency>>>()?;
let manifest = local_pkg.manifest();
let manifest = publish_pkg.manifest();
let ManifestMetadata {
ref authors,
ref description,
Expand All @@ -395,54 +394,39 @@ pub(crate) fn prepare_transmit(
ref rust_version,
} = *manifest.metadata();
let rust_version = rust_version.as_ref().map(ToString::to_string);
let readme_content = readme
let readme_content = local_pkg
.manifest()
.metadata()
.readme
.as_ref()
.map(|readme| {
paths::read(&local_pkg.root().join(readme)).with_context(|| {
format!("failed to read `readme` file for package `{}`", local_pkg)
})
})
.transpose()?;
if let Some(ref file) = *license_file {
if let Some(ref file) = local_pkg.manifest().metadata().license_file {
if !local_pkg.root().join(file).exists() {
bail!("the license file `{}` does not exist", file)
}
}

let deps_set = deps
.iter()
.map(|dep| dep.name.clone())
.collect::<BTreeSet<String>>();

let string_features = match manifest.resolved_toml().features() {
Some(features) => features
.iter()
.map(|(feat, values)| {
(
feat.to_string(),
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(),
values.iter().map(|fv| fv.to_string()).collect(),
)
})
.collect::<BTreeMap<String, Vec<String>>>(),
None => BTreeMap::new(),
};

Ok(NewCrate {
name: local_pkg.name().to_string(),
vers: local_pkg.version().to_string(),
name: publish_pkg.name().to_string(),
vers: publish_pkg.version().to_string(),
deps,
features: string_features,
authors: authors.clone(),
Expand All @@ -464,13 +448,14 @@ pub(crate) fn prepare_transmit(

fn transmit(
gctx: &GlobalContext,
ws: &Workspace<'_>,
pkg: &Package,
tarball: &File,
registry: &mut Registry,
registry_id: SourceId,
dry_run: bool,
) -> CargoResult<()> {
let new_crate = prepare_transmit(gctx, pkg, registry_id)?;
let new_crate = prepare_transmit(gctx, ws, pkg, registry_id)?;

// Do not upload if performing a dry run
if dry_run {
Expand Down
4 changes: 2 additions & 2 deletions tests/testsuite/inheritable_workspace_fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -752,11 +752,11 @@ You may press ctrl-c to skip waiting; the crate should be available shortly.
"homepage": "https://www.rust-lang.org",
"keywords": ["cli"],
"license": "MIT",
"license_file": "../LICENSE",
"license_file": "LICENSE",
"links": null,
"name": "bar",
"readme": "README.md",
"readme_file": "../README.md",
"readme_file": "README.md",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a side effect that fixes another bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I considered tweaking things to not have this happen but it seemed more correct now.

"repository": "https://github.com/example/example",
"rust_version": "1.60",
"vers": "1.2.3"
Expand Down
3 changes: 2 additions & 1 deletion tests/testsuite/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1865,7 +1865,8 @@ You may press ctrl-c to skip waiting; the crate should be available shortly.
"target-build-only/cat",
"target-normal-and-dev/cat",
"optional-dep-feature/cat",
"dep:optional-namespaced"
"dep:optional-namespaced",
"dep:optional-renamed-namespaced10"
]
},
"homepage": "foo",
Expand Down