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

Fix bazel package #9008

Merged
merged 1 commit into from
Aug 6, 2015
Merged

Fix bazel package #9008

merged 1 commit into from
Aug 6, 2015

Conversation

philandstuff
Copy link
Contributor

bazelbuild/bazel@981b7bc1 depends on protobuf-2.5 and won't work with 2.6 (and in
bazelbuild/bazel@bbe84fe3d upgraded straight to protobuf 3.0.0-alpha3); this commit fixes
the dependency to depend on protobuf2_5 specifically.

The bazel compile.sh needs which on the PATH; so this commit adds that
as a dependency.

Since this package didn't have a listed maintainer, I'm claiming it.

@wkennington
Copy link
Contributor

We have protobuf3 alpha so feel free to bump this to the latest version and use that dependency.

@philandstuff
Copy link
Contributor Author

@wkennington #9011 adds 3.0.0-alpha3.1 but bazel depends on, and vendors the jar for, 3.0.0-alpha3. I don't know if the jar for 3.0.0-alpha3 will work with the native code for 3.0.0-alpha3.1, and the effort required to find out if it does doesn't seem appropriate for the reward. I'd rather wait til the official protobuf-3.0.0 comes out and bump bazel then.

@philandstuff
Copy link
Contributor Author

I updated the commit with an extra piece which removes setting of JAVA_HOME. As I documented in the commit message:

Setting JAVA_HOME to ${jdk} broke bazel when used with openjdk, with the
message:

Problem with java installation: couldn't find/access rt.jar in /nix/store/z9vc0vzyzhnpl5l5inmqdnvdnbxmmmg7-openjdk-8u60b24

This is because if you set JAVA_HOME, bazel will look for rt.jar in
$JAVA_HOME/lib and $JAVA_HOME/jre/lib, but the nixpkgs openjdk
distribution puts rt.jar in ${jdk}/lib/openjdk/jre/lib for some reason.

If you leave JAVA_HOME unset, bazel will instead look for rt.jar
relative to where it finds javac, after resolving all symlinks. This
strategy works for both oraclejdk and openjdk within nixpkgs. So the
way to make bazel work with both oraclejdk and openjdk is to just remove
the wrapProgram invocation which sets JAVA_HOME.

https://github.com/google/bazel/blob/981b7bc1ce793a484f9a39178d57f9e24bfc487a/src/main/cpp/blaze_util_linux.cc#L143-L161

@rbvermaa
Copy link
Member

Can we make openjdk the default jdk for bazel?

@wkennington
Copy link
Contributor

We have a passthrough variable which designates the java home inside of the
jdk called home. So just set JAVA_HOME=${jdk.home} and it should work for
both jdks.

On Tue, Jul 28, 2015, 04:23 Rob Vermaas [email protected] wrote:

Can we make openjdk the default jdk for bazel?


Reply to this email directly or view it on GitHub
#9008 (comment).

@philandstuff
Copy link
Contributor Author

Can we make openjdk the default jdk for bazel?

Sure, I don't see any reason why not. I will do this tonight (BST).

So just set JAVA_HOME=${jdk.home} and it should work for both jdks.

Given it already works for both jdks, what benefit would we get from explicitly setting JAVA_HOME in a wrapper?

@wkennington
Copy link
Contributor

Does it not depend on the jdk being available in the path? What happens if
I build bazel and try and run it without implicitly having the jdk
available?

On Tue, Jul 28, 2015, 07:13 Philip Potter [email protected] wrote:

Can we make openjdk the default jdk for bazel?

Sure, I don't see any reason why not. I will do this tonight (BST).

So just set JAVA_HOME=${jdk.home} and it should work for both jdks.

Given it already works for both jdks, what benefit would we get from
explicitly setting JAVA_HOME in a wrapper?


Reply to this email directly or view it on GitHub
#9008 (comment).

@philandstuff
Copy link
Contributor Author

Does it not depend on the jdk being available in the path? What happens if I build bazel and try and run it without implicitly having the jdk available?

Good point. I'll re-add the wrapProgram using ${jdk.home} as well then.

@philandstuff
Copy link
Contributor Author

PR updated to re-wrap with ${jdk.home} and make openjdk the default.

Bazel 981b7bc1 depends on protobuf-2.5 and won't work with 2.6 (and in
bbe84fe3d upgraded straight to protobuf 3.0.0-alpha3); this commit fixes
the dependency to depend on protobuf2_5 specifically.

The bazel compile.sh needs `which` on the PATH; so this commit adds that
as a dependency.

Setting JAVA_HOME to ${jdk} broke bazel when used with openjdk, with the
message:

    Problem with java installation: couldn't find/access rt.jar in /nix/store/z9vc0vzyzhnpl5l5inmqdnvdnbxmmmg7-openjdk-8u60b24

This is because if you set JAVA_HOME, bazel will look for rt.jar in
$JAVA_HOME/lib and $JAVA_HOME/jre/lib, but the nixpkgs openjdk
distribution puts rt.jar in ${jdk}/lib/openjdk/jre/lib for some reason.

To fix this, this commit uses the ${jdk.home} passthru value to use the
appropriate JAVA_HOME for the given jdk.

As bazel now works with openjdk, and openjdk is free while oraclejdk is
not, this commit changes the default jdk for bazel to openjdk.

Since this package didn't have a listed maintainer, I'm claiming it.
@philandstuff
Copy link
Contributor Author

rebased to master to see if this fixes travis. was getting merge conflicts in unrelated files ¯(°_o)/¯

@copumpkin
Copy link
Member

Yay no implicit dependency on JDK!

lucabrunox pushed a commit that referenced this pull request Aug 6, 2015
@lucabrunox lucabrunox merged commit dad54b3 into NixOS:master Aug 6, 2015
@lucabrunox
Copy link
Contributor

Looks ok to me.

@philandstuff philandstuff deleted the fix-bazel branch November 29, 2015 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants