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

run-make: extract bootstrap cargo invocations to a proper Command #128734

Closed
jieyouxu opened this issue Aug 6, 2024 · 3 comments
Closed

run-make: extract bootstrap cargo invocations to a proper Command #128734

jieyouxu opened this issue Aug 6, 2024 · 3 comments
Assignees
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jieyouxu
Copy link
Member

jieyouxu commented Aug 6, 2024

Current bootstrap cargo incantations are nothing short of magic and are indecipherable. We should try to refactor them to proper Command wrappers.

let bootstrap_cargo = env_var("BOOTSTRAP_CARGO");
let mut cmd = cmd(bootstrap_cargo);
cmd.args(&[
    "build",
    "--manifest-path",
    manifest_path.to_str().unwrap(),
    "-Zbuild-std=core",
    "--target",
    &target,
])
.env("PATH", path)
.env("RUSTC", rustc)
...
@jieyouxu jieyouxu added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-testsuite Area: The testsuite used to check the correctness of rustc T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-run-make Area: port run-make Makefiles to rmake.rs labels Aug 6, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 6, 2024
@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 6, 2024
@lolbinarycat
Copy link
Contributor

@rustbot claim

@lolbinarycat
Copy link
Contributor

any idea why these are explicitly inheriting environment variables? it's not like they call env_clear or anything...

@jieyouxu
Copy link
Member Author

Sorry I didn't notice this issue, it's because at the time when I wrote run-make infra I just copied whatever the previous Makefile implementation did: it simply passed the already-available bootstrap cargo to run-make tests.

As discovered during a beta backport we found out that run-make -Z build-std tests fail on compiler_builtins on older branches #130634 because we added some new Cargo.lock setup that the bootstrap cargo did not know how to handle, but which is present in a cargo that is built from sources by a stage1 rustc.

This particular issue is fixed by a combination of #130642 and #130739 where we properly ensuring a suitably-staged cargo in bootstrap is available, and passing a path to that cargo in compiletest (see #130739 for details). We removed any BOOTSTRAP_CARGO in the test suite, run-make-support and compiletest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Status: Done
Development

No branches or pull requests

3 participants