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

WIP: OpenJDK 9 ea #27194

Merged
merged 11 commits into from
Sep 22, 2017
Merged

WIP: OpenJDK 9 ea #27194

merged 11 commits into from
Sep 22, 2017

Conversation

jerith666
Copy link
Contributor

@jerith666 jerith666 commented Jul 7, 2017

Motivation for this change

Start figuring out how to build it before it's officially released. :)

Things done
  • OpenJDK Specific Tasks
    • Test minimal (jre9_headless)
    • Figure out whether infinality patches are still relevant
    • Figure out whether *.pf symlink fix is still necessary
    • Decide if a bootstrap JDK external to nixpkgs is required
    • Test 32-bit
    • Keep up with ongoing ea builds (http://jdk.java.net/9/, now at 180)
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests) (no tests for jdk ... would be nice to write some, but I think that's better done separately)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
    • obviously nothing depends on 9 yet, but we could try updating some 8 pkgs?
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

My current branch has all the gory details of my false starts, etc., and of course will need to some rebase -iing before being merged. It does build though -- ./result/bin/java -version and ./result/bin/jshell both work. :)

@NeQuissimus
Copy link
Member

very cool, September release and we're basically ready?! :D


prePatch = ''
ls | grep jdk | grep -v '^jdk9' | awk -F- '{print $1}' | while read p; do
mv $p-* $(ls | grep '^jdk9')/$p
Copy link
Contributor

Choose a reason for hiding this comment

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

ls is not meant to be used in scripts. Use find.


preConfigure = ''
chmod +x configure
substituteInPlace configure --replace /bin/bash "$shell"
Copy link
Contributor

Choose a reason for hiding this comment

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

bash != sh

+ lib.optionalString minimal "\"--disable-headful\""
+ ");";

NIX_LDFLAGS= lib.optionals (!minimal) [
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use pkg-config --libs for all these libs such that it will continue to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inside a nix-shell, my best guess at what you mean doesn't seem to work for most of the libs being used (fontconfig is the only one it likes, apparently):

[nix-shell:/tmp/jdk/8]$ pkg-config --libs fontconfig Xinerama cups Xrandr magic
Package Xinerama was not found in the pkg-config search path.
Perhaps you should add the directory containing `Xinerama.pc'
to the PKG_CONFIG_PATH environment variable
No package 'Xinerama' found
Package cups was not found in the pkg-config search path.
Perhaps you should add the directory containing `cups.pc'
to the PKG_CONFIG_PATH environment variable
No package 'cups' found
Package Xrandr was not found in the pkg-config search path.
Perhaps you should add the directory containing `Xrandr.pc'
to the PKG_CONFIG_PATH environment variable
No package 'Xrandr' found
Package magic was not found in the pkg-config search path.
Perhaps you should add the directory containing `magic.pc'
to the PKG_CONFIG_PATH environment variable
No package 'magic' found

Can you explain further? (I'm a Java guy mostly, not too familiar with C build tools and conventions.)

Copy link
Contributor

Choose a reason for hiding this comment

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

@0xABAB was removed from the NixOS organisation for being too helpful from what I can see. :')

Let me see if I help...

/usr/lib/x86_64-linux-gnu/pkgconfig/xinerama.pc is in Debian unstable amd64. If the nix packaging doesn't have such file, that would seem surprising to me at least. If there really is no such file, then that should ideally be filed as a bug.

Just add a # FIXME replace with pkg-config call once <referred issue> is fixed` and call it done is my recommendation.

rm $out/lib/openjdk/bin/{policytool,appletviewer}
''}

# Copy the JRE to a separate output and setup fallback fonts
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean you haven't tested this with a real application? I.e., that it shows attractive fonts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

policytool and jconsole are both GUI apps that are part of the JDK, and the fonts looked fine to me in them. I have not tested any other apps yet, but certainly will before removing the "WIP" from the title. :)

done

# Generate certificates.
pushd $jre/lib/openjdk/jre/lib/security
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer to see subshell instead of a bashism.

# any package that depends on the JRE has $CLASSPATH set up
# properly.
mkdir -p $jre/nix-support
echo -n "${setJavaClassPath}" > $jre/nix-support/propagated-native-build-inputs
Copy link
Contributor

Choose a reason for hiding this comment

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

echo -n -> printf %s


postFixup = ''
# Build the set of output library directories to rpath against
LIBDIRS=""
Copy link
Contributor

Choose a reason for hiding this comment

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

Those quotes are optional.

# Build the set of output library directories to rpath against
LIBDIRS=""
for output in $outputs; do
LIBDIRS="$(find $(eval echo \$$output) -name \*.so\* -exec dirname {} \; | sort | uniq | tr '\n' ':'):$LIBDIRS"
Copy link
Contributor

Choose a reason for hiding this comment

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

There are less powerful versions of eval that are safer to use in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Use the + version of find, not ;.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see any variants of eval in the bash manpage ... ?


# Add the local library paths to remove dependencies on the bootstrap
for output in $outputs; do
OUTPUTDIR="$(eval echo \$$output)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Idem

Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant quotes too

# Add the local library paths to remove dependencies on the bootstrap
for output in $outputs; do
OUTPUTDIR="$(eval echo \$$output)"
BINLIBS="$(find $OUTPUTDIR/bin/ -type f; find $OUTPUTDIR -name \*.so\*)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant quotes.


# Test to make sure that we don't depend on the bootstrap
for output in $outputs; do
if grep -q -r '${bootjdk}' $(eval echo \$$output); then
Copy link
Contributor

Choose a reason for hiding this comment

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

More eval

return "com.sun.java.swing.plaf.windows.WindowsLookAndFeel";
} else {
- String desktop = AccessController.doPrivileged(new GetPropertyAction("sun.desktop"));
Toolkit toolkit = Toolkit.getDefaultToolkit();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't this be done upstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe for 10 -- I'm sure the window for this sort of change for 9 is closed by now though.

@0xABAB
Copy link
Contributor

0xABAB commented Jul 8, 2017

This is going in the right direction.

@jerith666
Copy link
Contributor Author

jerith666 commented Jul 14, 2017

@0xABAB since most of your cleanup suggestions aren't specific to 9, I made a new commit f79e71b to apply them to 8.nix, then rebased my work on 9.nix (still starting w/ a copy of 8) on top of it. If we should do that cleanup as a separate PR, LMK.

@jerith666
Copy link
Contributor Author

1st round of rebase -iing complete, leaving the commits with outstanding TODOs as children of the commits with more solid foundations.

@jerith666
Copy link
Contributor Author

The cleanup commit conflicts with 3cb745d. It'll be trivial to resolve, just waiting for what I assume is the resulting mass-rebuild to happen before rebasing on top of it. :)

as recommended by 0xABAB in NixOS#27194
this starts with copy of 8.nix and just updates hashes and replaces 8
with 9.  it also tweaks the version handling because we aren't dealing
with an update version yet.
fix-java-home: surrounding code changed slightly

swing-use-gtk-jdk9: location of the file being patched changed due to
modularization

read-truststore-from-env: the code that handles the trustStore was
refactored out into a helper class in upstream commit
http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/904861872c0e

adlc_updater: this isn't present anymore
this requires that we switch to configureFlagsArray to deal with
whitespace

the errors being suppressed are show below:

* For target support_native_java.desktop_libawt_xawt_awt_Robot.o:
/tmp/nix-build-openjdk-9ea-b176.drv-0/jdk9-jdk-9+176/jdk/src/java.desktop/unix/native/libawt_xawt/awt/awt_Robot.c: In function 'isXCompositeDisplay':
/tmp/nix-build-openjdk-9ea-b176.drv-0/jdk9-jdk-9+176/jdk/src/java.desktop/unix/native/libawt_xawt/awt/awt_Robot.c:152:50: error: embedded '\0' in format
[-Werror=format-contains-nul]
     snprintf(NET_WM_CM_Sn, sizeof(NET_WM_CM_Sn), "_NET_WM_CM_S%d\0", screenNumber);
                                                  ^
/tmp/nix-build-openjdk-9ea-b176.drv-0/jdk9-jdk-9+176/jdk/src/java.desktop/unix/native/libawt_xawt/awt/awt_Robot.c:152:50: error: embedded '\0' in format
[-Werror=format-contains-nul]
cc1: all warnings being treated as errors
* For target support_native_jdk.hotspot.agent_libsa_ps_core.o:
/tmp/nix-build-openjdk-9ea-b176.drv-0/jdk9-jdk-9+176/hotspot/src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c: In function 'read_exec_segments':
/tmp/nix-build-openjdk-9ea-b176.drv-0/jdk9-jdk-9+176/hotspot/src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c:834:7: error: ignoring return value of 'pread', declared
with attribute warn_unused_result [-Werror=unused-result]
       pread(ph->core->exec_fd, interp_name, exec_php->p_filesz, exec_php->p_offset);
       ^
cc1: all warnings being treated as errors
* separate jdk and jre images are now present under build/*/images

* samples have been removed (JEP 298)
  -- TODO that JEP says demos will be gone too, but it seems some are still present?

* bina directory is no longer present
in particular, the name of the config option for headless has changed,
per https://bugs.openjdk.java.net/browse/JDK-8163102
the code being patched here seems to have changed substantially or
perhaps even disappeared altogether.  need to investigate whether
these patches are still relevant.
@NeQuissimus
Copy link
Member

I am merging this in, we can improve it later.

@NeQuissimus NeQuissimus merged commit 02fe120 into NixOS:master Sep 22, 2017
@jerith666
Copy link
Contributor Author

Sorry for letting it sit so long!

NeQuissimus pushed a commit that referenced this pull request Sep 27, 2017
* openjdk 8: code cleanup

as recommended by 0xABAB in #27194

* openjdk 9: init at ea build 176

this starts with copy of 8.nix and just updates hashes and replaces 8
with 9.  it also tweaks the version handling because we aren't dealing
with an update version yet.

* openjdk 9: adapt patches from openjdk 8

fix-java-home: surrounding code changed slightly

swing-use-gtk-jdk9: location of the file being patched changed due to
modularization

read-truststore-from-env: the code that handles the trustStore was
refactored out into a helper class in upstream commit
http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/904861872c0e

adlc_updater: this isn't present anymore

* openjdk 9: make two more warnings-as-errors non-fatal

this requires that we switch to configureFlagsArray to deal with
whitespace

the errors being suppressed are show below:

* For target support_native_java.desktop_libawt_xawt_awt_Robot.o:
/tmp/nix-build-openjdk-9ea-b176.drv-0/jdk9-jdk-9+176/jdk/src/java.desktop/unix/native/libawt_xawt/awt/awt_Robot.c: In function 'isXCompositeDisplay':
/tmp/nix-build-openjdk-9ea-b176.drv-0/jdk9-jdk-9+176/jdk/src/java.desktop/unix/native/libawt_xawt/awt/awt_Robot.c:152:50: error: embedded '\0' in format
[-Werror=format-contains-nul]
     snprintf(NET_WM_CM_Sn, sizeof(NET_WM_CM_Sn), "_NET_WM_CM_S%d\0", screenNumber);
                                                  ^
/tmp/nix-build-openjdk-9ea-b176.drv-0/jdk9-jdk-9+176/jdk/src/java.desktop/unix/native/libawt_xawt/awt/awt_Robot.c:152:50: error: embedded '\0' in format
[-Werror=format-contains-nul]
cc1: all warnings being treated as errors
* For target support_native_jdk.hotspot.agent_libsa_ps_core.o:
/tmp/nix-build-openjdk-9ea-b176.drv-0/jdk9-jdk-9+176/hotspot/src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c: In function 'read_exec_segments':
/tmp/nix-build-openjdk-9ea-b176.drv-0/jdk9-jdk-9+176/hotspot/src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c:834:7: error: ignoring return value of 'pread', declared
with attribute warn_unused_result [-Werror=unused-result]
       pread(ph->core->exec_fd, interp_name, exec_php->p_filesz, exec_php->p_offset);
       ^
cc1: all warnings being treated as errors

* openjdk 9: ea+176 -> ea+180

* openjdk 9: TODO disable infinality patches, at least to start

the code being patched here seems to have changed substantially or
perhaps even disappeared altogether.  need to investigate whether
these patches are still relevant.

* openjdk 9: update installPhase for modularization

* separate jdk and jre images are now present under build/*/images

* samples have been removed (JEP 298)
  -- TODO that JEP says demos will be gone too, but it seems some are still present?

* bina directory is no longer present

* openjdk 9: TODO handle *.pf files or purge this code completely

* openjdk 9: update minimal jre components

in particular, the name of the config option for headless has changed,
per https://bugs.openjdk.java.net/browse/JDK-8163102

* TODO about echo -n vs printWords, #27427

(cherry picked from commit 02fe120)
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.

5 participants