Re: RFR: 8325444: GHA: JDK-8325194 causes a regression
On Wed, 7 Feb 2024 19:50:51 GMT, Christoph Langer wrote: > The > [change](https://github.com/openjdk/jdk/commit/d1c82156ba6ede4b798ac15f935289cfcc99d1a0) > for [JDK-8325194](https://bugs.openjdk.org/browse/JDK-8325194) broke > get-jtreg because the way to determine the build jdk was not correct. > > I then tried to use the bootstrap JDK with the correct wiring but it turned > out that the build of JTReg fails with the current JDK, see > [here](https://github.com/RealCLanger/jdk/actions/runs/7820130826/job/21334120478). > > So I propose to fix it by going to the originally proposed solution using > `$JAVA_HOME_17_`. Okay, whatever. - Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17756#pullrequestreview-1870266919
Re: RFR: 8325444: GHA: JDK-8325194 causes a regression
On Thu, 8 Feb 2024 07:31:15 GMT, Magnus Ihse Bursie wrote: >> The >> [change](https://github.com/openjdk/jdk/commit/d1c82156ba6ede4b798ac15f935289cfcc99d1a0) >> for [JDK-8325194](https://bugs.openjdk.org/browse/JDK-8325194) broke >> get-jtreg because the way to determine the build jdk was not correct. >> >> I then tried to use the bootstrap JDK with the correct wiring but it turned >> out that the build of JTReg fails with the current JDK, see >> [here](https://github.com/RealCLanger/jdk/actions/runs/7820130826/job/21334120478). >> >> So I propose to fix it by going to the originally proposed solution using >> `$JAVA_HOME_17_`. > > I don't like this approach. There must be better ways to achieve this, like > inputting the correct value as input to the action, or setting it as a global > gh variable. Where does even the `$JAVA_HOME_17_arm64` come from? Is it > provided by GH? I don't recall setting variables with that weird case style > in our GHA files. OK, I'm ready to integrate. Waiting for blessing from @magicus. - PR Comment: https://git.openjdk.org/jdk/pull/17756#issuecomment-1934128826
Re: RFR: 8325444: GHA: JDK-8325194 causes a regression
On Wed, 7 Feb 2024 19:50:51 GMT, Christoph Langer wrote: > The > [change](https://github.com/openjdk/jdk/commit/d1c82156ba6ede4b798ac15f935289cfcc99d1a0) > for [JDK-8325194](https://bugs.openjdk.org/browse/JDK-8325194) broke > get-jtreg because the way to determine the build jdk was not correct. > > I then tried to use the bootstrap JDK with the correct wiring but it turned > out that the build of JTReg fails with the current JDK, see > [here](https://github.com/RealCLanger/jdk/actions/runs/7820130826/job/21334120478). > > So I propose to fix it by going to the originally proposed solution using > `$JAVA_HOME_17_`. Noting the discussion here: https://github.com/actions/runner-images/discussions/9266 GitHub are considering removing the architecture prefix - PR Comment: https://git.openjdk.org/jdk/pull/17756#issuecomment-1933883736
Re: RFR: 8325444: GHA: JDK-8325194 causes a regression
On Wed, 7 Feb 2024 19:50:51 GMT, Christoph Langer wrote: > The > [change](https://github.com/openjdk/jdk/commit/d1c82156ba6ede4b798ac15f935289cfcc99d1a0) > for [JDK-8325194](https://bugs.openjdk.org/browse/JDK-8325194) broke > get-jtreg because the way to determine the build jdk was not correct. > > I then tried to use the bootstrap JDK with the correct wiring but it turned > out that the build of JTReg fails with the current JDK, see > [here](https://github.com/RealCLanger/jdk/actions/runs/7820130826/job/21334120478). > > So I propose to fix it by going to the originally proposed solution using > `$JAVA_HOME_17_`. I would prefer us to unbreak GHA for everyone first, and then look for the cleaner solutions to this problem. Current fix, while ugly, is essentially what we did before: using the runner image var. The `JAVA_HOME_17_arm64` is documented here for `macos-14` runner image: https://github.com/actions/runner-images/blob/main/images/macos/macos-14-arm64-Readme.md - PR Comment: https://git.openjdk.org/jdk/pull/17756#issuecomment-1933625985
Re: RFR: 8325444: GHA: JDK-8325194 causes a regression
On Thu, 8 Feb 2024 07:40:50 GMT, Christoph Langer wrote: > > I don't like this approach. There must be better ways to achieve this, like > > inputting the correct value as input to the action, or setting it as a > > global gh variable. Where does even the `$JAVA_HOME_17_arm64` come from? Is > > it provided by GH? I don't recall setting variables with that weird case > > style in our GHA files. > > I think (as @gdams told me) these are paths to the Java installation in the > Github Runners which happen to be there. > > I agree, to avoid the if statement, we could properly set it as input to > get-jtreg. > > We probably could also use the setup-java action here - however, this will > then lead to downloading another JDK, not sure if this is a performance > concern. @magicus I looked a bit more into the alternatives. So, regarding using the correct value as input to the action, I actually don't see that it buys us anything. We'd have switch statements further up when invoking the actions and we'll likely have even more of them. And as for setup-java, this also would only set up a `JAVA_HOME_{{ MAJOR_VERSION }}_{{ ARCHITECTURE }}` environment which we'd have to reference then. So no gain. Unless you have some great other idea, I don't see how we could resolve this more elegantly. - PR Comment: https://git.openjdk.org/jdk/pull/17756#issuecomment-1933615018
Re: RFR: 8325444: GHA: JDK-8325194 causes a regression
On Wed, 7 Feb 2024 19:50:51 GMT, Christoph Langer wrote: > The > [change](https://github.com/openjdk/jdk/commit/d1c82156ba6ede4b798ac15f935289cfcc99d1a0) > for [JDK-8325194](https://bugs.openjdk.org/browse/JDK-8325194) broke > get-jtreg because the way to determine the build jdk was not correct. > > I then tried to use the bootstrap JDK with the correct wiring but it turned > out that the build of JTReg fails with the current JDK, see > [here](https://github.com/RealCLanger/jdk/actions/runs/7820130826/job/21334120478). > > So I propose to fix it by going to the originally proposed solution using > `$JAVA_HOME_17_`. Looks good. - Marked as reviewed by shade (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17756#pullrequestreview-1869529331
Re: RFR: 8325444: GHA: JDK-8325194 causes a regression
On Thu, 8 Feb 2024 07:31:15 GMT, Magnus Ihse Bursie wrote: > I don't like this approach. There must be better ways to achieve this, like > inputting the correct value as input to the action, or setting it as a global > gh variable. Where does even the `$JAVA_HOME_17_arm64` come from? Is it > provided by GH? I don't recall setting variables with that weird case style > in our GHA files. I think (as @gdams told me) these are paths to the Java installation in the Github Runners which happen to be there. I agree, to avoid the if statement, we could properly set it as input to get-jtreg. We probably could also use the setup-java action here - however, this will then lead to downloading another JDK, not sure if this is a performance concern. - PR Comment: https://git.openjdk.org/jdk/pull/17756#issuecomment-1933513448
Re: RFR: 8325444: GHA: JDK-8325194 causes a regression
On Wed, 7 Feb 2024 19:50:51 GMT, Christoph Langer wrote: > The > [change](https://github.com/openjdk/jdk/commit/d1c82156ba6ede4b798ac15f935289cfcc99d1a0) > for [JDK-8325194](https://bugs.openjdk.org/browse/JDK-8325194) broke > get-jtreg because the way to determine the build jdk was not correct. > > I then tried to use the bootstrap JDK with the correct wiring but it turned > out that the build of JTReg fails with the current JDK, see > [here](https://github.com/RealCLanger/jdk/actions/runs/7820130826/job/21334120478). > > So I propose to fix it by going to the originally proposed solution using > `$JAVA_HOME_17_`. I don't like this approach. There must be better ways to achieve this, like inputting the correct value as input to the action, or setting it as a global gh variable. Where does even the `$JAVA_HOME_17_arm64` come from? Is it provided by GH? I don't recall setting variables with that weird case style in our GHA files. - PR Comment: https://git.openjdk.org/jdk/pull/17756#issuecomment-1933499696
RFR: 8325444: GHA: JDK-8325194 causes a regression
The [change](https://github.com/openjdk/jdk/commit/d1c82156ba6ede4b798ac15f935289cfcc99d1a0) for [JDK-8325194](https://bugs.openjdk.org/browse/JDK-8325194) broke get-jtreg because the macro to determine the build jdk was not correct. I fix this by reverting to the originally proposed way to use `$JAVA_HOME_17_`. I tried to use the bootstrap JDK in first place with the correct wiring but it turned out that the build of JTReg fails with that version, see [here](https://github.com/RealCLanger/jdk/actions/runs/7820130826/job/21334120478). - Commit messages: - JDK-8325444 Changes: https://git.openjdk.org/jdk/pull/17756/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=17756&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8325444 Stats: 7 lines in 1 file changed: 6 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17756.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17756/head:pull/17756 PR: https://git.openjdk.org/jdk/pull/17756
Re: RFR: 8325444: GHA: JDK-8325194 causes a regression
On Wed, 7 Feb 2024 19:50:51 GMT, Christoph Langer wrote: > The > [change](https://github.com/openjdk/jdk/commit/d1c82156ba6ede4b798ac15f935289cfcc99d1a0) > for [JDK-8325194](https://bugs.openjdk.org/browse/JDK-8325194) broke > get-jtreg because the macro to determine the build jdk was not correct. > > I fix this by reverting to the originally proposed way to use > `$JAVA_HOME_17_`. I tried to use the bootstrap JDK in first place with > the correct wiring but it turned out that the build of JTReg fails with that > version, see > [here](https://github.com/RealCLanger/jdk/actions/runs/7820130826/job/21334120478). Marked as reviewed by gdams (Author). - PR Review: https://git.openjdk.org/jdk/pull/17756#pullrequestreview-1868656028