Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v12]
> 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 filenames and line numbers as they are subject to > change (especially line number can quickly become different). I therefore > just added some sanity
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 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. - 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
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? Is the problem that on Windows we get -Xinternalversion in local timezone but on Linux we get UTC? That's a difference that may warrant fixing, but I don't think this is the way to do it. I don't think it makes a practical difference if the timestamp is taken at the start of the build or when a certain src file is compiled. - PR: https://git.openjdk.org/jdk/pull/9081
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: 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. (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 c onsidering switching stance to recommend using MSYS2 instead of Cygwin as the primary winenv for building the JDK. >> >> ### 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 >> >>
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. Bot, wake up! Will it help? - PR: https://git.openjdk.org/jdk/pull/9130
Re: RFR: 8288001: SOURCE_DATE_EPOCH should not be set by default
On Mon, 13 Jun 2022 07:14:15 GMT, Magnus Ihse Bursie 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"? @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, > [...] - 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. (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 c onsidering switching stance to recommend using MSYS2 instead of Cygwin as the primary winenv for building the JDK. >> >> ### 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 >> >>
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 erikj (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9130
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: 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? 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. - 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. (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 c onsidering switching stance to recommend using MSYS2 instead of Cygwin as the primary winenv for building the JDK. >> >> ### 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 >> >>
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v12]
> 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 Cygwin as the primary winenv for building the JDK. > > ### 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 >
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v11]
> 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 Cygwin as the primary winenv for building the JDK. > > ### 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 >
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: 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. Looks good - thanks! - Marked as reviewed by mcimadamore (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]
> 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 Cygwin as the primary winenv for building the JDK. > > ### 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 >
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v9]
> 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 Cygwin as the primary winenv for building the JDK. > > ### 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 >
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]
> 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 Cygwin as the primary winenv for building the JDK. > > ### 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 >
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v7]
> 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 Cygwin as the primary winenv for building the JDK. > > ### 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 >
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]
> 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 Cygwin as the primary winenv for building the JDK. > > ### 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 >
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: 8288114: Update JIRA link in vcs.xml
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. - Commit messages: - 8288114: Update JIRA link in vcs.xml Changes: https://git.openjdk.org/jdk/pull/9130/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9130=00 Issue: https://bugs.openjdk.org/browse/JDK-8288114 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/9130.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9130/head:pull/9130 PR: https://git.openjdk.org/jdk/pull/9130
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v5]
On Fri, 10 Jun 2022 09:34:53 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 c onsidering switching stance to recommend using MSYS2 instead of Cygwin as the primary winenv for building the JDK. >> >> ### 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 >> >>
Re: RFR: 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)). Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9122
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v4]
On Fri, 10 Jun 2022 09:45:15 GMT, Magnus Ihse Bursie wrote: > (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. - 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:39:29 GMT, Magnus Ihse Bursie wrote: >> .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. Nevermind then. >> .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. 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. - PR: https://git.openjdk.org/jdk/pull/9063
Re: RFR: 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)). Looks fine! - Marked as reviewed by shade (Reviewer). PR: https://git.openjdk.org/jdk/pull/9122
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]
> 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 Cygwin as the primary winenv for building the JDK. > > ### 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 >
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v4]
On Thu, 9 Jun 2022 12:57:05 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 c onsidering switching stance to recommend using MSYS2 instead of Cygwin as the primary winenv for building the JDK. >> >> ### 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 >> >>
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v4]
On Fri, 10 Jun 2022 07:13:32 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/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.
-
PR: https://git.openjdk.org/jdk/pull/9063
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? I appreciate all reviews. I hope this change is integrated. - PR: https://git.openjdk.org/jdk/pull/9099
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v4]
> 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 Cygwin as the primary winenv for building the JDK. > > ### 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 >
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]
> 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 Cygwin as the primary winenv for building the JDK. > > ### 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 >
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. (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 c onsidering switching stance to recommend using MSYS2 instead of Cygwin as the primary winenv for building the JDK. >> >> ### 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 >> >>
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: 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 erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9099
RFR: 8283724: Incorrect description for jtreg-failure-handler option
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? - Commit messages: - 8283724: Incorrect description for jtreg-failure-handler option Changes: https://git.openjdk.java.net/jdk/pull/9099/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9099=00 Issue: https://bugs.openjdk.org/browse/JDK-8283724 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/9099.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9099/head:pull/9099 PR: https://git.openjdk.java.net/jdk/pull/9099
Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v6]
On Thu, 9 Jun 2022 01:03:47 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 44 additional commits since > the last revision: > > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - Merge branch 'master' into JDK-8284858 > - 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 > - ... and 34 more: > https://git.openjdk.java.net/jdk/compare/705cfa36...1b29a17c Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8236
Re: RFR: JDK-8284858: Start of release updates for JDK 20 [v6]
> 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 44 additional commits since the last revision: - Merge branch 'master' into JDK-8284858 - Merge branch 'master' into JDK-8284858 - Merge branch 'master' into JDK-8284858 - Merge branch 'master' into JDK-8284858 - 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 - ... and 34 more: https://git.openjdk.java.net/jdk/compare/530d16c1...1b29a17c - Changes: - all: https://git.openjdk.java.net/jdk/pull/8236/files - new: https://git.openjdk.java.net/jdk/pull/8236/files/e495a579..1b29a17c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8236=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8236=04-05 Stats: 16423 lines in 539 files changed: 12382 ins; 2143 del; 1898 mod Patch: https://git.openjdk.java.net/jdk/pull/8236.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8236/head:pull/8236 PR: https://git.openjdk.java.net/jdk/pull/8236
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v2]
> 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 Cygwin as the primary winenv for building the JDK. > > ### 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 >
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: 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``` Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9070
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 Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9028
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: 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? It's not clear **why** you don't want `SOURCE_DATE_EPOCH` set by default. Could you explain? It would be good to make the OpenJDK build reproducible by default. The way it works currently, is a first step towards that goal. - PR: https://git.openjdk.java.net/jdk/pull/9081
RFR: 8288001: SOURCE_DATE_EPOCH should not be set by default
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? - Commit messages: - 8288001: SOURCE_DATE_EPOCH should not be set by default - 8288001: SOURCE_DATE_EPOCH should not be set by default Changes: https://git.openjdk.java.net/jdk/pull/9081/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9081=00 Issue: https://bugs.openjdk.org/browse/JDK-8288001 Stats: 5 lines in 2 files changed: 4 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/9081.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9081/head:pull/9081 PR: https://git.openjdk.java.net/jdk/pull/9081
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests
On Tue, 7 Jun 2022 20:05:26 GMT, Jonathan Gibbons 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 c onsidering switching stance to recommend using MSYS2 instead of Cygwin as the primary winenv for building the JDK. >> >> ### 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
Re: RFR: 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 Cygwin as the primary winenv for building the JDK. > > ### 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
RFR: 8287894: Use fixed timestamp as an alternative of __DATE__ macro in jdk.jdi to make Windows build reproducible
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``` - Commit messages: - 8287894: use fixed timestamp instead of __DATE__ macro to fix reproducible Windows build Changes: https://git.openjdk.java.net/jdk/pull/9070/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9070=00 Issue: https://bugs.openjdk.org/browse/JDK-8287894 Stats: 21 lines in 3 files changed: 14 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/9070.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9070/head:pull/9070 PR: https://git.openjdk.java.net/jdk/pull/9070
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v4]
On Tue, 7 Jun 2022 17:48:30 GMT, Ioi Lam wrote: >> Leo Korinth has updated the pull request incrementally with one additional >> commit since the last revision: >> >> wip > > make/RunTests.gmk line 357: > >> 355: $(subst .java,,$(subst .java,,$(suffix $(notdir $1 >> 356: >> 357: # The test selector starting with a hash (#testid) will be stripped by >> all > > Small nit - I think "selection" and "test selector" in the comments should be > changed to "test id" so you don't call the same thing with 3 different names. > But otherwise the change looks good to me. I agree. - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v5]
> 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 - Changes: - all: https://git.openjdk.java.net/jdk/pull/9028/files - new: https://git.openjdk.java.net/jdk/pull/9028/files/ac9631e2..a6d51c2a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9028=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9028=03-04 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/9028.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9028/head:pull/9028 PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v4]
On Tue, 7 Jun 2022 17:39:14 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: > > wip Marked as reviewed by iklam (Reviewer). make/RunTests.gmk line 357: > 355: $(subst .java,,$(subst .java,,$(suffix $(notdir $1 > 356: > 357: # The test selector starting with a hash (#testid) will be stripped by > all Small nit - I think "selection" and "test selector" in the comments should be changed to "test id" so you don't call the same thing with 3 different names. But otherwise the change looks good to me. - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make
On Mon, 6 Jun 2022 16:20:18 GMT, Ioi Lam wrote: >> Let me ask the obvious "dumb" question ... why does this have to be so >> complicated? Why isn't the name of the test simply passed through to jtreg >> as typed? > >> Let me ask the obvious "dumb" question ... why does this have to be so >> complicated? Why isn't the name of the test simply passed through to jtreg >> as typed? > > Is it because `#` is treated as comment by the shell? Could it be encoded by > something like `TestName.java%38testID`? After input from @iklam, I made this new version that gives a much cleaner diff. It calls the old code unmodified from a wrapper function. I think this is better. - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v4]
> 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: wip - Changes: - all: https://git.openjdk.java.net/jdk/pull/9028/files - new: https://git.openjdk.java.net/jdk/pull/9028/files/26a62eb0..ac9631e2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9028=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9028=02-03 Stats: 44 lines in 1 file changed: 18 ins; 16 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/9028.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9028/head:pull/9028 PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v3]
On Tue, 7 Jun 2022 13:45:47 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: > > wip I will add a comment: # The test selector starting with a hash (#testid) will be stripped by all evals # and will be reinserted in the last line by calling TestID (if it is present). Arguably I should also remove the hash part explicitly, would you like that? I think it might be more clear what the code does, but on the other hand it would maybe signal that it is a parsing strategy which it is not. - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Fri, 3 Jun 2022 07:56:38 GMT, Tim Prinzing wrote: > Fixed JtregNativeJdk.gmk to include c++ libs for NullCallerTest There are a few C++ tests under `test/jdk/java/foreign` as well. - PR: https://git.openjdk.java.net/jdk/pull/9010
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v3]
On Tue, 7 Jun 2022 13:45:47 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: > > wip I think this looks good, but Magnus should still take a look. - Marked as reviewed by erikj (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v3]
On Mon, 6 Jun 2022 07:55:09 GMT, Erik Joelsson wrote: >> Leo Korinth has updated the pull request incrementally with one additional >> commit since the last revision: >> >> wip > > make/RunTests.gmk line 444: > >> 442: $(strip $(foreach parser,ParseCustomTestSelection >> ParseGtestTestSelection ParseMicroTestSelection ParseJtregTestSelection >> ParseSpecialTestSelection \ >> 443: ,$(call $(parser),$1))) >> 444: endef > > As this is logically a one-liner macro, please format according to > guidelines. Also try to keep lines shortish. Something like this: > > Suggestion: > > ParseTestSelection = \ > $(strip $(foreach parser, ParseCustomTestSelection ParseGtestTestSelection > ParseMicroTestSelection \ > ParseJtregTestSelection ParseSpecialTestSelection, \ > $(call $(parser), $1) \ > )) I made it a one-liner, and I refactored the code to use `or` - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v3]
On Tue, 7 Jun 2022 13:45:47 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: > > wip @magicus could you also take a look please? - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v3]
> 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: wip - Changes: - all: https://git.openjdk.java.net/jdk/pull/9028/files - new: https://git.openjdk.java.net/jdk/pull/9028/files/704158c8..26a62eb0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9028=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9028=01-02 Stats: 14 lines in 2 files changed: 0 ins; 4 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/9028.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9028/head:pull/9028 PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Mon, 6 Jun 2022 15:56:47 GMT, Tim Prinzing wrote: > The idea was to reduce duplicate code. Changing to use objects to encapsulate > the up calls got rid of a lot of repeated code and made things simpler and > clearer. Objects are created with the class, method, and signature strings > and then calls can be made and the the error reporting has a context. It was > simply easier to do in c++ and there was no reason not to use it. Fair enough. At least we will now have a good example of what is needed to build a C++ test like this. - PR: https://git.openjdk.java.net/jdk/pull/9010
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v2]
On Tue, 7 Jun 2022 13:26:52 GMT, Leo Korinth wrote: >> make/RunTests.gmk line 47: >> >>> 45: define IfPrepend >>> 46: $(if $(strip $1),$(strip $2)$(strip $1),) >>> 47: endef >> >> These two probably fits better in make/common/Utils.gmk. >> >> Also please have a look at the [Code Conventions for the Build >> System](http://openjdk.java.net/groups/build/doc/code-conventions.html). One >> liner macros like this we like to format like this: >> >> >> IfPrepend = \ >> $(if $(strip $1),$(strip $2)$(strip $1),) > > I moved them both to `make/common/Utils.gmk`. (and made them one-liners, but with indentation four to match the code around) - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v2]
On Mon, 6 Jun 2022 06:50:44 GMT, Erik Joelsson wrote: >> Leo Korinth has updated the pull request incrementally with one additional >> commit since the last revision: >> >> wip > > make/RunTests.gmk line 39: > >> 37: >> >> 38: >> 39: HASH:=\# > > HASH is already defined in MakeBase.gmk so should be available here. thanks, in the new version I do not even use it. > make/RunTests.gmk line 47: > >> 45: define IfPrepend >> 46: $(if $(strip $1),$(strip $2)$(strip $1),) >> 47: endef > > These two probably fits better in make/common/Utils.gmk. > > Also please have a look at the [Code Conventions for the Build > System](http://openjdk.java.net/groups/build/doc/code-conventions.html). One > liner macros like this we like to format like this: > > > IfPrepend = \ > $(if $(strip $1),$(strip $2)$(strip $1),) I moved them both to `make/common/Utils.gmk`. > make/RunTests.gmk line 51: > >> 49: define TestID >> 50: $(subst .java,,$(subst .java$(HASH),,$(suffix $(notdir $1 >> 51: endef > > This macro deserves a comment explaining what it's trying to do. I added a description and made it a one-liner as well > make/RunTests.gmk line 53: > >> 51: endef >> 52: >> 53: define HashTestID > > This one too. I removed HashTestID and added the functionality directly into TestID > make/RunTests.gmk line 377: > >> 375: # either absolute or relative to any of the TEST_BASEDIRS or test roots. >> 376: define ParseJtregTestSelection >> 377: $(call IfAppend, \ > > Please adjust indentation of the whole body here. Fixed > make/RunTests.gmk line 448: > >> 446: # Now intelligently convert the test selection given by the user in TEST >> 447: # into a list of fully qualified test descriptors of the tests to run. >> 448: TESTS_TO_RUN:=$(strip $(foreach test,$(TEST),$(call >> ParseTestSelection,$(test > > I think this rewrite is more elegant, but logically there is a difference as > it no longer breaks early. I would like to have Magnus' opinion on this as he > wrote this originally. > > Style wise, please leave spaces around operators and space after comma when > possible. The top level strip should handle any whitespace issues caused by > this. I changed the code to use `$(or)` instead of `$(foreach)`, and I added spaces. - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make [v2]
> 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: wip - Changes: - all: https://git.openjdk.java.net/jdk/pull/9028/files - new: https://git.openjdk.java.net/jdk/pull/9028/files/553ecc35..704158c8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=9028=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9028=00-01 Stats: 82 lines in 2 files changed: 35 ins; 31 del; 16 mod Patch: https://git.openjdk.java.net/jdk/pull/9028.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9028/head:pull/9028 PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make
On Sat, 4 Jun 2022 01:51:20 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. `#` is treated as a comment by make itself. And the make file is using `eval` to set variables. Thus the test case selection will be regarded as comment (and stripped). - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v9]
On Thu, 2 Jun 2022 11:29:58 GMT, Christian Hagedorn wrote: >>> Thanks Thomas for doing the testing! >> >> Hi Christian, >> >> I can see no problems on ppcle attributable to your test. However, I may >> overlook something since tests are still failing all over the place because >> of loom. >> >> I see test errors in TestDwarf on ppcle and x64, however. On x64: >> >> >> --System.out:(52/5198)-- >> Command line: >> [/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/sapjvm_19/bin/java -cp >> /priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/jtreg_hotspot_tier1_work/JTwork/classes/runtime/ErrorHandling/TestDwarf.d:/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/grmpf/testdata/jtreg/jtreg_test_19/test/hotspot/jtreg/runtime/ErrorHandling:/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/jtreg_hotspot_tier1_work/JTwork/classes:/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/grmpf/testdata/jtreg/jtreg_test_19/test/hotspot/jtreg:/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/jtreg_hotspot_tier1_work/JTwork/classes/test/lib:/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/grmpf/testdata/jtreg/jtreg_test_19/test/lib:/net/sapmnt.sapjvm_work/openjdk/tools/jtreg-6+2/lib/javatest.jar:/net/sapmnt.sapjvm_work/openjdk/tools/jtreg-6+2/lib/jtreg.jar >> -Xmx768m -Djava.awt.headless=true >> -Djava.util.prefs.userRoot=/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/jtreg_ho tspot_tier1_work/tmp -Djava.io.tmpdir=/priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/jtreg_hotspot_tier1_work/tmp -Dtest.getfreeport.max.tries=40 -ea -esa -Xlog:dwarf=info -XX:-CreateCoredumpOnCrash -Xcomp -XX:CICrashAt=1 --version ] >> [2022-05-24T18:37:00.974121691Z] Gathering output for process 44490 >> [2022-05-24T18:37:02.872100892Z] Waiting for completion for process 44490 >> [2022-05-24T18:37:02.872338192Z] Waiting for completion finished for process >> 44490 >> Output and diagnostic info for process 44490 was saved into >> 'pid-44490-output.log' >> [2022-05-24T18:37:02.889455876Z] Waiting for completion for process 44490 >> [2022-05-24T18:37:02.889604189Z] Waiting for completion finished for process >> 44490 >> # To suppress the following error report, specify this argument >> # after -XX: or in .hotspotrc: SuppressErrorAt=/c1_Compilation.cpp:616 >> # >> # A fatal error has been detected by the Java Runtime Environment: >> # >> # Internal Error >> (/sapmnt/sapjvm_work/openjdk/nb/linuxx86_64/jdk-dev/src/hotspot/share/c1/c1_Compilation.cpp:616), >> pid=44490, tid=44505 >> # assert(CICrashAt < 0 || (uintx)_env->compile_id() != (uintx)CICrashAt) >> failed: just as planned >> # >> # JRE version: OpenJDK Runtime Environment (19.0) (fastdebug build >> 19-internal-adhoc.openjdk.jdk-dev) >> # Java VM: OpenJDK 64-Bit Server VM (fastdebug >> 19-internal-adhoc.openjdk.jdk-dev, compiled mode, sharing, tiered, >> compressed oops, compressed class ptrs, g1 gc, linux-amd64) >> # Problematic frame: >> # V [libjvm.so+0x73ca34] Compilation::~Compilation()+0x84 >> # >> # CreateCoredumpOnCrash turned off, no core file dumped >> # >> # An error report file with more information is saved as: >> # >> /priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/jtreg_hotspot_tier1_work/JTwork/scratch/hs_err_pid44490.log >> [1.835s][info][dwarf] Open DWARF file: >> /priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/sapjvm_19/lib/server/libjvm.debuginfo >> [1.842s][info][dwarf] Failed to parse the line number program header >> correctly. >> [1.842s][info][dwarf] Failed to process the line number program correctly. >> [1.842s][info][dwarf] Failed to retrieve file and line number information >> for >> /priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/sapjvm_19/lib/server/libjvm.so >> at offset: 0x0074176a >> [1.843s][info][dwarf] Failed to parse the line number program header >> correctly. >> [1.843s][info][dwarf] Failed to process the line number program correctly. >> [1.843s][info][dwarf] Failed to retrieve file and line number information >> for >> /priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/sapjvm_19/lib/server/libjvm.so >> at offset: 0x00a05011 >> [1.845s][info][dwarf] Failed to parse the line number program header >> correctly. >> [1.845s][info][dwarf] Failed to process the line number program correctly. >> [1.845s][info][dwarf] Failed to retrieve file and line number information >> for >> /priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/sapjvm_19/lib/server/libjvm.so >> at offset: 0x00a05b78 >> [1.849s][info][dwarf] Failed to parse the line number program header >> correctly. >> [1.849s][info][dwarf] Failed to process the line number program correctly. >> [1.849s][info][dwarf] Failed to retrieve file and line number information >> for >> /priv/jvmtests/output_openjdk19_dev_dbgU_linuxx86_64/sapjvm_19/lib/server/libjvm.so >> at offset: 0x018d49d3 >> [1.852s][info][dwarf] Failed to parse the line number program header >> correctly. >> [1.852s][info][dwarf]
Re: RFR: 8242181: [Linux] Show source information when printing native stack traces in hs_err files [v11]
> 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 filenames and line numbers as they are subject to > change (especially line number can quickly become different). I therefore > just added some sanity
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: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Mon, 6 Jun 2022 07:45:16 GMT, David Holmes wrote: > Hi Tim, > > Sorry but I have to ask why this test was created as a C++ program instead of > keeping it as a C program likes it predecessors? No need for C++ libs or > special exception handling flags in that case. The idea was to reduce duplicate code. Changing to use objects to encapsulate the up calls got rid of a lot of repeated code and made things simpler and clearer. Objects are created with the class, method, and signature strings and then calls can be made and the the error reporting has a context. It was simply easier to do in c++ and there was no reason not to use it. - PR: https://git.openjdk.java.net/jdk/pull/9010
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make
On Mon, 6 Jun 2022 10:48:05 GMT, David Holmes wrote: > Let me ask the obvious "dumb" question ... why does this have to be so > complicated? Why isn't the name of the test simply passed through to jtreg as > typed? Is it because `#` is treated as comment by the shell? Could it be encoded by something like `TestName.java%38testID`? - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287811: JFR: jfr configure error message should not print stack trace
On Fri, 3 Jun 2022 15:54:47 GMT, Erik Gahlin wrote: > Could I have a review of PR that removes a printStackTrace() for the jfr > tool. > > Testing: jdk/jfr/tool > > Thanks > Erik Marked as reviewed by mgronlun (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/9018
Re: RFR: 8287366: Improve test failure reporting in GHA
On Thu, 26 May 2022 12:04:41 GMT, Magnus Ihse Bursie wrote: > 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. - PR: https://git.openjdk.java.net/jdk/pull/8901
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make
On Sat, 4 Jun 2022 01:51:20 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. Let me ask the obvious "dumb" question ... why does this have to be so complicated? Why isn't the name of the test simply passed through to jtreg as typed? - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287828: Fix so that one can select jtreg test case by ID from make
On Sat, 4 Jun 2022 01:51:20 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. I think this is a nice fix. I mostly have style opinions. I would also like to have Magnus look at this as he wrote this originally. As for testing, it would be good to verify a decently sized job in Mach5, as well as trying to use the TestID selection on multiple platforms. make/RunTests.gmk line 39: > 37: > > 38: > 39: HASH:=\# HASH is already defined in MakeBase.gmk so should be available here. make/RunTests.gmk line 47: > 45: define IfPrepend > 46: $(if $(strip $1),$(strip $2)$(strip $1),) > 47: endef These two probably fits better in make/common/Utils.gmk. Also please have a look at the [Code Conventions for the Build System](http://openjdk.java.net/groups/build/doc/code-conventions.html). One liner macros like this we like to format like this: IfPrepend = \ $(if $(strip $1),$(strip $2)$(strip $1),) make/RunTests.gmk line 51: > 49: define TestID > 50: $(subst .java,,$(subst .java$(HASH),,$(suffix $(notdir $1 > 51: endef This macro deserves a comment explaining what it's trying to do. make/RunTests.gmk line 53: > 51: endef > 52: > 53: define HashTestID This one too. make/RunTests.gmk line 377: > 375: # either absolute or relative to any of the TEST_BASEDIRS or test roots. > 376: define ParseJtregTestSelection > 377: $(call IfAppend, \ Please adjust indentation of the whole body here. make/RunTests.gmk line 444: > 442: $(strip $(foreach parser,ParseCustomTestSelection > ParseGtestTestSelection ParseMicroTestSelection ParseJtregTestSelection > ParseSpecialTestSelection \ > 443: ,$(call $(parser),$1))) > 444: endef As this is logically a one-liner macro, please format according to guidelines. Also try to keep lines shortish. Something like this: Suggestion: ParseTestSelection = \ $(strip $(foreach parser, ParseCustomTestSelection ParseGtestTestSelection ParseMicroTestSelection \ ParseJtregTestSelection ParseSpecialTestSelection, \ $(call $(parser), $1) \ )) make/RunTests.gmk line 448: > 446: # Now intelligently convert the test selection given by the user in TEST > 447: # into a list of fully qualified test descriptors of the tests to run. > 448: TESTS_TO_RUN:=$(strip $(foreach test,$(TEST),$(call > ParseTestSelection,$(test I think this rewrite is more elegant, but logically there is a difference as it no longer breaks early. I would like to have Magnus' opinion on this as he wrote this originally. Style wise, please leave spaces around operators and space after comma when possible. The top level strip should handle any whitespace issues caused by this. - PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Fri, 3 Jun 2022 07:56:38 GMT, Tim Prinzing wrote: > Fixed JtregNativeJdk.gmk to include c++ libs for NullCallerTest Hi Tim, Sorry but I have to ask why this test was created as a C++ program instead of keeping it as a C program likes it predecessors? No need for C++ libs or special exception handling flags in that case. - PR: https://git.openjdk.java.net/jdk/pull/9010
RFR: 8287828: Fix so that one can select jtreg test case by ID from make
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. - Commit messages: - 8287828: Fix so that one can select jtreg test case by ID from make Changes: https://git.openjdk.java.net/jdk/pull/9028/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9028=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8287828 Stats: 48 lines in 1 file changed: 24 ins; 21 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/9028.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/9028/head:pull/9028 PR: https://git.openjdk.java.net/jdk/pull/9028
Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Fri, 3 Jun 2022 22:40:52 GMT, Tim Prinzing wrote: >> make/test/JtregNativeJdk.gmk line 67: >> >>> 65: BUILD_JDK_JTREG_EXECUTABLES_LIBS_exeNullCallerTest := $(LIBCXX) >>> jvm.lib >>> 66: BUILD_JDK_JTREG_EXECUTABLES_LIBS_exerevokeall := advapi32.lib >>> 67: BUILD_JDK_JTREG_EXECUTABLES_CFLAGS_exeNullCallerTest := /EHsc >> >> Does this test need this flag? or the default exception handling behavior >> is adequate? > > iostreams don't compile without it. While the tests currently have printf, > the 8281001 restores back to using iostream and this file doesn't have to > change. Ok, thanks. - PR: https://git.openjdk.java.net/jdk/pull/9010
Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Fri, 3 Jun 2022 18:16:53 GMT, Mandy Chung wrote: >> Fixed JtregNativeJdk.gmk to include c++ libs for NullCallerTest > > make/test/JtregNativeJdk.gmk line 67: > >> 65: BUILD_JDK_JTREG_EXECUTABLES_LIBS_exeNullCallerTest := $(LIBCXX) jvm.lib >> 66: BUILD_JDK_JTREG_EXECUTABLES_LIBS_exerevokeall := advapi32.lib >> 67: BUILD_JDK_JTREG_EXECUTABLES_CFLAGS_exeNullCallerTest := /EHsc > > Does this test need this flag? or the default exception handling behavior is > adequate? iostreams don't compile without it. While the tests currently have printf, the 8281001 restores back to using iostream and this file doesn't have to change. - PR: https://git.openjdk.java.net/jdk/pull/9010
Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Fri, 3 Jun 2022 07:56:38 GMT, Tim Prinzing wrote: > Fixed JtregNativeJdk.gmk to include c++ libs for NullCallerTest Thanks for doing the run, Dan. - PR: https://git.openjdk.java.net/jdk/pull/9010
Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Fri, 3 Jun 2022 07:56:38 GMT, Tim Prinzing wrote: > Fixed JtregNativeJdk.gmk to include c++ libs for NullCallerTest I did a Mach5 Tier1 test run on the v00 version of this patch. There were no failures and all 6 runs of jni/nullCaller/NullCallerTest.java passed. - PR: https://git.openjdk.java.net/jdk/pull/9010
Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"
On Fri, 3 Jun 2022 07:56:38 GMT, Tim Prinzing wrote: > Fixed JtregNativeJdk.gmk to include c++ libs for NullCallerTest Marked as reviewed by mchung (Reviewer). make/test/JtregNativeJdk.gmk line 67: > 65: BUILD_JDK_JTREG_EXECUTABLES_LIBS_exeNullCallerTest := $(LIBCXX) jvm.lib > 66: BUILD_JDK_JTREG_EXECUTABLES_LIBS_exerevokeall := advapi32.lib > 67: BUILD_JDK_JTREG_EXECUTABLES_CFLAGS_exeNullCallerTest := /EHsc Does this test need this flag? or the default exception handling behavior is adequate? - PR: https://git.openjdk.java.net/jdk/pull/9010
