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

Detect JDKs provisioned by Github actions automatically #14903

Closed
bmuskalla opened this issue Oct 19, 2020 · 15 comments
Closed

Detect JDKs provisioned by Github actions automatically #14903

bmuskalla opened this issue Oct 19, 2020 · 15 comments
Labels
a:feature A new functionality has:workaround Indicates that the issue has a workaround in:toolchains Java Toolchains

Comments

@bmuskalla
Copy link
Contributor

We should have a look at common actions on GitHub actions that provision JDKs.
If we can find a pattern, we may be able to automatically detect those without additional setup

Example: actions/setup-java@v1

@bmuskalla bmuskalla added the @jvm label Oct 19, 2020
@bmuskalla bmuskalla added this to the 6.8 RC1 milestone Oct 19, 2020
@ljacomet ljacomet removed this from the 6.8 RC1 milestone Nov 10, 2020
@bmuskalla bmuskalla added the in:toolchains Java Toolchains label Nov 23, 2020
@jjohannes jjohannes removed the @jvm label Mar 22, 2021
@evpaassen
Copy link

evpaassen commented May 3, 2021

This feature would be very useful to me.

The runners hosted by GitHub seem to have a Hosted Tool Cache with some JDKs preinstalled.

An example path to a JDK in the cache is:

/opt/hostedtoolcache/Java_Adopt_jdk/16.0.1-9/x64

So the pattern seems to be something like:

/opt/hostedtoolcache/<distribution>/<version>/<architecture>

The path to the tool cache directory resides in the environment variable RUNNER_TOOL_CACHE.

However, the setup-java action also allows to provision JDKs that are not in the cache and I don't know where those would be installed. actions/setup-java#44 contains some relevant information on this topic. B.t.w. notice that setup-java@v2 allows selecting the JDK distribution, while v1 did not.

Please let me know if I can help in any way.

@devinrsmith
Copy link

While I didn't solve this issues "automatically", there is a way to point gradle at it:

      - name: Setup JDK
        id: setup-java-8
        uses: actions/setup-java@v2.2.0
        with:
          distribution: 'zulu'
          java-version: '8'
      ...
      - name: Setup gradle properties
        run: |
          echo "org.gradle.java.installations.paths=${{ steps.setup-java-8.outputs.path }}" >> gradle.properties

rognan added a commit to rognan/deno-gradle-plugin that referenced this issue Nov 28, 2021
Gradle will by default use the same JDK for building the project as it
does for running Gradle. Newer JDKs are assumed to be more performant,
contain fewer bugs and to come with desirable features that we would
like to utilize in our build, but doing so should not endanger
backwards compatibility for the plugin itself. We can do that now by
utilizing Gradle toolchains, which allows us to use one JDK/JRE for
running Gradle and the other for building the project, i.e. executing
`javac`.

Doing this requires us to make multiple JDKs available to the Gradle
build action for each individual execution of the build. The setup-java
action doesn't support this at the moment, and the multiple build steps
configured for actions/setup-java in this commit is at best a
workaround for this, and at worst a bad assumption with respect to no
environment variables or other settings being overwritten. Also,
there's no handling of duplicate invocations of the action with the end
result being duplicate JDKs installed. Hopefully both actions/setup-java
and gradle/gradle-build-action will handle this case gracefully.

For more details, see:
- actions/setup-java#44
- gradle/gradle#14903
rognan added a commit to rognan/deno-gradle-plugin that referenced this issue Nov 28, 2021
Gradle will by default use the same JDK for building the project as it
does for running Gradle. Newer JDKs are assumed to be more performant,
contain fewer bugs and to come with desirable features that we would
like to utilize in our build, but doing so should not endanger
backwards compatibility for the plugin itself. We can do that now by
utilizing Gradle toolchains, which allows us to use one JDK/JRE for
running Gradle and the other for building the project, i.e. executing
`javac`.

Doing this requires us to make multiple JDKs available to the Gradle
build action for each individual execution of the build. The setup-java
action doesn't support this at the moment, and the multiple build steps
configured for actions/setup-java in this commit is at best a
workaround for this, and at worst a bad assumption with respect to no
environment variables or other settings being overwritten. Also,
there's no handling of duplicate invocations of the action with the end
result being duplicate JDKs installed. Hopefully both actions/setup-java
and gradle/gradle-build-action will handle this case gracefully.

For more details, see:
- actions/setup-java#44
- gradle/gradle#14903
rognan added a commit to rognan/deno-gradle-plugin that referenced this issue Nov 28, 2021
This commit decouples the JDK running Gradle from the one building the
project.

Gradle will by default use the same JDK for building the project as it
does for running Gradle. Newer JDKs are assumed to be more performant,
contain fewer bugs and to come with desirable features that we would
like to utilize in our build, but doing so should not endanger
backwards compatibility for the plugin itself. We can do that now by
utilizing Gradle toolchains, which allows us to use one JDK/JRE for
running Gradle and the other for building the project, i.e. executing
`javac`.

Doing this requires us to make multiple JDKs available to the Gradle
build action for each individual execution of the build. The setup-java
action doesn't support this at the moment, and the multiple build steps
configured for actions/setup-java in this commit is at best a
workaround for this, and at worst a bad assumption with respect to
environment variables or other settings being overwritten. Furthermore,
there's no handling of duplicate invocations of the action.

Hopefully both actions/setup-java and gradle/gradle-build-action will
handle all of this gracefully.

For more details, see:
- actions/setup-java#44
- gradle/gradle#14903
rognan added a commit to rognan/deno-gradle-plugin that referenced this issue Nov 29, 2021
This commit decouples the JDK running Gradle from the one building the
project.

Gradle will by default use the same JDK for building the project as it
does for running Gradle. Newer JDKs are assumed to be more performant,
contain fewer bugs and to come with desirable features that we would
like to utilize in our build, but doing so should not endanger
backwards compatibility for the plugin itself. We can do that now by
utilizing Gradle toolchains, which allows us to use one JDK/JRE for
running Gradle and the other for building the project, i.e. executing
`javac`.

Doing this requires us to make multiple JDKs available to the Gradle
build action for each individual execution of the build. The setup-java
action doesn't support this at the moment, and the multiple build steps
configured for actions/setup-java in this commit is at best a
workaround for this, and at worst a bad assumption with respect to
environment variables or other settings being overwritten. Furthermore,
there's no handling of duplicate invocations of the action.

Hopefully both actions/setup-java and gradle/gradle-build-action will
handle all of this gracefully.

For more details, see:
- actions/setup-java#44
- gradle/gradle#14903
rognan added a commit to rognan/deno-gradle-plugin that referenced this issue Sep 17, 2022
This commit decouples the JDK running Gradle from the one building the
project.

Gradle will by default use the same JDK for building the project as it
does for running Gradle. Newer JDKs are assumed to be more performant,
contain fewer bugs and to come with desirable features that we would
like to utilize in our build, but doing so should not endanger
backwards compatibility for the plugin itself. We can do that now by
utilizing Gradle toolchains, which allows us to use one JDK/JRE for
running Gradle and the other for building the project, i.e. executing
`javac`.

Doing this requires us to make multiple JDKs available to the Gradle
build action for each individual execution of the build. The setup-java
action doesn't support this at the moment, and the multiple build steps
configured for actions/setup-java in this commit is at best a
workaround for this, and at worst a bad assumption with respect to
environment variables or other settings being overwritten. Furthermore,
there's no handling of duplicate invocations of the action.

Hopefully both actions/setup-java and gradle/gradle-build-action will
handle all of this gracefully.

For more details, see:
- actions/setup-java#44
- gradle/gradle#14903
rognan added a commit to rognan/deno-gradle-plugin that referenced this issue Sep 17, 2022
This commit decouples the JDK running Gradle from the one building the
project.

Gradle will by default use the same JDK for building the project as it
does for running Gradle. Newer JDKs are assumed to be more performant,
contain fewer bugs and to come with desirable features that we would
like to utilize in our build, but doing so should not endanger
backwards compatibility for the plugin itself. We can do that now by
utilizing Gradle toolchains, which allows us to use one JDK/JRE for
running Gradle and the other for building the project, i.e. executing
`javac`.

Doing this requires us to make multiple JDKs available to the Gradle
build action for each individual execution of the build. The setup-java
action doesn't support this at the moment, and the multiple build steps
configured for actions/setup-java in this commit is at best a
workaround for this, and at worst a bad assumption with respect to
environment variables or other settings being overwritten. Furthermore,
there's no handling of duplicate invocations of the action.

Hopefully both actions/setup-java and gradle/gradle-build-action will
handle all of this gracefully.

For more details, see:
- actions/setup-java#44
- gradle/gradle#14903
@joffrey-bion
Copy link

Are there any plans to move this forward? This is what makes me reluctant to use JVM toolchains.

Note that the setup-java action is instant when using the temurin distribution because this distribution is already on the runner. If Gradle could find those pre-installed distributions automatically, we wouldn't even need the setup-java action!

@ov7a ov7a added a:feature A new functionality 🌱 internal-onboarding Good issues to onboard new Gradle team members has:workaround Indicates that the issue has a workaround and removed 🌱 internal-onboarding Good issues to onboard new Gradle team members labels Aug 10, 2023
@ov7a
Copy link
Member

ov7a commented Aug 10, 2023

The issue is in the backlog of the relevant team, but this area of Gradle is currently not a focus one, so it might take a while before a fix is made.

@joffrey-bion
Copy link

Ok thanks for providing a status, I'm looking forward to this! 🙏

@joffrey-bion
Copy link

joffrey-bion commented Aug 11, 2023

About this:

The path to the tool cache directory resides in the environment variable RUNNER_TOOL_CACHE.

However, the setup-java action also allows to provision JDKs that are not in the cache and I don't know where those would be installed. actions/setup-java#44 contains some relevant information on this topic. B.t.w. notice that setup-java@v2 allows selecting the JDK distribution, while v1 did not.

It looks like those are 2 different features. I believe Gradle could by default look into the default hosted tool cache of GitHub runners, but not necessarily be aware of specific GitHub actions (like setup-java) and where they place things (as this path is technically dynamic and can be retrieved as an output of the setup-java action).

@joffrey-bion
Copy link

joffrey-bion commented Aug 11, 2023

As a workaround, GitHub currently provides env variables for the Java home of the all current LTS versions: JAVA_HOME_8_X64, JAVA_HOME_11_X64, and JAVA_HOME_17_X64 (see the runner environment doc). So I believe we could provide those env variables as JDK installation dirs in a project's gradle.properties like this:

org.gradle.java.installations.fromEnv=JAVA_HOME_8_X64,JAVA_HOME_11_X64,JAVA_HOME_17_X64

Now that I think of it, maybe GitHub could provide such a default gradle.properties instead of Gradle implementing this "outsider knowledge" as a built-in location (unless those env vars are standard-ish). Given the current precedence order, GitHub cannot do this with the user home, otherwise users can no longer use this Gradle property themselves at the project level. Is this correct? Why is the order of precedence this way?

If GitHub pre-installed some Gradle (not the case now) and put this property in the Gradle installation dir, would this affect projects that use the Gradle wrapper? (it might be worth clarifying this in the docs, by the way)

@ov7a
Copy link
Member

ov7a commented Aug 11, 2023

@joffrey-bion That's a great idea! If it can be resolved at the action level, it would be nice.

users can no longer use this Gradle property themselves at the project level

Yes, that is correct, however, it can be overridden again by a command line. I don't think it's a big problem.

If GitHub pre-installed some Gradle (not the case now) and put this property in the Gradle installation dir, would this affect projects that use the Gradle wrapper? (it might be worth clarifying this in the docs, by the way)

It shouldn't matter how Gradle is invoked: as local ./gradlew or global gradle in that context.

@ov7a
Copy link
Member

ov7a commented Aug 11, 2023

Regarding the gradle action, PTAL at the issue there

@ov7a
Copy link
Member

ov7a commented Aug 11, 2023

Why is the order of precedence this way?

The idea here is user preferences should have higher priority than project preferences. So CLI has a clear intent, then user preferences, project preferences, and lastly, "system" preferences.

@joffrey-bion
Copy link

joffrey-bion commented Aug 11, 2023

Yes, that is correct, however, it can be overridden again by a command line. I don't think it's a big problem.

Well it is definitely a problem if GitHub were to define some properties in the home dir of the runners, because users setting gradle.properties in their project would see (some of) their settings properties ignored (because overridden) and might not understand why.

Why is the order of precedence this way?

The idea here is user preferences should have higher priority than project preferences. So CLI has a clear intent, then user preferences, project preferences, and lastly, "system" preferences.

While I see the point, I never ever needed to express it this way. But every single time I wanted to express user defaults, hoping to override them in a per-project basis... and it doesn't work this way.

If GitHub pre-installed some Gradle (not the case now) and put this property in the Gradle installation dir, would this affect projects that use the Gradle wrapper? (it might be worth clarifying this in the docs, by the way)

It shouldn't matter how Gradle is invoked: as local ./gradlew or global gradle in that context.

If it doesn't matter how Gradle is run, does that mean that my C:\gradle\gradle.properties affect a ./gradlew build run from projects that don't use this gradle installation at all? That would be insane, wouldn't it?
If it does matter how Gradle is run, what constitutes the "gradle properties in the installation dir" in case of gradle wrapper? And more importantly how can I specify default values of Gradle properties for a given machine (e..g memory or JDK locations), but overridable per project?

Anyway, this whole topic is out of scope for this discussion, sorry for the off topic, I just wanted to confirm that GitHub cannot and thus shouldn't try to do this.

@joffrey-bion
Copy link

Regarding the gradle action, PTAL at the gradle/gradle-build-action#561 there

Oh, I forgot about this action. It could definitely be provided as an extra CLI argument by default in this GitHub action, I'll head over to that issue. Thanks!

@ov7a
Copy link
Member

ov7a commented Aug 11, 2023

users setting gradle.properties in their project would see their settings ignored and might not understand why.

No, they would be overridden, not ignored. And it only applies to settings set by action, not all settings.

As for your use case - let's jump into a separate issue. I think there is a clear need to have better documentation in this area. Would you like to create a documentation issue with a list of stuff that should be clarified in that area?

@joffrey-bion
Copy link

No, they would be overridden, not ignored. And it only applies to settings set by action, not all settings.

Yes, I was unclear about this. The problem is still there, though. GitHub cannot set this property as a default without overriding the same property defined at the project level by some users.

Would you like to create a documentation issue with a list of stuff that should be clarified in that area?

I opened the following:

@bigdaz
Copy link
Member

bigdaz commented Aug 20, 2023

I've done some testing, and toolchain detection is now mostly working on GitHub Actions runners, without any special handling in the gradle-build-action.

There are 2 types of JDKs that can be detected on a runner:

  1. Pre-installed JDKs, that exist without any use of setup-java
  2. JDKs that are downloaded and installed on demand with setup-java

On ubuntu-latest and macos-latest, both of these types are automatically detected. The pre-installed JDKs are symlinked into standard locations, and the setup-java JDKS are detected because the action is updating the Maven Toolchains file.

On windows-latest, only the second type are detected, via the Maven Toolchains mechanism. Unfortunately the pre-installed JDKs are not found in a standard location and are not detected.

So as long as setup-java is used to provision all of the required JDKs, then these will be detected by Gradle and not re-downloaded.

I will attempt to update the gradle-build-action so that the pre-installed JDKs are detected on Windows as well, by supplying the env var names as suggested.

I think it's safe to close this issue, since this is something that should be fixed in the GitHub Runner (or the gradle-build-action).

@bigdaz bigdaz closed this as completed Aug 20, 2023
@ljacomet ljacomet added this to the outside-gradle milestone Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:feature A new functionality has:workaround Indicates that the issue has a workaround in:toolchains Java Toolchains
Projects
None yet
Development

No branches or pull requests

8 participants