Omit unchanged options from config.toml in `configure.py` #108632

Teapot4195 · 2023-03-02T00:12:01Z

Leaves section tags, but removes options that are unchanged.
Change in config.toml.example is to prevent comments from sneaking in by being directly after a section tag

closes #108612

rustbot · 2023-03-02T00:12:09Z

r? @ozkanonur

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

jyn514 · 2023-03-02T04:27:05Z

src/bootstrap/configure.py

+                # only blocks with an uncommented line will be output
+                block = []
+                has_non_comment = False
                for line in targets[target]:
-                    f.write(line + "\n")
+                    block.append(line)
+                    if len(line) == 0:
+                        if has_non_comment:
+                            for l in block:
+                                f.write(l + "\n")
+                        block = []
+                        has_non_comment = False
+                        continue
+                    if not line.startswith('#'):
+                        has_non_comment = True
        else:
+            # only blocks with an uncommented line will be output
+            block = []
+            has_non_comment = False
            for line in sections[section]:
-                f.write(line + "\n")
+                block.append(line)
+                if len(line) == 0:
+                    if has_non_comment:
+                        for l in block:
+                            f.write(l + "\n")
+                    block = []
+                    has_non_comment = False
+                    continue
+                if not line.startswith('#'):
+                    has_non_comment = True


can you avoid duplicating this please? maybe with a helper function?

onur-ozkan · 2023-03-02T09:23:31Z

src/bootstrap/configure.py

@@ -494,11 +494,35 @@ def configure_top_level_key(lines, top_level_key, value):
    for section in section_order:
        if section == 'target':
            for target in targets:
+                # only blocks with an uncommented line will be output
+                block = []
+                has_non_comment = False


Suggested change

has_non_comment = False

is_comment = True

onur-ozkan · 2023-03-02T09:26:41Z

src/bootstrap/configure.py

+                    block.append(line)
+                    if len(line) == 0:
+                        if has_non_comment:
+                            for l in block:
+                                f.write(l + "\n")
+                        block = []
+                        has_non_comment = False
+                        continue
+                    if not line.startswith('#'):
+                        has_non_comment = True
        else:
+            # only blocks with an uncommented line will be output
+            block = []
+            has_non_comment = False


comment check can be simplified with:

Suggested change

block.append(line)

if len(line) == 0:

if has_non_comment:

for l in block:

f.write(l + "\n")

block = []

has_non_comment = False

continue

if not line.startswith('#'):

has_non_comment = True

else:

# only blocks with an uncommented line will be output

block = []

has_non_comment = False

block.append(line)

if len(line) == 0:

if not is_comment:

for l in block:

f.write(l + "\n")

block = []

is_comment = True

continue

is_comment = line.startswith('#'):

else:

# only blocks with an uncommented line will be output

block = []

is_comment = True

I don't think this will work if the comment is the last line in the block e.g.:

# some text some_config=true # some more text end of block

I haven't tested it. Just tried to reverse the idea of has_non_comment with is_comment and simplify the following block

if not line.startswith('#'): has_non_comment = True

with

is_comment = line.startswith('#'):

to make it easier to read.

Maybe I mischanged something, is it not possible to write it that way?

onur-ozkan · 2023-03-02T09:28:19Z

@rustbot author

Teapot4195 · 2023-03-02T15:00:47Z

should have fixed all the issues listed, let me know if i should squash

onur-ozkan · 2023-03-02T15:19:07Z

should have fixed all the issues listed, let me know if i should squash

The code look much better. But it fails the pipeline unfortunately https://github.com/rust-lang/rust/actions/runs/4315278739/jobs/7529435117#step:26:1670

(and yes, please squash the commits at the end)

Teapot4195 · 2023-03-02T18:33:28Z

should have fixed all the issues listed, let me know if i should squash

The code look much better. But it fails the pipeline unfortunately https://github.com/rust-lang/rust/actions/runs/4315278739/jobs/7529435117#step:26:1670

(and yes, please squash the commits at the end)

been looking at this, can't tell what is wrong builds just fine on both my win 11 install and on my ubuntu 22.04 install.

onur-ozkan · 2023-03-02T23:40:35Z

should have fixed all the issues listed, let me know if i should squash

The code look much better. But it fails the pipeline unfortunately https://github.com/rust-lang/rust/actions/runs/4315278739/jobs/7529435117#step:26:1670
(and yes, please squash the commits at the end)

been looking at this, can't tell what is wrong builds just fine on both my win 11 install and on my ubuntu 22.04 install.

It's not the build thats failing.

python3 x.py test tests/rustdoc-gui --stage 2 --test-args "'--no-sandbox --jobs 1'" this command is failing due to missing /tmp/toolstate/toolstates.json.

You can force push to PR and re-trigger the pipelines to see if the error actually related with your PR. If it does, you might miss something while refactoring(because it pipelines passed before)

…where it is not default

Teapot4195 · 2023-03-03T23:21:38Z

ah looks like an or turned into an and and it broke everything :)

it's actually fixed now

onur-ozkan

LGTM

onur-ozkan · 2023-03-03T23:51:28Z

@bors r+ rollup

bors · 2023-03-03T23:51:30Z

📌 Commit 00f98e6 has been approved by ozkanonur

It is now in the queue for this repository.

…anonur Omit unchanged options from config.toml in `configure.py` Leaves section tags, but removes options that are unchanged. Change in `config.toml.example` is to prevent comments from sneaking in by being directly after a section tag closes rust-lang#108612

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#108627 (Properly colorize multi-part suggestions in the same line) - rust-lang#108632 (Omit unchanged options from config.toml in `configure.py`) - rust-lang#108715 (Remove unclosed_delims from parser) - rust-lang#108723 (rustdoc: function signature search with traits in `where` clause) - rust-lang#108724 (field is not used outside the crate) - rust-lang#108734 (rustdoc: Note in a type's layout/size if it is uninhabited) - rust-lang#108736 (Remove `allow(potential_query_instability)` from `ast_passes`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned onur-ozkan Mar 2, 2023

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 Mar 2, 2023

Teapot4195 force-pushed the issue-108612-fix branch from a1173d2 to 0035e53 Compare March 2, 2023 00:24

jyn514 reviewed Mar 2, 2023

View reviewed changes

onur-ozkan reviewed Mar 2, 2023

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 2, 2023

This comment has been minimized.

Sign in to view

Teapot4195 force-pushed the issue-108612-fix branch from 21f930c to a2d1368 Compare March 3, 2023 21:49

./configure script should only show blocks (and associated comments) …

00f98e6

…where it is not default

Teapot4195 force-pushed the issue-108612-fix branch from a2d1368 to 00f98e6 Compare March 3, 2023 22:27

onur-ozkan approved these changes Mar 3, 2023

View reviewed changes

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 3, 2023

matthiaskrgr mentioned this pull request Mar 4, 2023

Rollup of 7 pull requests #108743

Closed

matthiaskrgr mentioned this pull request Mar 4, 2023

Rollup of 7 pull requests #108747

Merged

bors merged commit 538f19d into rust-lang:master Mar 5, 2023

rustbot added this to the 1.69.0 milestone Mar 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Omit unchanged options from config.toml in `configure.py` #108632

Omit unchanged options from config.toml in `configure.py` #108632

Teapot4195 commented Mar 2, 2023

rustbot commented Mar 2, 2023

jyn514 Mar 2, 2023

onur-ozkan Mar 2, 2023

onur-ozkan Mar 2, 2023

Teapot4195 Mar 2, 2023 •

edited

Loading

onur-ozkan Mar 2, 2023

onur-ozkan commented Mar 2, 2023

Teapot4195 commented Mar 2, 2023

onur-ozkan commented Mar 2, 2023 •

edited

Loading

Teapot4195 commented Mar 2, 2023

onur-ozkan commented Mar 2, 2023

This comment has been minimized.

Teapot4195 commented Mar 3, 2023

onur-ozkan left a comment

onur-ozkan commented Mar 3, 2023

bors commented Mar 3, 2023

Omit unchanged options from config.toml in configure.py #108632

Omit unchanged options from config.toml in configure.py #108632

Conversation

Teapot4195 commented Mar 2, 2023

rustbot commented Mar 2, 2023

jyn514 Mar 2, 2023

Choose a reason for hiding this comment

onur-ozkan Mar 2, 2023

Choose a reason for hiding this comment

onur-ozkan Mar 2, 2023

Choose a reason for hiding this comment

Teapot4195 Mar 2, 2023 • edited Loading

Choose a reason for hiding this comment

onur-ozkan Mar 2, 2023

Choose a reason for hiding this comment

onur-ozkan commented Mar 2, 2023

Teapot4195 commented Mar 2, 2023

onur-ozkan commented Mar 2, 2023 • edited Loading

Teapot4195 commented Mar 2, 2023

onur-ozkan commented Mar 2, 2023

This comment has been minimized.

Teapot4195 commented Mar 3, 2023

onur-ozkan left a comment

Choose a reason for hiding this comment

onur-ozkan commented Mar 3, 2023

bors commented Mar 3, 2023

Omit unchanged options from config.toml in `configure.py` #108632

Omit unchanged options from config.toml in `configure.py` #108632

Teapot4195 Mar 2, 2023 •

edited

Loading

onur-ozkan commented Mar 2, 2023 •

edited

Loading