Skip to content

Conversation

melix
Copy link
Contributor

@melix melix commented Aug 13, 2021

Context

The current Java toolchain mechanisms only allow downloading (provisioning) JDKs from AdoptOpenJDK. However, there are multiple JDKs which are available via different means, and could be automatically provisioned by Gradle too.

For this purpose, as discussed in Slack this pull request adds support for custom toolchain provisioning.

In a nutshell, a provisioning service is responsible for:

  • providing a list of candidates for provisioning, given a toolchain spec
  • providing a provisioner given a specific selected candidate

A plugin can then register such a provisioning service on the toolchains service. This would typically allow the GraalVM team to provide such a mechanism in the GraalVM native image plugin.

IMPORTANT: For legal reasons, this pull request must not be merged until approved by @graemerocher

Some things that I noticed while working on this:

  • the JvmImplementation class is inconsistent with the JvmVendorSpec class: it should in theory behave similarly: a spec should be provided, so that we can use other implementations than J9. Currently, it's impossible to request something different than J9, even if custom JDKs offer alternatives.
  • the JvmVendorSpec has been updated so that we can query if a vendor matches the spec. Without this, it's barely impossible for a provisioner to tell if it's compatible or not
  • the current implementation selects the first one which lists at least a candidate. This probably needs to be changed as typically this forces the AdoptOpenJDK provisioner to be put last in the list (because it always answers "yes, I have candidates" even if in practice we don't know if it's going to be available)
  • there is now an injectable service to provide downloads (similar to JdkDownloader with a more generic signature): RemoteResourceService
  • some complexity comes from the fact we want to report the name of the downloaded JDK before we actually start downloading it, and that we want to provide the provisioning service with the location where to write the file (so that it lives in cache)

This should make it possible for projects like @joschi 's Java Metadata to provide a list of candidates based on the contents of the JSON file, and perform the download lazily.

Contributor Checklist

  • Review Contribution Guidelines
  • Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
  • Make sure all contributed code can be distributed under the terms of the Apache License 2.0, e.g. the code was written by yourself or the original code is licensed under a license compatible to Apache License 2.0.
  • Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team
  • Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective
  • Provide unit tests (under <subproject>/src/test) to verify logic
  • Update User Guide, DSL Reference, and Javadoc for public-facing changes
  • Ensure that tests pass sanity check: ./gradlew sanityCheck
  • Ensure that tests pass locally: ./gradlew <changed-subproject>:quickTest

Gradle Core Team Checklist

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation
  • Recognize contributor in release notes

@melix melix force-pushed the cc/toolchain-provisioning branch from 1c9b6fc to 1b55127 Compare August 16, 2021 14:09
@melix
Copy link
Contributor Author

melix commented Aug 16, 2021

I added a service to perform downloads of external resources via HTTP. This should make it even more trivial to implement toolchain provisioning. At this stage I think the PR is ready for review.

@melix melix force-pushed the cc/toolchain-provisioning branch from 1b55127 to b5e65b2 Compare August 20, 2021 15:07
@ljacomet
Copy link
Member

@bot-gradle test QF

@bot-gradle
Copy link
Collaborator

OK, I've already triggered QuickFeedback build for you.

@melix melix force-pushed the cc/toolchain-provisioning branch from 56cc162 to e830f30 Compare August 23, 2021 09:54
@melix
Copy link
Contributor Author

melix commented Sep 1, 2021

@ljacomet Did you have time to discuss this internally? I'm open to chat if needed.

melix and others added 5 commits September 21, 2021 10:38
There wasn't anything specific to AdoptOpenJDK in this class, which could be
reused for other purposes.

Signed-off-by: Cedric Champeau <[email protected]>
This commit refactors provisioning of Java toolchains, so that we can provide
more provisioning services dynamically. For now, there's no public support for
adding a provisioning service, so the only available service is the standard
AdoptOpenJDK service.

Signed-off-by: Cedric Champeau <[email protected]>
This commit allows plugin authors to register new provisioning services
by the means of the `JavaToolchainService`. For now, lookup isn't fault
tolerant, in the way that if a service provides candidates, and that
it for whatever reason cannot provide the candidate, we wouldn't look
for another provisioner.

Signed-off-by: Cedric Champeau <[email protected]>
This commit adds a resource download service which can be used by
toolchain provisioning to list, for example, available JDKs, then
perform the download itself.

The remote resource service honors the traditional offline mode
and performs caching of resources. It's worth noting that toolchain
provisioning calls the provisioning methods twice, but this isn't
a new behavior: it's just exercised by this pull request, which adds
tests with fake HTTP servers, highlighting this behavior.

Signed-off-by: Cedric Champeau <[email protected]>
This caused an unexpected error:
   > SystemInfo is not supported on this operating system.

Signed-off-by: Cedric Champeau <[email protected]>
@melix melix force-pushed the cc/toolchain-provisioning branch from e830f30 to 5cfb53c Compare September 21, 2021 09:43
@melix
Copy link
Contributor Author

melix commented Sep 21, 2021

Rebased onto master

@stale
Copy link

stale bot commented Dec 5, 2021

This pull request has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be closed if no further activity occurs. If you intend to work on this pull request, please ask the team to reopen the PR or push a new PR. Thank you for your contributions.

@stale stale bot added the stale label Dec 5, 2021
@EdGue42
Copy link

EdGue42 commented Dec 6, 2021

What is the status here, why is this still open, not merged, and now about to be closed?

@stale stale bot removed the stale label Dec 6, 2021
@ljacomet
Copy link
Member

ljacomet commented Dec 6, 2021

That's our stale bot being a bit too eager. We still want to offer something like that but different priorities had to be handled first and we are still discussing the exact design for this feature.

@EdGue42
Copy link

EdGue42 commented Dec 6, 2021

Thanks.
Please make sure to have at least "some" support for IBM Semeru in 7.4 ... or well:
please keep in what you have right now at least. I did test with a nightly build some days ago, and
seeing that it actually do the auto-provisioning for Semeru was quite helpful (and is something that
our team is absolutely looking forward to).

@stale
Copy link

stale bot commented Feb 20, 2022

This pull request has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be closed if no further activity occurs. If you intend to work on this pull request, please ask the team to reopen the PR or push a new PR. Thank you for your contributions.

@stale stale bot added the stale label Feb 20, 2022
@melix
Copy link
Contributor Author

melix commented Feb 21, 2022

ping

@stale stale bot removed the stale label Feb 21, 2022
@ljacomet
Copy link
Member

This is for sure on the agenda in this form or another.

@melix
Copy link
Contributor Author

melix commented Feb 21, 2022

Thanks, ping was for the stale bot ;)

@big-guy big-guy added the from:contributor PR by an external contributor label Apr 1, 2022
@stale
Copy link

stale bot commented Jun 4, 2022

This pull request has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be closed if no further activity occurs. If you intend to work on this pull request, please ask the team to reopen the PR or push a new PR. Thank you for your contributions.

@stale stale bot added the stale label Jun 4, 2022
@EdGue42
Copy link

EdGue42 commented Jun 7, 2022

Staling the auto-stale.

We need this to really work.

@stale stale bot removed the stale label Jun 7, 2022
@melix
Copy link
Contributor Author

melix commented Jul 5, 2022

Relates to gradle/build-tool-roadmap#29

@sschuberth
Copy link
Contributor

sschuberth commented Oct 4, 2022

Is this still relevant now that #21082 is merged?

@melix
Copy link
Contributor Author

melix commented Oct 4, 2022

I think this is still relevant in the sense that this PR fixes issue that the support included in 7.6 doesn't (e.g fixes on JvmImplementation or the ability to select GraalVM EE vs CE). The PR, in itself, is irrelevant though.

@sschuberth
Copy link
Contributor

the ability to select GraalVM EE vs CE

That's indeed super-useful!

@ljacomet
Copy link
Member

Closing this PR as a part of it is integrated already.
Please upvote #18896 to enable finer grained selection of the toolchain.

@ljacomet ljacomet closed this Mar 30, 2023
@bric3
Copy link
Contributor

bric3 commented Mar 30, 2023

What is the proper upvote mechanism (does a thumb up count 👍)

@ljacomet
Copy link
Member

yes, adding 👍 counts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

from:contributor PR by an external contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants