-
Notifications
You must be signed in to change notification settings - Fork 751
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
V2 setup-java ADR #97
Conversation
@konradpabjan , probably, my point is more technical detail and outside of this PR but I think we need to define how
|
@maxim-lobanov I'd suggest to avoid
This action though may provide an option that the user may indicate a remote URL pointing to a very specific/custom binary. Perhaps even one the user built by itself for whatever reason. This argument could then be leveraged for those who want something like a |
A key comment/question: what is the motivation and justification behind the switch of the default from Zulu to AdoptOpenJDK? An argument for having no default would be one thing (i.e. force a choice of a distro and refuse to work without one), but switching the default from one free distro to another seems like a choice that would require at least some significant amount of open debate before taking action... I'd highlight some considerations that IMO stand out as important for a default (as long as one exists):
I'm looking forward to seeing some open discussion of counter-arguments and reasons for switching away from the current default. |
A couple of naming ("spelling") comments:
|
Hi Gil, Let me address this one first as it directly touches my comment. This action's goal is to set up Java - the JDK - for the purpose of a CICD build environment in GitHub Actions for general purpose Java applications. For this reason I recommend we keep it simple. Given that not all distributions provide a bundle with JavaFX, it would be inconvenient to have this action listing X vendors, but only X-N offering a particular package. For that reason, I support keeping Similar situation happens with JRE packages. Besides the fact that not all vendors provide a "JRE" package, the main reason I suggest we keep it out of this is that this action's goal, again, is to set up the JDK environment for CICD purposes. If someone wants to ship a JRE inside a container, they'd probably have to download the package again during the workflow. Or perhaps simply use an existing Docker image as the base for their work. This action works by downloading/caching/extracting the package, and it would be quite troublesome to document (and maintain) where the I really see no benefit here to have added complexity, expanded and non-complete matrix of vendors x packages, and using an Action just to download a JRE to be shipped into a Docker image. For the latter, there are many better ways to do that. bb. |
There are lots of CICD workflows today using jdk+fx and jre packages (several hundreds of each can be found by searching github right now). This sometimes happens when the package type is needed for some functionality, but is also (more often?) driven by wanting to make sure tests actually work with the range of versions and package types a project intends to be "actually work on".
I think that as we add support for multiple distros, we must accept that the common denominator available across all distros should not limit the options available for what can be spelled about a requested java setup. This is especially true when considering completely removing (across all distros) the ability to "spell" the need for functionality that is already being used by several hundred existing repos. Note that the same argument you make about jdk+fx and jre not being available from all distros could then be identically made about java versions and their updates. E.g. not all distros include 6u, or 7u, or 9u, 10u, 12u and 14u, or 14-ea, 15-ea and 16-ea. Some repos that supported some versions up to a point no longer update them to the latest OpenJDK update levels. And some distros that at one point offered some versions no longer keep things that were previously published available "forever". I don't think any of these distro-specific "missing parts" should be used to disqualify a distro from being optionally specified, but it should certainly not be used to stop the associated versions or update levels from being served up by other distros that do support them. Distro-specific limitations will be a thing. I think that we should accept this and think through what the propper and perhaps common-across-distros "error messages" should exist to make it clear that the thing you are requesting is not available in the distro that you chose, as opposed to the things that you are requesting being a syntax error. |
In my opinion, there are two key reasons here:
|
As I suggested before (missing link to comment) on the other PR, I think there shouldn't be a default vendor on v2. While the VM may provide a default JDK version from a specific vendor - today that's Java 8 from Adopt - once a user explicitly adds Setup Action to their workflow, they should provide at the very least two arguments: version and vendor, or a URL for an explicit package to be downloaded and used. It would be confusing if we end up having a default in the VM that does not match the default in this Action, and such link would make it really hard for us to evolve and upgrade things. |
Having a default vendor means that there is one less thing for developers to consider when composing a workflow. If there is no default vendor, the action would no longer "just-work" without any inputs. |
To be clear, existing users are going to have to explicitly bump their setup-java action to v2 so I'm not sure that people are going to be broken. I do agree though. On the other side, the fewer configuration options to made the action work, the better IMO. Setting a default is unlikely to cause any harm. |
@AustinShalit having this action "working" without any user input is no different than not using this action at all, since the VMs come with JDK installed OOTB. @gdams we can and should document recommendations in case users are uncertain. But we should also help them understand the JDK landscape. I don't see harm being done if we do require two parameters. |
There are over 40,000 workflows using setup-java right now. In its current v1.x form it has supported only one (default) distro, and that OpenJDK distro has consistently been free, 100% OSS, and widely used for over seven years, with a wide spectrum of supported java versions and a track re did for uninterrupted updates of all active OpenJDK project versions. With the welcome addition of a number of new, also free, also 100% OSS, also widely used OpenJDK distros over the past couple of years (with e.g. Corretto, Liberica, Adopt, DrangonWell, SapMachine, all coming on the scene), the addition of options for selecting other distros, regardless of whether or not they cover the spectrum of current use, makes perfect sense. Dramatically shrinking the spectrum of use does not, and taking away functionality from existing users does not either. The silly claims of “neutrality” by one vendor attempting to displace others (one that is clearly the least neutral of the bunch, and the only one slinging this newly invented neutrality mud around) are, frankly, offensive to the multitude of actual OpenJDK contributors that work to actively maintain or develop OpenJDK versions, update them regularly, and keep providing good free binary distros with continued improvements to security and stability. Calling a distro Neutralium does not make it more free, neutral, or deserving than other distros. I’d like to steer the discussion around default choices away from arguments that amount to “my distro name has the word OpenJDK in it” and towards actual technical subjects. There are very real functionality and use mode spectrum discussions to be had. Let me highlight a key part of the spectrum of current use, and let’s remember that some of the most common uses of setup-java is in CICD setups and in regression testing. Let’s also keep in mind that the action is widely used across a spectrum that spans not only team-focused development projects and applications that can control and focus their use cases or versions, but also covers a wide range of publicly available libraries and projects that are then leveraged by many others... This latter group is the one that tends to need a wide spectrum of versions and configurations to test against, and that spectrum includes many of the following:
The currently adopted behaviors and available package and version options did not evolve in a vacuum. Let’s not ignore the reasons they exist. The current default supports the spectrum above. The default distro that setup-java currently uses has a long and consistent track record for building a huge spectrum of versions, updates, and package types, rather than arguing that only some are needed, and that approach has led to wide adoption (over 40K workflows in under a year, and growing fast). I’d be happy to see additional distros step up and start doing the same (rather than argue that the things they don’t choose to build should be taken away from others). But until they do, switching the default to ones that don’t cover the current use spectrum would require one to make some coherent argument for e.g. removing 7u testing capability from the default available to maven central projects, or for not updating to the latest 13u, 15u, or 16-ea update as they come out in OpenJDK. Or for no longer working for workflows that explicitly test against 8u144, or 6u79, or 12.0.2, or 13.0.3 for regression testing and change isolation reasons (or for e.g. some ALPN version-specific compatibility reasons). Each of these is a current, non-hypothetical use in existing GitHub workflows... |
@giltene I am advocating for v2 to not have a default vendor. Are you in favor of that? Users would have to pick a vendor and a version: - name: Set up JDK 11
uses: actions/setup-java@v2
with:
java-version: 11
java-distro: zulu Frankly, I am not following why so much debate for an action that should be able to cover 80% of the cases in a very simplistic way, and for the other 20%, developers can easily set up JAVA_HOME using wget/curl. |
Switching the default (which is the initial proposal, and the main thing I am responding to) and eliminating the default are two separate things. You are correct that if chosen, the latter will obviously make the former moot. But unless serious momentum behind removing the default altogether starts forming, I'm focusing my discussion on the first one. As for removing the default: I think that it is a matter that warrants separate discussion. I can see some merit in it, and a logical argument that perhaps it should have been that way from the start, and that e.g. even when only one distro was supported with ways of spelling out JDK version and package requirements (and matrixes around them), the distro option should have been required. I'm on the fence on that one [i.e. what would the "right way to start" have been]: the simplicity of not spelling out the distro probably helped, and is clearly natural. But it also led us to the "oh, there is more than one OpenJDK distro, which one do you mean" discussions and to the sort of "this is the wrong default" arguments that inevitably arise from choice. But hindsight may not serve us well now. There are 40K+ workflows happily using the current scheme, and there are plenty of options (in the form of other actions) available for the small minority who are unhappy with its defaults right now. Many of the 40K+ workflows will likely break (or worse, choose to stick much longer to 1.x) if forced to add or change options to make their current workflows continue to work. The cleanest way to move people to new versions and capabilities is to avoid breaking changes wherever possible. That's not to say that breaking changes should never be made, but when you do make them, there should be some real reason and benefit to existing users (or to new users that were unable to use stuff before the change is introduced) for taking that action. When choosing to introduce a breaking change, trying to minimize the % of existing cases that would actually break should be a key design concern as well. The options we are discussing are: a) Eliminating the default (and requiring an extra option). This will create a breaking change for 100% of workflows that would switch to 2.x. An arguably simple to address breaking change, but 40K+ workflows will need to address it. b) Changing the default (as suggested at the top of this discussion). This will create a breaking change for some unpredictable portion of workflows that currently make use of anything that AdoptOpenJDK does not currently offer. This set currently includes 7u, 8u versions before some update level, 13 updates after some update levels, 15-ea or 16-ea, 6u, 9u, 10, jdk+fx packages, etc., each of which has actual uses in current workflows. c) Keeping the current default and adding the distro option, along with expanding the distro option to cover not just Zulu and AOJ, but also Corretto, Liberica, Dragonwell, Oracle OpenJDK, SAP Machine, etc... This will address the actual needs stated by people for having alternate distros, without forcing a breaking change on any of the current 40K+ workflows that use 1.x, and without forcing anyone to break up their matrixes or finding new hardcoded URI replacements to their current working matrix setups... To me, (c) seems the obvious right path. I'm waiting for an "(a) or (b) would be better because of X, and X is worth breaking 40K existing workflows to achieve." argument.
The removal of the need to point test workflows to specific URIs is one of the key drivers of setup-java's success IMO, and is the historical use mode in CICD systems. For the same reason that we usually look to avoid spelling out code dependencies in Maven or Gradle with URIs for specific jars. The ability to cleanly spell out and run a test matrix across e.g. [7, 7u242, 8, 8u232, 11, 11.0.4, 13, 13.0.4] (and add 15-ea, and 16-ea to that for future-proofers) is here today, and is much wider than 20% IMO. Most projects that produce code used by others (libraries, stuff on maven central, etc.) need this sort of test matrix. It is the likely best practice for projects that are looking to both test for regressions introduced in their own code (and thus should be testing on fixed JDK update levels) and for regressions or issues introduced in new updates to JDKs (and thus should be testing on the latest update of each). |
Here is a suggested [potential] staged approach:
Making an actual choice on 2.1 and 2.2 without actually having a working distro option that people can use, give feedback on, and which we can do work to improve through that feedback seems like a recipe for more hind sight discussions later on... |
I'd like to chime in as a maintainer of 2 different Java OSS projects that are using this action to verify that our projects are working on multiple Java versions. First I have to admit that before the discussion here and in #13, I was not aware of the different way the different JDK distributions work. Thanks to everyone for contributing to these discussions, I learned quite a lot. One of the key things for me as a maintainer of an OSS project is the easy way of setting up my CI/CD and having the trust that it will continue working without a breaking change, especially when I choose a different version such as 15-ea, 16-ea as a JDK version. If a vendor chooses to remove some JDK from being available for download this would mean that my CI/CD will break and I will have to rush and find the time to fix it, even though nothing in my project changed. From my current understanding it seems that only Zulu offers a perpetual URL for downloading those JDKs. Therefore I think that Zulu is a good option for multitude of OSS projects. The fact that you want to switch to v2, only because you want to change the default option of the JDK means that the users of this action will be split between v1 and v2 for quite some time. A lot of OSS maintainers are limited in their time they have to spend on the projects, they are not backed by companies and are doing this in their own free time. Therefore, I guess a lot of them will stay on v1 for quite some time, or something will break for them if they are using Having said all of this in my opinion as an OSS maintainer I think that the approach proposed by @giltene ( 1. Add distro option, 2. Decide if changing the default is OK or decide if distro should be mandiatory) is a really good one. In order to get the best feedback and adoption I would even say that "adding distro option" can be done in v1 safely. I would also like to ask to add that this ADR should mention the different rules a specific distro provides their versions. e.g. currently AdoptOpenJDK does not have the "X-ea" and pre 8 (this is actually a specific 8u252) builds, Oracle OpenJDK removes the X-ea builds once the X version is released. The reason for me mentioning this is that in my opinion as an OSS maintainer, I would like to be able to test with the versions I have defined in my action and expect it to work without the need to do anything specific. If I choose an explicit vendor then it is completely expected for something to stop working if that distro no longer provides the version I need. |
can anything be done to move this forward - as of today I have to depend on a fork for adoptopenjdk support - and would prefer using the official action. |
LGTM! I can not wait for it or contribute in some way. |
FYI I have created a github action for |
To help move this forward, I'd like to re-iterate my suggestion in #97 (comment) : Lets focus V2 on adding actual ability to support multiple distros in a non-breaking way for existing workflows, and remove the changes to the defaults from the goals (and punt discussion about them to a later time). |
This ADR stalled a bit because we haven't had time to pick this up. We should be revisiting this soon. @AlenaSviridenko will be helping out to move this forward I've made some small clarifications to the ADR based off of some of the feedback here and some internal discussions that we've had. For This will causes workflows to fail for users that are pinned to There are also a few comments about first adding a distro option and then changing the default behavior. We are going to be avoiding this. The main reason is because breaking changes should only be introduced when going to a new major version ( |
FYI, work towards a generic means of discovering bits for a distribution is being done within foojay.io, and will likely soon be exposed as an API useable from within the setup-java implementation. The API will cover the same sort of options setup-java now supports for specifying things (version specification in server format (extended to support JEP322), package type), as well as specifying a distribution and a target platform. Expected distros supported out of the gate will likely include Zulu, AOJ, Corretto, SapMachine, Liberia, Dragonwell, and Oracle OpenJDK (Adoptium will likely be added as soon as it starts making bits available). The community need for such an API has become evident in multiple areas, with actions/setup-java being one example out of many: IDEs and various other build toolchains share this need as well. Beyond being able to locate bits by specification as is the need in the setup-java use case (and in typical CI/CD setups), the API will provide natural flow for discovering e.g. available java versions and update levels, and the distributions that support a given version, update level, and platform (as is the likely need in e.g. an interactive IDE workflow, where a developer may browsing for bits for a given java version or update level, but may not know which distributions have such bits available). Given it's core purpose, foojay.io seems like a good place to host and maintain such an API. Once the foojay.io distro discovery API is up, we'll be able to address #13, #68, #69, #70, #71, #72 with a single, common logic path that will work for all, and will even allow for the inclusion of additional (future) distros with no further code changes. We'll be able to add a |
Merging in the ADR! Some CI is failing but it's unrelated to the changes in this PR. The tests are being overhauled significantly with |
Hello everyone! |
Where can I find tueclwtest version of the actual ADR? The link to a rendered version at the top of this PR seems to be broken. |
Overview
This PR outlines the basic roadmap & plan for the
v2
version ofsetup-java
.🖌 v2 ADR rendered 🎨
In order to support #13, the action will have to be overhauled pretty extensively to support downloading different distributions of java. The same work can then be used to support even more distributions in the future.
As part of
v2
, the default downloaded version will switch toadoptOpenJDK
instead ofZulu
. This is a major breaking change that warrants a major version upgrade (v1
->v2
).Some existing work was done by users to support
adoptOpenJDK
(#67). That work can be leveraged after a consensus is reached with this ADR.We need your help!
Any ideas, suggestions, or feedback will be invaluable during planing ❤️