Skip to content

opt-dist: explicitly pass --set build.metrics=true while running build #139686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

ognevny
Copy link
Contributor

@ognevny ognevny commented Apr 11, 2025

at first it's not obvious that build.metrics should be enabled to use opt-dist which will lead to confusion. probably it's better to add a good error message which instructs to set this option manually

edit: are there more places which need such change?

at first it's not obvious that build.metrics should be enabled to use
opt-dist which will lead to confusion. probably it's better to add a
good error message which instructs to set this option manually
@rustbot
Copy link
Collaborator

rustbot commented Apr 11, 2025

r? @Kobzol

rustbot has assigned @Kobzol.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 11, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 11, 2025

Some changes occurred in src/tools/opt-dist

cc @Kobzol

@Kobzol
Copy link
Contributor

Kobzol commented Apr 11, 2025

Hi, if I understand the problem correctly, you are trying to use opt-dist manually (outside our CI) wit ha config that doesn't enable build metrics, and opt-dist breaks because it expects that metrics are enabled?

@ognevny
Copy link
Contributor Author

ognevny commented Apr 11, 2025

yes, I use opt-dist with opt-dist local. probably I hadn't searched properly, but I didn't find any kind of manual on using it

@Kobzol
Copy link
Contributor

Kobzol commented Apr 11, 2025

It's slightly documented here, but it's really best effort, it's mostly designed for our internal CI usage, and potentially for distro people who are feeling courageous :)

You could fix the metrics problem by adding the flag in Bootstrap::build and also Bootstrap::dist, but wouldn't it be easier to simply create a config with build.metrics = true? We can document that this is required in the rustc-dev-guide.

@ognevny
Copy link
Contributor Author

ognevny commented Apr 11, 2025

yeah, documenting seems a better option in this case. I'm just "playing" with opt-dist as I try to compile Rust with PGO in msys2 :)

@Kobzol
Copy link
Contributor

Kobzol commented Apr 11, 2025

You can modify the link that I posted directly in this repo (src/doc/rustc-dev-guide). Would you like to do it in this PR?

@ognevny
Copy link
Contributor Author

ognevny commented Apr 11, 2025

thanks. I'll let you to change the docs, so it will be made prettier faster (than processing through multiple reviews 😄). feel free to close this PR

@Kobzol
Copy link
Contributor

Kobzol commented Apr 11, 2025

Well if you did the change, I could approve it, which probably makes it faster, because if I make the change, someone else will have to approve xD But it doesn't really matter. Sent #139691.

@Kobzol Kobzol closed this Apr 11, 2025
@ognevny ognevny deleted the opt-dist-metrics branch April 11, 2025 19:32
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Apr 11, 2025
Document that `opt-dist` requires metrics to be enabled

Suggested in rust-lang#139686.
ChrisDenton added a commit to ChrisDenton/rust that referenced this pull request Apr 12, 2025
Document that `opt-dist` requires metrics to be enabled

Suggested in rust-lang#139686.
ChrisDenton added a commit to ChrisDenton/rust that referenced this pull request Apr 12, 2025
Document that `opt-dist` requires metrics to be enabled

Suggested in rust-lang#139686.
ChrisDenton added a commit to ChrisDenton/rust that referenced this pull request Apr 13, 2025
Document that `opt-dist` requires metrics to be enabled

Suggested in rust-lang#139686.
ChrisDenton added a commit to ChrisDenton/rust that referenced this pull request Apr 13, 2025
Document that `opt-dist` requires metrics to be enabled

Suggested in rust-lang#139686.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 13, 2025
Rollup merge of rust-lang#139691 - Kobzol:opt-dist-docs, r=jieyouxu

Document that `opt-dist` requires metrics to be enabled

Suggested in rust-lang#139686.
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Apr 14, 2025
Document that `opt-dist` requires metrics to be enabled

Suggested in rust-lang/rust#139686.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants