Allow overwriting the sysroot compile flag via --rustc-args

[antoyo]

Hi.
As discussed on Zulip, this is a solution to allow the codegens to overwrite the sysroot as part of their test suite.
Thanks for the review.

[rustbot]

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

[Mark-Simulacrum]

My impression based on the Zulip thread is that the best way for this to be done out-of-tree is for you to build a proper sysroot for the stage0 rustc, such that you don't need --sysroot to redirect it elsewhere. That would look something like copying the full directory (bin, lib, etc.) into a different location and then replacing its regular target-specific load path with the one you want.

(Most of that could be hardlinked if you wanted to avoid extra disk space, at least on Linux).

If that doesn't work, then I'd prefer to see a patch like this instead in rustbuild/bootstrap where we're computing the initial-sysroot in the first place. One option there is to pass the rustflags (e.g. RUSTFLAGS_BOOTSTRAP) to the rustc we're running to get the sysroot, where you could then pass --sysroot X at that stage. That would mean that e.g. compiletest will get built with that sysroot, I think, but that seems OK? Presumably if you're running tests you're comfortable with that too.

[antoyo]

My impression based on the Zulip thread is that the best way for this to be done out-of-tree is for you to build a proper sysroot for the stage0 rustc, such that you don't need --sysroot to redirect it elsewhere. That would look something like copying the full directory (bin, lib, etc.) into a different location and then replacing its regular target-specific load path with the one you want.

I'm not sure I understand. Do you suggest I copy the gcc-built sysroot from rustc_codegen_gcc/build_sysroot/sysroot to rustc_codegen_gcc/rust/build/x86_64-unknown-linux-gnu/stage0-sysroot in order to not have to overwrite the --sysroot argument?

If that doesn't work, then I'd prefer to see a patch like this instead in rustbuild/bootstrap where we're computing the initial-sysroot in the first place. One option there is to pass the rustflags (e.g. RUSTFLAGS_BOOTSTRAP) to the rustc we're running to get the sysroot, where you could then pass --sysroot X at that stage. That would mean that e.g. compiletest will get built with that sysroot, I think, but that seems OK? Presumably if you're running tests you're comfortable with that too.

Again, I'm not sure I understand, so I might be wrong here. Wouldn't that require to build rustc?
If so, out-of-tree codegens don't usually need to build rustc since they download a nightly rustc specified in rust-toolchain and load the codegen as a shared object.

[Mark-Simulacrum]

I'm not sure I understand. Do you suggest I copy the gcc-built sysroot from rustc_codegen_gcc/build_sysroot/sysroot to rustc_codegen_gcc/rust/build/x86_64-unknown-linux-gnu/stage0-sysroot in order to not have to overwrite the --sysroot argument?

I don't think that's quite right. The default stage0 sysroot we're passing is $(rustc --print sysroot), so you would copy into $tmp/sysroot from ~/.rustup/toolchains/... (based on Zulip) and build_sysroot into the right directories.

Again, I'm not sure I understand, so I might be wrong here. Wouldn't that require to build rustc? If so, out-of-tree codegens don't usually need to build rustc since they download a nightly rustc specified in rust-toolchain and load the codegen as a shared object.

No, I don't think you would need to build rustc for this. It's essentially a different approach to the copying, I think.

[antoyo]

I still don't understand.

I don't think that's quite right. The default stage0 sysroot we're passing is $(rustc --print sysroot), so you would copy into $tmp/sysroot from ~/.rustup/toolchains/... (based on Zulip) and build_sysroot into the right directories.

The codegen builds the sysroot into rustc_codegen_gcc/build_sysroot/sysroot.
What is $tmp is this case?

Why would you copy from ~/.rustup/toolchains/? That wasn't built by cg_gcc.

No, I don't think you would need to build rustc for this. It's essentially a different approach to the copying, I think.

You said this would build compiletest with the correct sysroot. I'm not sure that's what I need: I want compiletest to send a different --sysroot argument to the UI tests.
To run cg_gcc's tests, I don't need to build compiletest (edit: or maybe ./x.py test will build compiletest?).

[Mark-Simulacrum]

$tmp is a random directory. Based on Zulip, I am under the impression that you configure (via config.toml) bootstrap/rustbuild to use ~/.rustup/toolchains/nightly-... as your stage0 compiler, but for tests you are telling that compiler to use a different sysroot (via `--sysroot). I am suggesting that instead of trying to alter the behavior of the compiler via --sysroot, you create a directory containing the necessary artifacts so that the compiler's own detection logic suffices, which will avoid needing any patches to the Rust tree (as far as I can tell).

Yes, x.py test generally builds tools as needed, including compiletest.

[antoyo]

Where would this directory needs to be to be detected by rustc?

Where is this detection logic?

[Mark-Simulacrum]

You don't need to pass --sysroot for rustc to use the parent(?) directory of the directory the rustc binary is in as the sysroot. This is how most compilations outside of bootstrap/compiletest/etc work, Cargo or rustup don't pass --sysroot themselves.

rust/compiler/rustc_session/src/filesearch.rs

Lines 203 to 205 in d0ee190

	/// This function checks if sysroot is found using env::args().next(), and if it
	/// is not found, finds sysroot from current rustc_driver dll.
	pub fn get_or_default_sysroot() -> Result<PathBuf, String> {

has the start of the detection logic.

[antoyo]

I'm still confused. rustc --print sysroot gives me the following:

/home/user/.rustup/toolchains/nightly-2023-06-03-x86_64-unknown-linux-gnu

and the rustc is in /home/user/.rustup/toolchains/nightly-2023-06-03-x86_64-unknown-linux-gnu/bin so I have no directory in between where I could copy my own sysroot.

[Mark-Simulacrum]

cp -r /home/user/.rustup/toolchains/nightly-2023-06-03-x86_64-unknown-linux-gnu/ foo (where foo didn't exist previously), then adjust the contents as needed (e.g., replacing lib/rustlib/$target/lib or so).

[antoyo]

This doesn't seem to work. The sysroot used by the UI tests is the following:

/home/user/rustc_codegen_gcc/rust/build/x86_64-unknown-linux-gnu/stage0-sysroot

So it's not the one in /home/user/.rustup/toolchains/nightly-2023-06-03-x86_64-unknown-linux-gnu.

[antoyo]

Let's see if you can instead update this PR to allow overwriting the sysroot.
Can you please point me at the correct file to overwrite this stage0 sysroot?

[Mark-Simulacrum]

Locally when I run x.py test --stage 0 I see compiletest get passed "--sysroot-base" "build/aarch64-apple-darwin/stage0", which aligns with my expectations that it's using the compiler's default, not stage0-sysroot. How are you running x.py test? Based on Zulip you were using --stage 0 as well, but maybe there's some gap I'm missing?

My point is that we should not need any changes in this repository to enable you to do what you want, so updating this PR does not make sense.

[antoyo]

Locally when I run x.py test --stage 0 I see compiletest get passed "--sysroot-base"

Maybe this is the misunderstanding. I don't want to pass the sysroot to compiletest. I want compiletest to send a different sysroot to the UI tests.

I test with the following command:

RUSTC_ARGS="-Csymbol-mangling-version=v0 -Zcodegen-backend="$(pwd)"/../target/"$CHANNEL"/librustc_codegen_gcc."$dylib_ext""
COMPILETEST_FORCE_STAGE0=1 ./x.py test --run always --stage 0 tests/ui/ --rustc-args "$RUSTC_ARGS"

[Mark-Simulacrum]

I don't want to pass the sysroot to compiletest. I want compiletest to send a different sysroot to the UI tests.

--sysroot-base is the argument that compiletest is using to determine which sysroot should get passed during compilation of tests:

rust/src/tools/compiletest/src/runtest.rs

Line 1956 in 34d64ab

rustc.arg("--sysroot").arg(&self.config.sysroot_base);

[antoyo]

This is exactly the code that this PR changed, so I'm not sure I understand your point.

Edit:
I see my sysroot_base is the wrong one:

--sysroot-base /home/user/rustc_codegen_gcc/rust/build/x86_64-unknown-linux-gnu/stage0-sysroot

so something must be wrong.

Does this config look good to you:

changelog-seen = 2

[rust]
codegen-backends = []
deny-warnings = false

[build]
cargo = "/home/user/.rustup/toolchains/my_toolchain/bin/cargo"
local-rebuild = true
rustc = "/home/user/.rustup/toolchains/my_toolchain/bin/rustc"

[target.x86_64-unknown-linux-gnu]
llvm-filecheck = ""

[llvm]
download-ci-llvm = false

[Mark-Simulacrum]

rust/src/bootstrap/test.rs

Line 1430 in 34d64ab

let sysroot = if builder.top_stage == 0 {

is where we pass it, and I think initial_sysroot is always the output of "/home/user/.rustup/toolchains/my_toolchain/bin/rustc" --print sysroot...

I'm not sure what is wrong. I guess the best bet is for you to debug on your end and try to figure out what is causing us to pass stage0-sysroot.

[antoyo]

Ok, I understood what you mean and I can now pass the correct sysroot.

However, I cannot build bootstrap anymore: generic-array panics in its build script. It looks like it might be an ABI incompatibility between cg_gcc's sysroot and code compiled with cg_llvm (this is not a new bug introduced by the sync because all the tests that should pass are passing with this current PR).

Since this issue was probably there for a long time and I might not be able to fix it quickly, I'd like to continue with this PR. Anything you'd like me to change in this PR?

[Mark-Simulacrum]

@bors r+

Seems ok for now, but let's try to get that fixed soon.

[bors]

📌 Commit 8d5e856 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

[antoyo]

Seems ok for now, but let's try to get that fixed soon.

Thanks. What would you suggest as a proper fix?

[bors]

⌛ Testing commit 8d5e856 with merge 3b2073f...