update build.extended comments in config.example.toml

[onur-ozkan]

Fixes #115502

[rustbot]

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

[Mark-Simulacrum]

I would prefer to avoid populating a fixed list, because that means future changes to the list lead to breakage even if the user just wanted the default. Can we change the comment to mention that tools may not exist or point to config.example.toml instead?

[onur-ozkan]

I would prefer to avoid populating a fixed list

How about reading this from config.example.toml instead having a fixed hard-coded list? So changes in example config will be reflected to here as well.

[Mark-Simulacrum]

I'm not sure I follow. My point is that I don't want to persist the current (default/non-default) list of tools into config.toml at the time of ./configure. So I don't think reading config.toml helps?

[onur-ozkan]

Can we change the comment to mention that tools may not exist or point to config.example.toml instead?

Didn't you mean this list

rust/src/bootstrap/configure.py

Lines 286 to 296 in 150e163

	# fill the build.tools with default values if build.extended is enabled
	if known_args.get('extended', False) and 'tools' not in known_args:
	tools = [
	"cargo",
	"clippy",
	"rustdoc",
	"rustfmt",
	"rust-analyzer",
	"analysis",
	"src",
	]

would be unsynchronized with the value in config.example.toml?

Did I missed your point?

[Mark-Simulacrum]

I think so. That list getting desynchronized is certainly a concern, but the larger concern I have is with the result of the configure script, i.e., the config.toml file. I don't want to persist an explicit configuration that we may change over time if the user hasn't explicitly requested that configuration.

[onur-ozkan]

Oh, I see.

Hmm, now I am thinking if handling this(e.g., if extended is true and tools is not defined, use tools from config.example.toml as default value) in bootstrap/config.rs would be a good idea. So we don't end up persisting this list to config.toml for people, and we can catch the issues with this list on CI pipelines.

[Mark-Simulacrum]

My impression is that we do that. The issue this is trying to close is just about a misleading comment, right? It's not actually about changing any functionality.

[onur-ozkan]

My impression is that we do that.

Yes, I just checked it. Sorry for any confusion/noises; I thought we were missing the functionality as well.

[Mark-Simulacrum]

@bors r+

[bors]

📌 Commit 838a97223211298fab13df2b44883e4fde4a56f7 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

[onur-ozkan]

@bors r-

[onur-ozkan]

@bors r=Mark-Simulacrum

[bors]

📌 Commit 685b5c2 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

[onur-ozkan]

r-
r=Mark-Simulacrum

(check config.example.toml to see its default value) part was pushed after half a second of r+

[bors]

⌛ Testing commit 685b5c2 with merge e39976f...

[bors]

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing e39976f to master...

[rust-timer]

Finished benchmarking commit (e39976f): 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)	-2.8%	[-2.8%, -2.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)	1.5%	[1.5%, 1.5%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.5%	[1.5%, 1.5%]	1

Binary size

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

Bootstrap: 629.513s -> 628.501s (-0.16%)
Artifact size: 317.49 MiB -> 317.64 MiB (0.05%)