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
8273072: Avoid using += in configure #5276
8273072: Avoid using += in configure #5276
Conversation
|
Webrevs
|
Please update the copyright year in Shall we also fix this one?
|
Thank you for spotting this! The exclude argument in the devkit builder looks like it has never worked. :( |
I wouldn't have considered the m4 files as "shell scripts" and I'm surprised that the same operator in makefiles adds extra spaces - that seems unintuitive. But using the expanded format is fine.
Thanks,
David
@magicus This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 12 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.
|
@dholmes-ora The configure script is mostly shell code, with some m4 abstractions in it. (Autoconf calls this mongrel language for "m4sh". I'd rate it a 1 out of 5. Recommendation: stay away. ;-)) Makefiles, otoh, is a proper language, albeit badly designed. I'm actually a bit unsure if a construct like |
/integrate |
Going to push as commit a033aa5.
Your commit was automatically rebased without conflicts. |
JDK-8272700 was created to fix a bug in a variable assignment in configure. While it fixed the bug, it kept the problematic syntax that caused the bug in the first place.
We do not use the
FOO+="appended"
syntax for appending to variables in shell scripts, since this differs from what you'd expect (and what make produces) in that no space is added before the appended text.Instead, we use the longer, but clearer
FOO="$FOO appended"
.Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/5276/head:pull/5276
$ git checkout pull/5276
Update a local copy of the PR:
$ git checkout pull/5276
$ git pull https://git.openjdk.java.net/jdk pull/5276/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 5276
View PR using the GUI difftool:
$ git pr show -t 5276
Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/5276.diff