openjdk18: init at 18+37#165354
Conversation
|
Error when running |
|
@ofborg eval |
|
Looking upstream it seems OpenJ9 builds (which nixpkgs pulls from AdoptOpenJDK) are now provided by Semeru. The missing packages should be all removed now. |
|
|
Could this be merged before the NixOS 22.05 feature freeze? #167025 |
There was a problem hiding this comment.
| rev = "jdk-${version.feature}+${version.build}"; | |
| rev = "jdk-${version}"; |
There was a problem hiding this comment.
version here refers to the attrset above, not the usual derivation.version attribute:
let
version = {
feature = "18";
build = "36";
};
...I can rename and/or inline those variables (also used in derivation version, src repo, and a configure flag) but I'm following convention of other openjdk derivations.
|
I've been test driving this for a few weeks. It's worked great until the recent glibc upgrade to 2.34 because this is still being built with glibc 2.33 (I've confirmed rebasing fixes this). Can we get this off the ground please? Is there anything else to do to get this into |
There was a problem hiding this comment.
Missing or throw unsupported system or eval fails on other platforms.
There was a problem hiding this comment.
With all respect, it a. doesn't get called for non-Darwin platforms, and b. is the same way that all OpenJDK derivations have done this. I can change it if required, but it seems unnecessary and out of line.
nixpkgs/pkgs/top-level/java-packages.nix
Lines 51 to 56 in 992d767
https://github.com/NixOS/nixpkgs/blob/a6b757a52bf2370540ffc2c0210e25e79b419a63/pkgs/top-level/java-packages.nix#L187-L193
There was a problem hiding this comment.
Thats not great. It should at least eval on any platform otherwise you can't check if your changes on break evaluation for another platform.
There was a problem hiding this comment.
Wouldn't you set the system anyway when testing evaluation for another platform, then stdenv.hostPlatform would show that?
Anyway I don't think we put too many expectations on the evaluation of packages if they're not checked on hydra / ofborg, and all the ofborg builds on the PR are green, so I think it's ok.
There was a problem hiding this comment.
I don't think we put too many expectations on the evaluation of packages if they're not checked on hydra / ofborg
Especially important to:
- Help reviewers be more effective, since they can rely on ofborg builds and focus on other aspects of the PR.
- Help contributors get faster signal on their PRs, and have a higher confidence that their code is good if the build is green.
|
Hello friends! This issue seems to be a bit stalled; is there any way to push this through please? Thank you! |
|
Please? |
|
@SuperSandro2000 are you happy to proceed? |
|
Do we have an update on this? thanks. |
|
Will proceed to merge, let me rebase this. |
|
@GrahamcOfBorg eval |
|
Now that openjdk19 is out, is there some mechanism to generate an equivalent pull request? Is it just a matter of copy+pasting the 18 files and pointing them at the latest releases + shas from here - https://github.com/adoptium/temurin19-binaries/releases ? Thanks! |
|
#195638 is the official packaging request for OpenJDK 19. |
this was originally replaced with temurin in c742218, which landed in staging in acf46b0 (NixOS#140364) but it was also added in b6cb656 (in support of openjdk 18 in da40a44), which landed directly on master in 9413ebb (NixOS#165354). those two conflicted when master was merged into staging-next in a5dfac8 (NixOS#191339), and adoptopenjdk 17 was mistakenly kept during the conflict resolution. the net result is that one would get: $ nix-build -A pkgs.adoptopenjdk-hotspot-bin-17 error: Alias adoptopenjdk-hotspot-bin-17 is still in all-packages.nix ... instead of the desired: $ nix-build -A pkgs.adoptopenjdk-hotspot-bin-17 error: AdoptOpenJDK is now Temurin. Use temurin-bin-17
Description of changes
OpenJDK 18 has moved into general availability (GA) and is considered by the OpenJDK developers to be stable.
This PR provides both
openjdk18andadoptopenjdk-17used for bootstrapping in line with other OpenJDK versions, but I'm happy to separate this into two PRs if required (or bootstrap with another JDK).The
ignore-LegalNoticeFilePluginpatch for Linux JDK17 has been renamed to add a JDK18 variant which has a different check (usingOptional#ifEmpty()rather than!Optional#ifPresent()).Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes