Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Bootstrap: Changelog not displayed when config fails to parse #121756

Closed
TimNN opened this issue Feb 28, 2024 · 0 comments · Fixed by #121787
Closed

Bootstrap: Changelog not displayed when config fails to parse #121756

TimNN opened this issue Feb 28, 2024 · 0 comments · Fixed by #121787
Assignees
@onur-ozkan
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@TimNN
Copy link
Contributor

TimNN commented Feb 28, 2024

Bootstrap's "changelog" functionality can be quite helpful in figuring out how to adapt a local config.toml file for bootstrap changes.

However, before checking whether the changelog needs to be displayed, bootstrap currently fully parses and validates the config. This means that if there is an error while parsing the config, the changelog is never displayed.

I ran into this recently when the codegen profile was removed (the profile must be successfully resolved to parse the config, so the changelog was never displayed), and again while writing #121754 (where I'd ideally like to remove a field from the config, but kept it for now to be able to provide a better error message).

IMO bootstrap would ideally check whether the changelog needs to be displayed before fully parsing the config.
@TimNN TimNN added the C-bug Category: This is a bug. label Feb 28, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 28, 2024
@jieyouxu jieyouxu added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) and removed C-bug Category: This is a bug. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Feb 28, 2024
@onur-ozkan onur-ozkan self-assigned this Feb 29, 2024
@onur-ozkan onur-ozkan mentioned this issue Feb 29, 2024
Merged
@bors bors closed this as completed in 2448162 Mar 17, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 17, 2024
@rust-timer
Unrolled build for rust-lang#121787
5b51f20 
Rollup merge of rust-lang#121787 - onur-ozkan:improve-change-tracker, r=albertlarsan68

run change tracker even when config parse fails

Please note that we are currently validating the build configuration on two entry points (e.g., profile validation is handled on the python side), and change tracker system is handled on the rust side. Once rust-lang#94829 is completed (scheduled for 2024), we will be able to handle this more effectively.

Fixes rust-lang#121756
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@onur-ozkan onur-ozkan

Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

run change tracker even when config parse fails onur-ozkan/rust
4 participants
@TimNN @jieyouxu @onur-ozkan @rustbot