Always compile rustdoc with debug logging enabled when download-rustc is set

[jyn514]

Previously, logging at DEBUG or below would always be silenced, because
rustc compiles tracing with the static_max_level_info feature. That
makes sense for release artifacts, but not for developing rustdoc.

Instead, this compiles two different versions of tracing: one in the
release artifacts, distributed in the sysroot, and a new version
compiled by rustdoc. Since rustc_driver is always linked to the
version of sysroot, this copy/pastes init_env_logging into rustdoc.

To avoid compiling an unnecessary version of tracing when
download-rustc isn't set, this adds a new using-ci-artifacts
feature for rustdoc and passes that feature in bootstrap.

Addresses #81930. This builds on #79540.

r? @Mark-Simulacrum

[Mark-Simulacrum]

(Looks like this could use a rebase; but I should be able to review 2nd and 3rd commits anyway)

[jyn514]

Looks like this could use a rebase

Done :) #79540 just got merged 🎉

[Mark-Simulacrum]

Does it actually take long to compile tracing? What is our win here wall clock wise vs. just always having rustdoc depend on tracing on its own?

[jyn514]

You're right, it doesn't have much effect. It goes (on my machine) from 55 seconds to 60, which seems reasonable given it's a one-time cost. timings.zip

However that means that rustdoc will either
a) never get logging from rustc (never call rustc_driver::init_env_logger), which IMO makes the cure worse than the disease, or
b) get warnings that rustc logging is disabled whenever debug-logging = false is set, even if rustdoc logging is enabled (always call rustc_driver::init_env_logger).

[Mark-Simulacrum]

b seems.. eminently reasonable though? If someone is requesting logging in a generic way that (might) include rustc, giving them a warning seems pretty good. We should endeavor to make the warning talk about rustc specifically but IIRC that was hard, so we'll just want to document it in debug-logging's comment in config.toml.example.

[jyn514]

b seems.. eminently reasonable though? If someone is requesting logging in a generic way that (might) include rustc, giving them a warning seems pretty good. We should endeavor to make the warning talk about rustc specifically but IIRC that was hard, so we'll just want to document it in debug-logging's comment in config.toml.example.

Hmm, ok, that sounds useful. The warning looks like this:

> RUSTDOC_LOG=debug rustdoc >/dev/null
warning: some trace filter directives would enable traces that are disabled statically
 | `debug` would enable the DEBUG level for all targets
 = note: the static max level is `info`
 = help: to enable DEBUG logging, remove the `max_level_info` feature

which seems ok - not great, but ok. Unfortunately I can't test much until the beta compiler gets bumped (I get the LLVM linker error from #81930).

[jyn514]

I updated this to compile tracing twice unconditionally. It works much better than I expected :) even when both versions are active and have DEBUG logging enabled, you still see each log exactly once.

[jyn514]

Hmm, so the test failure here is odd. What happened is that

rust/src/test/rustdoc-ui/deprecated-attrs.rs

Line 3 in a8486b6

#![doc(no_default_passes, passes = "collapse-docs unindent-comments")]

tells rustdoc to run the collapse-docs pass, which no longer exists (it was removed in #80261). Rustdoc doesn't actually give a proper diagnostic here; instead it prints an error! log. I think now that tracing is compiled unconditionally, the log is now being emitted by default, because it's at the error level?

Do you think printing error logging by default is a good change? If so, I can just bless the test output. If not I'll look into why it happens, I'm not 100% sure right now.

Either way, I think rustdoc shouldn't be using error! logging and should go through normal Diagnostics instead. I'll make that change in a separate PR (and change the test not to use a removed pass at the same time).

[Mark-Simulacrum]

I agree that rustdoc should not use error! to report errors to downstream users. That said for presumably an unstable feature that only std uses, if the error is not seen outside tests, I think it'd be fine to just bless, but it'd also be fine to comment out the error! line or adjust the default level.

[jyn514]

That said for presumably an unstable feature that only std uses, if the error is not seen outside tests, I think it'd be fine to just bless

Unfortunately this is stable, although it's deprecated:

rust/src/librustdoc/core.rs

Lines 590 to 595 in 43fc1b3

	if name == sym::no_default_passes {
	report_deprecated_attr("no_default_passes", diag);
	if default_passes == passes::DefaultPassOption::Default {
	default_passes = passes::DefaultPassOption::None;
	}
	}

That said, I don't know anyone using this in practice. For now I'll update the test not to use a removed pass so rustdoc doesn't emit the warning.

[Mark-Simulacrum]

@bors r+

Thanks!

[bors]

📌 Commit 6dc9934 has been approved by Mark-Simulacrum