Re: RFR: 8287906: Rewrite of GitHub Actions (GHA) sanity tests [v3]

2022-06-09 Thread Magnus Ihse Bursie
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]

2022-06-09 Thread Magnus Ihse Bursie
> 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 
>