RFR: 8273092: Sort classlist in JDK image
When the classlist is generated using build.tools.classlist.HelloClasslist, its contents may be non-deterministic due to Java thread execution order. We should sort the generated classlist to make the JDK image's contents more deterministic. Tested with Mach5 tier1, tier2, builds-tier5 - Commit messages: - 8273092: Sort classlist in JDK image Changes: https://git.openjdk.java.net/jdk/pull/5288/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5288&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273092 Stats: 114 lines in 3 files changed: 102 ins; 6 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/5288.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5288/head:pull/5288 PR: https://git.openjdk.java.net/jdk/pull/5288
Re: RFR: 8229031: Exporting CLASSPATH from shell can result in build failures
On Fri, 27 Aug 2021 14:28:07 GMT, Magnus Ihse Bursie wrote: > Having the environment variable CLASSPATH set when building can cause > hard-to-diagnose build errors. We should not silently accept the CLASSPATH > variable when building. Normally, the classpath should be correctly setup for > all invocations of java during the build. If the user wants to have a > hard-coded value for CLASSPATH, they must set it explicitly in `configure`. Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5280
Re: RFR: 8271148: static-libs-image target --with-native-debug-symbols=external doesn't produce debug info
On Thu, 22 Jul 2021 16:43:26 GMT, Severin Gehwolf wrote: > Hi! > > Please review this tiny patch which removes the special casing of > `--with-native-debug-symbols=external` for the static libs build. I don't see > why this is needed. If no debug symbols are wanted > `--with-native-debug-symbols=none` can be used to achieve the same effect. > Therefore, I propose to remove this hunk. > > Testing: Inspecting of the log files and seeing that `-g` switch is there. > Run the reproducer test on the resulting static libraries. > > Thoughts? We currently have: `--with-native-debug-symbols=internal`: Dynamic libs: debug symbols internal. Static libs: debug symbols internal `--with-native-debug-symbols=external`: Dynamic libs: debug symbols external. Static libs: No debug symbols With the suggested change: `--with-native-debug-symbols=external`: Dynamic libs: debug symbols external. Static libs: debug symbols **internal** I don't think that's a good idea. There are two important points when picking _external_. 1: that you get debug symbols, but at least equally important, 2: that debug symbols are not left around in the product bits (which are distributed). Yes, we at Oracle are indeed relying on release product bits being completely stripped and void of any debug symbols. I agree that it's clunky. We just never had a need for saving debug symbols for the static libs, so no effort was put into implementing that support. I would be ok with the patch if it instead changed to: `--with-native-debug-symbols=external`: Dynamic libs: debug symbols external. Static libs: debug symbols **external** Right now we don't have the necessary build logic implemented in NativeCompilation.gmk for moving debugsymbols to external files for static libs, so such a change would need to provide that implementation. - PR: https://git.openjdk.java.net/jdk/pull/4876
RFR: 8229031: Exporting CLASSPATH from shell can result in build failures
Having the environment variable CLASSPATH set when building can cause hard-to-diagnose build errors. We should not silently accept the CLASSPATH variable when building. Normally, the classpath should be correctly setup for all invocations of java during the build. If the user wants to have a hard-coded value for CLASSPATH, they must set it explicitly in `configure`. - Commit messages: - 8229031: Exporting CLASSPATH from shell can result in build failures Changes: https://git.openjdk.java.net/jdk/pull/5280/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5280&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8229031 Stats: 27 lines in 2 files changed: 27 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/5280.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5280/head:pull/5280 PR: https://git.openjdk.java.net/jdk/pull/5280
Re: RFR: 8273072: Avoid using += in configure [v2]
On Fri, 27 Aug 2021 13:15:36 GMT, Jie Fu wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fix code review comments > > make/devkit/createMacosxDevkit.sh line 105: > >> 103: >> 104: for ex in $EXCLUDE_DIRS; do >> 105: EXCLUDE_ARGS="$EXCLUDE_ARGS --exclude=$ex " > > Thanks for your update. > This copyright should be updated too. Oh, right... I really need to write a tool that checks that before pushing... Also, I found out why this actually ever worked. There were a trailing space hidden in the assignment... - PR: https://git.openjdk.java.net/jdk/pull/5276
Integrated: 8273072: Avoid using += in configure
On Fri, 27 Aug 2021 10:21:26 GMT, Magnus Ihse Bursie wrote: > 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"`. This pull request has now been integrated. Changeset: a033aa5a Author:Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/a033aa5a3d9c63d72d11af218b9896b037fbd8de Stats: 8 lines in 4 files changed: 1 ins; 0 del; 7 mod 8273072: Avoid using += in configure Reviewed-by: dholmes, jiefu - PR: https://git.openjdk.java.net/jdk/pull/5276
Re: RFR: 8273072: Avoid using += in configure [v2]
On Fri, 27 Aug 2021 13:22:31 GMT, David Holmes wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fix code review comments > > 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 @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 `FOO+=BAR` with no spaces would add a space (I still believe so, since make is so word-oriented), but we always use `FOO += BAR`. (Which is not possible in shells since that would not count as a variable assignment but a command execution...) - PR: https://git.openjdk.java.net/jdk/pull/5276
Re: RFR: 8273072: Avoid using += in configure [v3]
> 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"`. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Fix copyright year and remove space - Changes: - all: https://git.openjdk.java.net/jdk/pull/5276/files - new: https://git.openjdk.java.net/jdk/pull/5276/files/3083307d..5dbc5ed1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5276&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5276&range=01-02 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5276.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5276/head:pull/5276 PR: https://git.openjdk.java.net/jdk/pull/5276
Re: RFR: 8273072: Avoid using += in configure [v3]
On Fri, 27 Aug 2021 13:53:56 GMT, Magnus Ihse Bursie wrote: >> 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"`. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Fix copyright year and remove space Marked as reviewed by jiefu (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5276
Re: RFR: 8273072: Avoid using += in configure [v2]
On Fri, 27 Aug 2021 13:14:52 GMT, Magnus Ihse Bursie wrote: >> 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"`. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Fix code review comments 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 - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5276
Re: RFR: 8273072: Avoid using += in configure [v2]
On Fri, 27 Aug 2021 13:14:52 GMT, Magnus Ihse Bursie wrote: >> 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"`. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Fix code review comments make/devkit/createMacosxDevkit.sh line 105: > 103: > 104: for ex in $EXCLUDE_DIRS; do > 105: EXCLUDE_ARGS="$EXCLUDE_ARGS --exclude=$ex " Thanks for your update. This copyright should be updated too. - PR: https://git.openjdk.java.net/jdk/pull/5276
Integrated: 8258465: Headless build fails due to missing X11 headers on linux
On Fri, 27 Aug 2021 12:46:14 GMT, Magnus Ihse Bursie wrote: > It turned out that JDK-8255785 was incorrectly verified, and headless do > indeed need X headers. This fix is a revert (anti-delta) of JDK-8255785. This pull request has now been integrated. Changeset: 596b0755 Author:Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/596b075591c4b2fe01bee7142f4d0a5f892647ed Stats: 5 lines in 1 file changed: 1 ins; 3 del; 1 mod 8258465: Headless build fails due to missing X11 headers on linux Reviewed-by: shade - PR: https://git.openjdk.java.net/jdk/pull/5278
Re: RFR: 8273072: Avoid using += in configure [v2]
> 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"`. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Fix code review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/5276/files - new: https://git.openjdk.java.net/jdk/pull/5276/files/a8e0c35a..3083307d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5276&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5276&range=00-01 Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5276.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5276/head:pull/5276 PR: https://git.openjdk.java.net/jdk/pull/5276
Re: RFR: 8273072: Avoid using += in configure
On Fri, 27 Aug 2021 10:21:26 GMT, Magnus Ihse Bursie wrote: > 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"`. Thank you for spotting this! The exclude argument in the devkit builder looks like it has never worked. :( - PR: https://git.openjdk.java.net/jdk/pull/5276
Re: RFR: 8258465: Headless build fails due to missing X11 headers on linux
On Fri, 27 Aug 2021 12:46:14 GMT, Magnus Ihse Bursie wrote: > It turned out that JDK-8255785 was incorrectly verified, and headless do > indeed need X headers. This fix is a revert (anti-delta) of JDK-8255785. Looks good! Looks like a trivial reversal. - Marked as reviewed by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5278
Re: RFR: 8273072: Avoid using += in configure
On Fri, 27 Aug 2021 10:21:26 GMT, Magnus Ihse Bursie wrote: > 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"`. Please update the copyright year in `jdk-version.m4`. Shall we also fix this one? ./devkit/createMacosxDevkit.sh:EXCLUDE_ARGS+="--exclude=$ex " - PR: https://git.openjdk.java.net/jdk/pull/5276
RFR: 8258465: Headless build fails due to missing X11 headers on linux
It turned out that JDK-8255785 was incorrectly verified, and headless do indeed need X headers. This fix is a revert (anti-delta) of JDK-8255785. - Commit messages: - 8258465: Headless build fails due to missing X11 headers on linux Changes: https://git.openjdk.java.net/jdk/pull/5278/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5278&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8258465 Stats: 5 lines in 1 file changed: 1 ins; 3 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5278.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5278/head:pull/5278 PR: https://git.openjdk.java.net/jdk/pull/5278
RFR: 8273072: Avoid using += in configure
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"`. - Commit messages: - 8273072: Avoid using += in configure Changes: https://git.openjdk.java.net/jdk/pull/5276/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5276&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8273072 Stats: 5 lines in 3 files changed: 1 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/5276.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5276/head:pull/5276 PR: https://git.openjdk.java.net/jdk/pull/5276
