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 >
Integrated: 8288195: Prepare build system for GHA changes
On Fri, 10 Jun 2022 09:54:36 GMT, Magnus Ihse Bursie wrote: > A few changes to the build system is needed for the GHA rewrite > ([JDK-8287906](https://bugs.openjdk.org/browse/JDK-8287906)). This pull request has now been integrated. Changeset: d0c8ff8f Author:Magnus Ihse Bursie URL: https://git.openjdk.org/jdk/commit/d0c8ff8fdfe86a4251290d4c1c7b3dbd4cfaf018 Stats: 28 lines in 3 files changed: 13 ins; 0 del; 15 mod 8288195: Prepare build system for GHA changes Reviewed-by: shade, erikj - PR: https://git.openjdk.org/jdk/pull/9122
Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v4]
On Fri, 10 Jun 2022 07:23:12 GMT, Aleksey Shipilev wrote: >> Magnus Ihse Bursie has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Apparently that was not a legal reference to actions/checkout. Try another >> way. > > make/InitSupport.gmk line 456: > >> 454: $(PRINTF) "\n* All command lines available in >> $(MAKESUPPORT_OUTPUTDIR)/failure-logs.\n" ; \ >> 455: $(PRINTF) "=== End of repeated output ===\n" ; \ >> 456: ) >> $(MAKESUPPORT_OUTPUTDIR)/failure-summary.log \ > > Changes in this file look like a generic build system improvement, so maybe > it should be forked out as separate issue? In case this part of build system > change regresses something, I would be odd to track it down to GHA workflow > improvement. Integrated as JDK-8288195. - PR: https://git.openjdk.org/jdk/pull/9063
RFR: 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 >
Integrated: 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? This pull request has now been integrated. Changeset: 09015488 Author:KIRIYAMA Takuya Committer: Erik Joelsson URL: https://git.openjdk.org/jdk/commit/0901548833a0125f15fede64bc2e7dbe84fed42d Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8283724: Incorrect description for jtreg-failure-handler option Reviewed-by: erikj, ihse - PR: https://git.openjdk.org/jdk/pull/9099
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
