Re: RFR: 8326891: Prefer RPATH over RUNPATH for $ORIGIN rpaths in internal JDK binaries
On Wed, 28 Feb 2024 19:29:13 GMT, Erik Joelsson wrote: > There is no supported usecase that I can think of for injecting other > versions of such libraries in a JDK distribution. I can imagine it could be used to allow "hot patching" of the installed JDK/JRE. Whether anyone has ever needed to do so is another matter. I suggest at least adding a Release Note. - PR Comment: https://git.openjdk.org/jdk/pull/18050#issuecomment-1970497315
Re: RFR: 8326831 Clarify test harness control variables in make help
On Tue, 27 Feb 2024 13:55:53 GMT, Ludvig Janiuk wrote: > Clarifying text in `make help` output on how to list variables for e.g. > JTREG='...'. > I have no idea what causes it. Nevermind. The patch looks good. - Marked as reviewed by gli (Committer). PR Review: https://git.openjdk.org/jdk/pull/18028#pullrequestreview-1907859906
Integrated: 8326953: Race in creation of win-exports.def with static-libs
On Wed, 28 Feb 2024 12:26:54 GMT, Magnus Ihse Bursie wrote: > We have seen a build failure along the lines of: > > /usr/bin/mv: cannot move > '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def.tmp' > to > '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def': > No such file or directory > > on Windows. > > My guess is that this is a race between creating the win-export.def file for > the gtest jvm and the product jvm. This pull request has now been integrated. Changeset: 5fa2bdc6 Author:Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/5fa2bdc6c76d8f70c8d8582889e96b9c0d2b86b5 Stats: 10 lines in 1 file changed: 5 ins; 2 del; 3 mod 8326953: Race in creation of win-exports.def with static-libs Reviewed-by: jwaters, erikj, dholmes - PR: https://git.openjdk.org/jdk/pull/18043
Re: RFR: 8326953: Race in creation of win-exports.def with static-libs [v3]
On Wed, 28 Feb 2024 17:06:17 GMT, Magnus Ihse Bursie wrote: >> We have seen a build failure along the lines of: >> >> /usr/bin/mv: cannot move >> '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def.tmp' >> to >> '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def': >> No such file or directory >> >> on Windows. >> >> My guess is that this is a race between creating the win-export.def file for >> the gtest jvm and the product jvm. > > Magnus Ihse Bursie has updated the pull request incrementally with two > additional commits since the last revision: > > - Also restore newline > - Roll back gtest changes Seems reasonable. Thanks for jumping on this so quick! - Marked as reviewed by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18043#pullrequestreview-1907474544
RFR: 8326891: Prefer RPATH over RUNPATH for $ORIGIN rpaths in internal JDK binaries
Executables and dynamic libraries on Linux can encode a search path that the dynamic linker will use when looking up library dependencies, generally referred to as an "rpath". In the JDK we use this with the $ORIGIN feature to set search paths relative to the location of the binary itself. Typically executables in the bin/ directory have the rpath "$ORIGIN/../lib" to find libjli.so. Most of the libraries in lib/ have rpath set to just "$ORIGIN" to find each other. There are two different types of such rpaths, RPATH and RUNPATH. The former is the earlier incantation but RUNPATH has been around since about 2003 and has been default in prominent Linux distros for a long time, and now also seems to be default in the linker directly from binutils. The toolchain used by Oracle defaulted to RPATH until at least JDK 11, but since then with some toolchain upgrade, the default was flipped to RUNPATH. The main (relevant in this case) difference between the two is that RPATH is considered before the LD_LIBRARY_PATH environment variable, while RUNPATH is only considered after LD_LIBRARY_PATH. For libraries that are part of a Linux distribution, letting users, or the system, control and override builtin rpaths with LD_LIBRARY_PATH seems like a reasonable thing to prefer. However, for the JDK, there really is no usecase for having an externally configured LD_LIBRARY_PATH potentially getting in the way of the JDK libraries finding each other correctly. If a user environment sets LD_LIBRARY_PATH, and there is a library in that path with the same name as a JDK library (e.g. libnet.so or some other generically named library) that external library will be loaded instead of the JDK internal library and that is basically guaranteed to break the JDK. There is no supported usecase that I can think of for injecting other versions of such libraries in a JDK distribution. I propose that we explicitly configure the JDK build to set RPATH instead of RUNPATH for Linux binaries. This is done with the linker flag "--disable-new-dtags". - Commit messages: - JDK-8326891 Changes: https://git.openjdk.org/jdk/pull/18050/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18050&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8326891 Stats: 9 lines in 2 files changed: 3 ins; 0 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/18050.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18050/head:pull/18050 PR: https://git.openjdk.org/jdk/pull/18050
Re: RFR: 8325881: Require minimum gcc version 10
On Sat, 17 Feb 2024 08:28:56 GMT, Kim Barrett wrote: > Please review this change that updates the minimum supported version of gcc > to be used for building OpenJDK from 6.0 to 10.0. > > This permits enabling C++17 (JDK-8314488), though gcc 9.0 might suffice for > that. A minimum of gcc 10 also obtains the primitives needed to support a > work-alick for std::is_constant_evaluated (added in C++20). There are a bunch > of improvements that would be enabled by that. Having it would also allow the > elimination of a bit of a mess in the HotSpot assert macros that was needed to > work around the lack of that feature (JDK-8303805). Either current or proposed > minimum versions of other supported compilers also provide the needed > primitives. > > Testing: mach5 tier1 (uses gcc13.2 on gcc-based platforms) > Locally (linux-x64) built and ran tier1 with gcc10.3. No problem from our side, thanks! - Marked as reviewed by shade (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17899#pullrequestreview-1906924932
Re: RFR: 8326953: Race in creation of win-exports.def with static-libs [v3]
On Wed, 28 Feb 2024 17:06:17 GMT, Magnus Ihse Bursie wrote: >> We have seen a build failure along the lines of: >> >> /usr/bin/mv: cannot move >> '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def.tmp' >> to >> '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def': >> No such file or directory >> >> on Windows. >> >> My guess is that this is a race between creating the win-export.def file for >> the gtest jvm and the product jvm. > > Magnus Ihse Bursie has updated the pull request incrementally with two > additional commits since the last revision: > > - Also restore newline > - Roll back gtest changes Marked as reviewed by erikj (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18043#pullrequestreview-1906887941
Re: RFR: 8326953: Race in creation of win-exports.def with static-libs [v3]
On Wed, 28 Feb 2024 17:06:17 GMT, Magnus Ihse Bursie wrote: >> We have seen a build failure along the lines of: >> >> /usr/bin/mv: cannot move >> '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def.tmp' >> to >> '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def': >> No such file or directory >> >> on Windows. >> >> My guess is that this is a race between creating the win-export.def file for >> the gtest jvm and the product jvm. > > Magnus Ihse Bursie has updated the pull request incrementally with two > additional commits since the last revision: > > - Also restore newline > - Roll back gtest changes This seem to work fine now. - PR Comment: https://git.openjdk.org/jdk/pull/18043#issuecomment-1969516930
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v43]
On Wed, 28 Feb 2024 15:54:38 GMT, Hannes Wallnöfer wrote: >> Jonathan Gibbons has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Refactor most of TestMarkdown.java into separate tests, grouped by >> functionality > > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java > line 1601: > >> 1599: : eKind != ElementKind.OTHER ? 1 // module, >> package, class, interface >> 1600: : 0; // doc file >> 1601: return "h" + Math.min(heading.getLevel() + offset, 6); > > I really like that we adapt the heading level to the current context, but I > notice that the code kind of expects `h1` headings to be used everywhere, and > "punishes" use of contextually correct headings by generating smaller > headings. Maybe it would be more educational to only adjust the level if it > needs adjusting? Setext headings only come in "level 1" and "level 2" flavors. And, at the time the renderer sees the AST, the difference between ATX and setext headings is erased. They're just "headings". - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1506307115
Re: RFR: 8326953: Possible race in creation of win-exports.def [v3]
> We have seen a build failure along the lines of: > > /usr/bin/mv: cannot move > '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def.tmp' > to > '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def': > No such file or directory > > on Windows. > > My guess is that this is a race between creating the win-export.def file for > the gtest jvm and the product jvm. Magnus Ihse Bursie has updated the pull request incrementally with two additional commits since the last revision: - Also restore newline - Roll back gtest changes - Changes: - all: https://git.openjdk.org/jdk/pull/18043/files - new: https://git.openjdk.org/jdk/pull/18043/files/f574d063..18e03322 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18043&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18043&range=01-02 Stats: 33 lines in 2 files changed: 1 ins; 18 del; 14 mod Patch: https://git.openjdk.org/jdk/pull/18043.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18043/head:pull/18043 PR: https://git.openjdk.org/jdk/pull/18043
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v43]
On Fri, 23 Feb 2024 22:27:43 GMT, Jonathan Gibbons wrote: >> Please review a patch to add support for Markdown syntax in documentation >> comments, as described in the associated JEP. >> >> Notable features: >> >> * support for `///` documentation comments in `JavaTokenizer` >> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` >> library >> * updates to `DocCommentParser` to treat `///` comments as Markdown >> * updates to the standard doclet to render Markdown comments in HTML > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > Refactor most of TestMarkdown.java into separate tests, grouped by > functionality src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1601: > 1599: : eKind != ElementKind.OTHER ? 1 // module, > package, class, interface > 1600: : 0; // doc file > 1601: return "h" + Math.min(heading.getLevel() + offset, 6); I really like that we adapt the heading level to the current context, but I notice that the code kind of expects `h1` headings to be used everywhere, and "punishes" use of contextually correct headings by generating smaller headings. Maybe it would be more educational to only adjust the level if it needs adjusting? - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1506190847
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v43]
On Fri, 23 Feb 2024 22:27:43 GMT, Jonathan Gibbons wrote: >> Please review a patch to add support for Markdown syntax in documentation >> comments, as described in the associated JEP. >> >> Notable features: >> >> * support for `///` documentation comments in `JavaTokenizer` >> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` >> library >> * updates to `DocCommentParser` to treat `///` comments as Markdown >> * updates to the standard doclet to render Markdown comments in HTML > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > Refactor most of TestMarkdown.java into separate tests, grouped by > functionality src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1733: > 1731: return false; > 1732: } > 1733: The two methods below and some other methods in this class have too much indentation. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1506145051
Re: RFR: 8326964: Remove Eclipse Shared Workspaces [v2]
> WIP Julian Waters has updated the pull request incrementally with one additional commit since the last revision: Remove shared targets - Changes: - all: https://git.openjdk.org/jdk/pull/18046/files - new: https://git.openjdk.org/jdk/pull/18046/files/aefff3e8..0d29a355 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18046&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18046&range=00-01 Stats: 20 lines in 1 file changed: 0 ins; 20 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18046.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18046/head:pull/18046 PR: https://git.openjdk.org/jdk/pull/18046
RFR: 8326964: Remove Eclipse Shared Workspaces
WIP - Commit messages: - 8326964 Changes: https://git.openjdk.org/jdk/pull/18046/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18046&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8326964 Stats: 3 lines in 1 file changed: 0 ins; 3 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18046.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18046/head:pull/18046 PR: https://git.openjdk.org/jdk/pull/18046
Re: RFR: 8326953: Possible race in creation of win-exports.def [v2]
On Wed, 28 Feb 2024 14:53:13 GMT, Magnus Ihse Bursie wrote: >> We have seen a build failure along the lines of: >> >> /usr/bin/mv: cannot move >> '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def.tmp' >> to >> '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def': >> No such file or directory >> >> on Windows. >> >> My guess is that this is a race between creating the win-export.def file for >> the gtest jvm and the product jvm. > > Magnus Ihse Bursie has updated the pull request incrementally with four > additional commits since the last revision: > > - Use $(BUILD_LIBJVM_OBJECT_DIR) to work properly with static libs > - Better attempt with keeping the win-exports file for static libs > - Disable win-export.def on static libs > - Make it proper $(eval $(call ...)) The race was just between the dynamic and static build of libjvm. I don't think we need or should create a separate defs file for gtest, that's just unnecessary complexity. - PR Review: https://git.openjdk.org/jdk/pull/18043#pullrequestreview-1906513460
Re: RFR: 8326947: Minimize MakeBase.gmk
On Wed, 28 Feb 2024 11:24:06 GMT, Magnus Ihse Bursie wrote: > This is part of a general "spring cleaning" of the build system, addressing > old code that has bit-rotted, been subject to lava flow, or just had bad or > smelly code that we've never gotten around to fix. > > This particular patch tries to make MakeBase truly minimal; only including > the core parts of the build system that all makefiles will need. This is now > limited to essential functionality for named parameter functions, variable > dependency, tool execution, logging and fixpath functionality. MakeBase still > includes Utils.gmk and FileUtils.gmk, and thus "provides" this functionality > as well. Separating these out as well will be the subject of a future patch. The distinction was that BaseUtils.gmk are needed for MakeBase.gmk, Utils.gmk and FileUtils.gmk, but these three are not dependent on anything else. My initial goal was to stop having MakeBase.gmk include Utils.gmk as well, but it required a lot of extra includes and I did not want to do that in this PR. (Maybe it is not a good idea at all; I have not yet really decided). I agree that the split seems a bit arbitrary. Maybe it was a bad idea. Let me ponder it for a while. - PR Comment: https://git.openjdk.org/jdk/pull/18041#issuecomment-1969158791
Re: RFR: 8326953: Possible race in creation of win-exports.def [v2]
> We have seen a build failure along the lines of: > > /usr/bin/mv: cannot move > '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def.tmp' > to > '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def': > No such file or directory > > on Windows. > > My guess is that this is a race between creating the win-export.def file for > the gtest jvm and the product jvm. Magnus Ihse Bursie has updated the pull request incrementally with four additional commits since the last revision: - Use $(BUILD_LIBJVM_OBJECT_DIR) to work properly with static libs - Better attempt with keeping the win-exports file for static libs - Disable win-export.def on static libs - Make it proper $(eval $(call ...)) - Changes: - all: https://git.openjdk.org/jdk/pull/18043/files - new: https://git.openjdk.org/jdk/pull/18043/files/2fa57610..f574d063 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18043&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18043&range=00-01 Stats: 21 lines in 2 files changed: 7 ins; 1 del; 13 mod Patch: https://git.openjdk.org/jdk/pull/18043.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18043/head:pull/18043 PR: https://git.openjdk.org/jdk/pull/18043
Re: RFR: JDK-8298405: Support Markdown in Documentation Comments [v43]
On Fri, 23 Feb 2024 22:27:43 GMT, Jonathan Gibbons wrote: >> Please review a patch to add support for Markdown syntax in documentation >> comments, as described in the associated JEP. >> >> Notable features: >> >> * support for `///` documentation comments in `JavaTokenizer` >> * new module `jdk.internal.md` -- a private copy of the `commonmark-java` >> library >> * updates to `DocCommentParser` to treat `///` comments as Markdown >> * updates to the standard doclet to render Markdown comments in HTML > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > Refactor most of TestMarkdown.java into separate tests, grouped by > functionality src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1380: > 1378: > 1379: var useMarkdown = trees.stream().anyMatch(t -> t.getKind() == > Kind.MARKDOWN); > 1380: var markdownHandler = useMarkdown ? new > MarkdownHandler(element) : null; The `MarkdownHandler` and `HeadingNodeRenderer` classes must become aware of the current `TagletWriter.Context`. That's because headings and other block tags should only be generated if `TagletWriter.Context.isFirstSentence` is `false`. - PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1506084275
Re: RFR: 8326831 Clarify test harness control variables in make help
On Tue, 27 Feb 2024 13:55:53 GMT, Ludvig Janiuk wrote: > Clarifying text in `make help` output on how to list variables for e.g. > JTREG='...'. It's interesting that the order of those two lines can change. I have no idea what causes it. - PR Comment: https://git.openjdk.org/jdk/pull/18028#issuecomment-1969132107
Re: RFR: 8326953: Possible race in creation of win-exports.def
On Wed, 28 Feb 2024 12:26:54 GMT, Magnus Ihse Bursie wrote: > We have seen a build failure along the lines of: > > /usr/bin/mv: cannot move > '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def.tmp' > to > '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def': > No such file or directory > > on Windows. > > My guess is that this is a race between creating the win-export.def file for > the gtest jvm and the product jvm. This did not help. Either it is fully misdirected, or it is not enough. It seems reasonable that the gtest jvm should depend on the gtest ALL_OBJ; I think this was the case for the old mapfiles (even though the incestuous relationship between the proper jvm and the gtest jvm makes it hard to reason about them), so if that is a correct assumption, this would fix a potential regression. My next guess is that this is correlated to `hotspot-static-libs`. I have not seen the failure on GHA, but only on the Oracle CI, where we build with static libs. That seems more reasonable to produce a race; the static libs functionality appeared while I was away, but I think it just runs the entire CompileJvm.gmk once more with STATIC_LIBS set to true, with no coordination wrt the normal build, so it can very well end up executing the same rule twice. - PR Comment: https://git.openjdk.org/jdk/pull/18043#issuecomment-1969046252
Re: RFR: 8326953: Possible race in creation of win-exports.def
On Wed, 28 Feb 2024 13:12:14 GMT, Julian Waters wrote: >> We have seen a build failure along the lines of: >> >> /usr/bin/mv: cannot move >> '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def.tmp' >> to >> '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def': >> No such file or directory >> >> on Windows. >> >> My guess is that this is a race between creating the win-export.def file for >> the gtest jvm and the product jvm. > > make/hotspot/lib/CompileJvm.gmk line 151: > >> 149: WIN_EXPORT_FILE := $(JVM_OUTPUTDIR)/win-exports.def >> 150: >> 151: JVM_LDFLAGS_NO_GTEST += -def:$(WIN_EXPORT_FILE) > > Can't this just be named EXPORT_FILE_FLAG or something along those lines? > LDFLAGS_NO_GTEST seems a bit strange The thought was to clearly communicate why this must not be included in the gtest LDFLAGS; otherwise they must be exactly identical. - PR Review Comment: https://git.openjdk.org/jdk/pull/18043#discussion_r1506012594
Re: RFR: 8326953: Possible race in creation of win-exports.def
On Wed, 28 Feb 2024 12:26:54 GMT, Magnus Ihse Bursie wrote: > We have seen a build failure along the lines of: > > /usr/bin/mv: cannot move > '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def.tmp' > to > '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def': > No such file or directory > > on Windows. > > My guess is that this is a race between creating the win-export.def file for > the gtest jvm and the product jvm. Marked as reviewed by jwaters (Committer). Marked as reviewed by jwaters (Committer). make/hotspot/lib/CompileJvm.gmk line 151: > 149: WIN_EXPORT_FILE := $(JVM_OUTPUTDIR)/win-exports.def > 150: > 151: JVM_LDFLAGS_NO_GTEST += -def:$(WIN_EXPORT_FILE) Can't this just be named EXPORT_FILE_FLAG or something along those lines? LDFLAGS_NO_GTEST seems a bit strange - PR Review: https://git.openjdk.org/jdk/pull/18043#pullrequestreview-1906237581 PR Review: https://git.openjdk.org/jdk/pull/18043#pullrequestreview-1906240849 PR Review Comment: https://git.openjdk.org/jdk/pull/18043#discussion_r1505943413
Re: RFR: 8326947: Minimize MakeBase.gmk
On Wed, 28 Feb 2024 11:24:06 GMT, Magnus Ihse Bursie wrote: > This is part of a general "spring cleaning" of the build system, addressing > old code that has bit-rotted, been subject to lava flow, or just had bad or > smelly code that we've never gotten around to fix. > > This particular patch tries to make MakeBase truly minimal; only including > the core parts of the build system that all makefiles will need. This is now > limited to essential functionality for named parameter functions, variable > dependency, tool execution, logging and fixpath functionality. MakeBase still > includes Utils.gmk and FileUtils.gmk, and thus "provides" this functionality > as well. Separating these out as well will be the subject of a future patch. The distinction between Utils.gmk and BaseUtils.gmk seems a bit arbitrary at a glance, especially since both are still included from MakeBase.gmk. How would you know in which file to add a new macro? make/ReleaseFile.gmk line 40: > 38: # A file containing a way to uniquely identify the source code revision > that > 39: # the build was created from > 40: SOURCE_REVISION_TRACKER := > $(SUPPORT_OUTPUTDIR)/src-rev/source-revision-tracker Other file locations that need to be shared between different makefiles are defined in spec.gmk.in. Not ideal either, but perhaps better for now than having to define the value twice? - PR Review: https://git.openjdk.org/jdk/pull/18041#pullrequestreview-1906328783 PR Review Comment: https://git.openjdk.org/jdk/pull/18041#discussion_r1506000674
Re: RFR: 8326953: Possible race in creation of win-exports.def
On Wed, 28 Feb 2024 12:26:54 GMT, Magnus Ihse Bursie wrote: > We have seen a build failure along the lines of: > > /usr/bin/mv: cannot move > '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def.tmp' > to > '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def': > No such file or directory > > on Windows. > > My guess is that this is a race between creating the win-export.def file for > the gtest jvm and the product jvm. I am not 100% confident that this actually identifies the problem, nor that it really provides a fix, nor that this even is the best way to achieve this even if it should be the correct fix. Intermittent build problems are the worst. :-( They are hard to reproduce or know for sure if they are gone, but when they occur they cause a havoc in the CI pipeline. Hence this opportunistic fix. - PR Comment: https://git.openjdk.org/jdk/pull/18043#issuecomment-1968881256
RFR: 8326953: Possible race in creation of win-exports.def
We have seen a build failure along the lines of: /usr/bin/mv: cannot move '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def.tmp' to '.../build/windows-x64-open-debug/hotspot/variant-server/libjvm/win-exports.def': No such file or directory on Windows. My guess is that this is a race between creating the win-export.def file for the gtest jvm and the product jvm. - Commit messages: - 8326953: Possible race in creation of win-exports.def Changes: https://git.openjdk.org/jdk/pull/18043/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18043&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8326953 Stats: 23 lines in 2 files changed: 15 ins; 1 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/18043.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18043/head:pull/18043 PR: https://git.openjdk.org/jdk/pull/18043
Re: RFR: 8326831 Clarify test harness control variables in make help
On Tue, 27 Feb 2024 13:55:53 GMT, Ludvig Janiuk wrote: > Clarifying text in `make help` output on how to list variables for e.g. > JTREG='...'. I use the command `make test-only JTREG=help` locally, the outputed message seems an error (shown below). Does it work as expected? Building target 'test-only' in configuration 'linux-x86_64-server-fastdebug' Valid keywords for JTREG: RunTests.gmk:205: *** Re-run without 'help' to continue. Stop. JOBS TIMEOUT_FACTOR FAILURE_HANDLER_TIMEOUT TEST_MODE ASSERT VERBOSE RETAIN TEST_THREAD_FACTORY MAX_MEM RUN_PROBLEM_LISTS RETRY_COUNT REPEAT_COUNT MAX_OUTPUT REPORT OPTIONS JAVA_OPTIONS VM_OPTIONS KEYWORDS EXTRA_PROBLEM_LISTS LAUNCHER_OPTIONS. gmake[2]: *** [make/Main.gmk:797: test] Error 2 ERROR: Build failed for target 'test-only' in configuration 'linux-x86_64-server-fastdebug' (exit code 2) No indication of failed target found. HELP: Try searching the build log for '] Error'. HELP: Run 'make doctor' to diagnose build problems. make[1]: *** [/home/lgx/source/java/fast-jdk/make/Init.gmk:323: main] Error 2 make: *** [/home/lgx/source/java/fast-jdk/make/Init.gmk:189: test-only] Error 2 The location of the `RunTests.gmk:205: *** Re-run without 'help' to continue. Stop.` is wrong. But it sometimes works good (Shown below). Building target 'test-only' in configuration 'linux-x86_64-server-fastdebug' Valid keywords for JTREG: JOBS TIMEOUT_FACTOR FAILURE_HANDLER_TIMEOUT TEST_MODE ASSERT VERBOSE RETAIN TEST_THREAD_FACTORY MAX_MEM RUN_PROBLEM_LISTS RETRY_COUNT REPEAT_COUNT MAX_OUTPUT REPORT OPTIONS JAVA_OPTIONS VM_OPTIONS KEYWORDS EXTRA_PROBLEM_LISTS LAUNCHER_OPTIONS. RunTests.gmk:205: *** Re-run without 'help' to continue. Stop. gmake[2]: *** [make/Main.gmk:797: test] Error 2 ERROR: Build failed for target 'test-only' in configuration 'linux-x86_64-server-fastdebug' (exit code 2) No indication of failed target found. HELP: Try searching the build log for '] Error'. HELP: Run 'make doctor' to diagnose build problems. make[1]: *** [/home/lgx/source/java/fast-jdk/make/Init.gmk:323: main] Error 2 make: *** [/home/lgx/source/java/fast-jdk/make/Init.gmk:189: test-only] Error 2 - PR Comment: https://git.openjdk.org/jdk/pull/18028#issuecomment-1968862420
Re: RFR: 8326947: Minimize MakeBase.gmk
On Wed, 28 Feb 2024 11:24:06 GMT, Magnus Ihse Bursie wrote: > This is part of a general "spring cleaning" of the build system, addressing > old code that has bit-rotted, been subject to lava flow, or just had bad or > smelly code that we've never gotten around to fix. > > This particular patch tries to make MakeBase truly minimal; only including > the core parts of the build system that all makefiles will need. This is now > limited to essential functionality for named parameter functions, variable > dependency, tool execution, logging and fixpath functionality. MakeBase still > includes Utils.gmk and FileUtils.gmk, and thus "provides" this functionality > as well. Separating these out as well will be the subject of a future patch. I have verified using `COMPARE_BUILD` on windows-x64, linux-x64, linux-aarch64, macosx-x64 and macosx-aarch64 that there are no differences in the resulting build. - PR Comment: https://git.openjdk.org/jdk/pull/18041#issuecomment-1968778732
RFR: 8326947: Minimize MakeBase.gmk
This is part of a general "spring cleaning" of the build system, addressing old code that has bit-rotted, been subject to lava flow, or just had bad or smelly code that we've never gotten around to fix. This particular patch tries to make MakeBase truly minimal; only including the core parts of the build system that all makefiles will need. This is now limited to essential functionality for named parameter functions, variable dependency, tool execution, logging and fixpath functionality. MakeBase still includes Utils.gmk and FileUtils.gmk, and thus "provides" this functionality as well. Separating these out as well will be the subject of a future patch. - Commit messages: - Whitespace fix - MakeBase.gmk should not include MakeIO.gmk anymore - MakeBase.gmk should not include CopyFiles.gmk anymore - Reorder BaseUtils.gmk to make more sense - Move some more functionality to BaseUtils.gmk - Create BaseUtils.gmk with the most basic stuff - Move all file stuff from Utils.gmk to FileUtils.gmk - Document the purpose of MakeBase - Move SOURCE_REVISION_TRACKER to where it is used. - Move timers to InitSupport where they are used - ... and 3 more: https://git.openjdk.org/jdk/compare/1ab6bd43...3c86bcfe Changes: https://git.openjdk.org/jdk/pull/18041/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18041&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8326947 Stats: 1099 lines in 43 files changed: 607 ins; 480 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/18041.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18041/head:pull/18041 PR: https://git.openjdk.org/jdk/pull/18041
Integrated: 8326509: Clean up JNIEXPORT in Hotspot after JDK-8017234
On Thu, 22 Feb 2024 16:28:20 GMT, Magnus Ihse Bursie wrote: > Once [JDK-8017234](https://bugs.openjdk.org/browse/JDK-8017234) has been > integrated, it is possible to do some cleanup. The goal of > [JDK-8017234](https://bugs.openjdk.org/browse/JDK-8017234) was to not change > any behavior, even if that behavior seemed odd. > > Now let's try to fix that. We can: > > a) remove JNIEXPORT from c2v functions. > b) make debug.cpp functions exported similarly on all platforms. > c) remove JNIEXPORT from aarch64 asm debug function. > > Note that this PR is [dependent > on](https://mail.openjdk.org/pipermail/jdk-dev/2021-March/005232.html) > https://github.com/openjdk/jdk/pull/17955. This pull request has now been integrated. Changeset: e6b3bda2 Author:Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/e6b3bda2c30ea7932a8a20027e1ef2e805610f14 Stats: 35 lines in 4 files changed: 0 ins; 32 del; 3 mod 8326509: Clean up JNIEXPORT in Hotspot after JDK-8017234 Reviewed-by: djelinski, jwaters, dholmes - PR: https://git.openjdk.org/jdk/pull/17967
Re: RFR: 8325881: Require minimum gcc version 10
On Sat, 17 Feb 2024 08:28:56 GMT, Kim Barrett wrote: > Please review this change that updates the minimum supported version of gcc > to be used for building OpenJDK from 6.0 to 10.0. > > This permits enabling C++17 (JDK-8314488), though gcc 9.0 might suffice for > that. A minimum of gcc 10 also obtains the primitives needed to support a > work-alick for std::is_constant_evaluated (added in C++20). There are a bunch > of improvements that would be enabled by that. Having it would also allow the > elimination of a bit of a mess in the HotSpot assert macros that was needed to > work around the lack of that feature (JDK-8303805). Either current or proposed > minimum versions of other supported compilers also provide the needed > primitives. > > Testing: mach5 tier1 (uses gcc13.2 on gcc-based platforms) > Locally (linux-x64) built and ran tier1 with gcc10.3. I have no objections either. Let me check with our team if this breaks anything hard for our builds. - PR Review: https://git.openjdk.org/jdk/pull/17899#pullrequestreview-1905704494
Re: RFR: 8326831 Clarify test harness control variables in make help
On Tue, 27 Feb 2024 13:55:53 GMT, Ludvig Janiuk wrote: > Clarifying text in `make help` output on how to list variables for e.g. > JTREG='...'. Thanks for the insight Erik! - PR Comment: https://git.openjdk.org/jdk/pull/18028#issuecomment-1968538692
Integrated: 8326685: Linux builds not reproducible if two builds configured in different build folders
On Mon, 26 Feb 2024 16:00:42 GMT, Andrew Leonard wrote: > Adds Linux -fdebug-prefix-map'ing for SUPPORT_OUTPUTDIR and HOTSPOT_OUTPUTDIR > when absolute paths are not allowed in the binaries, thus making the building > of a JDK identically reproducible from different build directories. This pull request has now been integrated. Changeset: 3b90ddfe Author:Andrew Leonard URL: https://git.openjdk.org/jdk/commit/3b90ddfefea36d9f7f08ff11cd0cb099aa32b02b Stats: 5 lines in 1 file changed: 5 ins; 0 del; 0 mod 8326685: Linux builds not reproducible if two builds configured in different build folders Reviewed-by: ihse, erikj - PR: https://git.openjdk.org/jdk/pull/18009
Re: RFR: 8325881: Require minimum gcc version 10
On Sat, 17 Feb 2024 08:28:56 GMT, Kim Barrett wrote: > Please review this change that updates the minimum supported version of gcc > to be used for building OpenJDK from 6.0 to 10.0. > > This permits enabling C++17 (JDK-8314488), though gcc 9.0 might suffice for > that. A minimum of gcc 10 also obtains the primitives needed to support a > work-alick for std::is_constant_evaluated (added in C++20). There are a bunch > of improvements that would be enabled by that. Having it would also allow the > elimination of a bit of a mess in the HotSpot assert macros that was needed to > work around the lack of that feature (JDK-8303805). Either current or proposed > minimum versions of other supported compilers also provide the needed > primitives. > > Testing: mach5 tier1 (uses gcc13.2 on gcc-based platforms) > Locally (linux-x64) built and ran tier1 with gcc10.3. I have no objections. We already build with gcc 11. Some machines may need to get a newer gcc, but I think that's acceptable. - PR Comment: https://git.openjdk.org/jdk/pull/17899#issuecomment-1968440090
