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(build-std): remove hack on creating virtual std workspace #14358

Merged
merged 3 commits into from
Aug 6, 2024

Conversation

weihanglo
Copy link
Member

What does this PR try to resolve?

Starting from rust-lang/rust#128534 (nightly-2024-08-05),
standard library has its own Cargo workspace.
Hence -Zbuild-std no longer need to fake a virtual workspace.

This also adjusts Cargo.toml in mock-std to align with std's Cargo.toml

How should we test and review this PR?

I haven't done any e2e test for -Zbuild-std manually. Would appreciate if someone would like to.

Additional information

There might be more clean-up we could do.

Starting from rust-lang/rust#128534 (nightly-2024-08-05),
stnadard library has its own Cargo workspace.
Hence `-Zbuild-std` no longer need to fake a virtual workspace.

This also adjusts Cargo.toml in `mock-std` to align with std's Cargo.toml
It was designed for `-Zbuild-std`
@rustbot
Copy link
Collaborator

rustbot commented Aug 6, 2024

r? @ehuss

rustbot has assigned @ehuss.
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

@rustbot rustbot added A-workspaces Area: workspaces S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 6, 2024
@glandium
Copy link
Contributor

glandium commented Aug 6, 2024

---- expected: tests/build-std/main.rs:116:27
++++ actual:   stderr
        1 + [UPDATING] crates.io index

That's an interesting error. -Zbuild-std should probably not touch crates.io

// TODO: Consider doing something to enforce --locked? Or to prevent the
// lock file from being written, such as setting ephemeral.
let mut std_ws = Workspace::new_virtual(src_path, current_manifest, virtual_manifest, gctx)?;
let mut std_ws = Workspace::new(&std_ws_manifest_path, gctx)?;
Copy link
Member

Choose a reason for hiding this comment

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

Can you build this workspace with --locked to avoid masking any future case where the lockfile is not up to date for whatever reason?

Copy link
Member Author

Choose a reason for hiding this comment

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

I understand the needs, though that requires more tweaks. I slightly lean toward get this fix first to unblock Cargo CI pipeline and then we make it --locked.

Regardless, which one is preferable?

  • Build succeeded. Lockfile got update but was written to file system.
  • Build failed because of any lockfile update.

Note that tests/build-std/main.rs should have guarded basic cases that won't update registry index, though they are not run in rust-lang/rust main CI pipeline.

Copy link
Member

Choose a reason for hiding this comment

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

I think the build should fail. An outdated lockfile means that something went wrong when building the rust-src component (it should be a verbatim copy of the lockfile used while building the standard library in the rust build system, which uses --locked on CI afaik) or that the rust-src component was (accidentally) tampered with on the user's system.

Copy link
Member Author

Choose a reason for hiding this comment

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

Tracking issue of it: rust-lang/wg-cargo-std-aware#38

@epage
Copy link
Contributor

epage commented Aug 6, 2024

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 6, 2024

📌 Commit 7902bf8 has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 6, 2024
@bors
Copy link
Collaborator

bors commented Aug 6, 2024

⌛ Testing commit 7902bf8 with merge 2cd657c...

@bors
Copy link
Collaborator

bors commented Aug 6, 2024

☀️ Test successful - checks-actions
Approved by: epage
Pushing 2cd657c to master...

@bors bors merged commit 2cd657c into rust-lang:master Aug 6, 2024
24 checks passed
@weihanglo weihanglo deleted the build-std branch August 6, 2024 12:45
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 7, 2024
Update cargo

3 commits in fa646583675d7c140482bd906145c71b7fb4fc2b..94977cb1fab003d45eb5bb108cb5e2fa0149672a
2024-08-02 16:08:06 +0000 to 2024-08-06 21:42:10 +0000
- Don't specify the depedency name in the `cargo add` inferred name test (rust-lang/cargo#14357)
- Fix renamed disallowed cfg lint name (rust-lang/cargo#14352)
- fix(build-std): remove hack on creating virtual std workspace (rust-lang/cargo#14358)

r? ghost
@rustbot rustbot added this to the 1.82.0 milestone Aug 7, 2024
weihanglo added a commit to weihanglo/cargo that referenced this pull request Aug 8, 2024
rust-lang#14358 didn't check the correct Cargo.lock existence
Perhaps it was there so the test passed, but after a new nightly
is out it is gone.

```
    Blocking waiting for file lock on package cache
error: "/home/user/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/Cargo.lock" does not exist, unable to build with the standard library, try:
        rustup component add rust-src --toolchain nightly-aarch64-apple-darwin

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrac
```
weihanglo added a commit to weihanglo/cargo that referenced this pull request Aug 8, 2024
rust-lang#14358 didn't check the correct Cargo.lock existence
Perhaps it was there so the test passed, but after a new nightly
is out it is gone.

```
    Blocking waiting for file lock on package cache
error: "/home/user/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/Cargo.lock" does not exist, unable to build with the standard library, try:
        rustup component add rust-src --toolchain nightly-aarch64-apple-darwin

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
@weihanglo weihanglo added the Z-build-std Nightly: build-std label Aug 8, 2024
weihanglo added a commit to weihanglo/cargo that referenced this pull request Aug 8, 2024
rust-lang#14358 didn't check the correct Cargo.lock existence
Perhaps it was there so the test passed, but after a new nightly
is out it is gone.

```
    Blocking waiting for file lock on package cache
error: "/home/user/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/Cargo.lock" does not exist, unable to build with the standard library, try:
        rustup component add rust-src --toolchain nightly-aarch64-apple-darwin

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
bors added a commit that referenced this pull request Aug 8, 2024
fix: std Cargo.lock moved to `library` dir

#14358 didn't check the correct Cargo.lock existence
Perhaps it was there so the test passed,
but after a new nightly is out it is gone.

```
    Blocking waiting for file lock on package cache
error: "/home/user/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/Cargo.lock" does not exist, unable to build with the standard library, try:
        rustup component add rust-src --toolchain nightly-aarch64-apple-darwin

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

Fixes rust-lang/rust#128808

### How to test the change:

The current nightly `cargo 1.82.0-nightly (94977cb 2024-08-06)` would fail when running

```
cargo +nightly build -Zbuild-std --target <host-triple>
```

After this fix, it should just work

```
RUSTC=~/.rustup/toolchains/nightly-<host-triple>/bin/rustc ./target/debug/cargo build -Zbuild-std --target <host-triple>
```
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Aug 12, 2024
Update cargo

3 commits in fa646583675d7c140482bd906145c71b7fb4fc2b..94977cb1fab003d45eb5bb108cb5e2fa0149672a
2024-08-02 16:08:06 +0000 to 2024-08-06 21:42:10 +0000
- Don't specify the depedency name in the `cargo add` inferred name test (rust-lang/cargo#14357)
- Fix renamed disallowed cfg lint name (rust-lang/cargo#14352)
- fix(build-std): remove hack on creating virtual std workspace (rust-lang/cargo#14358)

r? ghost
@glandium
Copy link
Contributor

glandium commented Sep 3, 2024

FWIW, this didn't change anything wrt -Zbuild-std not working with the use of a vendoring tree. Parts of rust-lang/rust#78790 and/or #8834 are probably still necessary.

@weihanglo
Copy link
Member Author

@glandium
Could you open a new issue with reproduction if it is not supported?

@glandium
Copy link
Contributor

glandium commented Sep 3, 2024

The issue already exists as rust-lang/wg-cargo-std-aware#23

antoniospg pushed a commit to antoniospg/cargo that referenced this pull request Sep 8, 2024
rust-lang#14358 didn't check the correct Cargo.lock existence
Perhaps it was there so the test passed, but after a new nightly
is out it is gone.

```
    Blocking waiting for file lock on package cache
error: "/home/user/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/Cargo.lock" does not exist, unable to build with the standard library, try:
        rustup component add rust-src --toolchain nightly-aarch64-apple-darwin

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-workspaces Area: workspaces S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. Z-build-std Nightly: build-std
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants