-
Notifications
You must be signed in to change notification settings - Fork 753
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
java-version-file
with asdf
's .tool-versions
fails for Corretto, also non-strict semver versions
#615
Comments
Hello @rtyley |
This allows projects to specify, with an `asdf` (https://asdf-vm.com/)-formatted `.tool-versions` file, the Java version to be used by the workflow for building the project- `gha-scala-library-release-workflow` has always used a recent LTS version Java for the build (eg Java 17), and this has sometimes been _too recent_ (guardian/atom-maker#94) for some projects at the Guardian. We _want_ projects to update to Java 21 (see guardian/support-frontend#5792, guardian/scala-steward-public-repos#67 etc), but tying dozens of projects tightly to what `gha-scala-library-release-workflow` is using will make updating that version pretty hard. If, at some later point, _some_ projects want to experiment with Java 25, we shouldn't have to force all other projects to be compatible with that first. ## How to express the version of Java required... ### Configuration proliferation is a problem... One option would have been to simply add a new input parameter to the workflow, specifying the required Java version, but that has a downside: it proliferates the number of places in a project where the desired Java version is declared - and there are many in use at the Guardian, with no well-agreed canonical source-of-truth: * GHA `ci.yml`, in the [`java-version`](https://github.com/guardian/etag-caching/blob/7ecc04981f5a42a0f2ecb10631f28da571a49836/.github/workflows/ci.yml#L22) field of `actions/setup-java` - this has been my favourite in the past, as whatever CI runs with is usually pretty close to the truth * In sbt, the `scalacOptions` of `-target`, `-release`, `-java-output-version` (currently `-release` preferred, tho' once [support](scala/scala#10654) is pervasive, `-java-output-version` is probably best) and the `javacOptions` of `-target` & `-source` - these all effectively set lower-bounds on the version of Java supported, rather than guaranteeing a minimum upper bound of Java version the way CI does. * In apps running on Amigo AMI images; the Java version baked into the AMI, usually [referenced](https://github.com/guardian/mobile-apps-api/blob/3231e6bf064163c6d0e72c8dc862678c68eb0b62/mobile-fronts/conf/riff-raff.yaml#L10) by `riff-raff.yaml` * In AWS Lambdas; the cloudformation [`Runtime`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-lambda-function.html#cfn-lambda-function-runtime) parameter, often set [by CDK](https://github.com/guardian/mobile-save-for-later/blob/1ac12e4c0100edb976ebae9e2a9975ad2321e26e/cdk/lib/mobile-save-for-later.ts#L44) ### ...`asdf`'s `.tool-versions` flle offers some consolidation Benefits of using `.tool-versions`: * Developers will automatically get the right Java version if they have `asdf` installed * actions/setup-java#606 has added early support for reading the version of Java used in CI by `setup-java` from the `.tool-versions` flle - unfortunately, it's of limited use to us at the moment because of actions/setup-java#615, but it's a promising start _(`setup-java` also has support for `jEnv` files, but they are not commonly used at the Guardian)_ * Format of the file is simple enough that it can be easily understood and used, even if `asdf` is not in use - **including the file doesn't _force_ developers to use `asdf`** (I know some devs have been having some problems with it, and maybe there are/will be better alternatives out there), but it clearly documents the Java version to be used. This does mean that **all library repos need to add a `.tool-versions` file** before this PR is merged. Helpful error messaging has been added with this PR to handle cases where the file is missing or invalid: #### Only Java _major_ version is guaranteed Note that although `asdf` requires a fully-specified Java version (eg `21.0.3.9.1`) in the `.tool-versions` file, currently the workflow will only match the *major* version of Java specified in the file (eg `21`), and will _always_ use the AWS Corretto distribution of Java. This is due to [limitations](actions/setup-java#615) in [`actions/setup-java`](https://github.com/actions/setup-java).
Hello @rtyley, I hope this message finds you well. I wanted to kindly bring to your attention some observations regarding the adaptation of our support strategy for the asdf's tool versions file. Specifically, we've noted certain discrepancies with Java versions that do not entirely align with Semver standards. Moreover, it has come to our attention that some of the currently supported distributions are not listed in the asdf java available versions. To ensure consistency and adherence to best practices, our focus will now be directed towards maintaining .tool-versions files that fully comply with Semver standards. For your convenience, please note that we have also updated the documentation. Thank you for your attention to this matter. |
Hello @rtyley, this is a polite reminder to verify if your issue has been resolved with the revised documentation. |
Hi @rtyley, could you please confirm whether the revised documentation resolved your issue? |
Hi @mahabaleshwars!
I can see the appeal of choosing a standard, but I think there's a possibility that strict semver (MAJOR.MINOR.PATCH) is not an applicable standard, when major JDK distributors like AWS, Oracle, Microsoft, Azul & SAP are releasing JDK versions that do not confirm to semver. If the organisations releasing JDKs are not using semver, then what's so good about setup-java requiring semver?
Personally, I wouldn't say that the changes made to the documentation so far resolve the issue.
Incidentally, I appreciate you linked to the documentation, but as it's a large document, and you were asking me to comment on a change, it would actually be more helpful to include a link to the PR that made the change, which I've found is #622. So, I think there are two reasonable ways to address the issue, and updating the documentation is one of them - obviously, I'd much prefer that Unfortunately, I feel the changes in #622 aren't quite clear enough to help users to understand the limitations - essentially the explanation added in the PR was just adding ", adhering to Semantic Versioning (semver)" to the end of "...the Problems with this:
...personally, I don't think adhering to the semver standard here is a necessarily a gain for Note that even if AWS Corretto did use strict semver, |
As suggested in #209 (comment), we want the Lambda runtime to match the Java version used by developers. I've also updated CI to use Java 21. Ideally, we wouldn't have to do this separately anymore, as setup-java has added partial support for asdf's .tool-versions file with actions/setup-java#606, but unfortunately actions/setup-java#615 means that it won't work with Corretto version numbers.
As suggested in #209 (comment), we want the Lambda runtime to match the Java version used by developers. I've also updated CI to use Java 21. Ideally, we wouldn't have to do this separately anymore, as setup-java has added partial support for asdf's .tool-versions file with actions/setup-java#606, but unfortunately actions/setup-java#615 means that it won't work with Corretto version numbers.
As suggested in #209 (comment), we want the Lambda runtime to match the Java version used by developers. I've also updated CI to use Java 21. Ideally, we wouldn't have to do this separately anymore, as setup-java has added partial support for asdf's .tool-versions file with actions/setup-java#606, but unfortunately actions/setup-java#615 means that it won't work with Corretto version numbers.
Hello @rtyley, your efforts to pinpoint the problem are appreciated. The 'setup-java' currently adheres to semver, which includes the Major.Minor.Patch-'pre-release version'+metadata format. We included a reference link to the semver documentation for additional clarity. However, our support for .tool-versions is presently limited to versions compliant with semver (https://semver.org/). |
Hello @rtyley, we are closing this issue now. Please feel free to reach out to us for any other issues. |
github's setup-java support of asdf is pretty useless, so replicating version nr here actions/setup-java#615
github's setup-java support of asdf is pretty useless, so replicating version nr here actions/setup-java#615 : .tool-versions
* we can't build with 11 and 17 anymore, since we'll use 21 as the baseline * attempting to build with graalvm out of curiosity * github's setup-java support of asdf is pretty useless, so replicating version nr here -- see actions/setup-java#615
* we can't build with 11 and 17 anymore, since we'll use 21 as the baseline * attempting to build with graalvm out of curiosity * github's setup-java support of asdf is pretty useless, so replicating version nr here -- see actions/setup-java#615 * simplify matrix config, and ignore failures from now experimental matrix entries
Description:
Support for
asdf
's.tool-versions
file was added to thejava-version-file
parameter with PR #606 in response to issue #579 - unfortunately, there's a difference between whatasdf
requires for this file, vs whatsetup-java
supports:asdf
requires that the Java version in.tool-versions
is fully specifiedSo a file will typically look like this:
If you shorten the version, eg to
21.0.2
or21
, and runasdf install java
, you'll get this error fromasdf
:setup-java
only supports major versions or strict 3-part semversetup-java
supports version numbers of 1 to 3 dot-separated integers (egmajor
tomajor.minor.patch
). This excludes several valid version numbers - here are some examples:21.0.2.13.1
or8.322.06.4
)8.74.0.17
11.0.16.1
18.0.2.1
17.0.1.12.1
setup-java
only supports major versions for CorrettoThis is probably due to AWS Corretto only providing a simple API for looking up Java versions, that only supports getting the latest version for each major version of Java:
setup-java/src/distributions/corretto/installer.ts
Line 100 in 99b8673
So, Corretto with the
java-version-file
parameter is doubly unsupported:setup-java
to verify version numberscorretto/installer.ts
wouldn't be able to locate and download the specific Corretto version, because it can only support finding bymajor
version numbersetup-java/src/distributions/corretto/installer.ts
Lines 61 to 62 in 99b8673
Task version:
v4.2.1
Platform:
Runner type:
Repro steps:
Define an invocation of
actions/setup-java
in a workflow, and pointjava-version-file
to.tool-versions
:.github/workflows/ci.yml :
In
.tool-versions
, define a version likezulu-8.58.0.13
- and verify that works by executingasdf install java
:.tool-versions :
The let the
ci.yml
workflow run.Expected behavior:
Ideally, if the
.tool-versions
file is valid forasdf
,setup-java
should install that precise version of Java. This meanssetup-java
needs to accept all versions output byasdf list-all java
.Where a distribution has been specified that
setup-java
is unable to search for precise Java versions (ie Corretto), if the user has set ajava-version-accept-any-matching-major-version
parameter totrue
it should just install the latest version of Java for that major version.Actual behavior:
zulu-8.58.0.13
in.tool-versions
(guardian/etag-caching@959d4dc) :https://github.com/guardian/etag-caching/actions/runs/8470107933/job/23207054598#step:3:16
cc @aparnajyothi-y @mahabaleshwars @HarithaVattikuti
The text was updated successfully, but these errors were encountered: