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

cargo is unnecessarily rebuilt on x.py dist #130018

Closed
xry111 opened this issue Sep 6, 2024 · 4 comments
Closed

cargo is unnecessarily rebuilt on x.py dist #130018

xry111 opened this issue Sep 6, 2024 · 4 comments
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-cargo Relevant to the cargo team, which will review and decide on the PR/issue.

Comments

@xry111
Copy link
Contributor

xry111 commented Sep 6, 2024

Run

x.py build
x.py dist

And x.py dist will rebuild cargo for no good reason (the rebuilt cargo executable is unchanged at all). Using a cargo instrumented with debug output:

the rustflags changed from
["-C", "target-cpu=raptorlake", "--cfg=windows_raw_dylib", "-Csymbol-mangling-version=v0", "-Zunstable-options", "--check-cfg=cfg(bootstrap)", "--check-cfg=cfg(parallel_compiler)", "--check-cfg=cfg(rust_analyzer)", "-Zmacro-backtrace", "-Csplit-debuginfo=off", "-Clink-args=-Wl,-z,origin", "-Clink-args=-Wl,-rpath,$ORIGIN/../lib", "-Zunstable-options"] to
["-C", "target-cpu=raptorlake", "--cfg=windows_raw_dylib", "-Csymbol-mangling-version=v0", "-Zunstable-options", "--check-cfg=cfg(bootstrap)", "--check-cfg=cfg(parallel_compiler)", "--check-cfg=cfg(rust_analyzer)", "-Zmacro-backtrace", "-Csplit-debuginfo=off", "-Clink-args=-Wl,-z,origin", "-Clink-args=-Wl,-rpath,$ORIGIN/../lib", "-Zunstable-options", "-Zon-broken-pipe=kill"]

AFAIK -Zon-broken-pipe=kill shouldn't affect code generation, thus there's no need to rebuild cargo.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 6, 2024
@lolbinarycat lolbinarycat added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-cargo Relevant to the cargo team, which will review and decide on the PR/issue. labels Sep 7, 2024
@weihanglo
Copy link
Member

weihanglo commented Sep 7, 2024

RUSTFLAGS is opaque to Cargo. Cargo doesn't really parse the content of those flags. Here is an issue of reconsidering rustflags cache mechanism
rust-lang/cargo#8716. Close this in favor of that.

@weihanglo weihanglo closed this as not planned Won't fix, can't repro, duplicate, stale Sep 7, 2024
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 7, 2024
@xry111
Copy link
Contributor Author

xry111 commented Sep 8, 2024

I understand Cargo does not really parse the content of those flags. But then our bootstrap mechanism should then work around the issue, or at least make the inclusion of -Zon-broken-pipe=kill configurable.

@xry111
Copy link
Contributor Author

xry111 commented Sep 8, 2024

Oops removing -Zon-broken-pipe can break things :(. My idea is now removing the if check in

    // `-Zon-broken-pipe=kill` breaks cargo tests
    if !path.ends_with("cargo") {
        // If the output is piped to e.g. `head -n1` we want the process to be killed,
        // rather than having an error bubble up and cause a panic.
        cargo.rustflag("-Zon-broken-pipe=kill");
    }

and fix up the cargo tests...

@jieyouxu
Copy link
Member

jieyouxu commented Oct 12, 2024

@xry111 can you check if this is fixed for you by #131155? This isn't a cargo problem, this was a bootstrap problem + tool problem where -Zon-broken-pipe=kill is needed by all the tool binaries (except for cargo) to paper over broken pipe I/O errors, but it must not be set for cargo. Before #131155 this was passed to previous cargo via RUSTFLAGS which is tracked by cargo, but now we use an untracked env var to pass that to rustc so cargo doesn't know about it, so cargo won't feel the need to rebuild cargo if e.g. you build rustdoc first then cargo, then try to do that again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-cargo Relevant to the cargo team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants