Skip to content

config.mk: Added variants of valopt/opt that do not automatically putvar #17890

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

Closed
wants to merge 1 commit into from

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Oct 9, 2014

Fixes config.mk so that it should not contain multiple inconsistent entries for the same option.

Used aforementioned variants to extract options that have explicit putvar calls associated with them in the subsequent code. When the explicit putvar call was conditional on some potentially complex condition, moved the putvar call out to the main control flow of the script so that it always runs if necessary.


As a driveby fix, captured the error exit when doing the test run of rustc --version from CFG_LOCAL_RUST_ROOT, and signal explicit configure failure when it did not run successfully. (If we cannot run rustc, we really shouldn't try to keep going.)


Fix #17887.

@pnkfelix
Copy link
Member Author

pnkfelix commented Oct 9, 2014

r? @alexcrichton

@pnkfelix pnkfelix force-pushed the fsk-fix-issue-17887 branch from 0672a22 to db4c2f7 Compare October 9, 2014 15:12
opt_nosave manage-submodules 1 "let the build manage the git submodules"
opt_nosave clang 0 "prefer clang to gcc for building the runtime"
opt_nosave inject-std-version 1 "inject the current compiler version of libstd into programs"
opt_nosave jemalloc 1 "build liballoc with jemalloc"
Copy link
Member

Choose a reason for hiding this comment

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

It's fine to make this just opt and remove the putvar below

@alexcrichton
Copy link
Member

Looks like some of the putvar can be removed to migrate foo_nosave to foo, other than that r=me

@alexcrichton
Copy link
Member

ping @pnkfelix, any updates here?

@pnkfelix pnkfelix force-pushed the fsk-fix-issue-17887 branch from db4c2f7 to b354c32 Compare October 24, 2014 17:22
@pnkfelix
Copy link
Member Author

@alexcrichton (thanks for the prod. ;) )

bors added a commit that referenced this pull request Oct 26, 2014
Fixes `config.mk` so that it should not contain multiple inconsistent entries for the same option.

Used aforementioned variants to extract options that have explicit `putvar` calls associated with them in the subsequent code.  When the explicit `putvar` call was conditional on some potentially complex condition, moved the `putvar` call out to the main control flow of the script so that it always runs if necessary.

----

As a driveby fix, captured the error exit when doing the test run of `rustc --version` from `CFG_LOCAL_RUST_ROOT`, and signal explicit configure failure when it did not run successfully.  (If we cannot run `rustc`, we really shouldn't try to keep going.)

----

Fix #17887.
@pnkfelix pnkfelix force-pushed the fsk-fix-issue-17887 branch from b354c32 to 733e2c6 Compare October 27, 2014 11:13
… `putvar`.

Used aforementioned variants to extract options that have explicit
`putvar` calls associated with them in the subsequent code.  When the
explicit `putvar` call was conditional on some potentially complex
condition, moved the `putvar` call out to the main control flow of the
script so that it always runs if necessary.

----

As a driveby fix, captured the error exit when doing the test run of
`rustc --version` from `CFG_LOCAL_RUST_ROOT`, and signal explicit
configure failure when it did not run successfully.  (If we cannot run
`rustc`, we really shouldn't try to keep going.)

----

Finally, in response to review feedback, went through and identified
cases where we had been calling `putvar` manually (and thus my naive
translation used `opt_nosave`/`valopt_nosave`), and then verified
whether a manual `putvar` was necessary (i.e., was each variable in
question manually computed somewhere in the `configure` script).
In cases that did not meet this criteria, I revised the code to use
the `opt`/`valopt` directly and removed the corresponding `putvar`,
cleaning things up a teeny bit.

----

Fix rust-lang#17887.
@pnkfelix pnkfelix force-pushed the fsk-fix-issue-17887 branch from 733e2c6 to 6f1e627 Compare October 27, 2014 11:17
bors added a commit that referenced this pull request Oct 27, 2014
Fixes `config.mk` so that it should not contain multiple inconsistent entries for the same option.

Used aforementioned variants to extract options that have explicit `putvar` calls associated with them in the subsequent code.  When the explicit `putvar` call was conditional on some potentially complex condition, moved the `putvar` call out to the main control flow of the script so that it always runs if necessary.

----

As a driveby fix, captured the error exit when doing the test run of `rustc --version` from `CFG_LOCAL_RUST_ROOT`, and signal explicit configure failure when it did not run successfully.  (If we cannot run `rustc`, we really shouldn't try to keep going.)

----

Fix #17887.
@bors bors closed this Oct 27, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

configure sometimes saves same key multiple times in config.mk with differing values
3 participants