[107049] Recognise top level keys in config.toml.example #108229

lionellloh · 2023-02-19T07:39:38Z

Test Plan

Configure changelog-seen

lionellloh@lionellloh-mbp rust % ./configure --set changelog-seen=1
configure: processing command line
configure:
configure: changelog-seen       := 1
configure: build.configure-args := ['--set', 'changelog-seen=1']
configure:
configure: writing `config.toml` in current directory
configure:
configure: run `python /Users/lionellloh/rust/x.py --help`
lionellloh@lionellloh-mbp rust % diff config.toml config.toml.example
16c16
< changelog-seen = 1
---
> changelog-seen = 2
331c331
< configure-args = ['--set', 'changelog-seen=1']
---
> #configure-args = []
675c675
< [target.x86_64-apple-darwin]
---
> [target.x86_64-unknown-linux-gnu]
809d808
<

Configure profile

lionellloh@lionellloh-mbp rust % ./configure --set profile=xyz
configure: processing command line
configure:
configure: profile              := xyz
configure: build.configure-args := ['--set', 'profile=xyz']
configure:
configure: writing `config.toml` in current directory
configure:
configure: run `python /Users/lionellloh/rust/x.py --help`
lionellloh@lionellloh-mbp rust % diff config.toml config.toml.example
26c26
< profile = xyz
---
> #profile = <none>
331c331
< configure-args = ['--set', 'profile=xyz']
---
> #configure-args = []
675c675
< [target.x86_64-apple-darwin]
---
> [target.x86_64-unknown-linux-gnu]
809d808
<

rustbot · 2023-02-19T07:39:45Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

Please see the contribution instructions for more information.

lionellloh · 2023-02-19T07:44:41Z

src/bootstrap/configure.py

@@ -459,12 +465,23 @@ def configure_section(lines, config):
                raise RuntimeError("failed to find config line for {}".format(key))


-for section_key in config:
-    section_config = config[section_key]


removed this and used <dict>.items() instead.

lionellloh · 2023-02-19T07:45:06Z

src/bootstrap/configure.py

-        raise RuntimeError("config key {} not in sections".format(section_key))
+def configure_top_level_key(lines, top_level_key, value): 
+    for i, line in enumerate(lines): 
+        if line.startswith('#' + top_level_key + ' = ') or line.startswith(top_level_key + ' = '): 


account for commented and uncommented top-level key-values

lionellloh · 2023-02-19T07:45:33Z

src/bootstrap/configure.py

-    section_config = config[section_key]
-    if section_key not in sections:
-        raise RuntimeError("config key {} not in sections".format(section_key))
+def configure_top_level_key(lines, top_level_key, value): 


decided not to overload configure_section()

lionellloh · 2023-02-19T19:16:09Z

Noob here, any lints I can run to autoformat the configure.py code? Or do I have to fix it manually? @jyn514 / @zephaniahong

jyn514 · 2023-02-19T19:44:47Z

@lionellloh you could run black manually, but there's no formatting enforced by CI, no

lionellloh · 2023-02-20T04:19:28Z

@lionellloh you could run black manually, but there's no formatting enforced by CI, no

I am seeing this in the CI:

tidy error: /checkout/src/bootstrap/configure.py:481: trailing whitespace

Is that just a warning?

zephaniahong · 2023-02-20T05:25:24Z

@lionellloh There a tool called tidy that enforces some formatting: https://rustc-dev-guide.rust-lang.org/tests/intro.html?highlight=tidy#tidy
To check locally, run ./x.py test tidy.

zephaniahong · 2023-02-20T05:27:21Z

@jyn514 Are you okay with running black? Because the last time I made a PR for a python file (and was formatted by some formatter), I was told to undo the formatting.

lionellloh · 2023-02-20T05:57:35Z

@lionellloh There a tool called tidy that enforces some formatting: https://rustc-dev-guide.rust-lang.org/tests/intro.html?highlight=tidy#tidy To check locally, run ./x.py test tidy.

Awesome, that's what I am looking for!

zephaniahong · 2023-02-20T06:02:48Z

That's probably because of what you did when testing out your ./configure
Change your config.toml to this

profile = "compiler"
changelog-seen = 2

If you look at the error, its trying to find a file called config.xyz.toml which is the profile you gave it when testing your ./configure

The profile dictates the file to look for. In this case, by changing it to compiler, it will look for a config.compiler.toml which I'm assuming you have. It should have been created when you ran ./x.py build and chose the compiler option.

zephaniahong · 2023-02-20T07:53:06Z

@lionellloh Once your tests pass, do squash your commits.

lionellloh · 2023-02-20T07:56:04Z

@lionellloh Once your tests pass, do squash your commits.

Sure! Any comments on the PR?

zephaniahong · 2023-02-20T07:58:52Z

I'll leave that to the reviewer since I'm still relatively new to this. Just wanted to help you reach a point where it'd be easier for your reviewer!(: But FWIW, it looks good to me!

zephaniahong · 2023-02-20T08:02:43Z

Also, if you're interested, you could consider working on #107050 as its closely related to this issue. You might want to check if the current assignee is still interested in working on it though.

lionellloh · 2023-02-22T02:46:51Z

r? @jyn514

zephaniahong · 2023-02-22T02:49:22Z

I don't think Jyn is free to review PRs. Usually, give the reviewer about 2 weeks. If you don't get a response, then do ping your reviewer. Alternatively, you can join the Zulip channel as sometimes reviewers are more active there.

Mark-Simulacrum · 2023-02-25T15:05:37Z

@bors r+ rollup

bors · 2023-02-25T15:05:39Z

📌 Commit 5643706 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

lionellloh · 2023-02-25T19:00:38Z

Thanks everyone! I am keen to contribute more. Feel free to assign more issues to me. I will look at #107050 next

…mpiler-errors Rollup of 7 pull requests Successful merges: - rust-lang#105736 (Test that the compiler/library builds with validate-mir) - rust-lang#107291 ([breaking change] Remove a rustdoc back compat warning) - rust-lang#107675 (Implement -Zlink-directives=yes/no) - rust-lang#107848 (Split `x setup` sub-actions to CLI arguments) - rust-lang#107911 (Add check for invalid #[macro_export] arguments) - rust-lang#108229 ([107049] Recognise top level keys in config.toml.example) - rust-lang#108333 (Make object bound candidates sound in the new trait solver) Failed merges: - rust-lang#108337 (hir-analysis: make a helpful note) r? `@ghost` `@rustbot` modify labels: rollup

jyn514 · 2023-03-09T04:13:08Z

This doesn't actually work.

> ./configure --set profile=compiler
configure: processing command line
configure: 
configure: profile              := compiler
configure: build.configure-args := ['--set', 'profile=compiler']
configure: 
configure: writing `config.toml` in current directory
configure: 
configure: run `python /home/gh-jyn514/rust1/x.py --help`
(bash@dev-desktop-us-1.infra.rust-lang.org) ~/rust1 [04:08:29]
> x c
Building bootstrap
    Finished dev [unoptimized] target(s) in 0.04s
failed to parse TOML configuration 'config.toml': invalid TOML value, did you mean to use a quoted string? at line 11 column 11
Build completed unsuccessfully in 0:00:00

I think you need to call to_toml on the value. @lionellloh do you have time to fix this?

lionellloh · 2023-03-09T06:59:36Z

Hey sorry yeah do assign it to me. I can take a look and fix it.

[107049] Recognise top level keys in config.toml.example

960ac2e

rustbot assigned Mark-Simulacrum Feb 19, 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 Feb 19, 2023

lionellloh commented Feb 19, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

Removed trailing spaces to satisfy lint

5643706

lionellloh mentioned this pull request Feb 20, 2023

./configure should set profile = "user" by default #107050

Closed

rustbot assigned jyn514 and unassigned Mark-Simulacrum Feb 22, 2023

jyn514 assigned Mark-Simulacrum and unassigned jyn514 Feb 22, 2023

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 Feb 25, 2023

compiler-errors mentioned this pull request Feb 25, 2023

Rollup of 7 pull requests #108464

Merged

bors merged commit 0b6b373 into rust-lang:master Feb 26, 2023

rustbot added this to the 1.69.0 milestone Feb 26, 2023

jyn514 mentioned this pull request Mar 9, 2023

./configure still doesn't work on top-level keys #108928

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[107049] Recognise top level keys in config.toml.example #108229

[107049] Recognise top level keys in config.toml.example #108229

lionellloh commented Feb 19, 2023 •

edited

Loading

rustbot commented Feb 19, 2023

lionellloh Feb 19, 2023

lionellloh Feb 19, 2023

lionellloh Feb 19, 2023

This comment has been minimized.

lionellloh commented Feb 19, 2023

jyn514 commented Feb 19, 2023

lionellloh commented Feb 20, 2023

zephaniahong commented Feb 20, 2023

zephaniahong commented Feb 20, 2023

lionellloh commented Feb 20, 2023

zephaniahong commented Feb 20, 2023 •

edited

Loading

zephaniahong commented Feb 20, 2023

lionellloh commented Feb 20, 2023

zephaniahong commented Feb 20, 2023 •

edited

Loading

zephaniahong commented Feb 20, 2023

lionellloh commented Feb 22, 2023

zephaniahong commented Feb 22, 2023

Mark-Simulacrum commented Feb 25, 2023

bors commented Feb 25, 2023

lionellloh commented Feb 25, 2023

jyn514 commented Mar 9, 2023

lionellloh commented Mar 9, 2023

[107049] Recognise top level keys in config.toml.example #108229

[107049] Recognise top level keys in config.toml.example #108229

Conversation

lionellloh commented Feb 19, 2023 • edited Loading

rustbot commented Feb 19, 2023

lionellloh Feb 19, 2023

Choose a reason for hiding this comment

lionellloh Feb 19, 2023

Choose a reason for hiding this comment

lionellloh Feb 19, 2023

Choose a reason for hiding this comment

This comment has been minimized.

lionellloh commented Feb 19, 2023

jyn514 commented Feb 19, 2023

lionellloh commented Feb 20, 2023

zephaniahong commented Feb 20, 2023

zephaniahong commented Feb 20, 2023

lionellloh commented Feb 20, 2023

zephaniahong commented Feb 20, 2023 • edited Loading

zephaniahong commented Feb 20, 2023

lionellloh commented Feb 20, 2023

zephaniahong commented Feb 20, 2023 • edited Loading

zephaniahong commented Feb 20, 2023

lionellloh commented Feb 22, 2023

zephaniahong commented Feb 22, 2023

Mark-Simulacrum commented Feb 25, 2023

bors commented Feb 25, 2023

lionellloh commented Feb 25, 2023

jyn514 commented Mar 9, 2023

lionellloh commented Mar 9, 2023

lionellloh commented Feb 19, 2023 •

edited

Loading

zephaniahong commented Feb 20, 2023 •

edited

Loading

zephaniahong commented Feb 20, 2023 •

edited

Loading