Pass flags to rustdoc shim without env. vars

[Kobzol]

Discussed here: #116448 (comment). Since it was not really documented why these flags were passed through the shim, I guess that the only way to find out if it's really needed... is to remove it :)

r? @petrochenkov

[petrochenkov]

I recalled what the issue with doctests was - bootstrap passes options to rustdoc only when it calls rustdoc directly, not through cargo.
The options should also be passed when rustdoc is run through cargo (doctests are tested that way).

[petrochenkov]

This really needs to be tested separately, with full x.py test --stage 2 #116448 (comment).
It's also quite possible that it is broken on master.

[Kobzol]

I recalled what the issue with doctests was - bootstrap passes options to rustdoc only when it calls rustdoc directly, not through cargo. The options should also be passed when rustdoc is run through cargo (doctests are tested that way).

Yes, this accurately describes the state on master.

Do you have an example of a x.py test command or a subdirectory of tests where doc tests are executed through cargo? I tried using x.py test --doc, but couldn't really find a place where it would be applied. It seems to me that this only happens when we test tools and submodules (?).

[petrochenkov]

cargo test on standard library crates or rustc crates should run rustdoc with doctest building & running?
I don't know how to run it in a more fine-grained way, I just remember that it was included into full x.py test (at least back in 2017 :D).

[Kobzol]

I looked around and there are indeed a few places where rustdoc is being executed without the linker being set (CargoTest, Bootstrap at least). But this is a bigger issue I think - there are some places in bootstrap where we execute cargo that doesn't go through the main Cargo builder function. This means that there might be more places where the linker isn't set.

If we set use-lld or linker, should it mean that absolutely all relevant commands executed by bootstrap should change the linker/flavor? If yes, then this might need some more restructuring, rather than just adding it arbitrarily to a few places where rustdoc is invoked by cargo.

[petrochenkov]

If we set use-lld or linker, should it mean that absolutely all relevant commands executed by bootstrap should change the linker/flavor?

Yes, that was the case when I originally implemented linker and full use-lld support (EDIT: at least for all commands executed during full testing x.py test --stage 2).
If they are not used in some cases it's a later regression.

[Kobzol]

Okay so I investigated a bit and the only place where rustdoc is invoked manually (outside of Builder::rustdoc_cmd) is here:

rust/src/bootstrap/test.rs

Line 846 in acd1869

let rustdoc = builder.bootstrap_out.join("rustdoc");

. There's an issue that here we need to prepend the rustdoc-theme command before actually passing the rustdoc command, but I don't think that Command allows prepending arguments, so that we could call rustdoc_cmd and then prepend rustdoc-theme.

[Kobzol]

I have added two functions to add the LLD flags to both normal rustdoc and cargo rustdoc invocations. It's far from pretty..

[Kobzol]

Rebased and tested full x.py test --stage 2 locally.

@rustbot ready

[petrochenkov]

I'm also not sure whether the CI failure in #116449 (comment) is spurious or not.
@rustbot author

[Kobzol]

I'm also unsure about the CI failure, it happened repeatedly, but I can't reproduce it locally. Let's see what happens now.

[Kobzol]

There was a problem with empty RUSTDOCFLAGS. If I passed the env. var. with an empty value, one rustdoc-gui test failed. So I added a check to skip setting the environment variable if it's empty.

@rustbot ready

[petrochenkov]

@bors r+

[bors]

📌 Commit 4ba9586 has been approved by petrochenkov

It is now in the queue for this repository.

[bors]

⌛ Testing commit 4ba9586 with merge 98af42c...

[bors]

💔 Test failed - checks-actions

[Kobzol]

A redundant use statement was in windows cfg-gated code.

@bors r=petrochenkov

[bors]

📌 Commit 2e7d684 has been approved by petrochenkov

It is now in the queue for this repository.

[bors]

⌛ Testing commit 2e7d684 with merge e7b2285...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing e7b2285 to master...

[rust-timer]

Finished benchmarking commit (e7b2285): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.8%	[-1.8%, -1.8%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.3%	[2.3%, 2.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 677.592s -> 676.441s (-0.17%)
Artifact size: 313.72 MiB -> 313.75 MiB (0.01%)