Re: [Proposal] Add code signing on Windows to the JDK build system
On 2025-03-11 20:29, Frederic Thevenet wrote: Hi Magnus, As mentioned before, the core idea here is really just providing a hook for a script to be invoked after the native linker has been called. So while thinking about making this more generic and not Windows specific, it struck me that it might in fact be desirable to also drop the "signing" notion from this altogether and instead consider it a way to extend to the build system by plugging in custom scripts? And maybe extend it with other strategically placed hooks? Do you think this is a direction worth exploring further? Or that the original, more focused scope is preferable? Hm, that is definitely an interesting take. I'm a big fan of "right-sizing" generalization; I think both too much and too little generalization is detrimental. My gut feeling right now is that we should keep this focused on what it is, a way to allow signing. It will make the functionality discoverable by other parties that would like to add their own signing script; a general "hook" system might not. But it's good to keep in mind in case any future, non-signing needs arise. I was more thinking along the lines that the current macOS signing could be considered/rewritten in terms of a "signing hook", with the exception that we also ship a default implementation alongside in the build system. If we do that right, we could probably retrofit Oracle's internal macOS signing logic into the same framework, which would be nice. But then again, that is definitely feature creep wrt to your original request. So maybe we can come back and revisit that thought later. /Magnus Thanks, Frederic On 07/03/2025 16:49, Magnus Ihse Bursie wrote: Hi Frederic, I think your proposal makes sense, and as Erik says, the patch is very unobtrusive. I'm not at all sure if it is even possible, but if the signing logic for macOS and Windows can be sort of unified and made more abstract, that would certainly be a win. /Magnus On 2025-02-26 20:46, Erik Joelsson wrote: On 2/26/25 10:40 AM, Frederic Thevenet wrote: Hi Erik, Thanks for your comments. As a matter of fact, we at Red Hat also face similar constraints that prevent us from being able to sign the files on the machine that build them (and I suspect this is quite common-place). My initial idea to accommodate that is to have the jdk build invoke a user supplied script (via a --with configure argument) rather than having it call signtool directly, so as to provide a way for everyone to easily plug their specific configuration and logic (in our case, the script uploads the file to sign to the signing machine and download it once it is done, in a synchronous fashion). In effect, it's just a hook that is invoked after the native linker has been called. I have created a PR[1] where you can see what I've done in details. That's a small and unintrusive solution. Our solution is a lot more involved as I figured I wanted to expose a set of top level "phase" targets for signing for each module, so that building libs or launchers can be done independently from signing (e.g. java.base-libs, java.base-launchers and java.base-sign). This necessitates a separate set of output directories for signed libs and signed launchers, where we first build like normal to the regular output dirs and then the sign target copies and signs binaries into the *-signed outputdirs, and when building jmods, we need to pick up the files from those *-signed directories. It's quite messy, so I've kept it all in our internal makefile extensions. I'm also questioning the usefulness of this flexibility given how much complexity it adds, so your proposal is probably preferred. That said, I'm wondering if we could make it a bit more generic and not define it as "windows" specific? Then perhaps bake in the macos signing logic into this as well. The default implementation could be using the native OS tools while there is a configure option to completely replace it. /Erik Thanks, Frederic [1] https://github.com/openjdk/jdk/pull/23807 On 26/02/2025 15:29, Erik Joelsson wrote: Hello, I think this is a reasonable idea and you are welcome to implement it. At Oracle we have been solving this using custom extension makefiles. That will unfortunately not change with this proposal as our signing process does not involve direct access to the native tools of the OS. Perhaps this implementation could be made plug-able enough so that we could leverage it, I'm not sure. What I'm trying to say is that we are not against this, but we are also not inclined to invest resources into it. /Erik On 2/26/25 12:52 AM, Frederic Thevenet wrote: Hi, I would like to start a discussion about adding built-in support for code signing native executable file and dynamic library on Windows directly as part of the JDK build, in a similar fashion to what already exists for macOS. Most, if not all vendors already ensure that ever
Integrated: 8351542: LIBMANAGEMENT_OPTIMIZATION remove special optimization settings
On Mon, 10 Mar 2025 15:38:45 GMT, Matthias Baesken wrote: > On Linux there are some special settings for LIBMANAGEMENT_OPTIMIZATION that > are most likely not needed any more and could be removed. This pull request has now been integrated. Changeset: c3db6671 Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/c3db667156f7e6b7d05c76370973b9f2db9f0d55 Stats: 12 lines in 2 files changed: 0 ins; 10 del; 2 mod 8351542: LIBMANAGEMENT_OPTIMIZATION remove special optimization settings Reviewed-by: ihse, kevinw - PR: https://git.openjdk.org/jdk/pull/23966
Re: RFR: 8351542: LIBMANAGEMENT_OPTIMIZATION remove special optimization settings [v5]
On Wed, 12 Mar 2025 12:45:48 GMT, Matthias Baesken wrote: >> On Linux there are some special settings for LIBMANAGEMENT_OPTIMIZATION that >> are most likely not needed any more and could be removed. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > move code back in VMManagementImpl Thanks for the reviews ! - PR Comment: https://git.openjdk.org/jdk/pull/23966#issuecomment-2721123753
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public [v2]
On Thu, 13 Mar 2025 07:39:20 GMT, Christoph Langer wrote: >> This PR addresses an issue that can be observed when building on Windows >> with configure options `--enable-linkable-runtime` and >> `--with-external-symbols-in-bundles=public`. >> >> The problem is that with this special build configuration, we build two sets >> of .pdb files for the binaries. The first set is the standard debug symbols >> files named .pdb. The second set consists of stripped debug >> symbols file called .stripped.pdb which have less information >> but enough to present line numbers in hs-err files. >> >> During build we use the *.stripped.pdb files for compiling the jmods and >> also the bundle files. However, in the images folder, both sets of .pdb >> files exist. The tests for runtime linking will now, in the absence of jmod >> files, pick up the .pdb files (without *stripped*) from images, but in the >> runtime the hashes of the *stripped* files are stored. >> >> With this change, the standard .pdb files in the >> `--with-external-symbols-in-bundles=public` configuration are now the >> stripped files and we create a set of full pdb files named *.full.pdb. Jmods >> and Bundles still contain the stripped pdb files and we also fix the issue >> that the debug symbols bundle also contained stripped pdb files so far. With >> this fix, it will contain the full pdb files and extracting these over a JDK >> runtime will replace stripped pdbs with full pdbs. > > Christoph Langer has updated the pull request incrementally with one > additional commit since the last revision: > > Fix tests > I'll add here that the implementation of JEP 493, does nothing special. It > takes what's in the JMODs _at build time_ when `jlink` is being run and uses > that as the "source of truth" when it comes to hash sums. It sounds to me > that when `jlink` runs during the build it only sees `*.stripped.pdb` files > (renamed as `*.pdb`). If something is being done **after** the `jdk` image > has been created (by `jlink`) and changes files (like copying full `*.pdb` > files over the stripped ones that are actually in the JMODs) then this is the > root cause of this bug. Rightfully so, since the `*.pdb` files in the JDK > image no longer match the ones in the JMOD files. Is that the case? Yes, that is the root cause of this issue that I tried to allude to in my last comment. The "jdk image" that we produce in the build and run tests from isn't exactly what jlink produced, or what ends up inside the zip/tar.gz bundles. Historically we always put the debug symbols in that image, for developer convenience. When introducing the stripped/public pdb support, we managed to preserve this property of the jdk image. This is basically why in this case, the image isn't representative of the shipped product and the tests fail. - PR Comment: https://git.openjdk.org/jdk/pull/24012#issuecomment-2721258333
Re: RFR: 8351277: Remove pipewire from AIX build [v2]
On Thu, 13 Mar 2025 13:03:16 GMT, Matthias Baesken wrote: >> Seems Wayland is not supported on AIX, so we most likely do not need to >> build pipewire on AIX and can remove it from the build. >> Reason is that pipewire updates can break the AIX build, so better avoid it >> if possible. >> >> To check if it works, I move the 'pipewire/include' folder away, and the >> build still worked on AIX. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove AIX special handling from endian.h This looks good to me. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/24009#pullrequestreview-2681818338
Re: RFR: 8351277: Remove pipewire from AIX build [v2]
> Seems Wayland is not supported on AIX, so we most likely do not need to build > pipewire on AIX and can remove it from the build. > Reason is that pipewire updates can break the AIX build, so better avoid it > if possible. > > To check if it works, I move the 'pipewire/include' folder away, and the > build still worked on AIX. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: remove AIX special handling from endian.h - Changes: - all: https://git.openjdk.org/jdk/pull/24009/files - new: https://git.openjdk.org/jdk/pull/24009/files/84740c96..e7941d37 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=24009&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24009&range=00-01 Stats: 4 lines in 1 file changed: 0 ins; 4 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/24009.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/24009/head:pull/24009 PR: https://git.openjdk.org/jdk/pull/24009
Re: RFR: 8351323: Parameterize compiler and linker flags for iconv [v3]
On Thu, 13 Mar 2025 13:28:32 GMT, Magnus Ihse Bursie wrote: >> snake66 has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Unbreak libjdwp build > > Then I think the current solution is fine. Even if OpenBSD is not the primary > goal of your port, I see no reason to actively work against it either, so if > it needs `ICONV_LDFLAGS` then we should include it. @magicus @erikj79 Thanks for the reviews and help! - PR Comment: https://git.openjdk.org/jdk/pull/23995#issuecomment-2721341294
RFR: JDK-8348598 : Update Libpng to 1.6.47
Upgraded Libpng to v1.6.47 - Commit messages: - unused-function added for gcc - libpng upgrade Changes: https://git.openjdk.org/jdk/pull/24021/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=24021&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8348598 Stats: 4982 lines in 23 files changed: 1913 ins; 2285 del; 784 mod Patch: https://git.openjdk.org/jdk/pull/24021.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/24021/head:pull/24021 PR: https://git.openjdk.org/jdk/pull/24021
Re: RFR: JDK-8348598 : Update Libpng to 1.6.47
On Thu, 13 Mar 2025 00:27:46 GMT, Harshitha Onkar wrote: > Upgraded Libpng to v1.6.47 make/modules/java.desktop/lib/ClientLibraries.gmk line 249: > 247: DISABLED_WARNINGS_clang_dgif_lib.c := sign-compare, \ > 248: DISABLED_WARNINGS_clang_gzwrite.c := format-nonliteral, \ > 249: DISABLED_WARNINGS_clang_png.c := unused-function, \ This warning was disabled to fix the following build issue on linux and macOS. open/src/java.desktop/share/native/libsplashscreen/libpng/png.c:1558:1: error: 'png_icc_profile_error' defined but not used [-Werror=unused-function] [2025-03-12T23:57:44,575Z] 1558 | png_icc_profile_error(png_const_structrp png_ptr, png_const_charp name, [2025-03-12T23:57:44,575Z] | ^ - PR Review Comment: https://git.openjdk.org/jdk/pull/24021#discussion_r1993813117
Re: RFR: 8351323: Parameterize compiler and linker flags for iconv [v3]
On Thu, 13 Mar 2025 11:39:14 GMT, snake66 wrote: >> Allows for future support for platforms that require different flags for >> libiconv support. >> >> Sponsored-by: The FreeBSD Foundation > > snake66 has updated the pull request incrementally with one additional commit > since the last revision: > > Unbreak libjdwp build @snake66 Your change (at version a9fdfef5f18c63deebd64f37825b8b733997c588) is now ready to be sponsored by a Committer. - PR Comment: https://git.openjdk.org/jdk/pull/23995#issuecomment-2721348961
Re: RFR: 8351323: Parameterize compiler and linker flags for iconv [v3]
> Allows for future support for platforms that require different flags for > libiconv support. > > Sponsored-by: The FreeBSD Foundation snake66 has updated the pull request incrementally with one additional commit since the last revision: Unbreak libjdwp build - Changes: - all: https://git.openjdk.org/jdk/pull/23995/files - new: https://git.openjdk.org/jdk/pull/23995/files/d4c4cca5..a9fdfef5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=23995&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=23995&range=01-02 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/23995.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/23995/head:pull/23995 PR: https://git.openjdk.org/jdk/pull/23995
Re: RFR: 8351277: Remove pipewire from AIX build
On Thu, 13 Mar 2025 09:29:38 GMT, Magnus Ihse Bursie wrote: > Can you include a revert of JDK-8349751, which is not needed anymore after > this change? Okay I removed the AIX code from endian.h . - PR Comment: https://git.openjdk.org/jdk/pull/24009#issuecomment-2721177888
RFR: 8351603: Change to GCC 14.2.0 for building on Linux at Oracle
Background:
Oracle is updating the version of GCC for building the JDK on Linux to 14.2.0.
This change updates the versions of the components used for the (Linux) devkit.
Newer versions of ccache use cmake for the build, so some of the logic in
`make/devkit/Tools.gmk` had to be updated to support cmake based components.
This change also fixes JDK-8344272 (gcc devkit doesn't have lto-plugin where
needed).
Testing:
Manual builds, tier1-5 + additional func & performance testing.
-
Commit messages:
- Update doc/building.{md,html}
- Add missing liblto_plugin.so link
- Change to GCC 14.2.0 for building on Linux at Oracle
Changes: https://git.openjdk.org/jdk/pull/23975/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=23975&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8351603
Stats: 40 lines in 3 files changed: 30 ins; 0 del; 10 mod
Patch: https://git.openjdk.org/jdk/pull/23975.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/23975/head:pull/23975
PR: https://git.openjdk.org/jdk/pull/23975
Re: RFR: 8351323: Parameterize compiler and linker flags for iconv [v3]
On Thu, 13 Mar 2025 11:39:14 GMT, snake66 wrote: >> Allows for future support for platforms that require different flags for >> libiconv support. >> >> Sponsored-by: The FreeBSD Foundation > > snake66 has updated the pull request incrementally with one additional commit > since the last revision: > > Unbreak libjdwp build This looks good to me now. Then I think the current solution is fine. Even if OpenBSD is not the primary goal of your port, I see no reason to actively work against it either, so if it needs `ICONV_LDFLAGS` then we should include it. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/23995#pullrequestreview-2681916914 PR Comment: https://git.openjdk.org/jdk/pull/23995#issuecomment-2721263178
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public [v2]
On Thu, 13 Mar 2025 07:39:20 GMT, Christoph Langer wrote: >> This PR addresses an issue that can be observed when building on Windows >> with configure options `--enable-linkable-runtime` and >> `--with-external-symbols-in-bundles=public`. >> >> The problem is that with this special build configuration, we build two sets >> of .pdb files for the binaries. The first set is the standard debug symbols >> files named .pdb. The second set consists of stripped debug >> symbols file called .stripped.pdb which have less information >> but enough to present line numbers in hs-err files. >> >> During build we use the *.stripped.pdb files for compiling the jmods and >> also the bundle files. However, in the images folder, both sets of .pdb >> files exist. The tests for runtime linking will now, in the absence of jmod >> files, pick up the .pdb files (without *stripped*) from images, but in the >> runtime the hashes of the *stripped* files are stored. >> >> With this change, the standard .pdb files in the >> `--with-external-symbols-in-bundles=public` configuration are now the >> stripped files and we create a set of full pdb files named *.full.pdb. Jmods >> and Bundles still contain the stripped pdb files and we also fix the issue >> that the debug symbols bundle also contained stripped pdb files so far. With >> this fix, it will contain the full pdb files and extracting these over a JDK >> runtime will replace stripped pdbs with full pdbs. > > Christoph Langer has updated the pull request incrementally with one > additional commit since the last revision: > > Fix tests Another angle of this is that as soon as a user runs `jlink` to produce a custom runtime the debugging experience will be different to the JDK they started out with. - PR Comment: https://git.openjdk.org/jdk/pull/24012#issuecomment-2721421629
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public [v2]
On Thu, 13 Mar 2025 07:39:20 GMT, Christoph Langer wrote: >> This PR addresses an issue that can be observed when building on Windows >> with configure options `--enable-linkable-runtime` and >> `--with-external-symbols-in-bundles=public`. >> >> The problem is that with this special build configuration, we build two sets >> of .pdb files for the binaries. The first set is the standard debug symbols >> files named .pdb. The second set consists of stripped debug >> symbols file called .stripped.pdb which have less information >> but enough to present line numbers in hs-err files. >> >> During build we use the *.stripped.pdb files for compiling the jmods and >> also the bundle files. However, in the images folder, both sets of .pdb >> files exist. The tests for runtime linking will now, in the absence of jmod >> files, pick up the .pdb files (without *stripped*) from images, but in the >> runtime the hashes of the *stripped* files are stored. >> >> With this change, the standard .pdb files in the >> `--with-external-symbols-in-bundles=public` configuration are now the >> stripped files and we create a set of full pdb files named *.full.pdb. Jmods >> and Bundles still contain the stripped pdb files and we also fix the issue >> that the debug symbols bundle also contained stripped pdb files so far. With >> this fix, it will contain the full pdb files and extracting these over a JDK >> runtime will replace stripped pdbs with full pdbs. > > Christoph Langer has updated the pull request incrementally with one > additional commit since the last revision: > > Fix tests I did a little bit of an experiment. When we link today, we supply this argument to define the main/full pdb file: "-pdb:$$($1_SYMBOLS_DIR)/$$($1_BASENAME).pdb" And in the case of `--with-external-symbols-in-bundles=public`, we also add this: "-pdbstripped:$$($1_SYMBOLS_DIR)/$$($1_BASENAME).stripped.pdb" Just checking the contents of the resulting dll using `strings`, we can see that a reference to the pdb file is stored: $ strings build/windows-x64/images/jdk/bin/verify.dll | grep pdb verify.dll.pdb This reference is the same wether I configure with `--with-external-symbols-in-bundles=public` or not, as in, strings does not find any `verify.dll.stripped.pdb` reference. Now with this patch, we instead supply: "-pdb:$$($1_SYMBOLS_DIR)/$$($1_BASENAME).full.pdb" "-pdbstripped:$$($1_SYMBOLS_DIR)/$$($1_BASENAME).pdb" And then the stored reference becomes: $ strings build/windows-x64-public-patch/images/jdk/bin/verify.dll | grep pdb verify.dll.full.pdb This is of course just an indication of what the Microsoft linker does and how the tools will interpret these libraries and executables. I haven't actually tested all the scenarios with debugging tools or generating hserr files. I do think it's enough to warrant a proper investigation before we change the `-pdb` flag to point to a filename other than `$$($1_BASENAME).pdb` though, as that could potentially have consequences for loading any kind of symbols in the final binaries. - PR Comment: https://git.openjdk.org/jdk/pull/24012#issuecomment-2721877569
Re: RFR: JDK-8348598 : Update Libpng to 1.6.47 [v3]
On Thu, 13 Mar 2025 17:30:33 GMT, Harshitha Onkar wrote: >> Upgraded Libpng to v1.6.47 > > Harshitha Onkar has updated the pull request incrementally with one > additional commit since the last revision: > > header update Marked as reviewed by azvegint (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/24021#pullrequestreview-2683085368
Re: RFR: 8351277: Remove pipewire from AIX build [v2]
On Thu, 13 Mar 2025 13:06:38 GMT, Matthias Baesken wrote: >> Seems Wayland is not supported on AIX, so we most likely do not need to >> build pipewire on AIX and can remove it from the build. >> Reason is that pipewire updates can break the AIX build, so better avoid it >> if possible. >> >> To check if it works, I move the 'pipewire/include' folder away, and the >> build still worked on AIX. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove AIX special handling from endian.h Looks right to me, but I will do a test build. - PR Review: https://git.openjdk.org/jdk/pull/24009#pullrequestreview-2683234993
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public
On Wed, 12 Mar 2025 15:57:40 GMT, Christoph Langer wrote: > This PR addresses an issue that can be observed when building on Windows with > configure options `--enable-linkable-runtime` and > `--with-external-symbols-in-bundles=public`. > > The problem is that whith this special build configuration, we build two sets > of .pdb files for the binaries. The first set is the standard debug symbols > file named .pdb. The second set is a stripped debug symbols file > called .stripped.pdb which has less information but enought to > present line numbers in hs-err files. > > During build we use the *.stripped.pdb files for compiling the jmods and also > the bundle files. However, in the images folder, both sets of .pdb files > exist. The tests for runtime linking will now, in the absence of jmod files, > pick up the .pdb files (without *stripped*) from images, but in the runtime > the hashes of the *stripped* files are stored. > > With this change, the standard .pdb files in the > `--with-external-symbols-in-bundles=public` configuration are now the > stripped files and we create a set of full pdb files named *.full.pdb. Jmods > and Bundles still contain the stripped pdb files and we also fix the issue > that the debug symbols bundle also contained stripped pdb files so far. With > this fix, it will contain the full pdb files and extracting these over a JDK > runtime will replace stripped pdbs with full pdbs. Nice find! This sounds like a bug in the build machinery. - PR Comment: https://git.openjdk.org/jdk/pull/24012#issuecomment-2718972286
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public [v2]
On Thu, 13 Mar 2025 07:39:20 GMT, Christoph Langer wrote: >> This PR addresses an issue that can be observed when building on Windows >> with configure options `--enable-linkable-runtime` and >> `--with-external-symbols-in-bundles=public`. >> >> The problem is that with this special build configuration, we build two sets >> of .pdb files for the binaries. The first set is the standard debug symbols >> files named .pdb. The second set consists of stripped debug >> symbols file called .stripped.pdb which have less information >> but enough to present line numbers in hs-err files. >> >> During build we use the *.stripped.pdb files for compiling the jmods and >> also the bundle files. However, in the images folder, both sets of .pdb >> files exist. The tests for runtime linking will now, in the absence of jmod >> files, pick up the .pdb files (without *stripped*) from images, but in the >> runtime the hashes of the *stripped* files are stored. >> >> With this change, the standard .pdb files in the >> `--with-external-symbols-in-bundles=public` configuration are now the >> stripped files and we create a set of full pdb files named *.full.pdb. Jmods >> and Bundles still contain the stripped pdb files and we also fix the issue >> that the debug symbols bundle also contained stripped pdb files so far. With >> this fix, it will contain the full pdb files and extracting these over a JDK >> runtime will replace stripped pdbs with full pdbs. > > Christoph Langer has updated the pull request incrementally with one > additional commit since the last revision: > > Fix tests Thanks Erik for your investigation. However, the failing gtest that I workarounded here seems to be an indication that this might not work since a method name can not be found. On the other hand, I did not check the implementation of os::print_function_and_library_name and whether it looks for some hard coded pdb name. - PR Comment: https://git.openjdk.org/jdk/pull/24012#issuecomment-2722075785
Integrated: 8351323: Parameterize compiler and linker flags for iconv
On Tue, 11 Mar 2025 19:22:34 GMT, snake66 wrote: > Allows for future support for platforms that require different flags for > libiconv support. > > Sponsored-by: The FreeBSD Foundation This pull request has now been integrated. Changeset: 771e160d Author:Harald Eilertsen URL: https://git.openjdk.org/jdk/commit/771e160da4daa98bfe37bf1acba65454c088910c Stats: 35 lines in 5 files changed: 25 ins; 2 del; 8 mod 8351323: Parameterize compiler and linker flags for iconv Reviewed-by: ihse, erikj - PR: https://git.openjdk.org/jdk/pull/23995
Re: RFR: JDK-8348598 : Update Libpng to 1.6.47
On Thu, 13 Mar 2025 00:27:46 GMT, Harshitha Onkar wrote: > Upgraded Libpng to v1.6.47 Marked as reviewed by erikj (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/24021#pullrequestreview-2682753294
Re: RFR: 8351542: LIBMANAGEMENT_OPTIMIZATION remove special optimization settings [v5]
On Wed, 12 Mar 2025 12:45:48 GMT, Matthias Baesken wrote: >> On Linux there are some special settings for LIBMANAGEMENT_OPTIMIZATION that >> are most likely not needed any more and could be removed. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > move code back in VMManagementImpl As pointed out already, we have a similar special setting here , should we also adjust this ? make/modules/java.base/lib/CoreLibraries.gmk-36-ifeq ($(call isTargetOs, linux)+$(COMPILE_WITH_DEBUG_SYMBOLS), true+true) make/modules/java.base/lib/CoreLibraries.gmk:37: LIBVERIFY_OPTIMIZATION := LOW make/modules/java.base/lib/CoreLibraries.gmk-38-endif make/modules/java.base/lib/CoreLibraries.gmk-39- make/modules/java.base/lib/CoreLibraries.gmk-40-$(eval $(call SetupJdkLibrary, BUILD_LIBVERIFY, \ make/modules/java.base/lib/CoreLibraries.gmk-41-NAME := verify, \ make/modules/java.base/lib/CoreLibraries.gmk:42:OPTIMIZATION := $(LIBVERIFY_OPTIMIZATION), \ make/modules/java.base/lib/CoreLibraries.gmk-43- DISABLED_WARNINGS_gcc_check_code.c := unused-variable, \ make/modules/java.base/lib/CoreLibraries.gmk-44- DISABLED_WARNINGS_clang_check_code.c := unused-variable, \ - PR Comment: https://git.openjdk.org/jdk/pull/23966#issuecomment-2721937986
Re: RFR: JDK-8348598 : Update Libpng to 1.6.47 [v2]
> Upgraded Libpng to v1.6.47 Harshitha Onkar has updated the pull request incrementally with one additional commit since the last revision: header update - Changes: - all: https://git.openjdk.org/jdk/pull/24021/files - new: https://git.openjdk.org/jdk/pull/24021/files/b8f10b8d..e1954bcb Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=24021&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24021&range=00-01 Stats: 5 lines in 1 file changed: 5 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/24021.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/24021/head:pull/24021 PR: https://git.openjdk.org/jdk/pull/24021
Re: RFR: JDK-8348598 : Update Libpng to 1.6.47 [v3]
> Upgraded Libpng to v1.6.47 Harshitha Onkar has updated the pull request incrementally with one additional commit since the last revision: header update - Changes: - all: https://git.openjdk.org/jdk/pull/24021/files - new: https://git.openjdk.org/jdk/pull/24021/files/e1954bcb..2f0cea2d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=24021&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24021&range=01-02 Stats: 82 lines in 16 files changed: 80 ins; 2 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/24021.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/24021/head:pull/24021 PR: https://git.openjdk.org/jdk/pull/24021
Re: RFR: 8351542: LIBMANAGEMENT_OPTIMIZATION remove special optimization settings [v5]
On Wed, 12 Mar 2025 12:45:48 GMT, Matthias Baesken wrote: >> On Linux there are some special settings for LIBMANAGEMENT_OPTIMIZATION that >> are most likely not needed any more and could be removed. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > move code back in VMManagementImpl Yes seems likely the same history for libverify, so should be good to do the same update there. - PR Comment: https://git.openjdk.org/jdk/pull/23966#issuecomment-2722162206
Re: RFR: JDK-8348598 : Update Libpng to 1.6.47 [v3]
On Thu, 13 Mar 2025 17:30:33 GMT, Harshitha Onkar wrote: >> Upgraded Libpng to v1.6.47 > > Harshitha Onkar has updated the pull request incrementally with one > additional commit since the last revision: > > header update I was surprised by how many changes there are. - Marked as reviewed by prr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/24021#pullrequestreview-2683217837
Re: RFR: JDK-8348598 : Update Libpng to 1.6.47 [v3]
On Thu, 13 Mar 2025 15:45:12 GMT, Harshitha Onkar wrote: >> Harshitha Onkar has updated the pull request incrementally with one >> additional commit since the last revision: >> >> header update > > make/modules/java.desktop/lib/ClientLibraries.gmk line 249: > >> 247: DISABLED_WARNINGS_clang_dgif_lib.c := sign-compare, \ >> 248: DISABLED_WARNINGS_clang_gzwrite.c := format-nonliteral, \ >> 249: DISABLED_WARNINGS_clang_png.c := unused-function, \ > > This warning was disabled to fix the following build issue on linux and macOS. > > > open/src/java.desktop/share/native/libsplashscreen/libpng/png.c:1558:1: > error: 'png_icc_profile_error' defined but not used [-Werror=unused-function] > [2025-03-12T23:57:44,575Z] 1558 | png_icc_profile_error(png_const_structrp > png_ptr, png_const_charp name, > [2025-03-12T23:57:44,575Z] | ^ Might this be a bug in PNG? All usages of this function are now guarded by "PNG_READ_iCCP_SUPPORTED"? It would be useful to report this upstream. - PR Review Comment: https://git.openjdk.org/jdk/pull/24021#discussion_r1994558215
Re: RFR: 8351603: Change to GCC 14.2.0 for building on Linux at Oracle [v2]
> Background: > > Oracle is updating the version of GCC for building the JDK on Linux to 14.2.0. > > This change updates the versions of the components used for the (Linux) > devkit. Newer versions of ccache use cmake for the build, so some of the > logic in `make/devkit/Tools.gmk` had to be updated to support cmake based > components. This change also fixes JDK-8344272 (gcc devkit doesn't have > lto-plugin where needed). > > Testing: > > Manual builds, tier1-5 + additional func & performance testing. Mikael Vidstedt has updated the pull request incrementally with one additional commit since the last revision: Update jib-profiles.js - Changes: - all: https://git.openjdk.org/jdk/pull/23975/files - new: https://git.openjdk.org/jdk/pull/23975/files/d70f4a8c..ac2b44db Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=23975&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=23975&range=00-01 Stats: 5 lines in 1 file changed: 0 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/23975.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/23975/head:pull/23975 PR: https://git.openjdk.org/jdk/pull/23975
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public [v2]
On Thu, 13 Mar 2025 07:39:20 GMT, Christoph Langer wrote: >> This PR addresses an issue that can be observed when building on Windows >> with configure options `--enable-linkable-runtime` and >> `--with-external-symbols-in-bundles=public`. >> >> The problem is that with this special build configuration, we build two sets >> of .pdb files for the binaries. The first set is the standard debug symbols >> files named .pdb. The second set consists of stripped debug >> symbols file called .stripped.pdb which have less information >> but enough to present line numbers in hs-err files. >> >> During build we use the *.stripped.pdb files for compiling the jmods and >> also the bundle files. However, in the images folder, both sets of .pdb >> files exist. The tests for runtime linking will now, in the absence of jmod >> files, pick up the .pdb files (without *stripped*) from images, but in the >> runtime the hashes of the *stripped* files are stored. >> >> With this change, the standard .pdb files in the >> `--with-external-symbols-in-bundles=public` configuration are now the >> stripped files and we create a set of full pdb files named *.full.pdb. Jmods >> and Bundles still contain the stripped pdb files and we also fix the issue >> that the debug symbols bundle also contained stripped pdb files so far. With >> this fix, it will contain the full pdb files and extracting these over a JDK >> runtime will replace stripped pdbs with full pdbs. > > Christoph Langer has updated the pull request incrementally with one > additional commit since the last revision: > > Fix tests Please refrain from integrating this until we get consensus on the principal issue here, as pointed out by Erik. This is not just a build issue, it is an issue of differing expectations. It might even be that the solution is a modification in jlink rather than the build system. My take is that we should be able to both test and debug what we ship. I can't say straight up how to solve that, but I believe that if there is a will, there is a way; we just have to try harder to find it. - PR Comment: https://git.openjdk.org/jdk/pull/24012#issuecomment-2720552884 PR Comment: https://git.openjdk.org/jdk/pull/24012#issuecomment-2720556790
Re: RFR: 8351277: Remove pipewire from AIX build
On Wed, 12 Mar 2025 14:52:10 GMT, Matthias Baesken wrote: > Seems Wayland is not supported on AIX, so we most likely do not need to build > pipewire on AIX and can remove it from the build. > Reason is that pipewire updates can break the AIX build, so better avoid it > if possible. > > To check if it works, I move the 'pipewire/include' folder away, and the > build still worked on AIX. Can you include a revert of JDK-8349751, which is not needed anymore after this change? - PR Comment: https://git.openjdk.org/jdk/pull/24009#issuecomment-2720571221
Re: RFR: 8351542: LIBMANAGEMENT_OPTIMIZATION remove special optimization settings [v5]
On Wed, 12 Mar 2025 12:45:48 GMT, Matthias Baesken wrote: >> On Linux there are some special settings for LIBMANAGEMENT_OPTIMIZATION that >> are most likely not needed any more and could be removed. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > move code back in VMManagementImpl Yes looks good. I'm seeing -O3 in a Linux release build and -O2 in a fastdebug build. I see the release build libmanagement.so is 48 bytes smaller than before, both around 25k, so no real difference. Didn't see any new problems in all the management tests. - Marked as reviewed by kevinw (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/23966#pullrequestreview-2681208349
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public [v2]
> This PR addresses an issue that can be observed when building on Windows with > configure options `--enable-linkable-runtime` and > `--with-external-symbols-in-bundles=public`. > > The problem is that with this special build configuration, we build two sets > of .pdb files for the binaries. The first set is the standard debug symbols > files named .pdb. The second set consists of stripped debug > symbols file called .stripped.pdb which have less information > but enough to present line numbers in hs-err files. > > During build we use the *.stripped.pdb files for compiling the jmods and also > the bundle files. However, in the images folder, both sets of .pdb files > exist. The tests for runtime linking will now, in the absence of jmod files, > pick up the .pdb files (without *stripped*) from images, but in the runtime > the hashes of the *stripped* files are stored. > > With this change, the standard .pdb files in the > `--with-external-symbols-in-bundles=public` configuration are now the > stripped files and we create a set of full pdb files named *.full.pdb. Jmods > and Bundles still contain the stripped pdb files and we also fix the issue > that the debug symbols bundle also contained stripped pdb files so far. With > this fix, it will contain the full pdb files and extracting these over a JDK > runtime will replace stripped pdbs with full pdbs. Christoph Langer has updated the pull request incrementally with one additional commit since the last revision: Fix tests - Changes: - all: https://git.openjdk.org/jdk/pull/24012/files - new: https://git.openjdk.org/jdk/pull/24012/files/37037f4f..f560fd21 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=24012&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24012&range=00-01 Stats: 13 lines in 2 files changed: 8 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/24012.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/24012/head:pull/24012 PR: https://git.openjdk.org/jdk/pull/24012
Re: RFR: 8351323: Parameterize compiler and linker flags for iconv [v2]
On Wed, 12 Mar 2025 16:56:32 GMT, Magnus Ihse Bursie wrote: > Also, to double check, are the new variables `ICONV_CFLAGS` and > `ICONV_LDFLAGS` going to be non-empty on BSD? Yes, the current BSD port has the following in `libraries.m4`: if test "x$OPENJDK_TARGET_OS" = "xbsd"; then if test "x$OPENJDK_TARGET_OS_ENV" = "xbsd.openbsd"; then ICONV_CFLAGS="-I/usr/local/include" ICONV_LDFLAGS="-L/usr/local/lib" ICONV_LIBS=-liconv elif test "x$OPENJDK_TARGET_OS_ENV" = "xbsd.freebsd"; then ICONV_CFLAGS=-DLIBICONV_PLUG ICONV_LDFLAGS= ICONV_LIBS= else ICONV_CFLAGS= ICONV_LDFLAGS= ICONV_LIBS= fi else ICONV_CFLAGS= ICONV_LDFLAGS= ICONV_LIBS=-liconv fi AC_SUBST(ICONV_CFLAGS) AC_SUBST(ICONV_LDFLAGS) AC_SUBST(ICONV_LIBS) So strictly speaking we only need `ICONV_CFLAGS` for FreeBSD, and `ICONV_LIBS` for aix and macosx. That is if we don't want to include OpenBSD support at this time. > make/modules/jdk.jdwp.agent/Lib.gmk line 76: > >> 74: java.base:libjava, \ >> 75: JDK_LIBS := java.base:libjvm, \ >> 76: LIBS_linux := $(LIBDL), \ > > Something got wrong here. You should have kept the LIBS_linux line, and the > new LIBS line should be `LIBS := $(ICONV_LIBS)`. Ooops, I'll fix! - PR Comment: https://git.openjdk.org/jdk/pull/23995#issuecomment-2720660907 PR Review Comment: https://git.openjdk.org/jdk/pull/23995#discussion_r1993169985
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public
On Wed, 12 Mar 2025 20:00:29 GMT, Erik Joelsson wrote: > What happens with the local debugging experience in the exploded image if you > configure with ` --with-external-symbols-in-bundles=public`? Then the full > debug symbols are only present in the *.full.pdb files. Will the debugger > tools on Windows know automatically how to resolve that, or will this > configuration be inconvenient for local dev/debug usecases? As I remember it, > the current scheme was chosen to put the "full" pdb files as *.pdb in the > exploded image whenever possible. FWIW, from my work on https://github.com/openjdk/jdk/pull/16039 I remember verifying that debuggers on Windows do not rely on the filenames of pdb files to match the symbols and binaries, so AFAIK, the fact that the symbols for, say, `jvm.dll`, will be in a file named `jvm.dll.full.pdb` instead of `jvm.dll.pdb` does not imply it won't be found by the debugger (i.e. it used to be in a file named `jvm.pdb`) Now, if I understand the proposed change correctly, the exploded image will contain two distinct `.pdb` files for each binaries, so a question remain about how a debugger might decide to choose one over the other, but do we know for sure that this indeed affect the right symbols to be picked up? - PR Comment: https://git.openjdk.org/jdk/pull/24012#issuecomment-2720726016
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public [v2]
On Thu, 13 Mar 2025 07:39:20 GMT, Christoph Langer wrote: >> This PR addresses an issue that can be observed when building on Windows >> with configure options `--enable-linkable-runtime` and >> `--with-external-symbols-in-bundles=public`. >> >> The problem is that with this special build configuration, we build two sets >> of .pdb files for the binaries. The first set is the standard debug symbols >> files named .pdb. The second set consists of stripped debug >> symbols file called .stripped.pdb which have less information >> but enough to present line numbers in hs-err files. >> >> During build we use the *.stripped.pdb files for compiling the jmods and >> also the bundle files. However, in the images folder, both sets of .pdb >> files exist. The tests for runtime linking will now, in the absence of jmod >> files, pick up the .pdb files (without *stripped*) from images, but in the >> runtime the hashes of the *stripped* files are stored. >> >> With this change, the standard .pdb files in the >> `--with-external-symbols-in-bundles=public` configuration are now the >> stripped files and we create a set of full pdb files named *.full.pdb. Jmods >> and Bundles still contain the stripped pdb files and we also fix the issue >> that the debug symbols bundle also contained stripped pdb files so far. With >> this fix, it will contain the full pdb files and extracting these over a JDK >> runtime will replace stripped pdbs with full pdbs. > > Christoph Langer has updated the pull request incrementally with one > additional commit since the last revision: > > Fix tests Maybe, I misunderstood how this works... I'm not a Windows person. > My take is that we should be able to both test and debug what we ship. Agreed. What I don't understand why this inconsistency is being done to begin with? To appease testing maybe? Is that the idea? > During build we use the *.stripped.pdb files for compiling the jmods and also > the bundle files. However, in the images folder, both sets of .pdb files > exist. The tests for runtime linking will now, in the absence of jmod files, > pick up the .pdb files (without _stripped_) from images, but in the runtime > the hashes of the _stripped_ files are stored. I'll add here that the implementation of JEP 493, does nothing special. It takes what's in the JMODs *at build time* when `jlink` is being run and uses that as the "source of truth" when it comes to hash sums. It sounds to me that when `jlink` runs during the build it only sees `*.stripped.pdb` files (renamed as `*.pdb`). If something is being done **after** the `jdk` image has been created (by `jlink`) and changes files (like copying full `*.pdb` files over the stripped ones that are actually in the JMODs) then this is the root cause of this bug. Rightfully so, since the `*.pdb` files in the JDK image no longer match the ones in the JMOD files. Is that the case? @RealCLanger Could you please run these commands for me and let me know the output? Sorry, I don't know the windows equivalents of these: $ jmod extract --dir java.base-jmod-extract images\jmods\java.base.jmod $ sha512sum java.base-jmod-extract\bin\keytool.exe.pdb 5df21e93cd24a03e491debd469001fc73950812de2d2b5d6df0272b55e9d206b56a4332a9f77e19e8755da12dc43f1c353575503b1f53d3d894794e19632964a java.base-jmod-extract\bin\keytool.exe.pdb $ jimage extract --dir lib-modules-extract images\jdk\lib\modules $ grep keytool.exe.pdb lib-modules-extract\jdk.jlink\jdk\tools\jlink\internal\runtimelink\fs_java.base_files 6|0|5df21e93cd24a03e491debd469001fc73950812de2d2b5d6df0272b55e9d206b56a4332a9f77e19e8755da12dc43f1c353575503b1f53d3d894794e19632964a|bin\keytool.exe.pdb $ sha512sum images\jdk\bin\keytool.exe.pdb - PR Comment: https://git.openjdk.org/jdk/pull/24012#issuecomment-2720746544
Re: RFR: 8351542: LIBMANAGEMENT_OPTIMIZATION remove special optimization settings [v5]
On Wed, 12 Mar 2025 12:45:48 GMT, Matthias Baesken wrote: >> On Linux there are some special settings for LIBMANAGEMENT_OPTIMIZATION that >> are most likely not needed any more and could be removed. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > move code back in VMManagementImpl Thanks for the review and testing in your CI ! Kevin/Erik , are you fine with the change as it is now? - PR Comment: https://git.openjdk.org/jdk/pull/23966#issuecomment-2720437258
Re: RFR: 8351842: Test issues on Windows with combination of --enable-linkable-runtime and --with-external-symbols-in-bundles=public [v2]
On Thu, 13 Mar 2025 07:39:20 GMT, Christoph Langer wrote: >> This PR addresses an issue that can be observed when building on Windows >> with configure options `--enable-linkable-runtime` and >> `--with-external-symbols-in-bundles=public`. >> >> The problem is that with this special build configuration, we build two sets >> of .pdb files for the binaries. The first set is the standard debug symbols >> files named .pdb. The second set consists of stripped debug >> symbols file called .stripped.pdb which have less information >> but enough to present line numbers in hs-err files. >> >> During build we use the *.stripped.pdb files for compiling the jmods and >> also the bundle files. However, in the images folder, both sets of .pdb >> files exist. The tests for runtime linking will now, in the absence of jmod >> files, pick up the .pdb files (without *stripped*) from images, but in the >> runtime the hashes of the *stripped* files are stored. >> >> With this change, the standard .pdb files in the >> `--with-external-symbols-in-bundles=public` configuration are now the >> stripped files and we create a set of full pdb files named *.full.pdb. Jmods >> and Bundles still contain the stripped pdb files and we also fix the issue >> that the debug symbols bundle also contained stripped pdb files so far. With >> this fix, it will contain the full pdb files and extracting these over a JDK >> runtime will replace stripped pdbs with full pdbs. > > Christoph Langer has updated the pull request incrementally with one > additional commit since the last revision: > > Fix tests First of all, after giving this a spin in our testing I found that I needed to address two new test problems. If we go with this change, we need to align JmodExcludedFiles with the new naming (*.full.pdb instead of *.stripped.pdb), otherwise this test would fail in ` --with-external-symbols-in-bundles=public` mode. Also, with the standard pdb file not being the stripped one, the gtest `dll_address_to_function_and_library_name` fails in its current form because it doesn't print the symbol name any more but only an address. So I weakened the condition on Windows a little to make the test pass. Probably something for the eyes of @tstuefe. Now, regarding the principal point opened by Erik, I think it is somehow valid. If we were to build with `--with-external-symbols-in-bundles=public`, the debugging experience for a developer is a bit worse. On the other hand, I agree with Severin, developers should probably not use this for their local builds, it's rather a shipment configuration. The alternative would be to exclude some 50+ test cases when using linkable runtime on Windows, which is no good either. Or should there be an alternative image directory used for these kinds of test? Or should jlink get some workaround coding to check whether *stripped.pdb files are present and try to prefer these when building runtimes? - PR Comment: https://git.openjdk.org/jdk/pull/24012#issuecomment-2720268571
