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.

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

Add x.py setup #76631

Merged
merged 1 commit into from
Sep 26, 2020
Merged

Add x.py setup #76631

merged 1 commit into from
Sep 26, 2020

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Sep 12, 2020

Closes #76503.

  • Suggest x.py setup if config.toml doesn't exist yet
  • Prompt for a profile if not given on the command line
  • Print the configuration that will be used
  • Print helpful starting commands after setup
  • Link to the dev-guide after finishing

@jyn514 jyn514 added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Sep 12, 2020
@jyn514
Copy link
Member Author

jyn514 commented Sep 12, 2020

Example output:

$ ./x.py check
Updating only changed submodules
Submodules updated in 0.01 seconds
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
warning: you have not made a `config.toml`
help: consider running `x.py setup` or copying `config.toml.example`
...
$ ./x.py setup
Updating only changed submodules
Submodules updated in 0.01 seconds
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
Welcome to the Rust project! What do you want to do with x.py?
a) Contribute to the standard library
b) Contribute to the compiler
c) Contribute to the compiler, and also modify LLVM or codegen
d) Install Rust from source
Please choose one (a/b/c/d): a
`x.py` will now use the configuration at /home/joshua/rustc2/src/bootstrap/defaults/config.toml.library
To get started, try one of the following commands:
- `x.py check`
- `x.py build`
- `x.py test library/std`
- `x.py doc`
For more suggestions, see https://rustc-dev-guide.rust-lang.org/building/suggested.html
Build completed successfully in 0:00:05
$ ./x.py setup
Welcome to the Rust project! What do you want to do with x.py?
a) Contribute to the standard library
b) Contribute to the compiler
c) Contribute to the compiler, and also modify LLVM or codegen
d) Install Rust from source
Please choose one (a/b/c/d): a
error: you asked `x.py` to setup a new config file, but one already exists at `config.toml`
help: try adding `profile = "library"` at the top of config.toml
note: this will use the configuration in /home/joshua/rustc2/src/bootstrap/defaults/config.toml.library
failed to run: /home/joshua/rustc2/build/bootstrap/debug/bootstrap setup
Build completed unsuccessfully in 0:00:01

@jyn514 jyn514 changed the title [WIP] Add x.py setup [Blocked] Add x.py setup Sep 12, 2020
@bors

This comment has been minimized.

@jyn514 jyn514 force-pushed the x.py-setup branch 3 times, most recently from f221466 to 95bab52 Compare September 18, 2020 12:43
@jyn514

This comment has been minimized.

@jyn514 jyn514 force-pushed the x.py-setup branch 2 times, most recently from c4be7f5 to 8e9c40b Compare September 20, 2020 21:47
@jyn514 jyn514 added the A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself label Sep 20, 2020
@bors

This comment has been minimized.

@jyn514 jyn514 changed the title [Blocked] Add x.py setup Add x.py setup Sep 21, 2020
@jyn514 jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Sep 21, 2020
@est31
Copy link
Member

est31 commented Sep 21, 2020

In my personal workflow, I cp config.toml.example to config.toml and then adjust the settings I want adjusted. That way their documentation is next to them and I always know what they do. I regularly re-do this step so that it doesn't bitrot too much as new options are being added/old ones removed. But maybe it is smarter to just have that single line instead plus 1 comment.

@jyn514
Copy link
Member Author

jyn514 commented Sep 21, 2020

I regularly re-do this step so that it doesn't bitrot too much as new options are being added/old ones removed.

This is the main reason for having the defaults in a separate file tracked by git - I don't want people to have to rerun this command as options are changed. I also want to avoid overwhelming people with too many options, I've had a lot of people say config.toml.example is so big it's hard to find anything. The defaults in src/bootstrap/defaults only have a few options, and each of them are commented.

@jyn514
Copy link
Member Author

jyn514 commented Sep 21, 2020

Before merging this I'd like to also print the help message mentioned at the end of #76503:

Done.

@jyn514 jyn514 force-pushed the x.py-setup branch 2 times, most recently from 037aa4b to 0557dca Compare September 21, 2020 14:29
@jyn514
Copy link
Member Author

jyn514 commented Sep 21, 2020

r? @Mark-Simulacrum

@jyn514 jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 21, 2020
@jyn514
Copy link
Member Author

jyn514 commented Sep 21, 2020

cc @Lokathor and @thomcc - is there anything else you'd like this to do? Is the output too verbose?

@jyn514
Copy link
Member Author

jyn514 commented Sep 21, 2020

Ooh, this could also link to https://rustc-dev-guide.rust-lang.org/building/suggested.html once it finishes.

@jyn514
Copy link
Member Author

jyn514 commented Sep 24, 2020

@Mark-Simulacrum do you have suggestions for setting profile = "user" by default in configure.py? I tried this, but it's not working:

diff --git a/src/bootstrap/configure.py b/src/bootstrap/configure.py
index 47673ce1e87..ac213b55873 100755
--- a/src/bootstrap/configure.py
+++ b/src/bootstrap/configure.py
@@ -34,6 +34,8 @@ def v(*args):
 o("debug", "rust.debug", "enables debugging environment; does not affect optimization of bootstrapped code (use `--disable-optimize` for that)")
 o("docs", "build.docs", "build standard library documentation")
 o("compiler-docs", "build.compiler-docs", "build compiler documentation")
+v("profile", "profile", "the latest version of the x.py changelog viewed")
 o("optimize-tests", "rust.optimize-tests", "build tests with optimizations")
 o("parallel-compiler", "rust.parallel-compiler", "build a multi-threaded rustc")
 o("verbose-tests", "rust.verbose-tests", "enable verbose output when running tests")
@@ -279,6 +281,7 @@ def set(key, value):
     arr = config
     parts = key.split('.')
     for i, part in enumerate(parts):
+        print(i, part)
         if i == len(parts) - 1:
             arr[part] = value
         else:
@@ -362,6 +365,7 @@ set('build.configure-args', sys.argv[1:])
 sections = {}
 cur_section = None
 sections[None] = []
+sections[0] = ["profile"]
 section_order = [None]
 targets = {}
 
@@ -392,6 +396,8 @@ for target in configured_targets:
     targets[target] = sections['target'][:]
     targets[target][0] = targets[target][0].replace("x86_64-unknown-linux-gnu", target)
 
+# Set the default profile to 'user', since contributors won't be using `configure`
+set("profile", "user")
 
 def is_number(value):
     try:
$ ./configure 
configure: processing command line
configure: 
configure: build.configure-args := []
0 build
1 configure-args
configure: profile              := user
0 profile
Traceback (most recent call last):
  File "./src/bootstrap/configure.py", line 449, in <module>
    raise RuntimeError("config key {} not in sections".format(section_key))
RuntimeError: config key profile not in sections

@jyn514
Copy link
Member Author

jyn514 commented Sep 24, 2020

The other option is to leave that for later so we can land this quickly and then improve the defaults for distro maintainers in a follow-up.

@Mark-Simulacrum
Copy link
Member

Yeah, I'm not sure. I would leave it to a future PR.

- Suggest `x.py setup` if config.toml doesn't exist yet (twice, once
before and once after the build)
- Prompt for a profile if not given on the command line
- Print the configuration file that will be used
- Print helpful starting commands after setup
- Link to the dev-guide after finishing
- Note that distro maintainers will see the changelog warning
@jyn514
Copy link
Member Author

jyn514 commented Sep 24, 2020

Ok, in that case I think this is ready to merge :)

@jyn514
Copy link
Member Author

jyn514 commented Sep 24, 2020

Because github isn't showing it, the last change I made was

commit 7304b64d28415841c6d1b07501194471109f917e
Author: Joshua Nelson <[email protected]>
Date:   Thu Sep 24 10:31:54 2020 -0400

    Note that distro maintainers will see the changelog warning
    
    My previous TODO was incorrect

diff --git a/src/bootstrap/bin/main.rs b/src/bootstrap/bin/main.rs
index 8290420ec06..669dd7a33de 100644
--- a/src/bootstrap/bin/main.rs
+++ b/src/bootstrap/bin/main.rs
@@ -15,7 +15,8 @@ fn main() {
 
     let changelog_suggestion = check_version(&config);
 
-    // TODO: `./configure` should show a warning about the changelog, not `x.py setup`
+    // NOTE: Since `./configure` generates a `config.toml`, distro maintainers will see the
+    // changelog warning, not the `x.py setup` message.
     let suggest_setup = !config.config.exists() && !matches!(config.cmd, Subcommand::Setup { .. });
     if suggest_setup {
         println!("warning: you have not made a `config.toml`");

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Sep 24, 2020

📌 Commit 9baa601 has been approved by Mark-Simulacrum

@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 Sep 24, 2020
RalfJung added a commit to RalfJung/rust that referenced this pull request Sep 25, 2020
Add `x.py setup`

Closes rust-lang#76503.

- Suggest `x.py setup` if config.toml doesn't exist yet
- Prompt for a profile if not given on the command line
- Print the configuration that will be used
- Print helpful starting commands after setup
- Link to the dev-guide after finishing
RalfJung added a commit to RalfJung/rust that referenced this pull request Sep 25, 2020
Add `x.py setup`

Closes rust-lang#76503.

- Suggest `x.py setup` if config.toml doesn't exist yet
- Prompt for a profile if not given on the command line
- Print the configuration that will be used
- Print helpful starting commands after setup
- Link to the dev-guide after finishing
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 26, 2020
Rollup of 12 pull requests

Successful merges:

 - rust-lang#75454 (Explicitly document the size guarantees that Option makes.)
 - rust-lang#76631 (Add `x.py setup`)
 - rust-lang#77076 (Add missing code examples on slice iter types)
 - rust-lang#77093 (merge `need_type_info_err(_const)`)
 - rust-lang#77122 (Add `#![feature(const_fn_floating_point_arithmetic)]`)
 - rust-lang#77127 (Update mdBook)
 - rust-lang#77161 (Remove TrustedLen requirement from BuilderMethods::switch)
 - rust-lang#77166 (update Miri)
 - rust-lang#77181 (Add doc alias for pointer primitive)
 - rust-lang#77204 (Remove stray word from `ClosureKind::extends` docs)
 - rust-lang#77207 (Rename `whence` to `span`)
 - rust-lang#77211 (Remove unused #[allow(...)] statements from compiler/)

Failed merges:

 - rust-lang#77170 (Remove `#[rustc_allow_const_fn_ptr]` and add `#![feature(const_fn_fn_ptr_basics)]`)

r? `@ghost`
@bors bors merged commit c39598a into rust-lang:master Sep 26, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 26, 2020
@jyn514 jyn514 deleted the x.py-setup branch September 26, 2020 19:54
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 6, 2020
…n514

Fix suggestions for x.py setup

rust-lang#76631 introduced a new `setup` command to x.py

By default the command prompts for a profile to use:

```
Welcome to the Rust project! What do you want to do with x.py?
a) Contribute to the standard library
b) Contribute to the compiler
c) Contribute to the compiler, and also modify LLVM or codegen
d) Install Rust from source
```

and then displays command suggestions, depending on which profile was chosen. However [the mapping between chosen profile](https://github.com/rust-lang/rust/blob/9cba260df0f1c67ea3690035cd5611a7465a1560/src/bootstrap/setup.rs#L75-L85) and [suggestion](https://github.com/rust-lang/rust/blob/9cba260df0f1c67ea3690035cd5611a7465a1560/src/bootstrap/setup.rs#L42-L47) isn't exact, leading to suggestions not being shown if the user presses `c` or `d`. (because "c" is translated to "llvm" and "d" to "maintainer", but suggestions trigger for "codegen" and "user" respectively)

A more thorough refactor would stop using "strings-as-type" to make sure this kind of error doesn't happen, but it may be overkill for that kind of "script" program?

Tagging the setup command author: @jyn514
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself 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.

Add x.py setup or similar
5 participants