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

jdk: point attributes without version suffix to latest #89731

Merged
merged 82 commits into from Sep 20, 2020

Conversation

doronbehar
Copy link
Contributor

For too long jdk was pointing to the legacy, unmaintained openjdk8 and too many apps are using it just because it works. This PR aims at making all apps that have jdk in their inputs to use jdk8 explicitly. The end goal is to get from ofborg the "0:rebuild-linux" label, that's why it's still a draft.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@asbachb
Copy link
Contributor

asbachb commented Jun 17, 2020

I wonder if it's smart to default jdk to 14. I suspect that several java depended packages will break.

Maybe it would make sense to introduce some kind of lts (java 11) and legacy (java 8) aliases.

@doronbehar
Copy link
Contributor Author

I suspect that several java depended packages will break.

The current plan is to do something similar to what was done at #89264 :

  1. Make 14 the default at all-packages,nix
  2. Iterate (I did it with git grep but it's possible there's a smarter way) all usages of jdk without an explicit version suffix and replace these usages with jdk8 specifically.

The goal of part (2) is to make ofborg label the PR as 1:rebuild-linux where the only changed path is the rename jdk 8 ->14.

With ffmeg it was a 135 files changed, 279 insertions(+), 279 deletions(-) PR, but with jdk, I expect it to touch something like ~500 files ... If you are up to contribute, I'd love to get help with this tedious task.

Maybe it would make sense to introduce some kind of lts (java 11) and legacy (java 8) aliases.

The point is that whatever is called jdk should point to the latest version, with or without aliases. The fact that a certain jdk update isn't backwards compatible doesn't mean we should be stuck with it as Nixpkgs' default for ever. See https://discourse.nixos.org/t/nixpkgs-policy-regarding-libraries-available-in-multiple-versions/7026

@jerith666
Copy link
Contributor

I agree it'd be great to have jdk etc. point to 14 (and then 15 etc.); and introduce a jdk_lts that points to 11 (and then 17 (?) etc.).

I also agree that this is the right approach, to just do the rename and make sure it doesn't cause any rebuilds. Then we can gradually replace all the resulting jdk8s with jdk or jdk_lts as appropriate for the package.

Sadly I don't have much time right now to help with this effort, but if that changes I'll jump in as I can.

For too long `jdk` was pointing to the legacy, unmaintained openjdk8 and
too many apps are using it just because it works.
@doronbehar
Copy link
Contributor Author

Thanks 💚 @gebner .

@gebner
Copy link
Member

gebner commented Sep 19, 2020

I wanted to do this as well because of https://discourse.nixos.org/t/remove-hardcoded-dependency-of-sbt-on-jdk8/9053

I've rebased this PR to master and evaluation almost passes. Many packages fail to evaluate because openjdk14.jre doesn't exist (adding it to passthru doesn't work either because there's no jre directory now...).

@doronbehar
Copy link
Contributor Author

Many packages fail to evaluate because openjdk14.jre doesn't exist (adding it to passthru doesn't work either because there's no jre directory now...).

Now I remember why I stopped working on this - What bothered me even more then the attributes' names was that later jdk versions here in Nixpkgs weren't split packages.

Still the work you are putting is absolutely valuable.

@gebner gebner merged commit efa2089 into NixOS:master Sep 20, 2020
@gebner gebner mentioned this pull request Sep 20, 2020
10 tasks
@danieldk
Copy link
Contributor

danieldk commented Sep 20, 2020

That's a wider discussion which should be done outside of this PR. As I've said, the jdk = jdk11 pattern is already widespread (also for other packages).

IMO, having the versioned package as an argument in the derivation really is a step backwards. It makes overriding much less intuitive. JREs are generally backwards compatible, so if you want a newer version, you have to write strange thing like:

myapp.override { jdk8 = jdk11; }

as oposed to

myapp.override { jdk = jdk11; }

Or if you want a newer CUDA version for some machine learning library (if we'd use the same pattern as here):

pytorch.override { cudatoolkit_10_1 = cudatoolkit_11; }

So, to override any Java derivation, one would first need to read it to discover what jdk version is used. What is even worse, if e.g. the default version is changed from jdk_8 to jdk_11, the overrides break for all downstream users.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/package-version-arguments-vs-generic-package-arguments/9071/1

@danieldk
Copy link
Contributor

Discourse is probably a better place to discuss this, so opened a Discourse topic.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/remove-hardcoded-dependency-of-sbt-on-jdk8/9053/5

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/retirement-of-old-openjdk-releases/9929/8

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/package-version-arguments-vs-generic-package-arguments/9071/8

@jlesquembre jlesquembre mentioned this pull request Dec 9, 2020
10 tasks
jlesquembre added a commit to jlesquembre/nixpkgs that referenced this pull request Dec 9, 2020
Clojure officially only supports Java LTS releases. Related to NixOS#89731.
As discussed in
https://discourse.nixos.org/t/package-version-arguments-vs-generic-package-arguments/9071
seems that the preferred way to set the jdk version is on
`all-packages.nix`
@doronbehar doronbehar mentioned this pull request Dec 18, 2020
10 tasks
@doronbehar doronbehar mentioned this pull request Dec 25, 2020
10 tasks
jerith666 added a commit to jerith666/nixpkgs that referenced this pull request Dec 26, 2020
* update two explicit references to jdk14 to just jdk, which no longer
  points at jdk8 after NixOS#89731.

* patch an explicit -XX:+UseConcMarkSweepGC to -XX:UseG1GC, as the
  former now throws an error (after having been deprecated since jdk 9)
balodja added a commit to balodja/nixpkgs that referenced this pull request May 12, 2021
balodja added a commit to balodja/nixpkgs that referenced this pull request Jul 23, 2021
Most of them are:
* separate packages for different openmodelica components,
* qt4 -> qt5,
* patches to instruct the OMEdit wrapper with stdenv executables
  location,
* adoption of NixOS#89731 and NixOS#109595,
* openblas -> blas, lapack according to NixOS#83888,
* parallel building,
* getting rid of spurious build phases,
* correct the license,
* cross-compilation,
* forcing compiler to clang++ according to OM build recommendations,
* drop of pangox_compat according to NixOS#75909 and NixOS#76412,
* better dependencies, and more.
balodja added a commit to balodja/nixpkgs that referenced this pull request Jul 24, 2021
Co-authored-by: Jaakko Luttinen <jaakko.luttinen@iki.fi>

Most of changes are:
* separate packages for different openmodelica components,
* qt4 -> qt5,
* patches to instruct the OMEdit wrapper with stdenv executables
  location,
* adoption of NixOS#89731 and NixOS#109595,
* openblas -> blas, lapack according to NixOS#83888,
* parallel building,
* getting rid of spurious build phases,
* correct the license,
* cross-compilation,
* forcing compiler to clang++ according to OM build recommendations,
* drop of pangox_compat according to NixOS#75909 and NixOS#76412,
* better dependencies, and more.
@panicgh panicgh mentioned this pull request Aug 31, 2022
13 tasks
@doronbehar doronbehar deleted the jdk_rename branch March 2, 2023 10:38
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