x.py rebuilds "bootstrap" a lot

[RalfJung]

When x.py is invoked both by vscode and from the terminal directly, "bootstrap" is rebuilt each time. I think in the end this is due to that line:

rust/src/bootstrap/build.rs

Line 7 in 10cc7a6

println!("cargo:rerun-if-env-changed=PATH");

Indeed PATH is almost certainly different between the two environments. (vscode lives inside a sandbox, it just runs too many untrusted things to be granted access to my files and keys.)

Rebuilding on every PATH change seems excessive, is that truly needed?
Cc @jyn514 @Mark-Simulacrum

[Mark-Simulacrum]

We could probably move that into this if:

rust/src/bootstrap/build.rs

Lines 13 to 21 in 10cc7a6

	if rustc.is_relative() {
	for dir in env::split_paths(&env::var_os("PATH").unwrap_or_default()) {
	let absolute = dir.join(&rustc);
	if absolute.exists() {
	rustc = absolute;
	break;
	}
	}
	}

I think that would still be fine and maybe enough to avoid trouble in practice? That if is the reason we added it in the first place.

[RalfJung]

RUSTC is unset for me (in both environments, unless vscode sets it itself), so that would help for my particular case I think.

[Mark-Simulacrum]

RUSTC is likely set for you -- this is a build script environment, where it should be set to something. IIRC, I had some trouble assuming it was an absolute path in all cases which is why this ad-hoc canonicalization logic was added.

[bjorn3]

RUSTC is set by x.py:

rust/src/bootstrap/bootstrap.py

Line 1001 in 10cc7a6

env["RUSTC"] = self.rustc(True)

[jyn514]

The fix in #94409 seems wrong to me - if bootstrap isn't rerun when RUSTC changes, it will still break moving the build directory, even if the path is absolute. If you want to fix it properly, you'd probably have to remove this handling from the build script altogether and into rustbuild instead.

[Mark-Simulacrum]

I'm not sure I follow your logic -- bootstrap is still rerun when RUSTC changes, just not when it is a relative path. My assumption (perhaps wrong, happy to be corrected) there is that a relative path is only encountered when we're being invoked with a more global rustc (e.g., /usr/bin/rustc, which is in PATH normally).

[RalfJung]

Nothing changed about rebuilds when RUSTC changes. The PR only changes things about rebuilds when PATH changes but RUSTC stays the same.

Put differently: we still emit rerun-if-env-changed for every env var we actually read. If we had build script helpers for var_os that do the rerun-if-env-changed automatically, then they would behave like the post-#94409 code.

[jyn514]

ah, I missed that only PATH was changed and not RUSTC. Yes, that fix seems right to me then.

Probably you can rerun even less, right, only when it's a lookup (rustc) and not when it's a relative path (target/release/rustc)? Maybe that's not happening in practice though.

[bjorn3]

It checks for is_relative() and not for a lookup.