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

R: Remove jdk dependency on aarch32/64 #50179

Merged
merged 1 commit into from Nov 12, 2018
Merged

Conversation

Thra11
Copy link
Member

@Thra11 Thra11 commented Nov 10, 2018

Remove jdk from R dependencies on arm platforms, as there is no free jdk on arm.

Motivation for this change

Despite being free itself, R is unavailable on aarch64 (wtihout setting allowUnfree to true) because it has an optional dependency on jdk and there is no free jdk package on arm platforms.

Things done

Made jdk optional, exclude it from the build inputs on aarch32 and aarch64.

  • 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/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

cc: @peti


@Thra11 Thra11 requested a review from peti as a code owner November 10, 2018 14:37
@Thra11
Copy link
Member Author

Thra11 commented Nov 10, 2018

I'm not sure if it's worth trying to do it differently, so that R can still depend on jdk on aarch64 if allowUnfree is true.

@timokau
Copy link
Member

timokau commented Nov 10, 2018

What does R need jdk for?

@Thra11
Copy link
Member Author

Thra11 commented Nov 10, 2018

For "java support" apparently. https://cran.r-project.org/doc/manuals/r-release/R-admin.html#Java-support Looks like you can write R packages which can call java code or use java for graphics output.

@Thra11
Copy link
Member Author

Thra11 commented Nov 10, 2018

I appear to have messed something up. This shouldn't trigger any rebuilds on other platforms.

It's probably the configureFlags which has changed.

@timokau
Copy link
Member

timokau commented Nov 10, 2018

Okay, not sure if this is a good idea or not. Building R on aarch64 would be great, but not supporting java is a tough tradeoff. I'll defer to someone who actually uses aarch64.

@peti
Copy link
Member

peti commented Nov 10, 2018

@GrahamcOfBorg build R

Copy link
Member

@peti peti left a comment

Choose a reason for hiding this comment

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

Looks good to me. I'm happy to see this PR merged assuming the test builds succeed.

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: R

Partial log (click to expand)

shrinking /nix/store/g8ac5fs0gc6xm9ff5yw31ar3wjg0dzwj-R-3.5.1/lib/R/library/methods/libs/methods.so
shrinking /nix/store/g8ac5fs0gc6xm9ff5yw31ar3wjg0dzwj-R-3.5.1/lib/R/library/graphics/libs/graphics.so
shrinking /nix/store/g8ac5fs0gc6xm9ff5yw31ar3wjg0dzwj-R-3.5.1/lib/R/library/utils/libs/utils.so
shrinking /nix/store/g8ac5fs0gc6xm9ff5yw31ar3wjg0dzwj-R-3.5.1/lib/R/library/grid/libs/grid.so
shrinking /nix/store/g8ac5fs0gc6xm9ff5yw31ar3wjg0dzwj-R-3.5.1/bin/Rscript
gzipping man pages under /nix/store/g8ac5fs0gc6xm9ff5yw31ar3wjg0dzwj-R-3.5.1/share/man/
strip is /nix/store/vcc4svb8gy29g4pam2zja6llkbcwsyiq-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/g8ac5fs0gc6xm9ff5yw31ar3wjg0dzwj-R-3.5.1/lib  /nix/store/g8ac5fs0gc6xm9ff5yw31ar3wjg0dzwj-R-3.5.1/bin
patching script interpreter paths in /nix/store/g8ac5fs0gc6xm9ff5yw31ar3wjg0dzwj-R-3.5.1
checking for references to /build in /nix/store/g8ac5fs0gc6xm9ff5yw31ar3wjg0dzwj-R-3.5.1...

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: R

Partial log (click to expand)

/nix/store/cmjfnswnjia8iing2flrvzm97ik9d353-R-3.5.1/lib/R/lib/libR.dylib: fixing dylib
/nix/store/cmjfnswnjia8iing2flrvzm97ik9d353-R-3.5.1/lib/R/lib/libR.dylib: fixing dylib
/nix/store/cmjfnswnjia8iing2flrvzm97ik9d353-R-3.5.1/lib/R/lib/libR.dylib: fixing dylib
/nix/store/cmjfnswnjia8iing2flrvzm97ik9d353-R-3.5.1/lib/R/lib/libR.dylib: fixing dylib
/nix/store/cmjfnswnjia8iing2flrvzm97ik9d353-R-3.5.1/lib/R/lib/libR.dylib: fixing dylib
/nix/store/cmjfnswnjia8iing2flrvzm97ik9d353-R-3.5.1/lib/R/lib/libR.dylib: fixing dylib
/nix/store/cmjfnswnjia8iing2flrvzm97ik9d353-R-3.5.1/lib/R/lib/libR.dylib: fixing dylib
/nix/store/cmjfnswnjia8iing2flrvzm97ik9d353-R-3.5.1/lib/R/lib/libR.dylib: fixing dylib
/nix/store/cmjfnswnjia8iing2flrvzm97ik9d353-R-3.5.1/lib/R/lib/libR.dylib: fixing dylib
/nix/store/cmjfnswnjia8iing2flrvzm97ik9d353-R-3.5.1/lib/R/lib/libR.dylib: fixing dylib

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: R

Partial log (click to expand)

shrinking /nix/store/2wawjqmks6qlkarn1w8vn5n1r5zgwswp-R-3.5.1/lib/R/modules/internet.so
shrinking /nix/store/2wawjqmks6qlkarn1w8vn5n1r5zgwswp-R-3.5.1/lib/R/lib/libR.so
shrinking /nix/store/2wawjqmks6qlkarn1w8vn5n1r5zgwswp-R-3.5.1/lib/R/bin/exec/R
shrinking /nix/store/2wawjqmks6qlkarn1w8vn5n1r5zgwswp-R-3.5.1/lib/R/bin/Rscript
gzipping man pages under /nix/store/2wawjqmks6qlkarn1w8vn5n1r5zgwswp-R-3.5.1/share/man/
strip is /nix/store/p9akxn2sfy4wkhqdqa3li97pc6jaz3r1-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/2wawjqmks6qlkarn1w8vn5n1r5zgwswp-R-3.5.1/lib  /nix/store/2wawjqmks6qlkarn1w8vn5n1r5zgwswp-R-3.5.1/bin
patching script interpreter paths in /nix/store/2wawjqmks6qlkarn1w8vn5n1r5zgwswp-R-3.5.1
checking for references to /build in /nix/store/2wawjqmks6qlkarn1w8vn5n1r5zgwswp-R-3.5.1...
/nix/store/2wawjqmks6qlkarn1w8vn5n1r5zgwswp-R-3.5.1

@matthewbauer
Copy link
Member

matthewbauer commented Nov 10, 2018

Could you make "javaSupport" an arg here? Just to emphasize that you really don't need jdk with R anyway - we can just enable it by default on all other platforms.

Add javaSupport as an argument for R. Default to false on arm as there is no free jdk on arm.
@peti peti merged commit 22a8025 into NixOS:master Nov 12, 2018
@Thra11 Thra11 deleted the r-without-jdk branch November 12, 2018 11:57
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

5 participants