Using cargo run with bootstrap uses the host rustfmt, not nightly rustfmt

[jyn514]

rust/src/bootstrap/config.rs

Lines 1095 to 1106 in c7ce69f

	config.initial_rustfmt = config.initial_rustfmt.or_else({
	let build = config.build;
	let initial_rustc = &config.initial_rustc;
	move || {
	// Cargo does not provide a RUSTFMT environment variable, so we
	// synthesize it manually.
	let rustfmt = initial_rustc.with_file_name(exe("rustfmt", build));
	if rustfmt.exists() { Some(rustfmt) } else { None }
	}
	});

The proper fix is to check if we're going through the cargo run entrypoint instead of x.py, and if so, download rustfmt from the nightly toolchain listed in src/stage0.json:

rust/src/stage0.json

Lines 8 to 11 in c7ce69f

	"rustfmt": {
	"date": "2022-02-23",
	"version": "nightly"
	},

Ideally we would eventually be able to move this into rustup, but that requires not only for rustup to add and release this feature, but also for all contributors to start using it. It shouldn't be too much extra work for bootstrap to do it itself in the meantime.

cc #94829, #94806 (comment), #94830 (comment)

[jyn514]

@rustbot label: +A-rustbuild +C-enhancement

[bjorn3]

You can use rustup run <toolchain> rustfmt. Or set the RUSTUP_TOOLCHAIN env var.

[jyn514]

Ah, interesting idea. Right now rustup component add --toolchain nightly-2022-02-23 rustfmt gives an error if the toolchain isn't already installed, but @kinnison suggests that rustup could support --profile none which would avoid installing an unused toolchain.

[jyn514]

I do wonder to what extent we can depend on rustup vs. supporting a pre-installed host toolchain. @cuviper is probably not running x.py fmt --check a whole lot while building from source, but it does make me slightly uncomfortable to have commands we know will fail if you use cargo run without rustup.

cc @Mark-Simulacrum

[Mark-Simulacrum]

I would prefer to avoid a dependency on rustup specifically being available for any of our commands; I suspect that manually downloading the tarball for rustfmt would be better.

In general my operating assumption is that we can reasonably expect rustup for "get me a beta toolchain", but should not look for anything more than that. (And of course, all that really means is that the bootstrap toolchain must be provided somehow - perhaps from the previous version in a distro, for example).

[cuviper]

You're right that I don't need fmt for distro builds -- and note that it currently isn't supported at all for stable/beta branches:

$ ./x.py fmt --check
Updating only changed submodules
  Submodules updated in 0.00 seconds
Building rustbuild
    Finished dev [unoptimized] target(s) in 0.06s
./x.py fmt is not supported on this channel
Build completed unsuccessfully in 0:00:00

[jyn514]

Only half-related to this issue, but I would love for cargo fmt to work out of the box. It looks like cargo fmt -- --unstable-options --skip-children already works as long as you have the right toolchain installed - I guess we don't use that so we can use @the8472's optimizations to make rustfmt faster? Could we possibly move those to cargo instead of special-casing x.py?

[Mark-Simulacrum]

My recollection is that we enable unstable rustfmt features in rustfmt.toml and such that mean we couldn't do cargo +beta fmt anyway (and RUSTC_BOOTSTRAP isn't supported by rustfmt).

[jyn514]

Ah right, people would still have to use two separate toolchains for cargo build vs cargo fmt (unless we get rustup to support this in rust-toolchain.toml somehow). Probably not worth investing time into, then.