Skip to content
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

Build library with Java version specified in .tool-versions #36

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rtyley
Copy link
Member

@rtyley rtyley commented Apr 26, 2024

This allows projects to specify, with an asdf-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 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 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 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 by riff-raff.yaml
  • In AWS Lambdas; the cloudformation Runtime parameter, often set by CDK

...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
  • Added .tool-versions file support 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 java-version-file with asdf's .tool-versions fails for Corretto, also non-strict semver versions 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:

image

image

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 in actions/setup-java.

rtyley added a commit to guardian/redirect-resolver that referenced this pull request Apr 26, 2024
@rtyley rtyley force-pushed the use-java-version-specified-by-tool-versions-for-project-build branch 2 times, most recently from e53d3f4 to 6225e8d Compare April 26, 2024 15:26
@rtyley rtyley force-pushed the use-java-version-specified-by-tool-versions-for-project-build branch 5 times, most recently from 2cbc98a to 6f7023e Compare April 26, 2024 16:53
rtyley added a commit to guardian/etag-caching that referenced this pull request Apr 26, 2024
@rtyley rtyley force-pushed the use-java-version-specified-by-tool-versions-for-project-build branch 2 times, most recently from 6d695c1 to 12d46a3 Compare April 26, 2024 17:17
@rtyley rtyley changed the title WIP Build library with Java version specified in .tool-versions Apr 26, 2024
@rtyley rtyley force-pushed the use-java-version-specified-by-tool-versions-for-project-build branch 3 times, most recently from 75e43c2 to bbd2dda Compare April 30, 2024 14:16
@rtyley rtyley marked this pull request as ready for review April 30, 2024 14:22
@rtyley rtyley force-pushed the use-java-version-specified-by-tool-versions-for-project-build branch from bbd2dda to a008534 Compare April 30, 2024 15:22
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).
@rtyley rtyley force-pushed the use-java-version-specified-by-tool-versions-for-project-build branch from a008534 to d967417 Compare April 30, 2024 15:24
echo "::error title=Missing .tool-versions file::gha-scala-library-release-workflow requires an asdf-format .tool-versions file to establish the Java version for the build."
exit 1
fi
LIBRARY_BUILD_MAJOR_JAVA_VERSION=$( grep -Eo 'java [[:alnum:]-]+-[[:digit:]]+' .tool-versions | rev | cut -d'-' -f1 | rev )
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This regex has been manually checked by running it on the output of asdf list-all java (1238 versions), ie:

asdf list-all java | sed -e 's/^/java /' | grep -Eo 'java [[:alnum:]-]+-[[:digit:]]+' | rev | cut -d'-' -f1 | rev > major-java-versions.csv

The spreadsheet of the results is here - a quick skim through doesn't show any obvious problems to me.

rtyley added a commit to rtyley/sample-project-using-gha-scala-library-release-workflow that referenced this pull request May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant