Skip to content

Commit

Permalink
Auto merge of #9393 - ehuss:build-std-updating, r=alexcrichton
Browse files Browse the repository at this point in the history
Fix build-std updating the index on every build.

rust-lang/rust#83776 has caused a problem where build-std will update the index on every build. That PR added `std_detect` from the `stdarch` submodule as a path dependency of `std`. However, since `stdarch` has a workspace of its own, an exclusion had to be added to `Cargo.toml` so that it does not treat `std_detect` as a workspace member (because nested workspaces are not supported).

The problem is that the std `Cargo.lock` file is built thinking that `std_detect` is *not* a workspace member. This means that its dev-dependencies are not included. However, when cargo resolves the std workspace, it doesn't know that `std_detect` should be excluded, so it considers it a workspace member (because it is a path dependency). This means that it expects the dev-dependencies to be in Cargo.lock. Because they are missing, it ends up poisoning the registry and triggering an update:

> poisoning registry `https://github.com/rust-lang/crates.io-index` because std_detect v0.1.5 (/Users/eric/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/stdarch/crates/std_detect) looks like it changed auxv

The solution here is to skip dev-dependencies if they are not actively being resolved, even if the package is a workspace member.

This has happened before (#8962), so I have updated the test to check for it.

There are some alternative solutions I considered:

* Add support for nested workspaces. 😄
* Use a symlink to `std_detect` in the `rust-lang/rust` repository so that it appears to cargo as-if it is "outside" of the stdarch workspace, and thus can be treated like a normal workspace member (and remove the "exclude"). That seems a little hacky.

Fixes #9390
  • Loading branch information
bors committed Apr 23, 2021
2 parents a85ae1a + 3b7cb69 commit 02103f9
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
10 changes: 8 additions & 2 deletions src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,12 +291,13 @@ pub fn resolve_with_previous<'cfg>(

let keep = |p: &PackageId| pre_patch_keep(p) && !avoid_patch_ids.contains(p);

let dev_deps = ws.require_optional_deps() || has_dev_units == HasDevUnits::Yes;
// In the case where a previous instance of resolve is available, we
// want to lock as many packages as possible to the previous version
// without disturbing the graph structure.
if let Some(r) = previous {
trace!("previous: {:?}", r);
register_previous_locks(ws, registry, r, &keep);
register_previous_locks(ws, registry, r, &keep, dev_deps);
}
// Everything in the previous lock file we want to keep is prioritized
// in dependency selection if it comes up, aka we want to have
Expand All @@ -320,7 +321,6 @@ pub fn resolve_with_previous<'cfg>(
registry.add_sources(Some(member.package_id().source_id()))?;
}

let dev_deps = ws.require_optional_deps() || has_dev_units == HasDevUnits::Yes;
let summaries: Vec<(Summary, ResolveOpts)> = ws
.members_with_features(specs, cli_features)?
.into_iter()
Expand Down Expand Up @@ -455,6 +455,7 @@ fn register_previous_locks(
registry: &mut PackageRegistry<'_>,
resolve: &Resolve,
keep: &dyn Fn(&PackageId) -> bool,
dev_deps: bool,
) {
let path_pkg = |id: SourceId| {
if !id.is_path() {
Expand Down Expand Up @@ -564,6 +565,11 @@ fn register_previous_locks(
continue;
}

// If dev-dependencies aren't being resolved, skip them.
if !dep.is_transitive() && !dev_deps {
continue;
}

// If this is a path dependency, then try to push it onto our
// worklist.
if let Some(pkg) = path_pkg(dep.source_id()) {
Expand Down
11 changes: 10 additions & 1 deletion tests/build-std/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,16 @@ fn basic() {
.build();

p.cargo("check").build_std().target_host().run();
p.cargo("build").build_std().target_host().run();
p.cargo("build")
.build_std()
.target_host()
// Importantly, this should not say [UPDATING]
// There have been multiple bugs where every build triggers and update.
.with_stderr(
"[COMPILING] foo v0.0.1 [..]\n\
[FINISHED] dev [..]",
)
.run();
p.cargo("run").build_std().target_host().run();
p.cargo("test").build_std().target_host().run();

Expand Down

0 comments on commit 02103f9

Please sign in to comment.