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

Add default configuration files for x.py #76619

Closed
jyn514 opened this issue Sep 11, 2020 · 4 comments · Fixed by #76628
Closed

Add default configuration files for x.py #76619

jyn514 opened this issue Sep 11, 2020 · 4 comments · Fixed by #76628
Assignees
@jyn514
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself C-feature-request Category: A feature request, i.e: not implemented / a PR. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jyn514
Copy link
Member

jyn514 commented Sep 11, 2020 •
edited
Loading

Currently, the options are 'use the defaults' or 'read the 600 line config.toml.example. Even with recommended defaults from rustc-dev-guide, this is not really ideal for new contributors. As an alternative, there could be different config files for each type of user:

  • distro maintainers and people compiling from source
  • contributors to the standard library or documentation
  • contributors to the compiler
  • contributors to the compiler + llvm

To avoid these getting out of date, they could be included instead of copied: include = "maintainer" in config.toml or something like that. This would also make it clear which changes are automatic and which were made by the user: the automatic changes are not in the same file.

I'm imaging that the files live in src/bootstrap/config.toml.maintainer or similar.
@jyn514 jyn514 added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Sep 11, 2020
@jyn514 jyn514 mentioned this issue Sep 11, 2020
Closed
@jyn514
Copy link
Member Author

jyn514 commented Sep 12, 2020

This will be annoying to implement by hand because of all the nested structs ... @Mark-Simulacrum do you mind if I use https://docs.rs/merge/0.1.0/merge/ or similar to implement it?

@jyn514 jyn514 mentioned this issue Sep 12, 2020
Merged
@jyn514 jyn514 added the A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself label Sep 13, 2020
@jyn514 jyn514 mentioned this issue Sep 13, 2020
Merged
@est31
Copy link
Member

est31 commented Sep 18, 2020

Another use case: contributions to documentation.

@jyn514
Copy link
Member Author

jyn514 commented Sep 18, 2020

@est31 hmm, when writing this I lumped that together with 'contributors to the standard library' in my mind - do you expect the defaults to be different between them? I guess one difference is that writing docs would only run x.py doc, not any other command, but that seems more like something to suggest in #76503 rather than having a different config file.

@jyn514 jyn514 self-assigned this Sep 18, 2020
@est31
Copy link
Member

est31 commented Sep 18, 2020

Good point, they fit into that category really well.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 21, 2020
@Dylan-DPC
Rollup merge of rust-lang#76628 - jyn514:default-config-files, r=Mark…
ee3743d 
…-Simulacrum

Add sample defaults for config.toml

- Allow including defaults in `src/bootstrap/defaults` using `profile = "..."`.
- Add default config files, with a README noting they're experimental and asking you to open an issue if you run into trouble. The config files have comments explaining why the defaults are set.
- Combine config files using the `merge` dependency.

This introduces a new dependency on `merge` that hasn't yet been vetted.

I want to improve the output when `include = "x"` isn't found:

```
thread 'main' panicked at 'fs::read_to_string(&file) failed with No such file or directory (os error 2) ("configuration file did not exist")', src/bootstrap/config.rs:522:28
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failed to run: /home/joshua/rustc/build/bootstrap/debug/bootstrap test tidy
Build completed unsuccessfully in 0:00:00
```

However that seems like it could be fixed in a follow-up.

Closes rust-lang#76619
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 21, 2020
@Dylan-DPC
Rollup merge of rust-lang#76628 - jyn514:default-config-files, r=Mark…
f820b58 
…-Simulacrum

Add sample defaults for config.toml

- Allow including defaults in `src/bootstrap/defaults` using `profile = "..."`.
- Add default config files, with a README noting they're experimental and asking you to open an issue if you run into trouble. The config files have comments explaining why the defaults are set.
- Combine config files using the `merge` dependency.

This introduces a new dependency on `merge` that hasn't yet been vetted.

I want to improve the output when `include = "x"` isn't found:

```
thread 'main' panicked at 'fs::read_to_string(&file) failed with No such file or directory (os error 2) ("configuration file did not exist")', src/bootstrap/config.rs:522:28
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failed to run: /home/joshua/rustc/build/bootstrap/debug/bootstrap test tidy
Build completed unsuccessfully in 0:00:00
```

However that seems like it could be fixed in a follow-up.

Closes rust-lang#76619
RalfJung added a commit to RalfJung/rust that referenced this issue Sep 21, 2020
@RalfJung
Rollup merge of rust-lang#76628 - jyn514:default-config-files, r=Mark…
eb0f3dd 
…-Simulacrum

Add sample defaults for config.toml

- Allow including defaults in `src/bootstrap/defaults` using `profile = "..."`.
- Add default config files, with a README noting they're experimental and asking you to open an issue if you run into trouble. The config files have comments explaining why the defaults are set.
- Combine config files using the `merge` dependency.

This introduces a new dependency on `merge` that hasn't yet been vetted.

I want to improve the output when `include = "x"` isn't found:

```
thread 'main' panicked at 'fs::read_to_string(&file) failed with No such file or directory (os error 2) ("configuration file did not exist")', src/bootstrap/config.rs:522:28
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failed to run: /home/joshua/rustc/build/bootstrap/debug/bootstrap test tidy
Build completed unsuccessfully in 0:00:00
```

However that seems like it could be fixed in a follow-up.

Closes rust-lang#76619
@bors bors closed this as completed in 8fa75a2 Sep 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jyn514 jyn514

Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself C-feature-request Category: A feature request, i.e: not implemented / a PR. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. 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.

Add sample defaults for config.toml jyn514/rust
2 participants
@est31 @jyn514