Skip to content

configure.py: Write last key in each section #143606

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

Merged
merged 4 commits into from
Jul 9, 2025

Conversation

lambdageek
Copy link
Contributor

@lambdageek lambdageek commented Jul 7, 2025

The loop that writes the keys in each section of bootstrap.toml accumulates all the commented lines before a given key and emits them when it reaches the next key in the section. This ends up dropping lines accumulated for the last key

Fixes #143605

The loop that writes the keys in each section of bootstrap.toml
accumulates all the commented lines before a given key and emits them
when it reaches the next key in the section.  This ends up dropping
lines accumulated for the last key
@rustbot
Copy link
Collaborator

rustbot commented Jul 7, 2025

r? @jieyouxu

rustbot has assigned @jieyouxu.
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 S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jul 7, 2025
@Kobzol
Copy link
Member

Kobzol commented Jul 7, 2025

r? kobzol

@lolbinarycat does it look right? :)

@rustbot rustbot assigned Kobzol and unassigned jieyouxu Jul 7, 2025
@lolbinarycat
Copy link
Contributor

I only changed parse_example_config so I don't think this is right.

@lolbinarycat
Copy link
Contributor

lolbinarycat commented Jul 7, 2025

Oh! I believe this is kinda an existing bug that just so happened to work before because the last block in a section was always the comments for the next section, while now those comments are considered technically part of the first item in a block.

Still, I'm not sure if any other user of write_uncommented is relying on this behavior, and evidently we don't have enough tests to catch a lot of edge cases...

It would arguably be safer to modify parse_example_config instead to add back the empty sections, then document this quirk on write_uncommented, but that's not exactly a satisfying solution...

In any case it would be nice to give write_uncommented a docstring if possible.

@Kobzol I'll leave it up to you to make the final call, should we do this the "right" way (what the PR does now), or the way that is less likely to cause further regressions (making parse_example_config produce dummy blocks)?

@rust-log-analyzer

This comment has been minimized.

@lambdageek lambdageek force-pushed the configure-write-last-key branch from 4ed5f06 to b6d2130 Compare July 7, 2025 19:56
@Kobzol
Copy link
Member

Kobzol commented Jul 8, 2025

Thanks for taking a look. I examined the function in detail and determined that it is too magical to be understood 😆 I tried to rewrite it like this, which hopefully makes it clearer:

def write_uncommented(target, f):
     """Writes each block in 'target' that is not composed entirely of comments to 'f'.
    A block is a sequence of non-empty lines separated by empty lines.
    """
    block = []

    def flush(last):
        # If the block is entiry made of comments, ignore it
        entire_block_comments = all(ln.startswith("#") or ln == "" for ln in block)
        if not entire_block_comments and len(block) > 0:
            for line in block:
                f.write(line + "\n")
            # Required to output a newline before the start of a new section
            if last:
                f.write("\n")
        block.clear()

    for line in target:
        block.append(line)
        if len(line) == 0:
            flush(last=False)

    flush(last=True)
    return f

What do you think of this?

@lambdageek
Copy link
Contributor Author

@Kobzol that looks much better. should I close the PR? Or add a Co-Authored-By?

@Kobzol
Copy link
Member

Kobzol commented Jul 8, 2025

Feel free to integrate the code into your PR (you can add me as a co-author).

@lambdageek lambdageek force-pushed the configure-write-last-key branch 2 times, most recently from c6f4c85 to 939b9a9 Compare July 8, 2025 11:37
move common code to a helper function

Co-Authored-By: Kobzol <[email protected]>
@lambdageek lambdageek force-pushed the configure-write-last-key branch from 939b9a9 to 3ba8e33 Compare July 8, 2025 11:37
@lambdageek
Copy link
Contributor Author

@Kobzol updated

@Kobzol
Copy link
Member

Kobzol commented Jul 8, 2025

Thanks! Much easier to understand now, if I do say so myself =D

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jul 8, 2025

📌 Commit 3ba8e33 has been approved by Kobzol

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 8, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 8, 2025
…y, r=Kobzol

configure.py: Write last key in each section

The loop that writes the keys in each section of bootstrap.toml accumulates all the commented lines before a given key and emits them when it reaches the next key in the section.  This ends up dropping lines accumulated for the last key

Fixes rust-lang#143605
bors added a commit that referenced this pull request Jul 8, 2025
Rollup of 8 pull requests

Successful merges:

 - #143402 (Port several linking (linkage?) related attributes the new attribute system )
 - #143555 (Don't mark `#[target_feature]` safe fns as unsafe in rustdoc JSON.)
 - #143593 (Port #[rustc_dummy])
 - #143600 (Update intro blurb in `wasm32-wasip1` docs)
 - #143603 (Clarify the meaning of `AttributeOrder::KeepFirst` and `AttributeOrder::KeepLast`)
 - #143606 (configure.py: Write last key in each section)
 - #143620 (fix: Remove newline from multiple crate versions note)
 - #143622 (Add target maintainer information for mips64-unknown-linux-muslabi64)

Failed merges:

 - #143403 (Port several trait/coherence-related attributes the new attribute system)

r? `@ghost`
`@rustbot` modify labels: rollup
@lambdageek lambdageek requested a review from matthiaskrgr July 8, 2025 18:19
@lolbinarycat lolbinarycat added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 8, 2025
@Kobzol
Copy link
Member

Kobzol commented Jul 8, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 8, 2025

📌 Commit 7c8a6d9 has been approved by Kobzol

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 8, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Jul 9, 2025
…y, r=Kobzol

configure.py: Write last key in each section

The loop that writes the keys in each section of bootstrap.toml accumulates all the commented lines before a given key and emits them when it reaches the next key in the section.  This ends up dropping lines accumulated for the last key

Fixes rust-lang#143605
bors added a commit that referenced this pull request Jul 9, 2025
Rollup of 11 pull requests

Successful merges:

 - #143177 (Remove false label when `self` resolve failure does not relate to macro)
 - #143339 (Respect endianness correctly in CheckEnums test suite)
 - #143426 (clippy fix: indentation)
 - #143499 (Don't call `predicates_of` on a dummy obligation cause's body id)
 - #143520 (Fix perf regression caused by tracing)
 - #143532 (More carefully consider span context when suggesting remove `&mut`)
 - #143606 (configure.py: Write last key in each section)
 - #143632 (fix: correct parameter names in LLVMRustBuildMinNum and LLVMRustBuildMaxNum FFI declarations)
 - #143644 (Add triagebot stdarch mention ping)
 - #143651 (Win: Use exceptions with empty data for SEH panic exception copies instead of a new panic)
 - #143660 (Disable docs for `compiler-builtins` and `sysroot`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Jul 9, 2025
Rollup of 9 pull requests

Successful merges:

 - #142357 (Simplify LLVM bitcode linker in bootstrap and add tests for it)
 - #143177 (Remove false label when `self` resolve failure does not relate to macro)
 - #143339 (Respect endianness correctly in CheckEnums test suite)
 - #143426 (clippy fix: indentation)
 - #143475 (tests: Use `cfg_target_has_reliable_f16_f128` in `conv-bits-runtime-const`)
 - #143499 (Don't call `predicates_of` on a dummy obligation cause's body id)
 - #143520 (Fix perf regression caused by tracing)
 - #143532 (More carefully consider span context when suggesting remove `&mut`)
 - #143606 (configure.py: Write last key in each section)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 45b63a4 into rust-lang:master Jul 9, 2025
11 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 9, 2025
rust-timer added a commit that referenced this pull request Jul 9, 2025
Rollup merge of #143606 - lambdageek:configure-write-last-key, r=Kobzol

configure.py: Write last key in each section

The loop that writes the keys in each section of bootstrap.toml accumulates all the commented lines before a given key and emits them when it reaches the next key in the section.  This ends up dropping lines accumulated for the last key

Fixes #143605
@lambdageek lambdageek deleted the configure-write-last-key branch July 9, 2025 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

configure.py doesn't write out the last set key in a section
8 participants