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

rustbuild: default values from config.mk now effectively override config.toml #43295

Closed
infinity0 opened this issue Jul 17, 2017 · 4 comments
Closed

rustbuild: default values from config.mk now effectively override config.toml #43295

infinity0 opened this issue Jul 17, 2017 · 4 comments
Labels
C-bug Category: This is a bug. P-medium Medium priority T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@infinity0
Copy link
Contributor

#42543 was fixed by unconditionally reading config.mk. However, sometimes this is populated by default values that the user did not give on the command line, for example

CFG_RELEASE_CHANNEL  := dev

when running from a location with a .git subdirectory (such as the Debian rustc git repo). This has the effect of rendering the corresponding command-line flags to x.py useless. For example even if I have

[rust]
channel = "stable"

and run

$ ./x.py build --config myconfig.toml

this will get ignored and the value from config.mk (which I did not set) takes precedence.

I can work around this by explicitly passing --release-channel to ./configure but this behaviour does not help rust's overall goal of eventually phasing out this file.

Other flags affected include possibly CFG_PREFIX, but probably not CFG_ENABLE_DEBUGINFO_* as bootstrap/config.rs does not appear to read them.

cc @Mark-Simulacrum
@Mark-Simulacrum Mark-Simulacrum added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jul 17, 2017
@Mark-Simulacrum
Copy link
Member

I'm going to mark as P-medium, cc @alexcrichton and @aidanhs. I think this should hopefully resolve to merging the defaults in before we parse command line params. Not sure if I'll have time to investigate in the short term, though, but certainly within a few days I hope.

@Mark-Simulacrum Mark-Simulacrum added the P-medium Medium priority label Jul 17, 2017
@retep998
Copy link
Member

Would this be fixed by simply not using config.mk at all and having configure create config.toml instead? #40730

@infinity0 infinity0 mentioned this issue Jul 17, 2017
Merged
@infinity0
Copy link
Contributor Author

That would probably work, yes.

@infinity0
Copy link
Contributor Author

As an amendment, this bug does affect CFG_ENABLE_DEBUGINFO_* etc, I was grepping for the full name and didn't notice this part.

TimNN added a commit to TimNN/rust that referenced this issue Jul 21, 2017
@TimNN
Rollup merge of rust-lang#43297 - infinity0:master, r=alexcrichton
8b35534 
configure: allow distros to disable debuginfo-only-std

This allows builders to generate debugging information for everything, even in a stable release build. This is useful for distros like Fedora (already carrying a [similar patch](https://src.fedoraproject.org/cgit/rpms/rust.git/tree/rust-1.16.0-configure-no-override.patch)) and Debian that automatically put all debuginfo in separate "debug symbol" packages.

This commit preserves the default behaviour of switching these on when a non-dev channel is selected, but allows the user to override this via the `./configure` command line.

In theory, one could also do this via `bootstrap/config.toml` but it doesn't work currently due to rust-lang#43295.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Jul 24, 2017
@Mark-Simulacrum
Rollup merge of rust-lang#43297 - infinity0:master, r=alexcrichton
953f381 
configure: allow distros to disable debuginfo-only-std

This allows builders to generate debugging information for everything, even in a stable release build. This is useful for distros like Fedora (already carrying a [similar patch](https://src.fedoraproject.org/cgit/rpms/rust.git/tree/rust-1.16.0-configure-no-override.patch)) and Debian that automatically put all debuginfo in separate "debug symbol" packages.

This commit preserves the default behaviour of switching these on when a non-dev channel is selected, but allows the user to override this via the `./configure` command line.

In theory, one could also do this via `bootstrap/config.toml` but it doesn't work currently due to rust-lang#43295.
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 28, 2017
@Mark-Simulacrum Mark-Simulacrum mentioned this issue Aug 26, 2017
Merged
bors added a commit that referenced this issue Aug 28, 2017
@bors
Auto merge of #44107 - alexcrichton:no-shell-configure, r=Mark-Simula…
469d147 
…crum

rustbuild: Rewrite the configure script in Python

This commit rewrites our ancient `./configure` script from shell into Python.
The impetus for this change is to remove `config.mk` which is just a vestige of
the old makefile build system at this point. Instead all configuration is now
solely done through `config.toml`.

The python script allows us to more flexibly program (aka we can use loops
easily) and create a `config.toml` which is based off `config.toml.example`.
This way we can preserve comments and munge various values as we see fit.

It is intended that the configure script here is a drop-in replacement for the
previous configure script, no functional change is intended. Also note that the
rationale for this is also because our build system requires Python, so having a
python script a bit earlier shouldn't cause too many problems.

Closes #40730
Closes #43295
Closes #42255
Closes #38058
Closes #32176
bors added a commit that referenced this issue Aug 28, 2017
@bors
Auto merge of #44107 - alexcrichton:no-shell-configure, r=Mark-Simula…
a0c3bd2 
…crum

rustbuild: Rewrite the configure script in Python

This commit rewrites our ancient `./configure` script from shell into Python.
The impetus for this change is to remove `config.mk` which is just a vestige of
the old makefile build system at this point. Instead all configuration is now
solely done through `config.toml`.

The python script allows us to more flexibly program (aka we can use loops
easily) and create a `config.toml` which is based off `config.toml.example`.
This way we can preserve comments and munge various values as we see fit.

It is intended that the configure script here is a drop-in replacement for the
previous configure script, no functional change is intended. Also note that the
rationale for this is also because our build system requires Python, so having a
python script a bit earlier shouldn't cause too many problems.

Closes #40730
Closes #43295
Closes #42255
Closes #38058
Closes #32176
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Category: This is a bug. P-medium Medium priority T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@infinity0 @retep998 @Mark-Simulacrum