Skip to content

configure sometimes saves same key multiple times in config.mk with differing values #17887

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
pnkfelix opened this issue Oct 9, 2014 · 1 comment

Comments

@pnkfelix
Copy link
Member

pnkfelix commented Oct 9, 2014

I don't mind our configure script, but it definitely has occasional warts.

For example, if I do: % ../configure --enable-local-rust --local-rust-root=/nonexistant
then I get a config.mk with the following:

CFG_ENABLE_LOCAL_RUST := 1
CFG_PREFIX           := /usr/local
CFG_LOCAL_RUST_ROOT  := /nonexistant
CFG_LLVM_ROOT        := 
CFG_JEMALLOC_ROOT    := 
[...]
CFG_LLDB_PYTHON_DIR  := /Applications/Xcode.app/Contents/SharedFrameworks/LLDB.framework/Versions/A/Resources/Python
CFG_ADB              := /Users/fklock/bin/adb
CFG_LOCAL_RUST_ROOT  := /Users/fklock
CFG_ENABLE_CLANG     := 1
CFG_USING_CLANG      := 1
....

Note that CFG_LOCAL_RUST_ROOT occurs multiple times; I believe the semantics are that the make run will pick up the last assigned value, but a reasonable human skimming the file is probably going to think that the first assignment is the only one in the file.

@pnkfelix
Copy link
Member Author

pnkfelix commented Oct 9, 2014

(The bug here is that we do a putvar when we first extract the local-rust-root from the command line options, and then we do a second putvar when we check its validity and update it with a different value (it found one in my $PATH, in a script I keep in /Users/fklock/bin/rustc, after finding the first was invalid.)

pnkfelix added a commit to pnkfelix/rust that referenced this issue Oct 24, 2014
… `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.
bors added a commit that referenced this issue 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.
bors added a commit that referenced this issue 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.
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 a pull request may close this issue.

1 participant