Re: RFR: 8288396: Always create reproducible builds [v2]
On Tue, 14 Jun 2022 11:54:40 GMT, Erik Joelsson wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fix exitTransportWithError signature > > make/autoconf/flags-ldflags.m4 line 132: > >> 130: >> 131: if test "x$TOOLCHAIN_TYPE" = xmicrosoft; then >> 132: REPRODUCIBLE_LDFLAGS="-experimental:deterministic" > > For the cflag, we check that the compiler supports it, but for the linker > flag you are just setting it without a check. Before this patch, if we got > here and ENABLE_REPRODUCIBLE_BUILD was true, it meant that the test had > passed for the compiler, from which we could assume it would also work for > the linker, but that is no longer the case. Good point. - PR: https://git.openjdk.org/jdk/pull/9152
Re: RFR: 8288396: Always create reproducible builds [v2]
On Tue, 14 Jun 2022 10:09:37 GMT, Magnus Ihse Bursie wrote: >> When we started introducing some possibly more intrusive compiler flags and >> functionality for reproducible builds, we also introduced a flag to turn >> this off out of an abundance of caution. But we have been been using this >> configuration for a year or so internally within Oracle, with no issues. So >> there's really no reason to be able to turn this off. (If you were to ask >> me, the fact that compilers and build tools ever started to produce >> non-deterministic output has been a bug from day one.) >> >> With this fix, all randomness should be gone from our builds, at least on >> linux and windows. There are no more `__DATE__` and `__TIME__` macros in the >> source code. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Fix exitTransportWithError signature I made some additional cleanup associated with shmem. I think that `sysAssert` can (and probably should) be replaced with `SHMEM_ASSERT`. I can fix that as well, if someone from serviceability says that I should do it. - PR: https://git.openjdk.org/jdk/pull/9152
Re: RFR: 8288396: Always create reproducible builds [v2]
> When we started introducing some possibly more intrusive compiler flags and > functionality for reproducible builds, we also introduced a flag to turn this > off out of an abundance of caution. But we have been been using this > configuration for a year or so internally within Oracle, with no issues. So > there's really no reason to be able to turn this off. (If you were to ask me, > the fact that compilers and build tools ever started to produce > non-deterministic output has been a bug from day one.) > > With this fix, all randomness should be gone from our builds, at least on > linux and windows. There are no more `__DATE__` and `__TIME__` macros in the > source code. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Fix exitTransportWithError signature - Changes: - all: https://git.openjdk.org/jdk/pull/9152/files - new: https://git.openjdk.org/jdk/pull/9152/files/8bc40ddb..e2f5fc05 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9152=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9152=00-01 Stats: 33 lines in 5 files changed: 0 ins; 27 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/9152.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9152/head:pull/9152 PR: https://git.openjdk.org/jdk/pull/9152
Re: RFR: 8288396: Always create reproducible builds
On Tue, 14 Jun 2022 09:48:25 GMT, Magnus Ihse Bursie wrote: > When we started introducing some possibly more intrusive compiler flags and > functionality for reproducible builds, we also introduced a flag to turn this > off out of an abundance of caution. But we have been been using this > configuration for a year or so internally within Oracle, with no issues. So > there's really no reason to be able to turn this off. (If you were to ask me, > the fact that compilers and build tools ever started to produce > non-deterministic output has been a bug from day one.) > > With this fix, all randomness should be gone from our builds, at least on > linux and windows. There are no more `__DATE__` and `__TIME__` macros in the > source code. This PR also include a more "radical" version of JDK-8287894, which probably should have been adopted by JDK-8287894 in the first place. There is no need to include the build date in the assert strings for shmem on Windows. - PR: https://git.openjdk.org/jdk/pull/9152
RFR: 8288396: Always create reproducible builds
When we started introducing some possibly more intrusive compiler flags and functionality for reproducible builds, we also introduced a flag to turn this off out of an abundance of caution. But we have been been using this configuration for a year or so internally within Oracle, with no issues. So there's really no reason to be able to turn this off. (If you were to ask me, the fact that compilers and build tools ever started to produce non-deterministic output has been a bug from day one.) With this fix, all randomness should be gone from our builds, at least on linux and windows. There are no more `__DATE__` and `__TIME__` macros in the source code. - Commit messages: - 8288396: Always create reproducible builds Changes: https://git.openjdk.org/jdk/pull/9152/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9152=00 Issue: https://bugs.openjdk.org/browse/JDK-8288396 Stats: 94 lines in 15 files changed: 11 ins; 63 del; 20 mod Patch: https://git.openjdk.org/jdk/pull/9152.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9152/head:pull/9152 PR: https://git.openjdk.org/jdk/pull/9152
Re: RFR: 8288001: SOURCE_DATE_EPOCH should not be set by default
On Mon, 13 Jun 2022 06:50:54 GMT, KIRIYAMA Takuya wrote: >> At default configuration, SOURCE_DATE_EPOCH is exported as environment >> variable in SetupReproducibleBuild. Then, gcc is affected of >> SOURCE_DATE_EPOCH environment variable. This value is used only to set >> SOURCE_DATE_ISO_8601 (except below), so I removed "export" for >> SOURCE_DATE_EPOCH in SetupReproducibleBuild. And, at building ct.sym, >> SOURCE_DATE_EPOCH environment variable is needed. So I added setting routine >> if SOURCE_DATE_EPOCH is not set. >> >> This fix works fine. With default configuration shows -Xinternalversion >> output same as Windows, and with --with-source-date configuration shows >> -Xinternalversion output specified timestamp. Would you please review this >> fix? > > Thank you for your comments. > I understood the goal of reproducible build. But now, > ENABLE_REPRODUCIBLE_BUILD is set to false in default configuration. > Then I think minimize the effort of SOURCE_DATE_EPOCH when reproducible build > is disabled. I wonder why build time information is different from Windows > and Linux. @tkiriyama I have created https://bugs.openjdk.org/browse/JDK-8288396 for the approach I championed above, i.e. to always build reproducible instead. As part of this, the `-Xinternalversion` time stamp should be of the same format for all platforms, given the same set of options to `configure`. The associated PR is https://github.com/openjdk/jdk/pull/9152. Can you verify if this solves your issues, and if so, close this bug? - PR: https://git.openjdk.org/jdk/pull/9081
Integrated: 8287906: Rewrite of GitHub Actions (GHA) sanity tests
On Tue, 7 Jun 2022 13:05:49 GMT, Magnus Ihse Bursie wrote: > With project Skara, the ability to run a set of sanity build and test jobs on > selected platforms was added. This functionality was driven by > `.github/workflows/submit.yml`. This file unfortunately lacks any real > structure, and contains a lot of code duplication and redundancy. This has > made it hard to add functionality, and new platforms to test, and it has made > it even harder to debug issues. (This is hard enough as it is, since we have > no direct access to the platforms that GHA runs on.) > > Since the GHA tests are important for a large subset of the community, we > need to do better. > > ## GitHub Actions framework rewrite > > This is a complete overhaul of the GHA testing framework. I started out > trying to just tease the old `submit.yml` apart, trying to de-duplicate code, > but I soon realized a much more thorough rework was needed. > > ### Design description > > The principle for the new design was to avoid code duplication, and to > improve readability of the code. The latter is extra important since the GHA > "language" is very limited, needs a lot of quirks and workarounds, and is > probably not well known by many OpenJDK developers. I've strived to find > useful layers of abstraction to make the expressions as clear as possible. > > Unfortunately, the Workflow/Action YAML language is quite limited. There are > two ways to avoid duplication, "local composite actions" and "callable > workflows". They both have several limitations: > > * "Callable workflows" can only be used in a single redirection. They are > (apparently) inlined into the "calling workflow" at run time, and as such, > they are present without having to check out the source code. (Which is a > lengthy process.) > > * "Local composite actions" can use other actions, but you must start by > checking out the repo. > > To use the strength of both kinds of sub-modules, I'm using "callable > workflows" from `main.yml` to call `build-.yml` and `test.yml`. It > is not allowed to mix "strategies" (that is, the method of automatically > creating a test matrix) when calling callable workflows, so I needed to have > some amount of duplication in `main.yml` that could have been avoided > otherwise. > > All the callable workflows need to check out the source code anyway, so there > is no real additional cost of using "local composite actions" for abstraction > of these workflows. (A bit of a lucky break.) I've created "high level" > actions, corresponding to something like a function call. The goal here was > both to avoid duplication, and to improve readability of the workflows. > > The four `build-.yml` files are very similar. But in the end of the > day, only like 50% of the source code is shared, and the platform specific > changes permeate the files. So I decided to keep them separately, since > mixing them all into one would have made a mess, due to the lack of proper > abstraction mechanisms. But that also mean that if we change platform > independent code in building, we need to remember to update it in all four > places. > > In the strictest sense, this is a "refactoring" in that the functionality > should be equal to the old `submit.yml`. The same platforms should build, > with the same arguments, and the same tests should run. When I look at the > code now, I see lots of potential for improvement here, by rethinking what we > do run. But let's save that discussion for the next PR. > > There is one major change, though. Windows is no longer running on Cygwin, > but on MSYS2. This was not really triggered by the recurring build issues on > Cygwin (though that certainly did help me in thinking I made the right > choice), but the sheer impossibility of getting Cygwin to behave as a normal > unix shell on GHA Windows hosts. I spent countless hours trying to work out > limitations, by setting `SHELLOPTS=igncr`, by running `set +x posix` to turn > of the POSIX compliance mode that kept turning on by itself and made bash > choke on several of our scripts, by playing tricks with the `PATH`, but in > the end to no avail. There were no single combination of hacks and > workarounds that could get us past the entire chain from configure, to build, > to testing. (The old solution user PowerShell instead to get around these > limitations.) I'm happy to report that I have had absolutely zero issues with > MSYS2 since I made the switch (and understood how to set the PATH properly), > and I'm seriously co nsidering switching stance to recommend using MSYS2 instead of Cygw
Re: RFR: 8288001: SOURCE_DATE_EPOCH should not be set by default
On Mon, 13 Jun 2022 09:47:48 GMT, Severin Gehwolf wrote: >> What do you mean by "build time information"? > > @magicus I think it's `-Xinternalversion` which has different output between > Windows and Linux of the same build. But to me that's a feature not a bug. > From the PR description: > >> With default configuration shows `-Xinternalversion` output same as Windows, >> [...] @jerboaa I agree. If anything, this PR makes me want to remove the option to *not* build reproducible. That way, there will be no discrepancies between platforms. - PR: https://git.openjdk.org/jdk/pull/9081
Re: RFR: 8288001: SOURCE_DATE_EPOCH should not be set by default
On Wed, 8 Jun 2022 07:47:24 GMT, KIRIYAMA Takuya wrote: > At default configuration, SOURCE_DATE_EPOCH is exported as environment > variable in SetupReproducibleBuild. Then, gcc is affected of > SOURCE_DATE_EPOCH environment variable. This value is used only to set > SOURCE_DATE_ISO_8601 (except below), so I removed "export" for > SOURCE_DATE_EPOCH in SetupReproducibleBuild. And, at building ct.sym, > SOURCE_DATE_EPOCH environment variable is needed. So I added setting routine > if SOURCE_DATE_EPOCH is not set. > > This fix works fine. With default configuration shows -Xinternalversion > output same as Windows, and with --with-source-date configuration shows > -Xinternalversion output specified timestamp. Would you please review this > fix? What do you mean by "build time information"? - PR: https://git.openjdk.org/jdk/pull/9081
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v12]
On Mon, 13 Jun 2022 06:45:49 GMT, Magnus Ihse Bursie wrote: >> With project Skara, the ability to run a set of sanity build and test jobs >> on selected platforms was added. This functionality was driven by >> `.github/workflows/submit.yml`. This file unfortunately lacks any real >> structure, and contains a lot of code duplication and redundancy. This has >> made it hard to add functionality, and new platforms to test, and it has >> made it even harder to debug issues. (This is hard enough as it is, since we >> have no direct access to the platforms that GHA runs on.) >> >> Since the GHA tests are important for a large subset of the community, we >> need to do better. >> >> ## GitHub Actions framework rewrite >> >> This is a complete overhaul of the GHA testing framework. I started out >> trying to just tease the old `submit.yml` apart, trying to de-duplicate >> code, but I soon realized a much more thorough rework was needed. >> >> ### Design description >> >> The principle for the new design was to avoid code duplication, and to >> improve readability of the code. The latter is extra important since the GHA >> "language" is very limited, needs a lot of quirks and workarounds, and is >> probably not well known by many OpenJDK developers. I've strived to find >> useful layers of abstraction to make the expressions as clear as possible. >> >> Unfortunately, the Workflow/Action YAML language is quite limited. There are >> two ways to avoid duplication, "local composite actions" and "callable >> workflows". They both have several limitations: >> >> * "Callable workflows" can only be used in a single redirection. They are >> (apparently) inlined into the "calling workflow" at run time, and as such, >> they are present without having to check out the source code. (Which is a >> lengthy process.) >> >> * "Local composite actions" can use other actions, but you must start by >> checking out the repo. >> >> To use the strength of both kinds of sub-modules, I'm using "callable >> workflows" from `main.yml` to call `build-.yml` and `test.yml`. It >> is not allowed to mix "strategies" (that is, the method of automatically >> creating a test matrix) when calling callable workflows, so I needed to have >> some amount of duplication in `main.yml` that could have been avoided >> otherwise. >> >> All the callable workflows need to check out the source code anyway, so >> there is no real additional cost of using "local composite actions" for >> abstraction of these workflows. (A bit of a lucky break.) I've created "high >> level" actions, corresponding to something like a function call. The goal >> here was both to avoid duplication, and to improve readability of the >> workflows. >> >> The four `build-.yml` files are very similar. But in the end of >> the day, only like 50% of the source code is shared, and the platform >> specific changes permeate the files. So I decided to keep them separately, >> since mixing them all into one would have made a mess, due to the lack of >> proper abstraction mechanisms. But that also mean that if we change platform >> independent code in building, we need to remember to update it in all four >> places. >> >> In the strictest sense, this is a "refactoring" in that the functionality >> should be equal to the old `submit.yml`. The same platforms should build, >> with the same arguments, and the same tests should run. When I look at the >> code now, I see lots of potential for improvement here, by rethinking what >> we do run. But let's save that discussion for the next PR. >> >> There is one major change, though. Windows is no longer running on Cygwin, >> but on MSYS2. This was not really triggered by the recurring build issues on >> Cygwin (though that certainly did help me in thinking I made the right >> choice), but the sheer impossibility of getting Cygwin to behave as a normal >> unix shell on GHA Windows hosts. I spent countless hours trying to work out >> limitations, by setting `SHELLOPTS=igncr`, by running `set +x posix` to turn >> of the POSIX compliance mode that kept turning on by itself and made bash >> choke on several of our scripts, by playing tricks with the `PATH`, but in >> the end to no avail. There were no single combination of hacks and >> workarounds that could get us past the entire chain from configure, to >> build, to testing. (T
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v12]
> ### Example run > > A good example on how a run looks like with the new GHA system is [the run > for this PR](https://github.com/magicus/jdk/actions/runs/2454577164). > > ### New features > > While the primary focus was to convert the old system to a new framework, > more accommodating to development, and to wait with further enhancements for > the future, I have made a few additional features already in this PR. Most of > them are related to needs that arose during development of this PR. > > * A build failure summary, similar to the recently added test failure > summary, is added when the build step fails > > * The test reporting has been extended to all platforms, including Windows > > * Test reporting has been improved slightly, and gotten multiple bug fixes > > * All artifacts are now available for individual download. This includes: > > * The build bundles, per platform > * The test results, per platform and test suite > * Build failure logs, in case of build failure > > The build bundles have a retention period of 24 h, but the rest uses > GitHub's default retention period (currently 90 days). The idea is that you > can use GHA to download builds for platforms you might not have access to, > but after that, conserving the builds does not make sense. GitHub currently > provides free, unlimited storage (within the retention period) for artifacts, > so we can afford this. > > * The GHA process starts up much faster, which mean that e.g. a build failure > on an exotic platform will show up earlier. This will not really affect the > overall run time though, since it is bounded by variables such as queuing for > workers, and waiting on tests with somewhat arbitrarily run times to finish. > > ### Additional changes outside GHA > > I also needed to make a few tweaks to the build system to play nice with the > new GHA code. > > * The build failure summary is now stored in > build/$BUILD/make-support/failure-summary.log > > * The configure summary now indicates what devkit or sysroot is used, if any > > * The --with-sysroot argument is now properly normalized > > ### Test failures > > A handful of tests, which relies on shell behavior, turned out to fail on > Windows when running under MSYS2. I have filed separate bugs, and submitted > PRs, to get these fixed: > > * https://bugs.openjdk.org/browse/JDK-8287902 > > * https://bugs.openjdk.org/browse/JDK-8287895 Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Remove bundle artifacts - Changes: - all: https://git.openjdk.org/jdk/pull/9063/files - new: https://git.openjdk.org/jdk/pull/9063/files/385bbd72..c0f9bb1e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9063=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9063=10-11 Stats: 47 lines in 1 file changed: 47 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/9063.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9063/head:pull/9063 PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v11]
> ### Example run > > A good example on how a run looks like with the new GHA system is [the run > for this PR](https://github.com/magicus/jdk/actions/runs/2454577164). > > ### New features > > While the primary focus was to convert the old system to a new framework, > more accommodating to development, and to wait with further enhancements for > the future, I have made a few additional features already in this PR. Most of > them are related to needs that arose during development of this PR. > > * A build failure summary, similar to the recently added test failure > summary, is added when the build step fails > > * The test reporting has been extended to all platforms, including Windows > > * Test reporting has been improved slightly, and gotten multiple bug fixes > > * All artifacts are now available for individual download. This includes: > > * The build bundles, per platform > * The test results, per platform and test suite > * Build failure logs, in case of build failure > > The build bundles have a retention period of 24 h, but the rest uses > GitHub's default retention period (currently 90 days). The idea is that you > can use GHA to download builds for platforms you might not have access to, > but after that, conserving the builds does not make sense. GitHub currently > provides free, unlimited storage (within the retention period) for artifacts, > so we can afford this. > > * The GHA process starts up much faster, which mean that e.g. a build failure > on an exotic platform will show up earlier. This will not really affect the > overall run time though, since it is bounded by variables such as queuing for > workers, and waiting on tests with somewhat arbitrarily run times to finish. > > ### Additional changes outside GHA > > I also needed to make a few tweaks to the build system to play nice with the > new GHA code. > > * The build failure summary is now stored in > build/$BUILD/make-support/failure-summary.log > > * The configure summary now indicates what devkit or sysroot is used, if any > > * The --with-sysroot argument is now properly normalized > > ### Test failures > > A handful of tests, which relies on shell behavior, turned out to fail on > Windows when running under MSYS2. I have filed separate bugs, and submitted > PRs, to get these fixed: > > * https://bugs.openjdk.org/browse/JDK-8287902 > > * https://bugs.openjdk.org/browse/JDK-8287895 Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Use jtreg 6.1+2 now that it is available - Changes: - all: https://git.openjdk.org/jdk/pull/9063/files - new: https://git.openjdk.org/jdk/pull/9063/files/34b5514b..385bbd72 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9063=10 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9063=09-10 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/9063.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9063/head:pull/9063 PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 8288114: Update JIRA link in vcs.xml
On Fri, 10 Jun 2022 17:11:41 GMT, Alexey Ivanov wrote: > Update the link to JBS in `vcs.xml` template to https://bugs.openjdk.org/ > > It will affect newly generated project files only. > Edit `vcs.xml` manually or in UI to update in existing projects. Marked as reviewed by ihse (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9130
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v4]
On Fri, 10 Jun 2022 07:29:49 GMT, Aleksey Shipilev wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Apparently that was not a legal reference to actions/checkout. Try another >> way. > > This all looks very nice. I have a few inline comments. > > The general one is that we need to make sure: > - jtreg needs to be built from its mainline at a given tag; > - tests need to pass (I assume you are waiting for test fixes to land first); @shipilev JDK-8287902 and JDK-8287895 are now fixed, and with that, all msys2-related test problems should be resolved. - PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v10]
> ### Example run > > A good example on how a run looks like with the new GHA system is [the run > for this PR](https://github.com/magicus/jdk/actions/runs/2454577164). > > ### New features > > While the primary focus was to convert the old system to a new framework, > more accommodating to development, and to wait with further enhancements for > the future, I have made a few additional features already in this PR. Most of > them are related to needs that arose during development of this PR. > > * A build failure summary, similar to the recently added test failure > summary, is added when the build step fails > > * The test reporting has been extended to all platforms, including Windows > > * Test reporting has been improved slightly, and gotten multiple bug fixes > > * All artifacts are now available for individual download. This includes: > > * The build bundles, per platform > * The test results, per platform and test suite > * Build failure logs, in case of build failure > > The build bundles have a retention period of 24 h, but the rest uses > GitHub's default retention period (currently 90 days). The idea is that you > can use GHA to download builds for platforms you might not have access to, > but after that, conserving the builds does not make sense. GitHub currently > provides free, unlimited storage (within the retention period) for artifacts, > so we can afford this. > > * The GHA process starts up much faster, which mean that e.g. a build failure > on an exotic platform will show up earlier. This will not really affect the > overall run time though, since it is bounded by variables such as queuing for > workers, and waiting on tests with somewhat arbitrarily run times to finish. > > ### Additional changes outside GHA > > I also needed to make a few tweaks to the build system to play nice with the > new GHA code. > > * The build failure summary is now stored in > build/$BUILD/make-support/failure-summary.log > > * The configure summary now indicates what devkit or sysroot is used, if any > > * The --with-sysroot argument is now properly normalized > > ### Test failures > > A handful of tests, which relies on shell behavior, turned out to fail on > Windows when running under MSYS2. I have filed separate bugs, and submitted > PRs, to get these fixed: > > * https://bugs.openjdk.org/browse/JDK-8287902 > > * https://bugs.openjdk.org/browse/JDK-8287895 Magnus Ihse Bursie has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision: - Merge branch 'master' into gha-refactor - Merge branch 'master' into gha-refactor - Clean up apt architecture handling, inspired by suggestions in code review - https instead of http - Clean up cross compilation according to code review - Use MSYS2 as name as consistently as possible - Fix test failure regex - Apparently that was not a legal reference to actions/checkout. Try another way. - Use Java 11 to build jtreg. Also temporarily switch out jtreg source ref to PR that fixes build on msys2. (This will be updated once the fix is in an official jtreg release.) - Build JTReg from source instead of downloading pre-built binary - ... and 1 more: https://git.openjdk.org/jdk/compare/836b39c5...34b5514b - Changes: - all: https://git.openjdk.org/jdk/pull/9063/files - new: https://git.openjdk.org/jdk/pull/9063/files/463f8862..34b5514b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9063=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9063=08-09 Stats: 10618 lines in 225 files changed: 9052 ins; 491 del; 1075 mod Patch: https://git.openjdk.org/jdk/pull/9063.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9063/head:pull/9063 PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v9]
> ### Example run > > A good example on how a run looks like with the new GHA system is [the run > for this PR](https://github.com/magicus/jdk/actions/runs/2454577164). > > ### New features > > While the primary focus was to convert the old system to a new framework, > more accommodating to development, and to wait with further enhancements for > the future, I have made a few additional features already in this PR. Most of > them are related to needs that arose during development of this PR. > > * A build failure summary, similar to the recently added test failure > summary, is added when the build step fails > > * The test reporting has been extended to all platforms, including Windows > > * Test reporting has been improved slightly, and gotten multiple bug fixes > > * All artifacts are now available for individual download. This includes: > > * The build bundles, per platform > * The test results, per platform and test suite > * Build failure logs, in case of build failure > > The build bundles have a retention period of 24 h, but the rest uses > GitHub's default retention period (currently 90 days). The idea is that you > can use GHA to download builds for platforms you might not have access to, > but after that, conserving the builds does not make sense. GitHub currently > provides free, unlimited storage (within the retention period) for artifacts, > so we can afford this. > > * The GHA process starts up much faster, which mean that e.g. a build failure > on an exotic platform will show up earlier. This will not really affect the > overall run time though, since it is bounded by variables such as queuing for > workers, and waiting on tests with somewhat arbitrarily run times to finish. > > ### Additional changes outside GHA > > I also needed to make a few tweaks to the build system to play nice with the > new GHA code. > > * The build failure summary is now stored in > build/$BUILD/make-support/failure-summary.log > > * The configure summary now indicates what devkit or sysroot is used, if any > > * The --with-sysroot argument is now properly normalized > > ### Test failures > > A handful of tests, which relies on shell behavior, turned out to fail on > Windows when running under MSYS2. I have filed separate bugs, and submitted > PRs, to get these fixed: > > * https://bugs.openjdk.org/browse/JDK-8287902 > > * https://bugs.openjdk.org/browse/JDK-8287895 Magnus Ihse Bursie has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 10 additional commits since the last revision: - Merge branch 'master' into gha-refactor - Clean up apt architecture handling, inspired by suggestions in code review - https instead of http - Clean up cross compilation according to code review - Use MSYS2 as name as consistently as possible - Fix test failure regex - Apparently that was not a legal reference to actions/checkout. Try another way. - Use Java 11 to build jtreg. Also temporarily switch out jtreg source ref to PR that fixes build on msys2. (This will be updated once the fix is in an official jtreg release.) - Build JTReg from source instead of downloading pre-built binary - 8287906: Rewrite of GitHub Actions (GHA) sanity tests - Changes: - all: https://git.openjdk.org/jdk/pull/9063/files - new: https://git.openjdk.org/jdk/pull/9063/files/2ebeee99..463f8862 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9063=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9063=07-08 Stats: 1839 lines in 93 files changed: 1186 ins; 242 del; 411 mod Patch: https://git.openjdk.org/jdk/pull/9063.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9063/head:pull/9063 PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v4]
On Fri, 10 Jun 2022 11:23:57 GMT, Aleksey Shipilev wrote: >> If I do that, there need to be some kind of if statement in the called >> workflow, since if that input argument is left out, we'd get a command line >> like `sudo dpkg --add-architecture` which I assume is illegal syntax (or, >> possibly worse, does something other than a no-op). >> >> I think there can be room for additional improvement in especially the >> "special" linux builds, but I had to draw the line somewhere, to be able to >> finish this PR. > > I think you can make a conditionalized step that runs in case the value is > not empty, and skip the step otherwise. If that is hard, it can be done in > the followup, sure. I cleaned up the entire "apt architecture" handling a bit. Some part of the code got more complex, but overall I think it was worth it due to the higher level of abstraction. - PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v8]
> ### Example run > > A good example on how a run looks like with the new GHA system is [the run > for this PR](https://github.com/magicus/jdk/actions/runs/2454577164). > > ### New features > > While the primary focus was to convert the old system to a new framework, > more accommodating to development, and to wait with further enhancements for > the future, I have made a few additional features already in this PR. Most of > them are related to needs that arose during development of this PR. > > * A build failure summary, similar to the recently added test failure > summary, is added when the build step fails > > * The test reporting has been extended to all platforms, including Windows > > * Test reporting has been improved slightly, and gotten multiple bug fixes > > * All artifacts are now available for individual download. This includes: > > * The build bundles, per platform > * The test results, per platform and test suite > * Build failure logs, in case of build failure > > The build bundles have a retention period of 24 h, but the rest uses > GitHub's default retention period (currently 90 days). The idea is that you > can use GHA to download builds for platforms you might not have access to, > but after that, conserving the builds does not make sense. GitHub currently > provides free, unlimited storage (within the retention period) for artifacts, > so we can afford this. > > * The GHA process starts up much faster, which mean that e.g. a build failure > on an exotic platform will show up earlier. This will not really affect the > overall run time though, since it is bounded by variables such as queuing for > workers, and waiting on tests with somewhat arbitrarily run times to finish. > > ### Additional changes outside GHA > > I also needed to make a few tweaks to the build system to play nice with the > new GHA code. > > * The build failure summary is now stored in > build/$BUILD/make-support/failure-summary.log > > * The configure summary now indicates what devkit or sysroot is used, if any > > * The --with-sysroot argument is now properly normalized > > ### Test failures > > A handful of tests, which relies on shell behavior, turned out to fail on > Windows when running under MSYS2. I have filed separate bugs, and submitted > PRs, to get these fixed: > > * https://bugs.openjdk.org/browse/JDK-8287902 > > * https://bugs.openjdk.org/browse/JDK-8287895 Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Clean up apt architecture handling, inspired by suggestions in code review - Changes: - all: https://git.openjdk.org/jdk/pull/9063/files - new: https://git.openjdk.org/jdk/pull/9063/files/79f5ee64..2ebeee99 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9063=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9063=06-07 Stats: 19 lines in 3 files changed: 10 ins; 5 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/9063.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9063/head:pull/9063 PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v7]
> ### Example run > > A good example on how a run looks like with the new GHA system is [the run > for this PR](https://github.com/magicus/jdk/actions/runs/2454577164). > > ### New features > > While the primary focus was to convert the old system to a new framework, > more accommodating to development, and to wait with further enhancements for > the future, I have made a few additional features already in this PR. Most of > them are related to needs that arose during development of this PR. > > * A build failure summary, similar to the recently added test failure > summary, is added when the build step fails > > * The test reporting has been extended to all platforms, including Windows > > * Test reporting has been improved slightly, and gotten multiple bug fixes > > * All artifacts are now available for individual download. This includes: > > * The build bundles, per platform > * The test results, per platform and test suite > * Build failure logs, in case of build failure > > The build bundles have a retention period of 24 h, but the rest uses > GitHub's default retention period (currently 90 days). The idea is that you > can use GHA to download builds for platforms you might not have access to, > but after that, conserving the builds does not make sense. GitHub currently > provides free, unlimited storage (within the retention period) for artifacts, > so we can afford this. > > * The GHA process starts up much faster, which mean that e.g. a build failure > on an exotic platform will show up earlier. This will not really affect the > overall run time though, since it is bounded by variables such as queuing for > workers, and waiting on tests with somewhat arbitrarily run times to finish. > > ### Additional changes outside GHA > > I also needed to make a few tweaks to the build system to play nice with the > new GHA code. > > * The build failure summary is now stored in > build/$BUILD/make-support/failure-summary.log > > * The configure summary now indicates what devkit or sysroot is used, if any > > * The --with-sysroot argument is now properly normalized > > ### Test failures > > A handful of tests, which relies on shell behavior, turned out to fail on > Windows when running under MSYS2. I have filed separate bugs, and submitted > PRs, to get these fixed: > > * https://bugs.openjdk.org/browse/JDK-8287902 > > * https://bugs.openjdk.org/browse/JDK-8287895 Magnus Ihse Bursie has updated the pull request incrementally with two additional commits since the last revision: - https instead of http - Clean up cross compilation according to code review - Changes: - all: https://git.openjdk.org/jdk/pull/9063/files - new: https://git.openjdk.org/jdk/pull/9063/files/b17e031c..79f5ee64 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9063=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9063=05-06 Stats: 18 lines in 1 file changed: 13 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/9063.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9063/head:pull/9063 PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v4]
On Fri, 10 Jun 2022 11:22:00 GMT, Aleksey Shipilev wrote: >> I can merge the two `apt-get install` lines, if you say that it is not >> necessary to call `update-alternatives` before the second install line. (But >> does it really speed things up?) > >> (But does it really speed things up?) > > Yes, I think it does: `apt` would read the package database once, and do the > post-install actions once. `update-alternatives` does not have to happen > between those two lines. I hope I made the abstractions of the apt version strings correct. If not, we'll have to adjust it when we upgrade compilers. - PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v6]
> ### Example run > > A good example on how a run looks like with the new GHA system is [the run > for this PR](https://github.com/magicus/jdk/actions/runs/2454577164). > > ### New features > > While the primary focus was to convert the old system to a new framework, > more accommodating to development, and to wait with further enhancements for > the future, I have made a few additional features already in this PR. Most of > them are related to needs that arose during development of this PR. > > * A build failure summary, similar to the recently added test failure > summary, is added when the build step fails > > * The test reporting has been extended to all platforms, including Windows > > * Test reporting has been improved slightly, and gotten multiple bug fixes > > * All artifacts are now available for individual download. This includes: > > * The build bundles, per platform > * The test results, per platform and test suite > * Build failure logs, in case of build failure > > The build bundles have a retention period of 24 h, but the rest uses > GitHub's default retention period (currently 90 days). The idea is that you > can use GHA to download builds for platforms you might not have access to, > but after that, conserving the builds does not make sense. GitHub currently > provides free, unlimited storage (within the retention period) for artifacts, > so we can afford this. > > * The GHA process starts up much faster, which mean that e.g. a build failure > on an exotic platform will show up earlier. This will not really affect the > overall run time though, since it is bounded by variables such as queuing for > workers, and waiting on tests with somewhat arbitrarily run times to finish. > > ### Additional changes outside GHA > > I also needed to make a few tweaks to the build system to play nice with the > new GHA code. > > * The build failure summary is now stored in > build/$BUILD/make-support/failure-summary.log > > * The configure summary now indicates what devkit or sysroot is used, if any > > * The --with-sysroot argument is now properly normalized > > ### Test failures > > A handful of tests, which relies on shell behavior, turned out to fail on > Windows when running under MSYS2. I have filed separate bugs, and submitted > PRs, to get these fixed: > > * https://bugs.openjdk.org/browse/JDK-8287902 > > * https://bugs.openjdk.org/browse/JDK-8287895 Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Use MSYS2 as name as consistently as possible - Changes: - all: https://git.openjdk.org/jdk/pull/9063/files - new: https://git.openjdk.org/jdk/pull/9063/files/8ce043c1..b17e031c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9063=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9063=04-05 Stats: 94 lines in 4 files changed: 44 ins; 46 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/9063.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9063/head:pull/9063 PR: https://git.openjdk.org/jdk/pull/9063
Integrated: 8288195: Prepare build system for GHA changes
On Fri, 10 Jun 2022 09:54:36 GMT, Magnus Ihse Bursie wrote: > A few changes to the build system is needed for the GHA rewrite > ([JDK-8287906](https://bugs.openjdk.org/browse/JDK-8287906)). This pull request has now been integrated. Changeset: d0c8ff8f Author:Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/d0c8ff8fdfe86a4251290d4c1c7b3dbd4cfaf018 Stats: 28 lines in 3 files changed: 13 ins; 0 del; 15 mod 8288195: Prepare build system for GHA changes Reviewed-by: shade, erikj - PR: https://git.openjdk.org/jdk/pull/9122
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v4]
On Fri, 10 Jun 2022 07:23:12 GMT, Aleksey Shipilev wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Apparently that was not a legal reference to actions/checkout. Try another >> way. > > make/InitSupport.gmk line 456: > >> 454: $(PRINTF) "\n* All command lines available in >> $(MAKESUPPORT_OUTPUTDIR)/failure-logs.\n" ; \ >> 455: $(PRINTF) "=== End of repeated output ===\n" ; \ >> 456: ) >> $(MAKESUPPORT_OUTPUTDIR)/failure-summary.log \ > > Changes in this file look like a generic build system improvement, so maybe > it should be forked out as separate issue? In case this part of build system > change regresses something, I would be odd to track it down to GHA workflow > improvement. Integrated as JDK-8288195. - PR: https://git.openjdk.org/jdk/pull/9063
RFR: 8288195: Prepare build system for GHA changes
A few changes to the build system is needed for the GHA rewrite ([JDK-8287906](https://bugs.openjdk.org/browse/JDK-8287906)). - Commit messages: - 8288195: Prepare build system for GHA changes Changes: https://git.openjdk.org/jdk/pull/9122/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9122=00 Issue: https://bugs.openjdk.org/browse/JDK-8288195 Stats: 28 lines in 3 files changed: 13 ins; 0 del; 15 mod Patch: https://git.openjdk.org/jdk/pull/9122.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9122/head:pull/9122 PR: https://git.openjdk.org/jdk/pull/9122
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v4]
On Fri, 10 Jun 2022 07:18:34 GMT, Aleksey Shipilev wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Apparently that was not a legal reference to actions/checkout. Try another >> way. > > .github/workflows/main.yml line 138: > >> 136: apt-gcc-version: '10-multilib' >> 137: apt-architecture: ':i386' >> 138: apt-add-architecture: 'sudo dpkg --add-architecture i386' > > Feels a bit weird to have the entire command line here. I'd expect to see > something like `apt-add-architecture: i386`. If I do that, there need to be some kind of if statement in the called workflow, since if that input argument is left out, we'd get a command line like `sudo dpkg --add-architecture` which I assume is illegal syntax (or, possibly worse, does something other than a no-op). I think there can be room for additional improvement in especially the "special" linux builds, but I had to draw the line somewhere, to be able to finish this PR. - PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v4]
On Fri, 10 Jun 2022 09:43:47 GMT, Magnus Ihse Bursie wrote: >> Also, I think we can speed up this part by merging two `apt-get install` >> invocation lines together. It was separate before, because it was two steps, >> unnecessary now. > > Ideally, all version information should be centralized somewhere. I plan to > take a second round on this code when it's been pushed to try to figure out > the best way to achieve this. (Possibly in github-versions.conf, possibly in > main.yml, depending on what makes the most sense given the GHA limitations). > > But I can extract out the cross-compilation suffix already in this PR, yes. I can merge the two `apt-get install` lines, if you say that it is not necessary to call `update-alternatives` before the second install line. (But does it really speed things up?) - PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v4]
On Fri, 10 Jun 2022 07:14:33 GMT, Aleksey Shipilev wrote:
>> .github/workflows/build-cross-compile.yml line 89:
>>
>>> 87: sudo apt-get install gcc-${{ inputs.apt-gcc-version }}
>>> g++-${{ inputs.apt-gcc-version }} libxrandr-dev${{ inputs.apt-architecture
>>> }} libxtst-dev${{ inputs.apt-architecture }} libcups2-dev${{
>>> inputs.apt-architecture }} libasound2-dev${{ inputs.apt-architecture }}
>>> 88: sudo update-alternatives --install /usr/bin/gcc gcc
>>> /usr/bin/gcc-10 100 --slave /usr/bin/g++ g++ /usr/bin/g++-10
>>> 89: sudo apt-get install gcc-10-${{ matrix.gnu-arch
>>> }}-linux-gnu${{ matrix.gnu-abi}}=10.3.0-1ubuntu1~20.04cross1 g++-10-${{
>>> matrix.gnu-arch }}-linux-gnu${{ matrix.gnu-abi}}=10.3.0-1ubuntu1~20.04cross1
>>
>> Should `10.3.0-1ubuntu1~20.04cross1` also go into common "var", like
>> `apt-gcc-version` did?
>
> Also, I think we can speed up this part by merging two `apt-get install`
> invocation lines together. It was separate before, because it was two steps,
> unnecessary now.
Ideally, all version information should be centralized somewhere. I plan to
take a second round on this code when it's been pushed to try to figure out the
best way to achieve this. (Possibly in github-versions.conf, possibly in
main.yml, depending on what makes the most sense given the GHA limitations).
But I can extract out the cross-compilation suffix already in this PR, yes.
-
PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v4]
On Fri, 10 Jun 2022 07:09:28 GMT, Aleksey Shipilev wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Apparently that was not a legal reference to actions/checkout. Try another >> way. > > .github/actions/get-bootjdk/action.yml line 26: > >> 24: # >> 25: >> 26: name: 'Get BootJDK' > > Here and later, polishing: "BootJDK" -> "boot JDK"? I think we've mostly been using "BootJDK" as a specialized term in the build system, but we've not been very consistent about it: ihse@mercurius:~/git/jdk-ALT/open/make$ gr -i "Boot JDK" | wc 61 6735606 ihse@mercurius:~/git/jdk-ALT/open/make$ gr -i BootJDK | wc 79 2985947 but I can switch it out to `Boot JDK` if that makes you happier. > .github/actions/get-msys/action.yml line 26: > >> 24: # >> 25: >> 26: name: 'Get msys' > > Here and later, polishing: call it consistently "msys2" or "MSYS2"? The official name is `MSYS2` so I've tried to use that, unless lower case were needed/preferred to keep things consistent. I'll check over the code to see what places I've missed. (I agree that in this description it should definitely be MSYS2). - PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v4]
On Fri, 10 Jun 2022 07:07:11 GMT, Aleksey Shipilev wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Apparently that was not a legal reference to actions/checkout. Try another >> way. > > .github/actions/get-bundles/action.yml line 103: > >> 101: jdk_dir="$(cygpath $jdk_dir)" >> 102: symbols_dir="$(cygpath $symbols_dir)" >> 103: tests_dir="$(cygpath $tests_dir)" > > Here, and maybe later: how safe it is to use Cygwin utilities like `cygpath` > in MSYS2 context? Perfectly safe and officially supported. MSYS2 is related to MSYS(1) by name only, it is in reality a fork/clone of Cygwin. The principal difference is that MSYS2 is more pragmatic when it comes to choosing between perfect POSIX compliance, or have tooling that is helpful in creating a POSIX-like build environment on Windows. - PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v5]
> ### Example run > > A good example on how a run looks like with the new GHA system is [the run > for this PR](https://github.com/magicus/jdk/actions/runs/2454577164). > > ### New features > > While the primary focus was to convert the old system to a new framework, > more accommodating to development, and to wait with further enhancements for > the future, I have made a few additional features already in this PR. Most of > them are related to needs that arose during development of this PR. > > * A build failure summary, similar to the recently added test failure > summary, is added when the build step fails > > * The test reporting has been extended to all platforms, including Windows > > * Test reporting has been improved slightly, and gotten multiple bug fixes > > * All artifacts are now available for individual download. This includes: > > * The build bundles, per platform > * The test results, per platform and test suite > * Build failure logs, in case of build failure > > The build bundles have a retention period of 24 h, but the rest uses > GitHub's default retention period (currently 90 days). The idea is that you > can use GHA to download builds for platforms you might not have access to, > but after that, conserving the builds does not make sense. GitHub currently > provides free, unlimited storage (within the retention period) for artifacts, > so we can afford this. > > * The GHA process starts up much faster, which mean that e.g. a build failure > on an exotic platform will show up earlier. This will not really affect the > overall run time though, since it is bounded by variables such as queuing for > workers, and waiting on tests with somewhat arbitrarily run times to finish. > > ### Additional changes outside GHA > > I also needed to make a few tweaks to the build system to play nice with the > new GHA code. > > * The build failure summary is now stored in > build/$BUILD/make-support/failure-summary.log > > * The configure summary now indicates what devkit or sysroot is used, if any > > * The --with-sysroot argument is now properly normalized > > ### Test failures > > A handful of tests, which relies on shell behavior, turned out to fail on > Windows when running under MSYS2. I have filed separate bugs, and submitted > PRs, to get these fixed: > > * https://bugs.openjdk.org/browse/JDK-8287902 > > * https://bugs.openjdk.org/browse/JDK-8287895 Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Fix test failure regex - Changes: - all: https://git.openjdk.org/jdk/pull/9063/files - new: https://git.openjdk.org/jdk/pull/9063/files/9cbf5875..8ce043c1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9063=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9063=03-04 Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/9063.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9063/head:pull/9063 PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v4]
> ### Example run > > A good example on how a run looks like with the new GHA system is [the run > for this PR](https://github.com/magicus/jdk/actions/runs/2454577164). > > ### New features > > While the primary focus was to convert the old system to a new framework, > more accommodating to development, and to wait with further enhancements for > the future, I have made a few additional features already in this PR. Most of > them are related to needs that arose during development of this PR. > > * A build failure summary, similar to the recently added test failure > summary, is added when the build step fails > > * The test reporting has been extended to all platforms, including Windows > > * Test reporting has been improved slightly, and gotten multiple bug fixes > > * All artifacts are now available for individual download. This includes: > > * The build bundles, per platform > * The test results, per platform and test suite > * Build failure logs, in case of build failure > > The build bundles have a retention period of 24 h, but the rest uses > GitHub's default retention period (currently 90 days). The idea is that you > can use GHA to download builds for platforms you might not have access to, > but after that, conserving the builds does not make sense. GitHub currently > provides free, unlimited storage (within the retention period) for artifacts, > so we can afford this. > > * The GHA process starts up much faster, which mean that e.g. a build failure > on an exotic platform will show up earlier. This will not really affect the > overall run time though, since it is bounded by variables such as queuing for > workers, and waiting on tests with somewhat arbitrarily run times to finish. > > ### Additional changes outside GHA > > I also needed to make a few tweaks to the build system to play nice with the > new GHA code. > > * The build failure summary is now stored in > build/$BUILD/make-support/failure-summary.log > > * The configure summary now indicates what devkit or sysroot is used, if any > > * The --with-sysroot argument is now properly normalized > > ### Test failures > > A handful of tests, which relies on shell behavior, turned out to fail on > Windows when running under MSYS2. I have filed separate bugs, and submitted > PRs, to get these fixed: > > * https://bugs.openjdk.org/browse/JDK-8287902 > > * https://bugs.openjdk.org/browse/JDK-8287895 Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Apparently that was not a legal reference to actions/checkout. Try another way. - Changes: - all: https://git.openjdk.java.net/jdk/pull/9063/files - new: https://git.openjdk.java.net/jdk/pull/9063/files/220f9209..9cbf5875 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9063=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9063=02-03 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/9063.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9063/head:pull/9063 PR: https://git.openjdk.java.net/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v3]
On Tue, 7 Jun 2022 22:47:32 GMT, Jonathan Gibbons wrote: >> make/conf/github-actions.conf line 31: >> >>> 29: >>> 30: >>> JTREG_URL=https://ci.adoptopenjdk.net/view/Dependencies/job/dependency_pipeline/330/artifact/jtreg/jtreg-6.1+1.tar.gz >>> 31: >>> JTREG_SHA256=ccfa21f54bb173f818a5a8d93f77d49301f275f0677c9f914297046c910c5129 >> >> This seems questionable, and adds a very suspect blocking dependency on that >> website for when we want to update the version of jtreg to be used. > > There are many issues at play in this PR. Can we handle "where to get jtreg" > separately from the other issues here: rewriting the actions, and whether to > switch to use MSYS2 instead of Cygwin. I reverted to the old method of building jtreg locally, with some changes. I do not attempt to build it before starting all runs, but instead each build job will start by building it if it is not in the cache. This adds < 60 seconds of build time so I think this is fully acceptable to avoid the complexity of the old solution. Most of the time a cache hit is likely to happen. GHA caches are indirectly keyed on runner OS (this also means that the explicit `runner.os` in the cache key for bootjdk is not strictly necessary, but only there to make it more obvious), so in theory this needs to be built at least once for Windows, macOS and Linux. After this, it is cached, and only removed from the cache if not having been accessed for 7 days. The 6.1+1 release of jtreg can not be built on msys2. There is a PR open (https://github.com/openjdk/jtreg/pull/87) with a fix. For the time being, while I address some additional issues in this PR, I have set the JTReg ref to checkout to this PR. Hopefully this fix will be included in a proper JTReg release when this PR is ready for integration. I still think it would be superior to download a prebuilt binary, though. - PR: https://git.openjdk.java.net/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v3]
> ### Example run > > A good example on how a run looks like with the new GHA system is [the run > for this PR](https://github.com/magicus/jdk/actions/runs/2454577164). > > ### New features > > While the primary focus was to convert the old system to a new framework, > more accommodating to development, and to wait with further enhancements for > the future, I have made a few additional features already in this PR. Most of > them are related to needs that arose during development of this PR. > > * A build failure summary, similar to the recently added test failure > summary, is added when the build step fails > > * The test reporting has been extended to all platforms, including Windows > > * Test reporting has been improved slightly, and gotten multiple bug fixes > > * All artifacts are now available for individual download. This includes: > > * The build bundles, per platform > * The test results, per platform and test suite > * Build failure logs, in case of build failure > > The build bundles have a retention period of 24 h, but the rest uses > GitHub's default retention period (currently 90 days). The idea is that you > can use GHA to download builds for platforms you might not have access to, > but after that, conserving the builds does not make sense. GitHub currently > provides free, unlimited storage (within the retention period) for artifacts, > so we can afford this. > > * The GHA process starts up much faster, which mean that e.g. a build failure > on an exotic platform will show up earlier. This will not really affect the > overall run time though, since it is bounded by variables such as queuing for > workers, and waiting on tests with somewhat arbitrarily run times to finish. > > ### Additional changes outside GHA > > I also needed to make a few tweaks to the build system to play nice with the > new GHA code. > > * The build failure summary is now stored in > build/$BUILD/make-support/failure-summary.log > > * The configure summary now indicates what devkit or sysroot is used, if any > > * The --with-sysroot argument is now properly normalized > > ### Test failures > > A handful of tests, which relies on shell behavior, turned out to fail on > Windows when running under MSYS2. I have filed separate bugs, and submitted > PRs, to get these fixed: > > * https://bugs.openjdk.org/browse/JDK-8287902 > > * https://bugs.openjdk.org/browse/JDK-8287895 Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Use Java 11 to build jtreg. Also temporarily switch out jtreg source ref to PR that fixes build on msys2. (This will be updated once the fix is in an official jtreg release.) - Changes: - all: https://git.openjdk.java.net/jdk/pull/9063/files - new: https://git.openjdk.java.net/jdk/pull/9063/files/d75222c2..220f9209 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9063=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9063=01-02 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/9063.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9063/head:pull/9063 PR: https://git.openjdk.java.net/jdk/pull/9063
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v2]
On Wed, 8 Jun 2022 14:46:13 GMT, Magnus Ihse Bursie wrote: >> With project Skara, the ability to run a set of sanity build and test jobs >> on selected platforms was added. This functionality was driven by >> `.github/workflows/submit.yml`. This file unfortunately lacks any real >> structure, and contains a lot of code duplication and redundancy. This has >> made it hard to add functionality, and new platforms to test, and it has >> made it even harder to debug issues. (This is hard enough as it is, since we >> have no direct access to the platforms that GHA runs on.) >> >> Since the GHA tests are important for a large subset of the community, we >> need to do better. >> >> ## GitHub Actions framework rewrite >> >> This is a complete overhaul of the GHA testing framework. I started out >> trying to just tease the old `submit.yml` apart, trying to de-duplicate >> code, but I soon realized a much more thorough rework was needed. >> >> ### Design description >> >> The principle for the new design was to avoid code duplication, and to >> improve readability of the code. The latter is extra important since the GHA >> "language" is very limited, needs a lot of quirks and workarounds, and is >> probably not well known by many OpenJDK developers. I've strived to find >> useful layers of abstraction to make the expressions as clear as possible. >> >> Unfortunately, the Workflow/Action YAML language is quite limited. There are >> two ways to avoid duplication, "local composite actions" and "callable >> workflows". They both have several limitations: >> >> * "Callable workflows" can only be used in a single redirection. They are >> (apparently) inlined into the "calling workflow" at run time, and as such, >> they are present without having to check out the source code. (Which is a >> lengthy process.) >> >> * "Local composite actions" can use other actions, but you must start by >> checking out the repo. >> >> To use the strength of both kinds of sub-modules, I'm using "callable >> workflows" from `main.yml` to call `build-.yml` and `test.yml`. It >> is not allowed to mix "strategies" (that is, the method of automatically >> creating a test matrix) when calling callable workflows, so I needed to have >> some amount of duplication in `main.yml` that could have been avoided >> otherwise. >> >> All the callable workflows need to check out the source code anyway, so >> there is no real additional cost of using "local composite actions" for >> abstraction of these workflows. (A bit of a lucky break.) I've created "high >> level" actions, corresponding to something like a function call. The goal >> here was both to avoid duplication, and to improve readability of the >> workflows. >> >> The four `build-.yml` files are very similar. But in the end of >> the day, only like 50% of the source code is shared, and the platform >> specific changes permeate the files. So I decided to keep them separately, >> since mixing them all into one would have made a mess, due to the lack of >> proper abstraction mechanisms. But that also mean that if we change platform >> independent code in building, we need to remember to update it in all four >> places. >> >> In the strictest sense, this is a "refactoring" in that the functionality >> should be equal to the old `submit.yml`. The same platforms should build, >> with the same arguments, and the same tests should run. When I look at the >> code now, I see lots of potential for improvement here, by rethinking what >> we do run. But let's save that discussion for the next PR. >> >> There is one major change, though. Windows is no longer running on Cygwin, >> but on MSYS2. This was not really triggered by the recurring build issues on >> Cygwin (though that certainly did help me in thinking I made the right >> choice), but the sheer impossibility of getting Cygwin to behave as a normal >> unix shell on GHA Windows hosts. I spent countless hours trying to work out >> limitations, by setting `SHELLOPTS=igncr`, by running `set +x posix` to turn >> of the POSIX compliance mode that kept turning on by itself and made bash >> choke on several of our scripts, by playing tricks with the `PATH`, but in >> the end to no avail. There were no single combination of hacks and >> workarounds that could get us past the entire chain from configure, to >> build, to testing. (T
Re: RFR: 8283724: Incorrect description for jtreg-failure-handler option
On Thu, 9 Jun 2022 06:49:15 GMT, KIRIYAMA Takuya wrote: > The description for the jtreg-failure-handler option is incorrect, so I fixed > it to describe jtreg-failure-handler option. > Would you please review this fix? Marked as reviewed by ihse (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9099
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v2]
> ### Example run > > A good example on how a run looks like with the new GHA system is [the run > for this PR](https://github.com/magicus/jdk/actions/runs/2454577164). > > ### New features > > While the primary focus was to convert the old system to a new framework, > more accommodating to development, and to wait with further enhancements for > the future, I have made a few additional features already in this PR. Most of > them are related to needs that arose during development of this PR. > > * A build failure summary, similar to the recently added test failure > summary, is added when the build step fails > > * The test reporting has been extended to all platforms, including Windows > > * Test reporting has been improved slightly, and gotten multiple bug fixes > > * All artifacts are now available for individual download. This includes: > > * The build bundles, per platform > * The test results, per platform and test suite > * Build failure logs, in case of build failure > > The build bundles have a retention period of 24 h, but the rest uses > GitHub's default retention period (currently 90 days). The idea is that you > can use GHA to download builds for platforms you might not have access to, > but after that, conserving the builds does not make sense. GitHub currently > provides free, unlimited storage (within the retention period) for artifacts, > so we can afford this. > > * The GHA process starts up much faster, which mean that e.g. a build failure > on an exotic platform will show up earlier. This will not really affect the > overall run time though, since it is bounded by variables such as queuing for > workers, and waiting on tests with somewhat arbitrarily run times to finish. > > ### Additional changes outside GHA > > I also needed to make a few tweaks to the build system to play nice with the > new GHA code. > > * The build failure summary is now stored in > build/$BUILD/make-support/failure-summary.log > > * The configure summary now indicates what devkit or sysroot is used, if any > > * The --with-sysroot argument is now properly normalized > > ### Test failures > > A handful of tests, which relies on shell behavior, turned out to fail on > Windows when running under MSYS2. I have filed separate bugs, and submitted > PRs, to get these fixed: > > * https://bugs.openjdk.org/browse/JDK-8287902 > > * https://bugs.openjdk.org/browse/JDK-8287895 Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: Build JTReg from source instead of downloading pre-built binary - Changes: - all: https://git.openjdk.java.net/jdk/pull/9063/files - new: https://git.openjdk.java.net/jdk/pull/9063/files/665451a5..d75222c2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9063=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9063=00-01 Stats: 30 lines in 2 files changed: 8 ins; 9 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/9063.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9063/head:pull/9063 PR: https://git.openjdk.java.net/jdk/pull/9063
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v5]
On Tue, 7 Jun 2022 18:15:30 GMT, Leo Korinth wrote: >> One can select a testcase by ID when running a jtreg test case directly from >> jtreg (using the testcase.java#testID syntax). However, this has not been >> possible to do when launching jtreg indirectly from make. >> >> This fix attempts to address this issue. I have not tested this thoroughly >> yet, I wanted to show the code to get feedback first. The idea is to *not* >> emulated destructive imperative assignments through the use of eval. I >> instead try to handle it in a functional style without reassigning >> variables. I have tried to make the change as small as possible. >> >> I am not used to change the build system, so I would appreciate thorough >> review. >> >> `IfAppend` and `IfPrepend` are general purpose functions. If similar >> functions exists elsewhere inside the code base or in make itself I should >> probably use those instead. If they do not exist, they might be useful >> elsewhere and maybe should be placed in a common make file instead of the >> RunTests.gmk file. > > Leo Korinth has updated the pull request incrementally with one additional > commit since the last revision: > > use test id in comments I apologize for the late review. The latest version look very good indeed, arguably even much better than the original. (Apart from it fixing the bug, that is.) Thank you for contributing to the build system! You're welcome back any time you want. ;-) - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287894: Use fixed timestamp as an alternative of __DATE__ macro in jdk.jdi to make Windows build reproducible
On Tue, 7 Jun 2022 19:57:48 GMT, Alexey Pavlyutkin wrote: > Hi! > > Here is a fix to jdk.jdi that fixes reproducible build for Windows. The idea > of the fix is to re-use value of --with-hotspot-build-time option to generate > deterministic timestamp exactly like it's done to hotspot component. > > Verification (Windows-10/MSVS-2019): ```bash ./configure > --with-boot-jdk=c:/work/boot-jdk/jdk-18 --with-debug-level=fastdebug > --with-hotspot-build-time="6/7/2022 2:35pm" > --with-extra-cflags="/experimental:deterministic" > --with-extra-cxxflags="/experimental:deterministic"``` > > Regression (Windows-10/MSVS-2019): ```bash ./configure > --with-boot-jdk=c:/work/boot-jdk/jdk-18 --with-debug-level=fastdebug``` Looks good. Thanks for fixing this! - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/9070
Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v5]
On Thu, 2 Jun 2022 17:00:35 GMT, Joe Darcy wrote: >> Time to start getting ready for JDK 20... > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 40 additional commits since > the last revision: > > - Update symbols for JDK 19 b25. > - Merge branch 'master' into JDK-8284858 > - Adjust deprecated options test. > - Merge branch 'master' into JDK-8284858 > - Update deprecated options test. > - Merge branch 'master' into JDK-8284858 > - Respond to review feedback. > - Update symbol information for JDK 19 b24. > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - ... and 30 more: > https://git.openjdk.java.net/jdk/compare/6a77da51...e495a579 Build changes look good. Thanks for fixing the bug in `generate-symbol-data.sh`. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8236
Re: RFR: 8288001: SOURCE_DATE_EPOCH should not be set by default
On Wed, 8 Jun 2022 07:47:24 GMT, KIRIYAMA Takuya wrote: > At default configuration, SOURCE_DATE_EPOCH is exported as environment > variable in SetupReproducibleBuild. Then, gcc is affected of > SOURCE_DATE_EPOCH environment variable. This value is used only to set > SOURCE_DATE_ISO_8601 (except below), so I removed "export" for > SOURCE_DATE_EPOCH in SetupReproducibleBuild. And, at building ct.sym, > SOURCE_DATE_EPOCH environment variable is needed. So I added setting routine > if SOURCE_DATE_EPOCH is not set. > > This fix works fine. With default configuration shows -Xinternalversion > output same as Windows, and with --with-source-date configuration shows > -Xinternalversion output specified timestamp. Would you please review this > fix? No no no. The current design is very much intentional. We must export `SOURCE_DATE_EPOCH` for gcc to be able to see it. I wonder just like Severin: *why* do you think this is a problem? Also, the long term goal is to *only* make reproducible builds. The sole reason it is possible to do `--disable-reproducible-builds` is out of an abundance of caution, if the changes needed for full reproducibility had unintended negative consequences. - PR: https://git.openjdk.java.net/jdk/pull/9081
Re: RFR: 8287366: Improve test failure reporting in GHA
On Mon, 6 Jun 2022 12:57:25 GMT, Jaikiran Pai wrote: >> It is currently both tricky and tedious to figure out what went wrong when a >> jtreg test fails in GHA. >> >> We should utilize the full potential of GitHub Action summaries and error >> annotations to make finding failures easier and more discoverable. >> >> With this PR, the overview of failures are presented on the "Summary" page >> for the action (the top-most line to the left, with the outline house icon). >> Below the `submit.yml` dependency graph, you'll find the annotations, which >> will look like this: >> >> >> Linux x86 (jdk/tier1 part 1) >> Test run reported 34 test failure(s) and 0 error(s). See summary for details. >> >> >> Below the annotations follow the summaries. Go have a look at the runs for >> this PR to see what it looks like! In short, there is a separate summary per >> test job. The first part lists the names of the failed tests. This will >> always be included. Below this (with links from the summary list) are >> detailed information for each failed test. This include the jtreg output, >> and the `hs_err` file(s), if present. The latter part has a limit from >> Github on 1 MB. If this limit is broken, no detailed information at all is >> presented (sorry 'bout that; GitHub's rules). >> >> This PR is deliberately based on a commit prior to the fix for JDK-8287137 >> (Problemlist failing x86_32 tests after Loom integration), so you can see >> for yourself how the GHA runs looks in case of a "train wreck" testing >> situation, like on x86 after Loom. As you can see, most of the output part >> of the summaries got larger than the 1 MB limit, which means they were not >> shown. Only the summary for `Linux x86 (hs/tier1 runtime)` displays as >> intended. OTOH, this shows that the system has a "graceful degradation" mode >> for even large amount of failures like this. And, since I don't see a Loom >> v2.0 coming anytime soon, I believe this amount of failed tests are unlikely >> to be a realistic scenario. >> >> Finally: the duplication in submit.yml is really, really annoying. :-( I >> have copied the same code block to three places. The fourth place, for >> Windows, do not get any support at this time. Concurrently with this change, >> I have started a separate branch where I split up submit.yml into reusable >> parts, using "callable workflows" and "custom actions". As part of this >> effort, I will also change the windows jobs to use cygwin bash instead of >> PowerShell. Until then, I could not be bothered to even think about >> implementing this functionality in PS. When that change is integrated, >> Windows will get this functionality for free, too. > >> With this PR, the overview of failures are presented on the "Summary" page >> for the action (the top-most line to the left, with the outline house icon). > > @magicus, thank you. This is really useful. I didn't even know that this > "Summary" page existed. I now checked this page on one of my PRs (which > includes this commit) and it does indeed make it much simpler to analyze > these failures. @jaikiran Thanks for the kind words. I think I should perhaps do some tweaking to the Skara bots that link to the GHA runs, so it easier to go to the summary page. - PR: https://git.openjdk.java.net/jdk/pull/8901
Re: Warning about git from 'make test' on Windows
On 2022-06-02 21:26, Andrey Turbanov wrote: Hello. I noticed strange warnings produced by 'make test' recently on my Win11 installation. For example: $ make test TEST="tier1" Building target 'test' in configuration 'windows-x86_64-server-release' /usr/bin/bash: Files/Git/cmd/git: No such file or directory /usr/bin/bash: Files/Git/cmd/git: No such file or directory Test selection 'tier1', will run: Also found this in build\windows-x86_64-server-release\configure-support\config.log configure:29274: checking for git configure:29502: result: Files/Git/cmd/git Seems like space in git path is incorrectly handled. Is it a known issue or is it a limitation of autoconf? Spaces in paths is always tricky and is best avoided. That being said, we try to make autoconf robust for tools in directories with space in them. Are you running in Cygwin? Can you verify that your C: drive can use short paths. See https://openjdk.java.net/groups/build/doc/building.html#spaces-in-path /Magnus Andrey Turbanov
Integrated: 8287724: Fix various issues with msys2
On Thu, 2 Jun 2022 09:17:59 GMT, Magnus Ihse Bursie wrote: > I encountered a bunch of issues when running with msys2 on Windows (but one > of them could have happened on cygwin as well). > > * fixpath must set MSYS2_ARG_CONV_EXCL="*" before running cmd.exe to figure > out the temp directory, or msys might interfere with the command line to cmd. > > * Paths like "/c/s/source/jdk", meaning to point to "c:\s\source\jdk", would > be interpreted by fixpath as "/cs:\source\jdk", since the leading "/c" would > be considered a prefix. This is not a problem on cygwin, where the /cygpath > prefix makes paths unambiguous. I countered this by checking if the file > exists (as written, or just the basepath, or the first 3 parts of the path). > If so, I treat it as a filename, rather than a prefix. > > * Configure is supposed to handle windows-style input paths, but > `--with-bootjdk=c:\java\jdk-17` or similar would break, since we started to > look for files in that directory without having to normalized the path first. > > * Finally, configure.guess sometimes reports msys as `mingw` and sometimes as > `msys`, depending on the value of MSYSTEM. (And for some values, the old > autoconf-configure.guess breaks -- I did not bother fixing this.) This pull request has now been integrated. Changeset: bddef715 Author:Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/bddef7151849a213926ffdd86a7e228db66606b1 Stats: 38 lines in 3 files changed: 23 ins; 7 del; 8 mod 8287724: Fix various issues with msys2 Reviewed-by: erikj - PR: https://git.openjdk.java.net/jdk/pull/8989
RFR: 8287724: Fix various issues with msys2
I encountered a bunch of issues when running with msys2 on Windows (but one of them could have happened on cygwin as well). * fixpath must set MSYS2_ARG_CONV_EXCL="*" before running cmd.exe to figure out the temp directory, or msys might interfere with the command line to cmd. * Paths like "/c/s/source/jdk", meaning to point to "c:\s\source\jdk", would be interpreted by fixpath as "/cs:\source\jdk", since the leading "/c" would be considered a prefix. This is not a problem on cygwin, where the /cygpath prefix makes paths unambiguous. I countered this by checking if the file exists (as written, or just the basepath, or the first 3 parts of the path). If so, I treat it as a filename, rather than a prefix. * Configure is supposed to handle windows-style input paths, but `--with-bootjdk=c:\java\jdk-17` or similar would break, since we started to look for files in that directory without having to normalized the path first. * Finally, configure.guess sometimes reports msys as `mingw` and sometimes as `msys`, depending on the value of MSYSTEM. (And for some values, the old autoconf-configure.guess breaks -- I did not bother fixing this.) - Commit messages: - 8287724: Fix various issues with msys2 Changes: https://git.openjdk.java.net/jdk/pull/8989/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8989=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287724 Stats: 38 lines in 3 files changed: 23 ins; 7 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/8989.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8989/head:pull/8989 PR: https://git.openjdk.java.net/jdk/pull/8989
Integrated: 8287366: Improve test failure reporting in GHA
On Thu, 26 May 2022 12:04:41 GMT, Magnus Ihse Bursie wrote: > It is currently both tricky and tedious to figure out what went wrong when a > jtreg test fails in GHA. > > We should utilize the full potential of GitHub Action summaries and error > annotations to make finding failures easier and more discoverable. > > With this PR, the overview of failures are presented on the "Summary" page > for the action (the top-most line to the left, with the outline house icon). > Below the `submit.yml` dependency graph, you'll find the annotations, which > will look like this: > > > Linux x86 (jdk/tier1 part 1) > Test run reported 34 test failure(s) and 0 error(s). See summary for details. > > > Below the annotations follow the summaries. Go have a look at the runs for > this PR to see what it looks like! In short, there is a separate summary per > test job. The first part lists the names of the failed tests. This will > always be included. Below this (with links from the summary list) are > detailed information for each failed test. This include the jtreg output, and > the `hs_err` file(s), if present. The latter part has a limit from Github on > 1 MB. If this limit is broken, no detailed information at all is presented > (sorry 'bout that; GitHub's rules). > > This PR is deliberately based on a commit prior to the fix for JDK-8287137 > (Problemlist failing x86_32 tests after Loom integration), so you can see for > yourself how the GHA runs looks in case of a "train wreck" testing situation, > like on x86 after Loom. As you can see, most of the output part of the > summaries got larger than the 1 MB limit, which means they were not shown. > Only the summary for `Linux x86 (hs/tier1 runtime)` displays as intended. > OTOH, this shows that the system has a "graceful degradation" mode for even > large amount of failures like this. And, since I don't see a Loom v2.0 coming > anytime soon, I believe this amount of failed tests are unlikely to be a > realistic scenario. > > Finally: the duplication in submit.yml is really, really annoying. :-( I have > copied the same code block to three places. The fourth place, for Windows, do > not get any support at this time. Concurrently with this change, I have > started a separate branch where I split up submit.yml into reusable parts, > using "callable workflows" and "custom actions". As part of this effort, I > will also change the windows jobs to use cygwin bash instead of PowerShell. > Until then, I could not be bothered to even think about implementing this > functionality in PS. When that change is integrated, Windows will get this > functionality for free, too. This pull request has now been integrated. Changeset: e0e15def Author:Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/e0e15def24c4c93c863ff459788bea23ef99d790 Stats: 285 lines in 1 file changed: 264 ins; 0 del; 21 mod 8287366: Improve test failure reporting in GHA Reviewed-by: clanger - PR: https://git.openjdk.java.net/jdk/pull/8901
Re: RFR: 8287171: Refactor null caller tests to a single directory [v3]
On Mon, 30 May 2022 00:10:50 GMT, Tim Prinzing wrote: >> Created a test at test/jdk/jdk/nullCaller called NullCallerTest that creates >> a test module with some resources in it for the actual tests that occur at >> the native level. The native part was switched to c++ instead of c to make >> it easier to create helper objects that reduce the redundant code performing >> error checking. The two helper classes InstanceCall and StaticCall were >> placed in an include file called CallHelper.hpp. The main part of the tests >> are in exeNullCallerTest.cpp, and there is a separate function for each of >> the bug reports. > > Tim Prinzing has updated the pull request incrementally with one additional > commit since the last revision: > > problem with iostream on Windows, use printf Build changes look good. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8934
RFR: 8287366: Improve test failure reporting in GHA
It is currently both tricky and tedious to figure out what went wrong when a jtreg test fails in GHA. We should utilize the full potential of GitHub Action summaries and error annotations to make finding failures easier and more discoverable. With this PR, the overview of failures are presented on the "Summary" page for the action (the top-most line to the left, with the outline house icon). Below the `submit.yml` dependency graph, you'll find the annotations, which will look like this: Linux x86 (jdk/tier1 part 1) Test run reported 34 test failure(s) and 0 error(s). See summary for details. Below the annotations follow the summaries. Go have a look at the runs for this PR to see what it looks like! In short, there is a separate summary per test job. The first part lists the names of the failed tests. This will always be included. Below this (with links from the summary list) are detailed information for each failed test. This include the jtreg output, and the `hs_err` file(s), if present. The latter part has a limit from Github on 1 MB. If this limit is broken, no detailed information at all is presented (sorry 'bout that; GitHub's rules). This PR is deliberately based on a commit prior to the fix for JDK-8287137 (Problemlist failing x86_32 tests after Loom integration), so you can see for yourself how the GHA runs looks in case of a "train wreck" testing situation, like on x86 after Loom. As you can see, most of the output part of the summaries got larger than the 1 MB limit, which means they were not shown. Only the summary for `Linux x86 (hs/tier1 runtime)` displays as intended. OTOH, this shows that the system has a "graceful degradation" mode for even large amount of failures like this. And, since I don't see a Loom v2.0 coming anytime soon, I believe this amount of failed tests are unlikely to be a realistic scenario. Finally: the duplication in submit.yml is really, really annoying. :-( I have copied the same code block to three places. The fourth place, for Windows, do not get any support at this time. Concurrently with this change, I have started a separate branch where I split up submit.yml into reusable parts, using "callable workflows" and "custom actions". As part of this effort, I will also change the windows jobs to use cygwin bash instead of PowerShell. Until then, I could not be bothered to even think about implementing this functionality in PS. When that change is integrated, Windows will get this functionality for free, too. - Commit messages: - Extra commit to re-trigger the GHA - 8287366: Improve test failure reporting in GHA Changes: https://git.openjdk.java.net/jdk/pull/8901/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8901=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287366 Stats: 285 lines in 1 file changed: 264 ins; 0 del; 21 mod Patch: https://git.openjdk.java.net/jdk/pull/8901.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8901/head:pull/8901 PR: https://git.openjdk.java.net/jdk/pull/8901
Integrated: 8287254: Clean up Xcode sysroot logic
On Tue, 24 May 2022 17:09:10 GMT, Magnus Ihse Bursie wrote: > The logic in BASIC_SETUP_DEVKIT for setting a correct sysroot for Xcode is > hard to follow. This should be straightened out. We also expose variables > that are no longer used. So there's a bit of related cleanup. > > The new code is more or less functionally equivalent to the old. There were > some corner cases which the old code did not handle well; this has now been > improved. I've also added yet another method of trying to get the SDK root > before falling back to the system library, by using `xcrun --show-sdk-path`. > > In an ideal world, the sysroot flags to `mig` should be set in configure, > e.g. as `MIG_FLAGS` or `MIG_SYSROOT_FLAGS`. I can't be bothered to do that > for a single call to `mig`, though. > > As always, changes like this that depend on the environment is tricky to > test. I've tried running it on a couple of different macs, with and without a > devkit. This pull request has now been integrated. Changeset: d889319a Author:Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/d889319a86101e944aefd4ad7f300505abbe5b30 Stats: 200 lines in 4 files changed: 91 ins; 98 del; 11 mod 8287254: Clean up Xcode sysroot logic Reviewed-by: erikj - PR: https://git.openjdk.java.net/jdk/pull/8872
RFR: 8287254: Clean up Xcode sysroot logic
The logic in BASIC_SETUP_DEVKIT for setting a correct sysroot for Xcode is hard to follow. This should be straightened out. We also expose variables that are no longer used. So there's a bit of related cleanup. The new code is more or less functionally equivalent to the old. There were some corner cases which the old code did not handle well; this has now been improved. I've also added yet another method of trying to get the SDK root before falling back to the system library, by using `xcrun --show-sdk-path`. In an ideal world, the sysroot flags to `mig` should be set in configure, e.g. as `MIG_FLAGS` or `MIG_SYSROOT_FLAGS`. I can't be bothered to do that for a single call to `mig`, though. As always, changes like this that depend on the environment is tricky to test. I've tried running it on a couple of different macs, with and without a devkit. - Commit messages: - Make Xcode SDK locating more robust - * Clean up BASIC_SETUP_XCODE_SYSROOT - Don't export SDKROOT - Move sdk setup to BASIC_SETUP_XCODE_SYSROOT Changes: https://git.openjdk.java.net/jdk/pull/8872/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8872=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287254 Stats: 200 lines in 4 files changed: 91 ins; 98 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/8872.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8872/head:pull/8872 PR: https://git.openjdk.java.net/jdk/pull/8872
Re: Reproducible MacOS Codesign'ed builds?
On 2022-05-23 19:16, Andrew Leonard wrote: Hi, Has anyone looked into reproducible builds for codesign'd MacOS builds? Basically Apple codesigning is non-deterministic, which is not surprisingly I guess, so naturally makes reproducible builds a bit tricky. The general theme for this sort of issue seems to be to remove the signature before comparing (codesign --remove-signature X.dylib). Which i've attempted, and works to a degree. The single stumbling block being the signing of jpackageapplauncher in jdk.jpackage, which then gets placed in the jmod's classes resource section, leading to different module "hash" in java.base/../module-info.class, and also a different "modules" file. Has anyone else tried to tackle this problem? Could we store jpackageapplauncher somewhere that would not end up in the jmod classes...but still be securely loadable? ( https://github.com/openjdk/jdk/blob/646c8aaeeccb494c72ff84c6e0f303f790be0ba9/src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacAppImageBuilder.java#L284 Unfortunately, I think this is an unsolvable problem. :-( We need to package all needed resources with the jmod, and jdk.jpackage needs its launchers. If we were do to anything differently, then suddenly the jdk.jpackage module would be different -- you will not be able to create a stripped-down image containing jdk.jpackage that will work properly, without also copying the launcher on the side. If anything, I suggest you create an image without jdk.jpackage, if you want to have a reproducible build on macOS. Personally, I have not spent much time and effort yet looking at reproducibility on macOS. I think we have quite some way to go there, compared with Linux and Windows. /Magnus
Re: RFR: 8287202: GHA: Add macOS aarch64 to the list of default platforms for workflow_dispatch event
On Tue, 24 May 2022 06:41:52 GMT, Christoph Langer wrote: > It seems that it was forgotten to add the macOS aarch64 platform to the > default platforms of workflow_dispatch. Let's fix this. Marked as reviewed by ihse (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8861
Integrated: 8287174: Remove deprecated configure arguments
On Mon, 23 May 2022 17:25:30 GMT, Magnus Ihse Bursie wrote: > We have a bunch of configure arguments that has been deprecated for multiple > releases. These should be removed. In effect, this will raise an error > instead of a warning if these argument is included on the command line for > configure. The deprecated arguments stopped having any effect when they were > deprecated. This pull request has now been integrated. Changeset: cf57d72f Author: Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/cf57d72fe8f40810f386413fe6e8c3c5dafab01f Stats: 23 lines in 3 files changed: 0 ins; 23 del; 0 mod 8287174: Remove deprecated configure arguments Reviewed-by: shade, dholmes - PR: https://git.openjdk.java.net/jdk/pull/8852
Integrated: 8287155: Additional make typos
On Mon, 23 May 2022 10:04:14 GMT, Magnus Ihse Bursie wrote: > Testing ispell + shell magic to locate typos. It worked, but is not scalable > to the entire JDK. :-( Keep the fixes for the problems found in the make > directory, though. This pull request has now been integrated. Changeset: 02fec1e6 Author: Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/02fec1e6e5b6728c13763718c98cf5db68b1cce3 Stats: 15 lines in 13 files changed: 0 ins; 0 del; 15 mod 8287155: Additional make typos Reviewed-by: lancea, iris - PR: https://git.openjdk.java.net/jdk/pull/8837
RFR: 8287174: Remove deprecated configure arguments
We have a bunch of configure arguments that has been deprecated for multiple releases. These should be removed. In effect, this will raise an error instead of a warning if these argument is included on the command line for configure. The deprecated arguments stopped having any effect when they were deprecated. - Commit messages: - 8287174: Remove deprecated configure arguments Changes: https://git.openjdk.java.net/jdk/pull/8852/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8852=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287174 Stats: 23 lines in 3 files changed: 0 ins; 23 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8852.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8852/head:pull/8852 PR: https://git.openjdk.java.net/jdk/pull/8852
RFR: 8287155: Additional make typos
Testing ispell + shell magic to locate typos. It worked, but is not scalable to the entire JDK. :-( Keep the fixes for the problems found in the make directory, though. - Commit messages: - 8287155: Additional make typos Changes: https://git.openjdk.java.net/jdk/pull/8837/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8837=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287155 Stats: 15 lines in 13 files changed: 0 ins; 0 del; 15 mod Patch: https://git.openjdk.java.net/jdk/pull/8837.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8837/head:pull/8837 PR: https://git.openjdk.java.net/jdk/pull/8837
Re: RFR: 8284209: Replace remaining usages of 'a the' in source code
On Wed, 18 May 2022 14:46:42 GMT, Alexey Ivanov wrote: > Replaces usages of articles that follow each other in all combinations: > a/the, an?/an?, the/the… > > It's the last issue in the series, and it still touches different areas of > the code. Build changes look good. Thanks for the cleanup! - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8771
Re: RFR: 8282322: AArch64: Provide a means to eliminate all STREX family of instructions
On Wed, 18 May 2022 19:05:03 GMT, Dmitry Chuyko wrote: > On AArch64 it is sometimes convenient to have LSE atomics right from the > start. Currently they are enabled after feature detection and RR reverse > debugger works incorrectly. > > New build configuration feature 'hardlse' is added. If it is enabled for > aarch64 type of build, then statically compiled stubs replace the initial > pessimistic implementation and dynamically generated replacements (when LSE > support is detected). The feature works for builds of all debug levels. > > New file atomic_linux_aarch64_lse.S is derived from atomic_linux_aarch64.S > and inherits its copyright. This alternative static implementation > corresponds to the dynamically generated code. > > Note, this configuration part is necessary but not sufficient to fully avoid > strex instructions for practical purposes. Other parts are: > > * Run on the OS built without strex family instructions. E.g. Amazon Linux > 2022. > * Compile with outline atomics enabled and the configuration flag enabled. > E.g. configure with > --with-extra-cflags='-march=armv8.3-a+crc+crypto -moutline-atomics' > --with-extra-cxxflags='-march=armv8.3-a+crc+crypto -moutline-atomics' > --with-extra-ldflags='-Wl,--allow-multiple-definition' > --with-jvm-features=hardlse > > Testing: tier1, tier2 on linux-aarch64 release builds with feature off and > feature on. Configure files need a bit more work. make/autoconf/jvm-features.m4 line 47: > 45: ifdef([custom_jvm_features_valid], custom_jvm_features_valid) \ > 46: \ > 47: cds compiler1 compiler2 dtrace epsilongc g1gc hardlse jfr jni-check \ The feature should be named `hard-lse` to match the `HARD_LSE` define in Hotspot code. (Also, it improves readability.) make/autoconf/jvm-features.m4 line 442: > 440: JVM_FEATURES_VARIANT_FILTER="link-time-opt opt-size" > 441: fi > 442: # Filter out hardlse feature by default You should not set up a PLATFORM filter in `JVM_FEATURES_PREPARE_VARIANT`. In fact, you will need a `JVM_FEATURES_CHECK_HARD_LSE`, which verifies that it can only be enabled on aarch64. See e.g. `JVM_FEATURES_CHECK_JVMCI` foor inspiration on how to write this and where to call it. I suggest you set up `JVM_FEATURES_PLATFORM_FILTER` in that function as well. - Changes requested by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8779
Re: RFR: 8286262: Windows: Cleanup deprecation warning suppression
On Sun, 15 May 2022 20:50:25 GMT, Kim Barrett wrote: > Please review this cleanup of deprecation warning suppression when building > for Windows. > > This change consists of several parts. > > (1) Remove the global deprecation warning suppression when building HotSpot > for Windows. > > (2) Add macro definitions requesting suppression of selected sets of > deprecation warnings when building HotSpot for Windows. > > (3) Remove unnecessary forwarding macros for various POSIX functions in > globalDefinitions_visCPP.hpp. These were provided to avoid deprecation > warnings (that were previously also being suppressed by the global request). > They are now covered by the new macros provided by change (2) above. > > An alternative to item (3) is to not define _CRT_NONSTDC_NO_DEPRECATE (in item > (2)) and either retain the forwarding macros or define os:: wrapper functions > for all of the affected functions. We might eventually do the latter because > of other reasons for avoiding some of these functions, but the approach being > taken here is simpler. > > For documentation of _CRT_NONSTDC_NO_DEPRECATE, see: > https://docs.microsoft.com/en-us/cpp/c-runtime-library/compatibility > https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996 > > Similarly for _CRT_SECURE_NO_WARNINGS. > > Perhaps similarly for _WINSOCK_DEPRECATED_NO_WARNINGS (though I didn't find > any documentation for the latter). But it might be better to not supress the > warnings and instead use the alternatives (JDK-8286781). > > Testing: > mach5 tier1 Looks good to me, build-wise. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8718
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v8]
On Fri, 13 May 2022 08:43:35 GMT, Christian Hagedorn wrote: >> I'm googling around for some information about -gdwarf-4 but is mostly >> coming up empty handed. :( I found >> [this](https://www.phoronix.com/scan.php?page=news_item=GCC-11-DWARF-5-Possible-Default) >> saying that dwarf-5 became default in gcc11. It also claims dwarf-4 is >> about 10 years old. My guess is that all our supported gcc versions do >> support -gdwarf-4, but this needs to be verified. >> >> In practice, we only use gcc on linux so I'm not convinced we need to have >> an addition check for that. If we ever are going to support gcc on other >> OSes I think we'll have many more, much harder problems, than to remove the >> -gdwarf-4 flag. > > I'm back to work again. I also had a look but could not find something on > Google, either. I then skimmed through the old GCC manuals. I found the first > occurrence of `-gdwarf-4` in the manual for GCC 4.5.4 > [here](https://gcc.gnu.org/onlinedocs/gcc-4.5.4/gcc.pdf): > > > - gdwarf-version > Produce debugging information in DWARF format (if that is supported). This > is the format used by DBX on IRIX 6. The value of version may be either > 2, 3 > or 4; the default version is 2. > > While the manual for GCC 4.4.7 only mentions `-gdwarf-2`: > > -gdwarf-2 > Produce debugging information in DWARF version 2 format (if that is > supported). This is the format used by DBX on > IRIX 6. With this option, GCC > uses features of DWARF version 3 when they are useful; version 3 is upward > compatible with version 2, but may still cause problems for older > debuggers. > > > The minimum accepted GCC version is currently 5.0 according to: > https://github.com/openjdk/jdk/blob/d5ae3833b1b71eb84fadb69c0c92851400f8921c/doc/building.md?plain=1#L341-L344 > > This suggests that all our supported GCC versions should accept `-gdwarf-4`. @chhagedorn Thanks for the research. You provide more than necessary reason to accept `-gdwarf-4` without any further checks. - PR: https://git.openjdk.java.net/jdk/pull/7126
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v8]
On Fri, 8 Apr 2022 11:11:29 GMT, Christian Hagedorn wrote: >> When printing the native stack trace on Linux (mostly done for hs_err >> files), it only prints the method with its parameters and a relative offset >> in the method: >> >> Stack: [0x7f6e01739000,0x7f6e0183a000], sp=0x7f6e01838110, >> free space=1020k >> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native >> code) >> V [libjvm.so+0x620d86] Compilation::~Compilation()+0x64 >> V [libjvm.so+0x624b92] Compiler::compile_method(ciEnv*, ciMethod*, int, >> bool, DirectiveSet*)+0xec >> V [libjvm.so+0x8303ef] >> CompileBroker::invoke_compiler_on_method(CompileTask*)+0x899 >> V [libjvm.so+0x82f067] CompileBroker::compiler_thread_loop()+0x3df >> V [libjvm.so+0x84f0d1] CompilerThread::thread_entry(JavaThread*, >> JavaThread*)+0x69 >> V [libjvm.so+0x1209329] JavaThread::thread_main_inner()+0x15d >> V [libjvm.so+0x12091c9] JavaThread::run()+0x167 >> V [libjvm.so+0x1206ada] Thread::call_run()+0x180 >> V [libjvm.so+0x1012e55] thread_native_entry(Thread*)+0x18f >> >> This makes it sometimes difficult to see where exactly the methods were >> called from and sometimes almost impossible when there are multiple >> invocations of the same method within one method. >> >> This patch improves this by providing source information (filename + line >> number) to the native stack traces on Linux similar to what's already done >> on Windows (see >> [JDK-8185712](https://bugs.openjdk.java.net/browse/JDK-8185712)): >> >> Stack: [0x7f34fca18000,0x7f34fcb19000], sp=0x7f34fcb17110, >> free space=1020k >> Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native >> code) >> V [libjvm.so+0x620d86] Compilation::~Compilation()+0x64 >> (c1_Compilation.cpp:607) >> V [libjvm.so+0x624b92] Compiler::compile_method(ciEnv*, ciMethod*, int, >> bool, DirectiveSet*)+0xec (c1_Compiler.cpp:250) >> V [libjvm.so+0x8303ef] >> CompileBroker::invoke_compiler_on_method(CompileTask*)+0x899 >> (compileBroker.cpp:2291) >> V [libjvm.so+0x82f067] CompileBroker::compiler_thread_loop()+0x3df >> (compileBroker.cpp:1966) >> V [libjvm.so+0x84f0d1] CompilerThread::thread_entry(JavaThread*, >> JavaThread*)+0x69 (compilerThread.cpp:59) >> V [libjvm.so+0x1209329] JavaThread::thread_main_inner()+0x15d >> (thread.cpp:1297) >> V [libjvm.so+0x12091c9] JavaThread::run()+0x167 (thread.cpp:1280) >> V [libjvm.so+0x1206ada] Thread::call_run()+0x180 (thread.cpp:358) >> V [libjvm.so+0x1012e55] thread_native_entry(Thread*)+0x18f >> (os_linux.cpp:705) >> >> For Linux, we need to parse the debug symbols which are generated by GCC in >> DWARF - a standardized debugging format. This patch adds support for DWARF >> 4, the default of GCC 10.x, for 32 and 64 bit architectures (tested with >> x86_32, x86_64 and AArch64). DWARF 5 is not supported as it was still >> experimental and not generated for HotSpot. However, newer GCC version may >> soon generate DWARF 5 by default in which case this parser either needs to >> be extended or the build of HotSpot configured to only emit DWARF 4. >> >> The code follows the parsing steps described in the official DWARF 4 spec: >> https://dwarfstd.org/doc/DWARF4.pdf >> I added references to the corresponding sections throughout the code. >> However, I tried to explain the steps from the DWARF spec directly in the >> code (method names, comments etc.). This allows to follow the code without >> the need to actually deep dive into the spec. >> >> The comments at the `Dwarf` class in the `elf.hpp` file explain in more >> detail how a DWARF file is structured and how the parsing algorithm works to >> get to the filename and line number information. There are more class >> comments throughout the `elf.hpp` file about how different DWARF sections >> are structured and how the parsing algorithm needs to fetch the required >> information. Therefore, I will not repeat the exact workings of the >> algorithm here but refer to the code comments. I've tried to add as much >> information as possible to improve the readability. >> >> Generally, I've tried to stay away from adding any assertions as this code >> is almost always executed when already processing a VM error. Instead, the >> DWARF parser aims to just exit gracefully and possibly omit source >> information for a stack frame instead of risking to stop writing the hs_err >> file when an assertion would have failed. To debug failures, `-Xlog:dwarf` >> can be used with `info`, `debug` or `trace` which provides logging messages >> throughout parsing. >> >> **Testing:** >> Apart from manual testing, I've added two kinds of tests: >> - A JTreg test: Spawns new VMs to let them crash in various ways. The test >> reads the created hs_err files to check if the DWARF parsing could correctly >> find the filename and line number. For normal HotSpot files, I could not >> check against hardcoded
Re: RFR: 8286122: [macos]: App bundle cannot upload to Mac App Store due to info.plist embedded in java exe [v2]
On Thu, 12 May 2022 04:15:50 GMT, Alexander Matveev wrote: >> - It is not possible to support native JDK commands such as "java" inside >> Mac App Store bundles due to embedded info.plist. Workarounds suggested in >> JDK-8286122 does not seems to be visible. >> - With proposed fix we will enforce "--strip-native-commands" option for >> jlink, so native JDK commands are not included when generating Mac App Store >> bundles. >> - Custom runtime provided via --runtime-image should not contain native >> commands as well, otherwise jpackage will throw error. >> - Added two tests to validate fix. > > Alexander Matveev has updated the pull request incrementally with one > additional commit since the last revision: > > 8286122: [macos]: App bundle cannot upload to Mac App Store due to > info.plist embedded in java exe [v2] We have the `NSMicrophoneUsageDescription` permission on the `java` launcher in the JDK, since otherwise no Java program can access the mike, even though most won't care. I agree that the situation is different for a jpackaged app, where the developer knows if that permission is needed or not. Yes, plistbuddy is an official Apple program. My understanding of the PR was that native commands are removed by jlink if the user is packaging on a mac for the App Store. I thought this was a workaround that solved the immediate problem of not being able to submit the app to App Store. (However, I don't know how the app is supposed to be started without a launcher...) - PR: https://git.openjdk.java.net/jdk/pull/8666
Re: RFR: 8286122: [macos]: App bundle cannot upload to Mac App Store due to info.plist embedded in java exe
Aren't we embedding this bundle ID in every launcher, so you would need a .template for each possible launcher that could be included in an app? I naively assumed that only the java launcher was needed, since this is about packaging a Java app, so all you'd need is an entry to your main class. Is this not the case? What I think we need to do is first evaluate if we actually need to embed this bundle ID in the executables at all, or rather, what would the consequences be if we didn't. My understanding is that we only do this to be able to run them outside of a bundle directory structure, but this would need some pretty thorough testing of course. If we are to generate a special set of launchers for jpackage, maybe all we need to do is not embed any bundle ID in them, as they are meant to only be used within a jpackaged app, so they should be covered by Info.plist files anyway. What you say sounds good, although I feel I only partially understand it. :-) I assume the important point here is that the app get the NSMicrophoneUsageDescription property, and afaict this can be provided by the Info.plist file for the entire application, as you say. And possible the problem here is that we embed metadata in the java executable at the same time. There seems to be a lot of guessing here. :-) I assume we either need to read up on how this works (which might be difficult if this is seem as a badly documented corner case even by Apple tech support), or test some alternatives, or perhaps both. That solution make take some time to get correct, so the jpackage team needs to decide if they want to go with the workaround in this PR in the meantime. /Magnus /Erik
Re: RFR: 8286122: [macos]: App bundle cannot upload to Mac App Store due to info.plist embedded in java exe
On 2022-05-12 13:17, Michael Hall wrote: I had read this but possibly didn’t grok the issue myself. If I understand correctly now the conflict isn’t within the application but across applications to some other application that has already been added to the Mac App Store that included the commands with the given CFBundleIdentifier. Yes, that is indeed how I also interpret this. Presumably, the very first developer to submit a jpackaged application to App Store (from what I can tell now OpenJDK itself is not present there, which I mistakenly believed before) got everything working without troubles, but blocked all other developers from submitting their apps. A solution like including a bundle identifier something like net.java.openjdk.MYAPP.java would be possible at packaging time but not at build time. To fix this at build time you would need to generate a name unique to each installed jdk. Including release net.java.openjdk.JDK_RELEASE.java might avoid some conflicts but would still be open to conflict for two applications at the same release. So it can’t be addressed ‘before the fact’ either. The only thing I am currently thinking of that might work would be include a replaceable part in the identifier. So something like net.java.openjdk.java.XX Where jpackage could include something to change the X…. to a unique application name. If you don’t change the string size you could probably avoid some of the resizing issues Apple DTS mentions. Whether there is a standard Apple tool to do this I don’t know. As you say, we're a bit in a bind since the java executable needs to be created when the JDK is built, but the bundle ID needs to be determined when jpackage is run (and a suitable, per-application ID can be created), and there is no standard tooling to update the bundle ID after build time. I believe what you are describing is exactly solution #2 suggested by the Apple engineer. I don't think that would be horribly difficult to achieve, though. Sure, it's a bit of a hack, but not the ugliest I've seen in my days. If we go down this route, I suppose we do something like this: 1) When building the JDK, we create an additional copy of the java executable, and store it with the jdk.jpackage resources. Let's call it java.template, for the sake of it. This is identical to the real bin/java except for the fact that it contains a different bundle ID, with a large enough padding field, like X... This way, we don't have to mess around with the real java executable for the JDK. 2) At jpackage time, this java.template file is installed instead of bin/java, and the padding field is replaced by a unique value. The java executable is small (33kB on macOS, currently) so a simple search through the binary field for the pattern is likely to work alright. As long as there are no checksums being broken, this should be straightforward. My primary suggestion would to be to use an UUID for the unique ID. They are of fixed length, are for all intents and purposes unique and you can conjure them up from your hat. (An alternative is that the user needs to specify a unique ID, but that is probably a less ideal solution.) Presumably, we can have some kind of prefix like "org.openjdk.jpackage." before the UUID to make them a bit understandable, if someone where to inspect the package metadata. This seems like a fully workable solution to me. However, I'd really like to understand option #1 better, which was: "Package the `java` executable in a standard bundle, replacing `runtime/Contents/Home/bin/java` with a symlink to that." I don't know what a "standard bundle" is, or how you would go around to package the java executable in one. But on the surface, it sounds much nicer than binary editing. I also don't understand if that means that the standard bundle needs to be created at jpackage time, so it gives us the chance to set a proper ID, or if the standard bundle can be created at build time, and then the existence of this bundle just makes Apple avoid the bundle ID collision checks. Or if I'm just misunderstanding it all... /Magnus
Re: RFR: 8286562: GCC 12 reports some compiler warnings [v4]
On Thu, 12 May 2022 01:29:13 GMT, Yasumasa Suenaga wrote: >> Yasumasa Suenaga has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Calculate char offset before realloc() > > Thanks for all to review this PR! I think we should separate this issue as > following: > > * Suppress warnings > * make/modules/java.desktop/lib/Awt2dLibraries.gmk > * src/hotspot/share/classfile/bytecodeAssembler.cpp > * src/hotspot/share/classfile/classFileParser.cpp > * > src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp > * src/hotspot/share/opto/memnode.cpp > * src/hotspot/share/opto/type.cpp > * src/hotspot/share/utilities/compilerWarnings.hpp > * src/hotspot/share/utilities/compilerWarnings_gcc.hpp > * src/java.base/unix/native/libjli/java_md_common.c > * Bug fixes > * src/java.base/share/native/libjli/java.c > * src/java.base/share/native/libjli/parse_manifest.c > * src/jdk.jpackage/linux/native/applauncher/LinuxPackage.c > > I want to include the change of Awt2dLibraries.gmk (HarfBuzz) in this PR > because it is 3rd party library. > > I will separate in above if I do not hear any objections, and this issue (PR) > handles "suppress warnings" only. @YaSuenag From my PoV this sounds like a good suggestion. - PR: https://git.openjdk.java.net/jdk/pull/8646
Re: RFR: 8286623: Bundle zlib by default with JDK on macos aarch64
On Thu, 12 May 2022 08:43:56 GMT, Jaikiran Pai wrote: > Can I please get a review of this change to the build system which now makes > bundled zlib as the default for macos aarch64 systems? > > We have been running into various intermittent failures on macos aarch64 > systems for several months now. After investigation we have been able to > ascertain that the root cause of the issue lies within the zlib that's > shipped on macos aarch64 systems. The complete details about that issue is > available at https://bugs.openjdk.java.net/browse/JDK-8282954. > We have filed a bug with Apple for their inputs on what we can do to fix it > in that shipped library. Given the nature of that issue, we don't have a > timeline on when/if the solution for that will be available. Until that time, > at least, the proposal is to use bundled zlib (which doesn't reproduce those > failures) by default on macos aarch64. LGTM - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8675
Re: RFR: 8286122: [macos]: App bundle cannot upload to Mac App Store due to info.plist embedded in java exe
(cc:ing build-dev.) On 2022-05-12 00:17, Michael Hall wrote: Is this restricted somehow to Mac App Store applications? Is a warning issued as stripping native commands may break application functionality? Is it not possible for the user to provide their own Info.plist with a different bundle identifier that doesn’t collide? I’m not real familiar with the build process. But would it be possible for the user to build their own jdk that substitutes something else for the colliding identifier that gets embedded? This problem seem to come about as a clash between the the OpenJDK packages themselves being submitted to Apple, as well as a developer-specific jpackage containing JDK binaries, such as java. The well-written response from Apple tech support in the original web bug is very instructive. I'll quote it in its entirety: I’ve dealt with this message before, and it has a long and interesting history. The Mac App Store prevents two developers from submitting executables with the same bundle identifier. This is an important security check: We don’t want app A impersonating app B. This check applies to all executables, not just the app’s main executable. Historically the Mac App Store ingestion process had bugs that caused it to apply to other types of code, like frameworks and bundles. When I first saw this incident I was worried that these bugs had come back. However, that’s not the case. Let’s look at the main executables in your app: % find APP_NAME.app -type f -print0 | xargs -0 file | grep "Mach-O.*executable" APP_NAME.app/Contents/MacOS/APP_NAME: Mach-O 64-bit executable x86_64 APP_NAME.app/Contents/runtime/Contents/Home/bin/java: Mach-O 64-bit executable x86_64 APP_NAME.app/Contents/runtime/Contents/Home/bin/keytool: Mach-O 64-bit executable x86_64 APP_NAME.app/Contents/runtime/Contents/Home/lib/jspawnhelper: Mach-O 64-bit executable x86_64 Based on the error message it’s obvious we need to focus on the `java` executable. However, it’s placed in a location that doesn’t have a corresponding `Info.plist` file: % find APP_NAME.app -name "Info.plist" APP_NAME.app/Contents/Info.plist APP_NAME.app/Contents/runtime/Contents/Info.plist The first file here applies to your entire app and the second file applies to the Java runtime package as a whole. Moreover, neither of these have a bundle ID that matches the error message: % plutil -extract CFBundleIdentifier raw -o - "APP_NAME.app/Contents/Info.plist" UNIQUE.BUNDLE.ID % plutil -extract CFBundleIdentifier raw -o - "APP_NAME.app/Contents/runtime/Contents/Info.plist" com.oracle.java.com.UNIQUE.BUNDLE.ID So where is this bundle ID coming from? * * * Some further spelunking reveals the issue. Consider this: % otool -s __TEXT __info_plist -v APP_NAME.app/Contents/runtime/Contents/Home/bin/java … CFBundleIdentifier net.java.openjdk.java … This is an obscure corner case in the macOS bundle story: A standalone tool, like `java`, which isn’t in a bundle structure, and thus can’t have a standalone `Info.plist` file, can put its information property list in a `__TEXT` / `__info_plist` section within its executable. And it seems that the folks who created your Java runtime did just that. Given that, the Mac App Store error is valid: You are trying to submit an executable whose bundle ID matches some existing app. To get around this check you’ll need to give `java` a new bundle ID. That’s not easy to do. This `__TEXT` / `__info_plist` section is set up by a linker option (see `-sectcreate` in )> and there’s no good way to modify it after the fact [1]. At this point my advice is that you escalate this with your Java runtime vendor. I suspect that they’ve added this information property list to get around a TCC check — the only other interesting property in there is `NSMicrophoneUsageDescription` — and they probably didn’t notice this Mac App Store submission issue. There’s a couple of ways they could resolve this [2] but it’s really their issue to resolve. [1] And by “good” I mean “Using a standard tool supplied by Apple.” The Mach-O file format is reasonably well documented and thus you could build a custom tool to do that, but even that’s not easy. There are a couple of problems: * The new information property list may be larger than the previous one. * The `__info_plist` section can appear anywhere in the `__TEXT` segment. If you increase the size of the section then subsequent sections need to move up to accommodate it and, depending on which sections you have to move, that might require other cross-section links to be fixed up. Yergh! [2] The ones that spring to mind are: * Package the `java` executable in a standard bundle, replacing `runtime/Contents/Home/bin/java` with a symlink to that. * Add an `__info_plist` section with a bunch of padding and then build a tool to update the bundle ID in that section, taking advantage of that padding to avoid the need to
Re: RFR: 8286582: Build fails on macos aarch64 when using --with-zlib=bundled [v2]
On Wed, 11 May 2022 19:14:54 GMT, Lance Andersen wrote: >> make/autoconf/lib-bundled.m4 line 220: >> >>> 218: if test "x$USE_EXTERNAL_LIBZ" = "xfalse"; then >>> 219: LIBZ_CFLAGS="$LIBZ_CFLAGS >>> -I$TOPDIR/src/java.base/share/native/libzip/zlib" >>> 220: if test "x$OPENJDK_TARGET_OS" = xmacosx; then >> >> Please add a comment here as to why we are doing this > >> @LanceAndersen Is that really needed? We normally don't comment on the >> reason why certain code needs certain defines. We tried to document some >> compiler flags in the beginning, but it turned out that most command lines >> ended up as essays, and this were not helpful. I think it's quite obvious >> what this code does: libz requires the define HAVE_UNISTD_H on macOS. I'm >> not even sure how you'd formulate a "why" -- "otherwise it does not work >> properly"? > > The zlib configure script, which needs to be run prior running make to > build the upstream repository, will determine if HAVE_UNISTD_H is needed and > generate zconf.h replacing the macro with "1".My reason for adding a > comment as this is a variant from how zlib is built upstream. Perhaps just > updating open/src/java.base/share/native/libzip/zlib/ChangeLog is enough. I > was just trying to document why we are doing this. > > > Another question would it make sense to set LIBZ_DISABLED_WARNINGS_CLANG in > make/autoconf/lib-bundled.m4 so that the value in the case of zlib is set in > one location? In addition, I can log a request to address this in the > upstream project so we do not need to worry about this warning going forward. It would not make sense to set the disabled warning in the configure script, no. The current code looks perfectly fine. Disabled warnings per module are set in the makefiles. If you feel strongly that this needs to be documented more than in the JBS issue and this PR review, updating the zlib ChangeLog file is probably the way to go. I have no strong opinion on that. But from the build system PoV, the current code is fine as it is to be committed. - PR: https://git.openjdk.java.net/jdk/pull/8651
Re: RFR: 8286582: Build fails on macos aarch64 when using --with-zlib=bundled [v2]
On Wed, 11 May 2022 15:03:56 GMT, Lance Andersen wrote: >> Jaikiran Pai has updated the pull request incrementally with four additional >> commits since the last revision: >> >> - copyright years >> - disable format-nonliteral warning when building LIBSPLASHSCREEN with >> bundled zlib >> - Magnus' suggestion - make the LIBZ_CFLAGS more readable in the build file >> - Magnus' suggestion - Disable format-nonliteral in build section of zlib >> instead of source code > > make/autoconf/lib-bundled.m4 line 220: > >> 218: if test "x$USE_EXTERNAL_LIBZ" = "xfalse"; then >> 219: LIBZ_CFLAGS="$LIBZ_CFLAGS >> -I$TOPDIR/src/java.base/share/native/libzip/zlib" >> 220: if test "x$OPENJDK_TARGET_OS" = xmacosx; then > > Please add a comment here as to why we are doing this @LanceAndersen Is that really needed? We normally don't comment on the reason why certain code needs certain defines. We tried to document some compiler flags in the beginning, but it turned out that most command lines ended up as essays, and this were not helpful. I think it's quite obvious what this code does: libz requires the define HAVE_UNISTD_H on macOS. I'm not even sure how you'd formulate a "why" -- "otherwise it does not work properly"? > make/modules/java.base/lib/CoreLibraries.gmk line 139: > >> 137: DISABLED_WARNINGS_gcc := unused-function implicit-fallthrough, \ >> 138: DISABLED_WARNINGS_clang := format-nonliteral, \ >> 139: LDFLAGS := $(LDFLAGS_JDKLIB) \ > > A comment would be good here also as to the reasoning And once again, we never comment on why we disable warnings. The context is clear enough -- there is a compiler warning that is not applicable to this module. Especially for 3rd party code, which is not nearly as stringent as we are about enabling warnings. - PR: https://git.openjdk.java.net/jdk/pull/8651
Re: RFR: 8286582: Build fails on macos aarch64 when using --with-zlib=bundled [v2]
On Wed, 11 May 2022 14:24:38 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which fixes build failures on macos >> when using `--with-zlib=bundled`? >> >> With this change the build now passes (tested both with bundled and system >> zlib variants). >> >> tier1, tier2 and tier3 testing has been done and no related failures have >> been noticed. > > Jaikiran Pai has updated the pull request incrementally with four additional > commits since the last revision: > > - copyright years > - disable format-nonliteral warning when building LIBSPLASHSCREEN with > bundled zlib > - Magnus' suggestion - make the LIBZ_CFLAGS more readable in the build file > - Magnus' suggestion - Disable format-nonliteral in build section of zlib > instead of source code Looks good to me. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8651
Re: RFR: 8286562: GCC 12 reports some compiler warnings [v2]
On 2022-05-11 16:01, Kim Barrett wrote: Globally disabling use-after-free warnings for this package seems really questionable. If these are problems in the code, just suppressing the warning and leaving the problems to bite us seems like a bad idea. And the problems ought to be reported upstream to the HarfBuzz folks. I agree that an upstream report would be nice, but it has been a long-standing policy of not changing 3rd party code to fix warnings, but instead suppress them. (Most of the time we compile with a much larger set of warnings enabled than the upstream's own build does, so we trigger warnings they never even see.) /Magnus
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v3]
On Wed, 11 May 2022 13:05:45 GMT, Erik Gahlin wrote: >> make/modules/jdk.jfr/Java.gmk line 26: >> >>> 24: # >>> 25: >>> 26: DISABLED_WARNINGS_java += exports lossy-conversions >> >> Note that with the fix of JDK-8286392 (and JDK-8286396) the >> `lossy-conversions` warning should not be disabled for the JFR code. >> >> In general, you need to check which of the subtasks of JDK-8286374 that has >> been fixed, and adjust the makefiles accordingly, before pushing this fix. >> >> (In the future, it might be easier to push the fix which disables the >> warnings first, and then file follow-up bugs on aa per-component basis, and >> remind them to remove the disabling in the makefile. That way there won't be >> a race between individual fixes and a "master" bug like this.) > > I agree, but if it doesn't happen, I can follow up with a separate PR where I > remove the disablement. That's good to know. I think the tricky part is mostly about keeping track of all these disabled warnings, so they are not kept around longer than necessary. And that needs coordination with all the subtasks of the umbrella issue. - PR: https://git.openjdk.java.net/jdk/pull/8599
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v3]
On Wed, 11 May 2022 07:45:39 GMT, Adam Sotona wrote: >> Please review this patch adding new lint option, **lossy-conversions**, to >> javac to warn about type casts in compound assignments with possible lossy >> conversions. >> >> The new lint warning is shown if the type of the right-hand operand of a >> compound assignment is not assignment compatible with the type of the >> variable. >> >> The implementation of the warning is based on similar check performed to >> emit "possible lossy conversion" compilation error for simple assignments. >> >> Proposed patch also include complex matrix-style test with positive and >> negative test cases of lossy conversions in compound assignments. >> >> Proposed patch also disables this new lint option in all affected JDK >> modules and libraries to allow smooth JDK build. Individual cases to address >> possibly lossy conversions warnings in JDK are already addressed in a >> separate umbrella issue and its sub-tasks. >> >> Thanks for your review, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > 8244681: Add a warning for possibly lossy conversion in compound assignments > recommended correction of the warning description Check updates on JDK-8286374 subtasks. make/modules/jdk.jfr/Java.gmk line 26: > 24: # > 25: > 26: DISABLED_WARNINGS_java += exports lossy-conversions Note that with the fix of JDK-8286392 (and JDK-8286396) the `lossy-conversions` warning should not be disabled for the JFR code. In general, you need to check which of the subtasks of JDK-8286374 that has been fixed, and adjust the makefiles accordingly, before pushing this fix. (In the future, it might be easier to push the fix which disables the warnings first, and then file follow-up bugs on aa per-component basis, and remind them to remove the disabling in the makefile. That way there won't be a race between individual fixes and a "master" bug like this.) - Changes requested by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8599
Re: RFR: 8286582: Build fails on macos aarch64 when using --with-zlib=bundled
On Wed, 11 May 2022 11:38:31 GMT, Jaikiran Pai wrote: > Can I please get a review of this change which fixes build failures on macos > when using `--with-zlib=bundled`? > > With this change the build now passes (tested both with bundled and system > zlib variants). > > tier1, tier2 and tier3 testing has been done and no related failures have > been noticed. Changes requested by ihse (Reviewer). make/autoconf/lib-bundled.m4 line 224: > 222: LIBZ_LIBS="" > 223: if test "x$USE_EXTERNAL_LIBZ" = "xfalse"; then > 224: LIBZ_CFLAGS="$LIBZ_CFLAGS $APPLE_LIBZ_CFLAGS > -I$TOPDIR/src/java.base/share/native/libzip/zlib" Declaring APPLE_LIBZ_CFLAGS far away (and only conditionally) and then using it once here just makes for hard-to-read code. Suggestion: LIBZ_CFLAGS="$LIBZ_CFLAGS -I$TOPDIR/src/java.base/share/native/libzip/zlib" if test "x$OPENJDK_TARGET_OS" = xmacosx; then LIBZ_CFLAGS="$LIBZ_CFLAGS -DHAVE_UNISTD_H" fi ... and remove the assignment above. src/java.base/share/native/libzip/zlib/gzwrite.c line 452: > 450: len = strlen(next); > 451: # else > 452: # ifdef __APPLE__ // ignore format-nonliteral warning on macOS Instead of patching 3rd party code to fix a compilation warning, you should disable that warning instead. In `make/modules/java.base/lib/CoreLibraries.gmk`, add DISABLED_WARNINGS_clang := format-nonliteral, \ as line 138. - PR: https://git.openjdk.java.net/jdk/pull/8651
Re: RFR: 8286562: GCC 12 reports some compiler warnings [v2]
On Wed, 11 May 2022 08:40:21 GMT, Yasumasa Suenaga wrote: >> I saw some compiler warnings when I tried to build OpenJDK with GCC 12.0.1 >> on Fedora 36. >> As you can see, the warnings spreads several areas. Let me know if I should >> separate them by area. >> >> * -Wstringop-overflow >> * src/hotspot/share/oops/array.hpp >> * >> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp >> >> In member function 'void Array::at_put(int, const T&) [with T = unsigned >> char]', >> inlined from 'void ConstantPool::tag_at_put(int, jbyte)' at >> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:126:64, >> inlined from 'void ConstantPool::method_at_put(int, int, int)' at >> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:380:15, >> inlined from 'ConstantPool* >> BytecodeConstantPool::create_constant_pool(JavaThread*) const' at >> /home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/bytecodeAssembler.cpp:85:26: > > Yasumasa Suenaga has updated the pull request incrementally with one > additional commit since the last revision: > > Avoid pragma error in before GCC 12 The harfbuzz disabled warning looks good, so build changes are approved. You'll still need approval for the rest of the changes. While it's not my place really to say about the code changes, I think hiding the warnings with pragmas like this is the least attractive option. But if the code owners are okay with it... - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8646
Re: RFR: 8286430: make test TEST="gtest:" exits with error when it shouldn't [v2]
On Tue, 10 May 2022 08:46:44 GMT, Severin Gehwolf wrote: >> `make test TEST="gtest:> `1.10.0`. It expects `suites` to be present in the google test output >> whereas the OpenJDK build infra code expects `cases`. I'm not sure when/if >> that changed, but here is a proposed fix. >> >> Thoughts? > > Severin Gehwolf has updated the pull request incrementally with one > additional commit since the last revision: > > Handle both 'cases' and 'suites' Marked as reviewed by ihse (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8605
Re: RFR: 8286429: jpackageapplauncher build fails intermittently in Tier[45]
On Tue, 10 May 2022 01:36:06 GMT, David Holmes wrote: >> The way LauncherCommon.gmk is currently written, it's only meant to be >> included from "make/module//Launcher.gmk", or at least only from one >> single place for each module. This is because the man page generation that >> happens in LauncherCommon.gmk is module global. For the jdk.jpackage module, >> LauncherCommon.gmk is now called from two separate sub makefiles, both >> make/module/jdk.jpackage/Lib.gmk and make/module/jdk.jpackage/Launcher.gmk. >> These files are called from the top level targets jdk.jpackage-libs and >> jdk.jpackage-launchers respectively. These top level targets are assumed to >> be able to run independently of each other, which is normally fine, but now >> they define the same rules for the same files. This creates a race where one >> may be deleting files that the other one is creating, causing directories to >> disappear while files are being written to them. This can fail the build, >> but also risks silently corrupting the build. >> >> This patch fixes this by adding a conditional around the man page >> generation, which helps guarantee that man pages are only processed when >> called from make/module//Launcher.gmk. It's a bit of a hack, but >> it's building on top of the existing design of piggybacking man page >> generation on the launchers build. >> >> Also fixing broken whitespace further down in the file (tabs->space). > > I'm still unclear how `--disable-manpages` is supposed to affect this logic, > as they seemed to be the failing configs. @dholmes-ora That option is a bit weirdly named. It is used to enable new generation of man pages from markdown files instead of copying the static troff files. So without that option, this could not have failed. - PR: https://git.openjdk.java.net/jdk/pull/8618
Re: RFR: 8286429: jpackageapplauncher build fails intermittently in Tier[45]
On Mon, 9 May 2022 23:18:47 GMT, Erik Joelsson wrote: > The way LauncherCommon.gmk is currently written, it's only meant to be > included from "make/module//Launcher.gmk", or at least only from one > single place for each module. This is because the man page generation that > happens in LauncherCommon.gmk is module global. For the jdk.jpackage module, > LauncherCommon.gmk is now called from two separate sub makefiles, both > make/module/jdk.jpackage/Lib.gmk and make/module/jdk.jpackage/Launcher.gmk. > These files are called from the top level targets jdk.jpackage-libs and > jdk.jpackage-launchers respectively. These top level targets are assumed to > be able to run independently of each other, which is normally fine, but now > they define the same rules for the same files. This creates a race where one > may be deleting files that the other one is creating, causing directories to > disappear while files are being written to them. This can fail the build, but > also risks silently corrupting the build. > > This patch fixes this by adding a conditional around the man page generation, > which helps guarantee that man pages are only processed when called from > make/module//Launcher.gmk. It's a bit of a hack, but it's building on > top of the existing design of piggybacking man page generation on the > launchers build. > > Also fixing broken whitespace further down in the file (tabs->space). Marked as reviewed by ihse (Reviewer). I think this looks good. I'm just curious why this started to show up now. There have been no changes in this area lately, have it? Is it just pure (bad) luck that the race did not happen before? - PR: https://git.openjdk.java.net/jdk/pull/8618
Fwd: JDK 19+21 early-access build is reproducible
Just a FYI: The fix of the last remaining bug towards full reproducibility (on Linux, at least) of the JDK was given attention on the reproducible-builds mailing list. /Magnus Forwarded Message Subject:JDK 19+21 early-access build is reproducible Date: Fri, 6 May 2022 13:48:20 -0700 From: John Neffenger Reply-To: General discussions about reproducible builds Organization: Status Six Communications To: Reproducible Builds List Starting yesterday, for the first time, the JDK can create reproducible builds of the JDK! Pull request 8478 [1] was the last reproducibility bug remaining in my JDK builds on Linux, and it's included in the latest JDK 19+21 early-access build. [2] OpenJDK 19 will be generally available on September 20, 2022. That also means there's nothing in the JDK that's holding back any Java application from having reproducible builds. The link below lists all the "reproducible build" fixes for OpenJDK: https://bugs.openjdk.java.net/issues/?jql=labels+%3D+reproducible-build I tested on six Linux architectures (amd64, arm64, armhf, i386, ppc64el, s390x), and the entire JDK is reproducible including the command-line tools, demos, API documentation, JMOD archives, native libraries, and man pages -- even when using a different build path. Note that I didn't test on macOS or Windows. A big thank you to Magnus Ihse Bursie and Andrew Leonard for doing much of the work to make this possible. John [1]: https://github.com/openjdk/jdk/pull/8478 [2]: https://jdk.java.net/19/
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup Looks good, thanks for doing this cleanup. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: 8282828: CDS uncompressed oops archive is not deterministic
On Fri, 29 Apr 2022 22:50:45 GMT, Ioi Lam wrote: > During `java -Xshare:dump -XX:-UseCompressedOops`, the location of the Java > heap is chosen by the OS. Due to Address Space Layout Randomization, the heap > will always start at a different location. This causes the archive for > uncompressed oops ($JAVA_HOME/lib/server/classes_nocoops.jsa) to be > non-deterministic. > > The fix is to patch the archived object pointers to make it look like the > heap starts at a fixed address -- I chose 0x1000, but the exact value > doesn't really matter. > > At runtime, the object pointers will be patched again according to the real > location of the heap. > > Tested with tiers 1-5. I am running builds-tier5 several times to test the > xxx-cmp-baseline builds. Marked as reviewed by ihse (Reviewer). Looks good to me. But you need hotspot reviewers as well. And thanks for fixing this! I think this is the last major known non-reproducibility bug in the JDK! - PR: https://git.openjdk.java.net/jdk/pull/8478
Integrated: 8285919: Remove debug printout from JDK-8285093
On Fri, 29 Apr 2022 12:43:02 GMT, Magnus Ihse Bursie wrote: > A debug printout in configure was introduced in JDK-8285093. It should be > removed. This pull request has now been integrated. Changeset: 64225e19 Author:Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/64225e19995e81d2e836ce84befea1a01bb6c860 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod 8285919: Remove debug printout from JDK-8285093 Reviewed-by: erikj - PR: https://git.openjdk.java.net/jdk/pull/8467
RFR: 8285919: Remove debug printout from JDK-8285093
A debug printout in configure was introduced in JDK-8285093. It should be removed. - Commit messages: - 8285919: Remove debug printout from JDK-8285093 Changes: https://git.openjdk.java.net/jdk/pull/8467/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8467=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285919 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8467.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8467/head:pull/8467 PR: https://git.openjdk.java.net/jdk/pull/8467
Re: RFR: 8283324: CLDRConverter run time increased by 3x
On Mon, 18 Apr 2022 23:16:18 GMT, Naoto Sato wrote: > Fixing performance regression caused by the fix to > https://bugs.openjdk.java.net/browse/JDK-8176706. The fix introduced extra > looping through the resource map multiple times which was not necessary. The > execution time of the tool now got back on par with close to JDK18. Thanks for fixing this! - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8288
Integrated: 8285093: Introduce UTIL_ARG_WITH
On Tue, 19 Apr 2022 20:38:31 GMT, Magnus Ihse Bursie wrote: > Analogous to `UTIL_ARG_ENABLE`, we need a `UTIL_ARG_WITH` which wraps > ´AC_ARG_WITH`, provides a declarative rather than programmatic way of > handling configure arguments. It can also make sure that all edge cases are > covered, and that we treat arguments consistently. > > This PR contains the implementation of `UTIL_ARG_WITH`, and an example > conversation of the calls to `AC_ARG_WITH` in basic_tools.m4. There are some > 120-odd more places where `AC_ARG_WITH` is called that need to be converted, > but that is out of scope for this PR. Getting the m4 logic for `AC_ARG_WITH` > was hard enough for this PR, and verifying a whole bunch of configure > arguments is mind-numbingly boring. So I intend to attack those piecewise, > fixing them in large enough batches at a time, later on. > > I also fixed a bug and a documentation issue in `UTIL_ARG_ENABLE`. This pull request has now been integrated. Changeset: 94afb366 Author:Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/94afb366b2ec76669e1aac38dbadc223ccafda3d Stats: 381 lines in 2 files changed: 349 ins; 5 del; 27 mod 8285093: Introduce UTIL_ARG_WITH Reviewed-by: erikj - PR: https://git.openjdk.java.net/jdk/pull/8306
RFR: 8285093: Introduce UTIL_ARG_WITH
Analogous to `UTIL_ARG_ENABLE`, we need a `UTIL_ARG_WITH` which wraps ´AC_ARG_WITH`, provides a declarative rather than programmatic way of handling configure arguments. It can also make sure that all edge cases are covered, and that we treat arguments consistently. This PR contains the implementation of `UTIL_ARG_WITH`, and an example conversation of the calls to `AC_ARG_WITH` in basic_tools.m4. There are some 120-odd more places where `AC_ARG_WITH` is called that need to be converted, but that is out of scope for this PR. Getting the m4 logic for `AC_ARG_WITH` was hard enough for this PR, and verifying a whole bunch of configure arguments is mind-numbingly boring. So I intend to attack those piecewise, fixing them in large enough batches at a time, later on. I also fixed a bug and a documentation issue in `UTIL_ARG_ENABLE`. - Commit messages: - Use new default value in basic_tools.m4 - Better default for RESULT. - Fix UTIL_ARG_WITH in basic_tools.m4 - Create UTIL_ARG_WITH Changes: https://git.openjdk.java.net/jdk/pull/8306/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8306=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8285093 Stats: 382 lines in 2 files changed: 349 ins; 5 del; 28 mod Patch: https://git.openjdk.java.net/jdk/pull/8306.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8306/head:pull/8306 PR: https://git.openjdk.java.net/jdk/pull/8306
Integrated: 8284999: Remove remaining files in src/samples
On Tue, 19 Apr 2022 10:41:07 GMT, Magnus Ihse Bursie wrote: > JEP 298 was about removing demos and samples. Unfortunately, > [JDK-8173801](https://bugs.openjdk.java.net/browse/JDK-8173801) which should > have removed all files in src/samples, left a few non-source files (key > stores and images). These should be removed. This pull request has now been integrated. Changeset: 595c8b85 Author: Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/595c8b859890b5b439069a5aac6664b96b444580 Stats: 0 lines in 10 files changed: 0 ins; 0 del; 0 mod 8284999: Remove remaining files in src/samples Reviewed-by: erikj - PR: https://git.openjdk.java.net/jdk/pull/8294
Re: Why we use specific compiler versions - was: Re: JDK-8284772 - was RE: [jdk17] RFR: 8269148: Update minor GCC version in GitHub Actions pipeline
On 2022-04-14 19:42, Andrew Hughes wrote: On 12:57 Wed 13 Apr , Magnus Ihse Bursie wrote: I disagree completely. We had it this way in mainline originally, but it was fixed in https://bugs.openjdk.java.net/browse/JDK-8256393. Prior to this patch, it seems there were no GCC version requirements. That's not what I'm suggesting. What I'm suggesting is that we replace gcc-10=10.2.0-5ubuntu1~20.04 (or whatever it is now) with just gcc-10. Is it possible to set the requirement to 10.2? That'd be okay for me, since it matches how we check for gcc versions in configure (currently it is at least "6.0"), and how we have traditionally described our requirements. Or are the only two options "just major version" or "major, minor, patch and ubuntu release"? :( /Magnus That still selects a specific major version of GCC. It is between major versions that we see new optimisations introduced, deprecations, etc. I would certainly not suggest that we allow it to be switched from e.g. GCC 10 to G11 behind our backs. I have dealt with such transitions in Fedora and know the changes they bring. What I don't see the advantage of is requiring a very specific package version. If this was OpenJDK, it would be like asking to build 8u with specifically 8u332-b05 rather than just 8u. I can't see that we would do that without a very good reason. I don't see such a good reason for requiring the same of GCC. These two are handled differently on the GCC development side too. Breakage is expected with the move to a new major version. This is why three stable versions are currently being maintained [0]. If breakage were to happen between minor releases of a stable version, however, that would be a bug. I've yet to see a case of it happening, though of course it's possible. As you might know, I'm not too fond of the GHA solution, since we can't debug issues with Github's hosts. Nevertheless, many users look at the GHA results as a way to sanity check their code. Any and all spurious build failures is a problem then, since it will present a red marker -- even if the new code in the PR is okay. This specific versioning is producing precisely these spurious failures. The reason I started digging into this was because my PR failed on Linux x86_64. There were no code changes in the PR (I was backporting the GHA setup to our Shenandoah fork of 8u). Having only just added support to 8u mainline, I found this very odd. It turns out it failed because it could no longer download the specific version of GCC. [1] I agree GHA can be painful to debug - it took me weeks to get 8u working in full - but it is useful for testing on architectures and operating systems one doesn't have easy access to. The less control we have over the build platform, the greater the chance for odd and non-reproducible build failures. Selecting a specific version of the compiler is a way to guarantee reproducible build results. If the build succeeds in mainline, and I submit correct code, chances are higher that the build also succeeds in my PR. In contrast, if the gcc version suddenly were changed behind my back, the mainline build might succeed, and my PR build fail, not due to anything I've done wrong, but just due to the fact that the compiler was switched by the Ubuntu team in the meantime. Yes, it means a bit more annoyance when upgrading the compiler, but that also means it is a conscious (and hopefully well tested) choice. I'll take that any day over re-introducing more uncertainty into an already-unstable testing procedure. As I say, I'm not suggesting we don't select a specific version, just that we are not *too* specific to the point we are constantly changing the version specification every time the Ubuntu maintainer fixes a typo. /Magnus [0] https://gcc.gnu.org/ [1] https://github.com/openjdk-bots/shenandoah-jdk8u/runs/5889665932?check_suite_focus=true Thanks
Re: RFR: 8284890: Support for Do not fragment IP socket options [v2]
On Fri, 15 Apr 2022 11:49:29 GMT, Michael McMahon wrote: >> Hi, >> >> Could I get the following PR review please? It adds a new JDK specific >> extended socket option >> called IP_DONTFRAGMENT, which disables IP packet fragmentation in both IPv4 >> and IPv6 >> UDP sockets (NIO DatagramChannels). For IPv4 in particular, it sets the DF >> (Dont Fragment) bit >> in the IP header. There is no equivalent in the IPv6 packet header as >> fragmentation is implemented >> exclusively by the sending and receiving nodes. >> >> Thanks, >> Michael > > Michael McMahon has updated the pull request incrementally with one > additional commit since the last revision: > > builds in github action now Build changes look OK. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8245
Re: RFR: 8284539: Configure --with-source-date=version fails on MacOS
On Thu, 14 Apr 2022 16:13:59 GMT, Andrew Leonard wrote: > JDK-8282769 added support for more ISO-8601 formats, but remove handling of > just a date "-MM-DD" being present, which is the case for a configure > using --with-source-date=version which uses the date string from > version-numbers.conf. > Also, the first date parse had an invalid format string "%FZ %TZ", with too > many Zs. > This PR corrects the first date parse to parse a standard ISO-8601 Zulu > date: "%FT%TZ" > Then it adds the final check for no time being specified. > > Signed-off-by: Andrew Leonard Marked as reviewed by ihse (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8247
Re: RFR: 8284949: riscv: Add Zero support for the 32-bit RISC-V architecture [v2]
On Tue, 19 Apr 2022 08:47:18 GMT, Feilong Jiang wrote: >> This patch adds Zero support for the 32-bit RISC-V architecture. >> >> Additional tests: >> >> - [x] Linux zero RISCV32 cross-compilation >> - [x] Resulting binaries run on QEMU User mode without problems > > Feilong Jiang has updated the pull request incrementally with one additional > commit since the last revision: > > adjust SYS_futex define for RISCV32 Marked as reviewed by ihse (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8284
RFR: 8284999: Remove remaining files in src/samples
JEP 298 was about removing demos and samples. Unfortunately, [JDK-8173801](https://bugs.openjdk.java.net/browse/JDK-8173801) which should have removed all files in src/samples, left a few non-source files (key stores and images). These should be removed. - Commit messages: - 8284999: Remove remaining files in src/samples Changes: https://git.openjdk.java.net/jdk/pull/8294/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8294=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284999 Stats: 0 lines in 10 files changed: 0 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8294.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8294/head:pull/8294 PR: https://git.openjdk.java.net/jdk/pull/8294
Integrated: 8284891: Fix typos in build system files
On Thu, 14 Apr 2022 16:05:48 GMT, Magnus Ihse Bursie wrote: > I ran `codespell` on the `make` directory, and accepted those changes where > it indeed discovered real typos. > > (Due to false positives this can unfortunately not be run automatically) > > Most of the fixes are in comments. A few are in messages aimed at the user. This pull request has now been integrated. Changeset: 160eb2bd Author:Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/160eb2bd392fea29dd690ee9781174d14dc0b659 Stats: 72 lines in 46 files changed: 0 ins; 0 del; 72 mod 8284891: Fix typos in build system files Reviewed-by: erikj - PR: https://git.openjdk.java.net/jdk/pull/8246
RFR: 8284891: Fix typos in build system files
I ran `codespell` on the `make` directory, and accepted those changes where it indeed discovered real typos. (Due to false positives this can unfortunately not be run automatically) Most of the fixes are in comments. A few are in messages aimed at the user. - Commit messages: - 8284891: Fix typos in build system files Changes: https://git.openjdk.java.net/jdk/pull/8246/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8246=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284891 Stats: 72 lines in 46 files changed: 0 ins; 0 del; 72 mod Patch: https://git.openjdk.java.net/jdk/pull/8246.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8246/head:pull/8246 PR: https://git.openjdk.java.net/jdk/pull/8246
Re: RFR: 8284853: Fix various 'expected' typo [v2]
On Thu, 14 Apr 2022 09:28:17 GMT, Andrey Turbanov wrote: >> Found various typos of expected: `exepected`, `exept`, `epectedly`, >> `expeced`, `Unexpeted`, etc. > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 8284853: Fix various 'expected' typo > improve test log Build changes look good. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8231
Re: RFR: 8284661: Reproducible assembly builds without relative linking [v6]
On Thu, 14 Apr 2022 12:07:06 GMT, Andrew Leonard wrote: >> This PR removes the need for relative object file linking introduced by >> JDK-8284437 for linux libraries, by specifying >> .file directives in the linux .S source files. The >> source files specify a .file ASSEMBLY_SRC_FILE >> where ASSEMBLY_SRC_FILE is defined by the NativeCompliation.gmk. >> >> Signed-off-by: Andrew Leonard > > Andrew Leonard has updated the pull request incrementally with one additional > commit since the last revision: > > 8284661: Reproducible assembly builds without relative linking > > Signed-off-by: Andrew Leonard Looks great now! Thanks for pulling this through. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8177
Re: fixpath seems to be deleting path separators when paths are read from files
On 2022-04-14 04:15, Julian Waters wrote: Hi Magnus, > So this is only in @-files? > Sounds like it could be a newline issue then. (CR/LF vs LF) Is this in > cygwin? > Have you checked if "make doctor" finds autocrlf issues? Yes, it only happens when files containing commands are tagged as @; the current builds are tested on MSYS2 (ucrt.exe). make doctor doesn't find any issues oddly enough, apart from the obligatory warning to run "make clean" after the build failed > You can try setting the variable DEBUG_FIXPATH to any non-empty value to > get a bit more debug info out of fixpath. It ended up spamming the console with a lot of output, but the (I think) relevant log was: fixpath: debug: input: /ucrt64/bin/g++.exe -Wl,--warn-unresolved-symbols -Wl,-O1 -m64 -shared -O3 -flto -fuse-linker-plugin -fno-strict-aliasing -m64 -Wl,-version-script=/d/eclipse/workspace/hotspot/jdk/build/windows-x86_64-server-release/hotspot/variant-server/libjvm/mapfile -Wl,-soname=jvm.dll -o /d/eclipse/workspace/hotspot/jdk/build/windows-x86_64-server-release/support/modules_libs/java.base/server/jvm.dll @/d/eclipse/workspace/hotspot/jdk/build/windows-x86_64-server-release/hotspot/variant-server/libjvm/objects/_BUILD_LIBJVM_objectfilenames.txt /d/eclipse/workspace/hotspot/jdk/build/windows-x86_64-server-release/hotspot/variant-server/libjvm/objects/jvm.dll.res fixpath: debug: output: /ucrt64/bin/g++.exe -Wl,--warn-unresolved-symbols -Wl,-O1 -m64 -shared -O3 -flto -fuse-linker-plugin -fno-strict-aliasing -m64 -Wl,-version-script=d:\eclipse\workspace\hotspot\jdk\build\windows-x86_64-server-release\hotspot\variant-server\libjvm\mapfile -Wl,-soname=jvm.dll -o d:\eclipse\workspace\hotspot\jdk\build\windows-x86_64-server-release\support\modules_libs\java.base\server\jvm.dll @C:\Users\vertig0\Downloads\msys64\tmp\fixpath.UeMmxB\atfile d:\eclipse\workspace\hotspot\jdk\build\windows-x86_64-server-release\hotspot\variant-server\libjvm\objects\jvm.dll.res The redirected @file was removed too quickly for me to see its contents though You can modify make/scripts/fixpath.sh, and remove/comment out the line rm -rf $TEMPDIRS in the cleanup trap to keep it for post-mortem debugging. /Magnus best regards, Julian On Wed, Apr 13, 2022 at 7:04 PM Magnus Ihse Bursie wrote: On 2022-04-13 05:39, Julian Waters wrote: > Recently I've been getting build failures on my Windows device that go > something along the lines of: > /msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/.../x86_64-w64-mingw32/bin/ld.exe: > cannot find > d:eclipseworkspacehotspotjdkbuildwindows-x86_64-server-releasehotspotvariant-serverlibjvmobjectsabstractCompiler.o: > No such file or directory > > The issue results from fixpath apparently deleting path separators when it > transforms the path styles for Windows; It does add the proper drive name > but ends up wiping all of the slashes in the path > > /d/eclipse/workspace/hotspot/jdk/build/windows-x86_64-server-release/hotspot/variant-server/libjvm/objects/abstractCompiler.o > > ends up becoming (As seen above) > > d:eclipseworkspacehotspotjdkbuildwindows-x86_64-server-releasehotspotvariant-serverlibjvmobjectsabstractCompiler.o > > Curiously, it works fine only for the last path listed in the @file, so > given a file containing: > > /d/eclipse/workspace/hotspot/jdk/build/windows-x86_64-server-release/hotspot/variant-server/libjvm/objects/zWorkers.o > /d/eclipse/workspace/hotspot/jdk/build/windows-x86_64-server-release/hotspot/variant-server/libjvm/objects/jvm.dll.res > > the latter would properly convert to > > d:\eclipse\workspace\hotspot\jdk\build\windows-x86_64-server-release\hotspot\variant-server\libjvm\objects\jvm.dll.res > > but the former and everything before it would be affected So this is only in @-files? Sounds like it could be a newline issue then. (CR/LF vs LF) Is this in cygwin? Have you checked if "make doctor" finds autocrlf issues? > > The full generated build command (with the absolute paths truncated) is as > follows > > fixpath exec g++.exe -Wl,--warn-unresolved-symbols -Wl,-O1 -m64 -shared -O3 > -fuse-linker-plugin -fno-strict-aliasing -m64 > -Wl,-version-script=/hotspot/variant-server/libjvm/mapfile > -Wl,-soname=jvm.dll -o /support/modules_libs/java.base/server/jvm.dll > @/hotspot/variant-server/libjvm/objects/_BUILD_LIBJVM_objectfilenames.txt > /hotspot/variant-server/libjvm/objects/jvm.dll.res > > There aren't any peculiarities between the 2 compilers (I'm currently > experimenting with the ucrt linking port of gcc) that should be
Re: RFR: 8284661: Reproducible assembly builds without relative linking [v4]
On Wed, 13 Apr 2022 11:30:14 GMT, Andrew Leonard wrote: >> This PR removes the need for relative object file linking introduced by >> JDK-8284437 for linux libraries, by specifying >> .file directives in the linux .S source files. The >> source files specify a .file ASSEMBLY_SRC_FILE >> where ASSEMBLY_SRC_FILE is defined by the NativeCompliation.gmk. >> >> Signed-off-by: Andrew Leonard > > Andrew Leonard has updated the pull request incrementally with one additional > commit since the last revision: > > 8284661: Reproducible assembly builds without relative linking > > Signed-off-by: Andrew Leonard The latest version looks much better! Fix the test to check for linux, and the indentation in the new header file, and I'd say you're good to go! make/data/autoheaders/assemblyprefix.h line 24: > 22: # > 23: > 24: // ASSEMBLY_SRC_FILE gets replaced by relative or absolute file > path While not critical, the indentation here looks surprising and odd. - PR: https://git.openjdk.java.net/jdk/pull/8177
Re: RFR: 8284752: Zero does not build on Mac OS X due to missing os::current_thread_enable_wx implementation
On 2022-04-12 15:21, David Holmes wrote: On Tue, 12 Apr 2022 11:31:03 GMT, Johannes Bechberger wrote: Adds an implementation of the missing method (guarded with `defined(AARCH64) && defined(__APPLE__)`) and fixes the compilation issues on Mac M1. Looks good and trivial. It seems apparent that nobody has been building Zero in this environment. I'm a bit surprised at this. I thought macos/aarch64 was added to GHA some time ago? (And also to Oracle's internal CI pipeline) /Magnus
Re: fixpath seems to be deleting path separators when paths are read from files
On 2022-04-13 05:39, Julian Waters wrote: Recently I've been getting build failures on my Windows device that go something along the lines of: /msys64/ucrt64/bin/../lib/gcc/x86_64-w64-mingw32/.../x86_64-w64-mingw32/bin/ld.exe: cannot find d:eclipseworkspacehotspotjdkbuildwindows-x86_64-server-releasehotspotvariant-serverlibjvmobjectsabstractCompiler.o: No such file or directory The issue results from fixpath apparently deleting path separators when it transforms the path styles for Windows; It does add the proper drive name but ends up wiping all of the slashes in the path /d/eclipse/workspace/hotspot/jdk/build/windows-x86_64-server-release/hotspot/variant-server/libjvm/objects/abstractCompiler.o ends up becoming (As seen above) d:eclipseworkspacehotspotjdkbuildwindows-x86_64-server-releasehotspotvariant-serverlibjvmobjectsabstractCompiler.o Curiously, it works fine only for the last path listed in the @file, so given a file containing: /d/eclipse/workspace/hotspot/jdk/build/windows-x86_64-server-release/hotspot/variant-server/libjvm/objects/zWorkers.o /d/eclipse/workspace/hotspot/jdk/build/windows-x86_64-server-release/hotspot/variant-server/libjvm/objects/jvm.dll.res the latter would properly convert to d:\eclipse\workspace\hotspot\jdk\build\windows-x86_64-server-release\hotspot\variant-server\libjvm\objects\jvm.dll.res but the former and everything before it would be affected So this is only in @-files? Sounds like it could be a newline issue then. (CR/LF vs LF) Is this in cygwin? Have you checked if "make doctor" finds autocrlf issues? The full generated build command (with the absolute paths truncated) is as follows fixpath exec g++.exe -Wl,--warn-unresolved-symbols -Wl,-O1 -m64 -shared -O3 -fuse-linker-plugin -fno-strict-aliasing -m64 -Wl,-version-script=/hotspot/variant-server/libjvm/mapfile -Wl,-soname=jvm.dll -o /support/modules_libs/java.base/server/jvm.dll @/hotspot/variant-server/libjvm/objects/_BUILD_LIBJVM_objectfilenames.txt /hotspot/variant-server/libjvm/objects/jvm.dll.res There aren't any peculiarities between the 2 compilers (I'm currently experimenting with the ucrt linking port of gcc) that should be causing this from what I can tell, out of curiosity, has this been an issue reported elsewhere before? No. I haven't figured out what's causing this so far unfortunately :( You can try setting the variable DEBUG_FIXPATH to any non-empty value to get a bit more debug info out of fixpath. /Magnus best regards, Julian
Why we use specific compiler versions - was: Re: JDK-8284772 - was RE: [jdk17] RFR: 8269148: Update minor GCC version in GitHub Actions pipeline
I disagree completely. We had it this way in mainline originally, but it was fixed in https://bugs.openjdk.java.net/browse/JDK-8256393. As you might know, I'm not too fond of the GHA solution, since we can't debug issues with Github's hosts. Nevertheless, many users look at the GHA results as a way to sanity check their code. Any and all spurious build failures is a problem then, since it will present a red marker -- even if the new code in the PR is okay. The less control we have over the build platform, the greater the chance for odd and non-reproducible build failures. Selecting a specific version of the compiler is a way to guarantee reproducible build results. If the build succeeds in mainline, and I submit correct code, chances are higher that the build also succeeds in my PR. In contrast, if the gcc version suddenly were changed behind my back, the mainline build might succeed, and my PR build fail, not due to anything I've done wrong, but just due to the fact that the compiler was switched by the Ubuntu team in the meantime. Yes, it means a bit more annoyance when upgrading the compiler, but that also means it is a conscious (and hopefully well tested) choice. I'll take that any day over re-introducing more uncertainty into an already-unstable testing procedure. /Magnus On 2022-04-13 08:25, Langer, Christoph wrote: Hi Andrew, One dummy question: Why do we need to specify the real package name here? If we install gcc-10, I think apt system will pick up the latest gcc-10 for us. IIRC the intent is to keep control over the gcc version and not randomly update whenever the distro updates. Upgrading compiler versions for the OpenJDK is actually a very involved process when done properly and we often find code changes need to be made, or warnings adjusted, when a new version of the compiler is to be used. This approach forces us to check the new version is okay before switching over to it. At least that is the theory. :) Cheers, David I'm in complete agreement with you as regards major versions of GCC. Fedora's eager adoption of them means we often encounter these early. JDK-8282231 is just the latest example from the introduction of GCC 12. However, the GHA workflow in OpenJDK doesn't just depend on a major version of GCC - which is actually contained in the Ubuntu package name of gcc-9 or gcc-10 itself - but the full revision number, even down to local packaging changes. I believe this is overkill and leads to valuable time being wasted on issues like JDK-8283778 where the GCC version itself didn't even change at all, just the Ubuntu version suffix. Having just encountered this with 8u, I've filed JDK-8284772 there to just use the package name, which includes the major GCC version. That's already how it is depending on the x86_32 GCC, which hasn't suffered broken dependencies in the same way as x86_64. I have yet to see an issue be specific to a minor GCC version bump, whereas the current setup is pretty much guaranteed to mean further fixes to the GitHub workflow every time the Ubuntu packager produces a new build. I'm happy to submit the change for other JDK versions if there is interest, but I at least don't want to be encountering this in maintaining 8u (and certainly not having to add fixes to a release branch in rampdown, as happened recently with 11u) I'm in full agreement with you and can't see any reason for but just additional trouble with hard maintenance of the GCC version suffix. I would love to see JDK-8284772 be done in head and backported to all active update releases. I had the same idea when doing JDK-8283778. Best regards Christoph
