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.

Sign up for GitHub

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

Jump to bottom

openjdk: fix build for !enableGnome2 #42187

Merged
merged 1 commit into from Jul 5, 2018
Merged

openjdk: fix build for !enableGnome2 #42187

merged 1 commit into from Jul 5, 2018

Conversation

jameysharp
Copy link
Contributor

Motivation for this change

This OpenJDK packaging has a headless build configuration controlled by the minimal flag, which is regularly build-tested by Hydra, and a non-headless configuration based on pure Xlib libraries without Gnome features, which is not normally tested.

Sometime before OpenJDK 8, the !enableGnome2 case broke, because it needs to link against libXrandr but that wasn't included in the buildInputs.

It might be nice to add packages with this flag set to all-packages.nix so Hydra tests this configuration regularly. This would be analogous to jre_headless and similar, but I couldn't decide what the best package name would be (jre_gnomeless?) so I haven't written such a patch. I also wasn't sure if it would be desirable since obviously most people aren't using this package with the Gnome support turned off, so it's kind of a waste of CI compute time...

If this patch is backported to NixOS 18.03 or earlier, the same fix needs to be applied to OpenJDK 9.

I tested openjdk9.override { enableGnome2 = false; } with an application I care about on NixOS 18.03 using .overrideAttrs to patch the buildInputs. I tested openjdk10 the same way against Nixpkgs commit 44f3a1d.

Before submitting this pull request, I ran:

nix-build --expr \
  '(import ./. {}).openjdk10.override { enableGnome2 = false; }' \
  '(import ./. {}).openjdk8.override { enableGnome2 = false; }'

and tested result/bin/java -version and result-2/bin/java -version.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@jameysharp
openjdk: fix build for !enableGnome2
f1c3a61 
This OpenJDK packaging has a headless build configuration controlled by
the `minimal` flag, which is regularly build-tested by Hydra, and a
non-headless configuration based on pure Xlib libraries without Gnome
features, which is not normally tested.

Sometime before OpenJDK 8, the !enableGnome2 case broke, because it
needs to link against libXrandr but that wasn't included in the
buildInputs.

If this patch is backported to NixOS 18.03 or earlier, the same fix
needs to be applied to OpenJDK 9.

I have tested OpenJDK versions 8, 9, and 10, but not any other versions.
@jameysharp
Copy link
Contributor Author

I didn't understand what the rebuild-* tags meant before. Should this PR go through staging as well?

It's kind of unfortunate to rebuild everything since this change shouldn't make any difference whatsoever for any packages that were building successfully before, but so it goes, I guess...

@fpletz
Copy link
Member

fpletz commented Jun 19, 2018

Over 100 rebuilds is rather large but I think we can squeeze this into master because most rebuilds are probably small because nothing is actually built (just uses java at runtime).

Alternatively, you can also only add libXrandr only if !enableGnome2 which would prevent those rebuilds.

You are right that this probably won't semantically change anything but Nix can't know for sure so everything gets rebuilt because the buildInputs have changed.

I've only approved this rather than merging right away because I'd like to wait for some Java people to chime in.

@matthewbauer matthewbauer merged commit d6a202f into NixOS:master Jul 5, 2018
@jameysharp jameysharp deleted the jdk-without-gnome branch July 8, 2019 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fpletz fpletz fpletz approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 101-500
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jameysharp @fpletz @matthewbauer @GrahamcOfBorg