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

bootstrap: read config from $RUST_BOOTSTRAP_CONFIG #73317

Merged
merged 1 commit into from
Jun 19, 2020

Conversation

davidtwco
Copy link
Member

@davidtwco davidtwco commented Jun 13, 2020

This PR modifies bootstrap so that config.toml is read first from RUST_BOOTSTRAP_CONFIG, then --config and finally config.toml in the current directory.

This is a subjective change, intended to improve the ergnomics when using "development shells" for rustc development (for example, using tools such as Nix) which set environment variables to ensure a reproducible environment (these development shells can then be version controlled, e.g. my rustc shell). By optionally reading config.toml from an environment variable, a config.toml can be defined in the development shell and a path to it exposed in the RUST_BOOTSTRAP_CONFIG environment variable - avoiding the need to manually symlink the contents of this file to config.toml in the working directory.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 13, 2020
@Mark-Simulacrum
Copy link
Member

This should adjust x.py (well, src/bootstrap/bootstrap.py) as well and likely entirely move the detection logic to there, exclusively relying on the environment variable in the Rust code.

I would also like to see the environment variable renamed to something like RUST_BOOTSTRAP_CONFIG since this isn't really rustc configuration.

Other than those two, this seems fine to add to me.

@davidtwco davidtwco changed the title bootstrap: read config from $RUSTC_CONFIG bootstrap: read config from $RUST_BOOTSTRAP_CONFIG Jun 14, 2020
@davidtwco
Copy link
Member Author

This should adjust x.py (well, src/bootstrap/bootstrap.py) as well and likely entirely move the detection logic to there, exclusively relying on the environment variable in the Rust code.

I would also like to see the environment variable renamed to something like RUST_BOOTSTRAP_CONFIG since this isn't really rustc configuration.

Other than those two, this seems fine to add to me.

These should both be fixed now.

Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me with nit fixed

src/bootstrap/flags.rs Outdated Show resolved Hide resolved
This commit modifies bootstrap so that `config.toml` is read first from
`RUST_BOOTSTRAP_CONFIG`, then `--config` and finally `config.toml` in the
current directory.

This is a subjective change, intended to improve the ergnomics when
using "development shells" for rustc development (for example, using tools
such as Nix) which set environment variables to ensure a reproducible
environment (these development shells can then be version controlled). By
optionally reading `config.toml` from an environment variable, a `config.toml`
can be defined in the development shell and a path to it exposed in the
`RUST_BOOTSTRAP_CONFIG` environment variable - avoiding the need to manually
symlink the contents of this file to `config.toml` in the working
directory.

Signed-off-by: David Wood <david@davidtw.co>
@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 16, 2020

📌 Commit 93022be 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 Jun 16, 2020
@RalfJung
Copy link
Member

@bors rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 19, 2020
Rollup of 10 pull requests

Successful merges:

 - rust-lang#72280 (Fix up autoderef when reborrowing)
 - rust-lang#72785 (linker: MSVC supports linking static libraries as a whole archive)
 - rust-lang#73011 (first stage of implementing LLVM code coverage)
 - rust-lang#73044 (compiletest: Add directives to detect sanitizer support)
 - rust-lang#73054 (memory access sanity checks: abort instead of panic)
 - rust-lang#73136 (Change how compiler-builtins gets many CGUs)
 - rust-lang#73280 (Add E0763)
 - rust-lang#73317 (bootstrap: read config from $RUST_BOOTSTRAP_CONFIG)
 - rust-lang#73350 (bootstrap/install.rs: support a nonexistent `prefix` in `x.py install`)
 - rust-lang#73352 (Speed up bootstrap a little.)

Failed merges:

r? @ghost
@bors bors merged commit 0404788 into rust-lang:master Jun 19, 2020
@davidtwco davidtwco deleted the bootstrap-config-env-var branch June 20, 2020 11:09
davidtwco added a commit to davidtwco/rust that referenced this pull request Jun 22, 2020
This commit fixes a regression introduced in rust-lang#73317 where an oversight
meant that `config.toml` was assumed to exist.

Signed-off-by: David Wood <david@davidtw.co>
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 22, 2020
…var, r=Mark-Simulacrum

bootstrap: no `config.toml` exists regression

Fixes rust-lang#73574.

This PR fixes a regression introduced in rust-lang#73317 where an oversight meant that `config.toml` was assumed to exist.
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 23, 2020
…var, r=Mark-Simulacrum

bootstrap: no `config.toml` exists regression

Fixes rust-lang#73574.

This PR fixes a regression introduced in rust-lang#73317 where an oversight meant that `config.toml` was assumed to exist.
davidtwco added a commit to davidtwco/veritas that referenced this pull request Jun 23, 2020
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants