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

gradle: Decouple from JDK 8 and support Java Toolchains #119444

Merged
merged 1 commit into from
Nov 22, 2021

Conversation

lorenzleutgeb
Copy link
Member

@lorenzleutgeb lorenzleutgeb commented Apr 14, 2021

This sets a default Java installation per Gradle version, and allows to specify Java Toolchains.

Gradle Version Attribute Default Java Version defaultJava
7.3 gradle_7 17 jdk8
6.9.1 gradle_6 11 jdk11
5.6.4 gradle_5 11 jdk11
4.10.3 gradle_4 8 jdk8

(see also Gradle's Compatibility Matrix)

E.g. with gradle_7.override { javaToolchains = [ jdk8 jdk11 ]; } I get

$ gradle -q javaToolchains

 + Options
     | Auto-detection:     Enabled
     | Auto-download:      Enabled

 + N/A JDK 17.0.1+12-nixos
     | Location:           /nix/store/1vi5p9gliab13jc3v56kz7z78x5bq0da-openjdk-17.0.1+12/lib/openjdk
     | Language Version:   17
     | Vendor:             N/A
     | Is JDK:             true
     | Detected by:        Current JVM

 + OpenJDK 1.8.0_272-b10
     | Location:           /nix/store/4f0d8c9lmj174hz3qvn1qhc70n22bai4-openjdk-8u272-b10
     | Language Version:   8
     | Vendor:             Oracle
     | Is JDK:             true
     | Detected by:        environment variable 'JAVA_TOOLCHAIN_NIX_0'

 + OpenJDK 11.0.12+0-adhoc..source
     | Location:           /nix/store/7r5b2r4kjsbflgxs8sy7i86c1wy773is-openjdk-11.0.12+7
     | Language Version:   11
     | Vendor:             Oracle
     | Is JDK:             true
     | Detected by:        environment variable 'JAVA_TOOLCHAIN_NIX_1'

@lorenzleutgeb lorenzleutgeb force-pushed the gradle-jdk branch 4 times, most recently from 8f5b126 to 8b290dd Compare April 14, 2021 18:50
@lorenzleutgeb

This comment has been minimized.

@SuperSandro2000
Copy link
Member

Hey, @SuperSandro2000 I hope you don't mind me pinging you after ~2weeks.

You can ping me if you have a question but it can take me a few days to answer if you are unlucky.

I'd still like to get this PR merged and would appreciate your advice on how we can get it done 🚀 😃 Any other changes to make?

ofborg is red because something does not eval correct. I don't really know why from ofborgs log. Maybe nix-env -f . -qaP --show-trace gives something more useful?

@lorenzleutgeb

This comment has been minimized.

@SuperSandro2000
Copy link
Member

expect it to eval now.

ofborg has a different opinion on that. I am not sure why it fails now but I assume the variable is not a derivation but I can't really help your with that.

@stale
Copy link

stale bot commented Oct 30, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 30, 2021
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 16, 2021
@lorenzleutgeb lorenzleutgeb force-pushed the gradle-jdk branch 12 times, most recently from 620909a to 3f2954e Compare November 16, 2021 09:23
@ofborg ofborg bot requested review from a user and wirew0rm November 19, 2021 14:51
@lorenzleutgeb lorenzleutgeb changed the title gradle: Decouple gradleGen and JDK gradle: Decouple from jdk8 and support Java Toolchains Nov 19, 2021
@lorenzleutgeb lorenzleutgeb changed the title gradle: Decouple from jdk8 and support Java Toolchains gradle: Decouple from JDK 8 and support Java Toolchains Nov 19, 2021
@lorenzleutgeb
Copy link
Member Author

lorenzleutgeb commented Nov 19, 2021

Pinging @tadfisher to review with respect to gradle2nix compatibility. I tried to keep everything compatible:

  • gradleGen will default to defaultJava = jdk8 if it's not given (but it would be a good idea to pass something better).
  • gradleGen is still exposed (even though I originally wanted to remove it).

let
gradle = (gradleGen.override (old: { java = jdk11; })).gradle_6_9;
gradle = gradle_6;
Copy link
Member Author

@lorenzleutgeb lorenzleutgeb Nov 19, 2021

Choose a reason for hiding this comment

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

Deleting this override is okay, because gradle_6 now uses jdk11 by default.

let
# The default one still uses jdk8 (#89731)
gradle = (gradleGen.override (old: { java = jdk; })).gradle_latest;
Copy link
Member Author

Choose a reason for hiding this comment

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

Deleting this override is okay, because gradle (which is the latest Gradle) currently uses jdk17, which is currently equal to jdk, by default.

@marvin-mk2
Copy link

marvin-mk2 bot commented Nov 22, 2021

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR will be should be put back in the needs_reviewer queue in one day.


Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

@kevincox
Copy link
Contributor

It looks like all concerns have been addressed. If any remain they can be fixed in post.

@kevincox kevincox merged commit dc44791 into NixOS:master Nov 22, 2021
@Mindavi Mindavi mentioned this pull request Nov 23, 2021
9 tasks
Mindavi added a commit to Mindavi/nixpkgs that referenced this pull request Nov 23, 2021
The PR NixOS#119444 broke the build for mindustry
even further. This patch fixes that again so it evals and builds properly.
mkg20001 pushed a commit to mkg20001/nixpkgs that referenced this pull request Nov 29, 2021
The PR NixOS#119444 broke the build for mindustry
even further. This patch fixes that again so it evals and builds properly.
@Profpatsch
Copy link
Member

This PR completely broke the gradleGen interface, breaking gradle2nix tadfisher/gradle2nix#53

@lorenzleutgeb
Copy link
Member Author

@Profpatsch yes. I did not understand that this PR would break gradle2nix. My apologies. I tried to reach @tadfisher, as evidenced in #119444 (comment) but never received a response. Overall I think that this PR still provided some convenient features. Thanks to @Mic92 for cleaning up.

@coderfromhere
Copy link

@lorenzleutgeb What's the proper way to use gradleGen with jdk overrides in the new interface? I.e. what would be the new solution for something like this:

{ jdk }:
{
  packageOverrides = p: {
    gradle = (p.gradleGen.override {
      java = p.${jdk};
    }).gradle_latest;
  };
}

@lorenzleutgeb
Copy link
Member Author

@coderfromhere Very sorry to take so long to reply.

Instead of using packageOverrides, I'd suggest using overlays. An overlay to use the latest version of Gradle with a given JDK would look like this:

(self: super: {
  gradle = super.gradle.override { inherit java; };
})

The above example requires a variable java to reference the JDK. An overlay without such a parameter could look like this:

(self: super: {
  gradle = super.gradle.override { java = super.jdk8; };
})

See also the arguments here.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/auto-detecting-java-installations/4677/9

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

Successfully merging this pull request may close these issues.

None yet

6 participants