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

Allow -java-output-version flag, matching Scala 3 #10654

Merged
merged 1 commit into from
Jan 13, 2024

Conversation

rtyley
Copy link
Contributor

@rtyley rtyley commented Jan 8, 2024

Following on from #9982 and discussion there, this change enables -java-output-version as an alternative name for the -release flag in Scala 2, as in Scala 3 -java-output-version is the preferred flag name (see scala/scala3#14606), with the alias of -release still permitted.

-Xunchecked-java-output-version is also included as an alternative for -target, though note use of this flag is discouraged in favour of -java-output-version/-release for Java 9 and above.

Following on from scala#9982 and
discussion in https://github.com/scala/scala/pull/9982/files#r1434172490,
this change enables `-java-output-version` as an alternative name for
the `-release` flag in Scala 2, as in Scala 3 `-java-output-version` is
the preferred flag name (see scala/scala3#14606 ),
with the alias of `-release` still permitted.

Correspondingly, `-Xunchecked-java-output-version` is also included as an
alternative for `-target`, though note use of this flag is discouraged
in favour of `-java-output-version`/`-release` for Java 9 and above.
Copy link
Contributor

@som-snytt som-snytt left a comment

Choose a reason for hiding this comment

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

Thanks! Historical note, at the time, Scala 3 hadn't quite hashed out their new options.

@som-snytt som-snytt merged commit 4deab53 into scala:2.13.x Jan 13, 2024
3 checks passed
rtyley added a commit to guardian/gha-scala-library-release-workflow that referenced this pull request Apr 30, 2024
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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants