fix(build-std): make Resolve align to what to build #14938

weihanglo · 2024-12-15T20:59:33Z

What does this PR try to resolve?

Blocked on #14943 (or can just merge this one).

#14935 failed because since 125e873
std_resolve only includes sysroot as primary package.
When any custom Cargo feature is provided via -Zbuild-std-feature,
the default feature set panic-unwind would be gone, so no
panic_unwind crate presents in std_resolve.

When then calling std_resolve.query with the default set of
crates from std_crates, which automatically includes
panic_unwind when std presents, it'll result in spec not found
because panic_unwind was not in std_resolve anyway.

How should we test and review this PR?

This patch is kinda a revert of 125e873
in terms of the behavior.

With this, now std_resolve is always resolved to the same set of
packages that Cargo will use to generate the unit graph, (technically
the same set of crates + sysroot), by sharing the same set of primary
packages via std_crates functions.

Note that when multiple --targets provided, if std is specified or there
is one might support std, Cargo will always resolve std dep graph.

To test it manually, run

RUSTFLAGS="-C panic=abort" cargo +nightly-2024-12-15 b -Zbuild-std=std,panic_abort -Zbuild-std-features=panic_immediate_abort

change to this PR's cargo with the same nightly rustc, it would succeed.

I am a bit reluctant to add an new end-2end build-std test, but I still did it.
A bit scared when mock-std gets out-of-sync of features in std in rust-lang/rust.

rustbot · 2024-12-15T20:59:39Z

r? @epage

rustbot has assigned @epage.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

src/cargo/ops/cargo_compile/mod.rs

weihanglo · 2024-12-17T05:10:03Z

@epage I hope this PR is in a more reviewable state :)

epage · 2024-12-17T22:25:39Z

src/cargo/core/compiler/build_context/target_info.rs

+    /// Checks if a target support core only.
+    ///
+    /// If no explictly stated in target spec json, we treat it as "maybe support".
+    pub fn support_core_only(&self) -> bool {
+        matches!(self.supports_std, Some(false))


Unsure if this is something inherent to build-std or something weird about how we have the code structured but looking at this function in isolated, it seems weird to say that something only supports core if std isn't supported. There is also alloc and others, right?

Also, outside of the context of buiild-std, this makes it read like we have native support for no_std crates but this is separate from that.

Thanks for calling all these out.

Just updated the function name to maybe_support_std and called out that it is for -Zbuild-std.

it seems weird to say that something only supports core if std isn't supported. There is also alloc and others, right?

Yes, but core is like the fundamental of everything and alloc depends on core.

Anyway, this only affects -Zbuild-std to determine the default set of crate to build, if no value is provided to -Zbuild-std.

…sages from concurrent runs 47c2095 didn't really fix the flakiness. build-std tests use the users `CARGO_HOME` for downloading registry dependencies of the standard library. This reduces disk needs of the tests, speeds up the tests, and reduces the number of network requests that could fail. However, this means all of the tests access the same locks for the package cache. In one test, we assert on the output and a `[BLOCKING]` message can show up, depending on test execution time from concurrent test runs. We are going to hack around this by having the one test that asserts on test output to use the standard `cargo-test-support` `CARGO_HOME`, rather than the users `CARGO_HOME`. There will then only be one process accessing the lock and no `[BLOCKING]` messages.

This is a preparation of reuse for subsequent fix.

This failed because since 125e873 [`std_resolve`][1] only includes `sysroot` as primary package. When any custom Cargo feature is provided via `-Zbuild-std-feature`, the default feature set `panic-unwind` would be gone, so no `panic_unwind` crate presents in `std_resolve`. When then calling [`std_resolve.query`][2] with the default set of crates from [`std_crates`][3], which automatically includes `panic_unwind` when `std` presents, it'll result in spec not found because `panic_unwind` was not in `std_resolve` anyway. [1]: https://github.com/rust-lang/cargo/blob/addcc8ca715bc7fe20df66afd6efbf3c77ef43f8/src/cargo/core/compiler/standard_lib.rs#L96 [2]: https://github.com/rust-lang/cargo/blob/addcc8ca715bc7fe20df66afd6efbf3c77ef43f8/src/cargo/core/compiler/standard_lib.rs#L158 [3]: https://github.com/rust-lang/cargo/blob/addcc8ca715bc7fe20df66afd6efbf3c77ef43f8/src/cargo/core/compiler/standard_lib.rs#L156 See rust-lang#14935

This is kinda a revert of 125e873 in terms of the behavior. After this, now `std_resolve` is always resolved by the same set of packages that Cargo will use to generate the unit graph, (technically the same set of crates + `sysroot`), by sharing the same set of primary packages via `std_crates` functions.

…essages from concurrent runs (#14943) ### What does this PR try to resolve? 47c2095 didn't really fix the flakiness. Spun off from <#14938> 2a54190 build-std tests use the users `CARGO_HOME` for downloading registry dependencies of the standard library. This reduces disk needs of the tests, speeds up the tests, and reduces the number of network requests that could fail. However, this means all of the tests access the same locks for the package cache. In one test, we assert on the output and a `[BLOCKING]` message can show up, depending on test execution time from concurrent test runs. We are going to hack around this by having the one test that asserts on test output to use the standard `cargo-test-support` `CARGO_HOME`, rather than the users `CARGO_HOME`. There will then only be one process accessing the lock and no `[BLOCKING]` messages. ### How should we test and review this PR? No more assertion errors like this: ``` ---- test_proc_macro stdout ---- running `/home/runner/work/cargo/cargo/target/debug/cargo fetch -Zbuild-std -Zpublic-dependency` running `/home/runner/work/cargo/cargo/target/debug/cargo test --lib -Zbuild-std -Zpublic-dependency` thread 'test_proc_macro' panicked at tests/build-std/main.rs:394:10: ---- expected: tests/build-std/main.rs:388:27 ++++ actual: stderr 1 + [BLOCKING] waiting for file lock on package cache error: test failed, to rerun pass `-p cargo --test build-std` 2 + [BLOCKING] waiting for file lock on package cache 1 3 | [COMPILING] foo v0.0.0 ([ROOT]/foo) 2 4 | [FINISHED] `test` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s 3 5 | [RUNNING] unittests src/lib.rs (target/debug/deps/foo-[HASH]) ```

Update cargo 6 commits in 769f622e12db0001431d8ae36d1093fb8727c5d9..99dff6d77db779716dda9ca3b29c26addd02c1be 2024-12-14 04:27:35 +0000 to 2024-12-18 00:55:17 +0000 - fix(build-std): make Resolve align to what to build (rust-lang/cargo#14938) - test(build-std): Isolate output test to avoid spurious `[BLOCKING]` messages from concurrent runs (rust-lang/cargo#14943) - docs: fix wrong changelog PR link (rust-lang/cargo#14947) - docs(unstable): Correct stabilization version for MSRV-resolver (rust-lang/cargo#14945) - Update release information for home 0.5.11 (rust-lang/cargo#14939) - Limit release trigger to 0.* tags (rust-lang/cargo#14940)

rustbot assigned epage Dec 15, 2024

rustbot added A-cfg-expr Area: Platform cfg expressions Command-fetch S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 15, 2024

weihanglo added Z-build-std Nightly: build-std and removed A-cfg-expr Area: Platform cfg expressions labels Dec 15, 2024

weihanglo changed the title ~~test(build-std): resolver too less deps~~ fix(build-std): make Resolve align to what to build Dec 15, 2024

weihanglo force-pushed the build-std branch from 283c4d4 to 2a54190 Compare December 16, 2024 00:11

rustbot added the A-cfg-expr Area: Platform cfg expressions label Dec 16, 2024

weihanglo commented Dec 16, 2024

View reviewed changes

src/cargo/ops/cargo_compile/mod.rs Show resolved Hide resolved

weihanglo mentioned this pull request Dec 16, 2024

test(build-std): Isolate output test to avoid spurious [BLOCKING] messages from concurrent runs #14943

Merged

weihanglo marked this pull request as draft December 17, 2024 02:51

weihanglo force-pushed the build-std branch 2 times, most recently from 89ee4f9 to 9d10ede Compare December 17, 2024 04:48

weihanglo marked this pull request as ready for review December 17, 2024 05:09

weihanglo mentioned this pull request Dec 17, 2024

error: package ID specification panic_unwind did not match any packages with -Zbuild-std #14935

Closed

epage reviewed Dec 17, 2024

View reviewed changes

epage approved these changes Dec 17, 2024

View reviewed changes

weihanglo force-pushed the build-std branch from 9d10ede to 5f273d1 Compare December 17, 2024 23:54

weihanglo added 3 commits December 17, 2024 19:02

refactor(build-std): extract support core only detection

cbdbfe4

This is a preparation of reuse for subsequent fix.

weihanglo force-pushed the build-std branch from 5f273d1 to 0149bca Compare December 18, 2024 00:02

epage approved these changes Dec 18, 2024

View reviewed changes

epage added this pull request to the merge queue Dec 18, 2024

Merged via the queue into rust-lang:master with commit 99dff6d Dec 18, 2024
20 checks passed

weihanglo mentioned this pull request Dec 18, 2024

Update cargo rust-lang/rust#134450

Merged

rustbot added this to the 1.85.0 milestone Dec 18, 2024

weihanglo deleted the build-std branch December 19, 2024 22:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(build-std): make Resolve align to what to build #14938

fix(build-std): make Resolve align to what to build #14938

weihanglo commented Dec 15, 2024 •

edited

Loading

rustbot commented Dec 15, 2024

weihanglo commented Dec 17, 2024

epage Dec 17, 2024

weihanglo Dec 18, 2024

fix(build-std): make Resolve align to what to build #14938

fix(build-std): make Resolve align to what to build #14938

Conversation

weihanglo commented Dec 15, 2024 • edited Loading

What does this PR try to resolve?

How should we test and review this PR?

rustbot commented Dec 15, 2024

weihanglo commented Dec 17, 2024

epage Dec 17, 2024

Choose a reason for hiding this comment

weihanglo Dec 18, 2024

Choose a reason for hiding this comment

weihanglo commented Dec 15, 2024 •

edited

Loading