Reland #132772: use download-rustc="if-unchanged" as a global default

[onur-ozkan]

Relands #132772 with the fix.

r? jieyouxu (knows the context).

[rustbot]

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

This PR modifies src/bootstrap/defaults.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

[jieyouxu]

I'm afraid for whatever reason this still is not sufficient, a local stage1 compiler build with this PR's changes shows:

PS X:\repos\rust> rustc +stage1 -vV
rustc 1.84.0-dev
binary: rustc
commit-hash: unknown
commit-date: unknown
host: x86_64-pc-windows-msvc
release: 1.84.0-dev
LLVM version: 19.1.3

[onur-ozkan]

I'm afraid for whatever reason this still is not sufficient, a local stage1 compiler build with this PR's changes shows:

PS X:\repos\rust> rustc +stage1 -vV

rustc 1.84.0-dev

binary: rustc

commit-hash: unknown

commit-date: unknown

host: x86_64-pc-windows-msvc

release: 1.84.0-dev

LLVM version: 19.1.3

You need to use different channel than "dev" in config.toml.

[jieyouxu]

You need to use different channel than "dev" in config.toml.

... Right, I use compiler profile by default. Changed to:

profile = "compiler"
change-id = 999999

[llvm]
#assertions = true
download-ci-llvm = true

[build]
build = "x86_64-pc-windows-msvc"
#profiler = true

[rust]
channel = "nightly"
download-rustc = false
codegen-backends = ["llvm"]
deny-warnings = true
debug = false
debuginfo-level = 1
#debug-logging = true
debug-assertions = true
debug-assertions-std = true

and building local stage1 rustc to double-check -vV output.

FTR, I find it a bit strange that dev-channel rustc builds omit git info, because when I do compiler stuff I would like to sometimes know which commit the compiler was built with (especially if it's local changes) (e.g. to run bare rustc +stage1 xxx to check its behavior).

[jieyouxu]

But anyway, I can confirm that rustc +stage1 -vV contains git info as expected by changing channel to something that's not dev, e.g. nightly:

PS X:\repos\rust> rustc +stage1 -vV
rustc 1.84.0-nightly (a9944bed3 2024-11-10)
binary: rustc
commit-hash: a9944bed3a8272cf6b1738d0d30aaf75ea7d079e
commit-date: 2024-11-10
host: x86_64-pc-windows-msvc
release: 1.84.0-nightly
LLVM version: 19.1.3

[jieyouxu]

The changes themselves LGTM and I did some manual testing and this seems to work properly for non-dev-channel builds.

[jieyouxu]

@bors r+

[bors]

📌 Commit a9944be has been approved by jieyouxu

It is now in the queue for this repository.

[jieyouxu]

@bors rollup=never (revert of a revert)

[jieyouxu]

(Renamed PR title to be slightly more informative)

[Zalathar]

Question: Does this mean that download-rustc="if-unchanged" becomes the default for the compiler profile?

If so, that seems like the wrong default for users of that profile. For example, trying to build the compiler on master might waste time downloading rustc, instead of populating the build cache for further changes as the user intended.

[jieyouxu]

If so, that seems like the wrong default for users of that profile. For example, trying to build the compiler on master might waste time downloading rustc, instead of populating the build cache for further changes as the user intended.

What do you mean by that? Wouldn't rustc download be skipped if rustc was modified?

[Zalathar]

E.g. if I do:

git checkout master
git reset --hard origin/master
git pull
./x build library

Would that build the compiler (as expected), or would it download a pre-built compiler instead?

[jieyouxu]

Would that build the compiler (as expected), or would it download a pre-built compiler instead?

AFAIK that would download a pre-built compiler if compiler/ tree has otherwise no local changes. Good point. I would've expected compiler profile to never download-rustc unless explicitly specified to be if-unchanged, but download-rustc = 'if-unchanged' for other profiles is reasonable as default. A local build AFAICT with a compiler profile but no explicitly specified download-rustc still downloads a pre-built rustc which I find surprising and almost never what I want because I usually want to explicitly build a rustc that corresponds to latest commit on master.

@onur-ozkan what do you think?

@bors r- (unresolved concerns)

[onur-ozkan]

E.g. if I do:

git checkout master
git reset --hard origin/master
git pull
./x build library

Would that build the compiler (as expected), or would it download a pre-built compiler instead?

If you haven't modified the compiler tree and your configuration doesn't conflict with the pre-compiled rustc, why would you want to compile the in-tree compiler? I'm also unsure how many people would actually prefer that. If you still want to build the in-tree compiler without any changes, you can always set download-rustc explicitly to false.

The goal here is to use the same default between library and compiler profiles to keep similar bootstrap behaviour on build process.

From @RalfJung on Zulip thread:

FWIW I think download-rustc=if-unchanged should also be made the right option for basically all cases, so there's no reason to have that different between "compiler" and "library" profiles, either

[jieyouxu]

If you haven't modified the compiler tree and your configuration doesn't conflict with the pre-compiled rustc, why would you want to compile the in-tree compiler? I'm also unsure how many people would actually prefer that. If you still want to build the in-tree compiler without any changes, you can always set download-rustc explicitly to false.

The goal here is to use the same default between library and compiler profiles to keep similar bootstrap behaviour on build process.

I'll double-check with other compiler team members, but I would personally prefer if we kept download-rustc = false for compiler profile and if-unchanged for other profiles at least in this PR. FWIW, I prefer always unconditionally building rustc from source locally when doing compiler work because I need to be certain that the compiler I have contains changes from the latest master commit.

I go around and ask compiler people if we have consensus, and I can send a follow-up to also make this the default for compiler profile if we do.

EDIT: t-compiler thread: https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/.60download-rustc.20.3D.20'if-unchanged'.60.20for.20.60compiler.60.20profile.3F. I'll bring this up during this week's compiler triage meeting as well unless we have obvious preferences beforehand already.

[RalfJung]

I think it would be a bad idea to use different settings for libs and compiler work. It is not uncommon for the same person to do both, and I certainly don't want to reconfigure my checkouts each time I switch between the two. We should provide a seamless transition for a new contributor to switch from a library patch to a compiler patch and vice versa. A compiler dev will basically always change the "compiler" dir, so things will behave exactly as they should.

[jieyouxu]

Thanks for the preset changes. You can r=me after PR CI is green.

[onur-ozkan]

@bors r=jieyouxu

[bors]

📌 Commit 0b30026 has been approved by jieyouxu

It is now in the queue for this repository.

[bors]

☔ The latest upstream changes (presumably #132954) made this pull request unmergeable. Please resolve the merge conflicts.

[onur-ozkan]

@bors r=jieyouxu

[bors]

📌 Commit db12ccd has been approved by jieyouxu

It is now in the queue for this repository.

[bors]

⌛ Testing commit db12ccd with merge 65b3877...

[bors]

☀️ Test successful - checks-actions
Approved by: jieyouxu
Pushing 65b3877 to master...

[rust-timer]

Finished benchmarking commit (65b3877): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

• If the regression was expected or you think it can be justified,
  please write a comment with sufficient written justification, and add
  @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
• If you think that you know of a way to resolve the regression, try to create
  a new PR with a fix for the regression.
• If you do not understand the regression or you think that it is just noise,
  you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
  were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.2%	[0.2%, 0.3%]	2
Improvements ✅ (primary)	-1.1%	[-1.1%, -1.0%]	3
Improvements ✅ (secondary)	-0.5%	[-0.9%, -0.3%]	6
All ❌✅ (primary)	-1.1%	[-1.1%, -1.0%]	3

Max RSS (memory usage)

Results (primary 4.1%)

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)	4.1%	[4.1%, 4.1%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.1%	[4.1%, 4.1%]	1

Cycles

Results (secondary 2.0%)

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.0%	[1.9%, 2.2%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 786.527s -> 784.717s (-0.23%)
Artifact size: 335.37 MiB -> 335.35 MiB (-0.01%)

[lqd]

Noise of course.

@rustbot label: +perf-regression-triaged

[lqd]

rust/config.example.toml

Lines 499 to 507 in dae7ac1

	# Whether to download the stage 1 and 2 compilers from CI.
	# This is useful if you are working on tools, doc-comments, or library (you will be able to build
	# the standard library without needing to build the compiler).
	#
	# Set this to "if-unchanged" if you are working on `src/tools`, `tests` or `library` (on CI, `library`
	# changes triggers in-tree compiler build) to speed up the build process.
	#
	# Set this to `true` to download unconditionally.
	#download-rustc = false

needs to be updated to the new default