search

bazel-contrib / rules_jvm_external

Bazel rules to resolve, fetch and export Maven artifacts
Apache License 2.0
332 stars 254 forks source link

Depending on embedded JDK #445

Open spietras opened 4 years ago

spietras commented 4 years ago

As far as I understand, Bazel by default provides an embedded JDK (or at least some rules to get it), so we don't have to have one locally.

rules_jvm_external use Coursier, which is invoked by running java:

https://github.com/bazelbuild/rules_jvm_external/blob/aed0eb3747682e64611bf98bb1ca590a44b470de/coursier.bzl#L109-L129

This assumes java is available at JAVA_HOME, which is unset with no local JDK. If I understand correctly Bazel sets JAVABASE to the location of the embedded JDK.

So is it possible to use the embedded JDK with rules_jvm_external? Maybe it is possible now and I am missing something? And if it's not currently possible, wouldn't it be sufficient to just check if JAVABASE is set too? I guess the order of checking is important because someone can have a local JDK, while still wanting to use the embedded one.

For now, a cheap workaround is to just set JAVA_HOME relatively, knowing that both maven and jdk are in the external workspace. I can confirm that adding this line:

build --repo_env=JAVA_HOME=../bazel_tools/jdk

to the .bazelrc file works and the embedded JDK is used.

jin commented 4 years ago

Looks like it's actually possible to use the downloaded JDK (not embedded one, since the embedded one is minimal enough to run Bazel, and probably not other Java applications) to execute Coursier, but I just haven't got to doing this yet. Would you wanna take a stab at this and submit a PR?

spietras commented 4 years ago

Would you wanna take a stab at this and submit a PR?

Sure, I'll give it a try soon.

Looks like it's actually possible to use the downloaded JDK (not embedded one, since the embedded one is minimal enough to run Bazel, and probably not other Java applications)

Yeah, sorry, by "embedded" I meant the downloaded JDK, not the embedded JRE. So from now on, I will refer to the downloaded JDK as the "remote JDK", because that's how it's called in the bazel_tools workspace.

spietras commented 4 years ago

Update:

It would be so easy to just use JAVABASE... if only we had access to it. But there is a problem.

JAVABASE is a make variable from Java toolchain. As such, the only rules that have access to the variable are those that have access to Java toolchains. Currently, only regular rules can access toolchains. It's not possible to pass a toolchain to repository rules and coursier_fetch (a rule invoked by maven_install) is a repository rule. So JAVABASE is simply not defined for coursier_fetch.

Could it possibly be available? Probably, but not at this moment. I guess Bazel itself would have to change the way toolchains are created. It's another story for another time. So are there any other possibilities? To an extent.

This issue came up in other Bazel ecosystems too. See this issue and this PR from rules_python. Here, pip is used as a way to get third party dependencies, similar to maven. pip_import is also a repository rule and therefore can't use toolchains. But the path to the Python interpreter can be passed as an argument to the rule in the WORKSPACE file in two ways:

  1. As a string: the path is used literally, with no modifications
  2. As a Label: Bazel will check if the target described by the label exists and resolve the path to it

Option 1 is ok if you know the path to the tool you want to use. That's fine if you want to use your local tool, but unsuitable for remote tools (as in remote JDK). Option 2 is better for remote tools, but note that inside the rule the path to the Label is resolved. Labels pointing to regular targets can't be resolved in repository rules (if ever), so the only way is to point to a file. This can be done for example by expose_files in the repository that provides the remote tools.

So, as I understand (and I might be wrong), the only way our goal can be achieved is for the remote JDK repository to expose the bin/java file, so we can pass the Label pointing to that file as an argument to maven_install, resolve the path to it in the rule and then invoke it as usual. I think it's not possible now as bin/java is not exposed, but I would like someone that knows it better to confirm that.

So I think I will leave it like that for now (unless someone points out that I'm wrong and it is possible - in that case I would willingly come back to it). I'm sure this issue will come up in the future, because fully hermetic Java builds are kind of desired, I guess. So that's just my analysis of the situation and some thoughts, maybe it will be useful.

jin commented 4 years ago

Thank you for taking the time to look into this, @spietras. It sounds like there's a usability gap between toolchains and repository rules, and the current API is not sufficient to express such a relationship.

I think that the approach to expose bin/java as a file target is feasible, but it runs the risk of revealing too much implementation details of the remote Java tools. We would have to check back with the Java tooling team in Bazel to see if that's something we can do, or if we should stick with extending the toolchain API to better support repository rules.

jin commented 4 years ago

Also reported here, with a repro example: https://github.com/bazelbuild/rules_jvm_external/issues/450

pcj commented 3 years ago

Just hit this as well... Really need a better story around toolchains and repository rules

ERROR: no such package '@maven//': Unable to run coursier: No Java runtime present, requesting install.
alexeagle commented 1 year ago

I just ran into this again, and the solution linked above from @brentleyjones is really nice:

Just add to .bazelrc:

# Don't depend on a JAVA_HOME pointing at a system JDK
# see https://github.com/bazelbuild/rules_jvm_external/issues/445
build --repo_env=JAVA_HOME=../bazel_tools/jdk
guw commented 1 year ago

Even better would be a way to make coursier use the same JDK as server_javabase (somehow). We have to use a JDK with modified cacerts file to allow connectivity.

alexeagle commented 1 year ago

@shs96c @jin assuming we won't make a "principled fix", WDYT about adding that workaround to the recommended install instructions? I keep finding that clients are tripping on this and their engineers think "Bazel makes you install Java" - a perception we've worked so hard to correct.

alexeagle commented 4 weeks ago

Pinging since I still run into this regularly.

shs96c commented 4 weeks ago

@alexeagle, if you set the resolver attribute to maven, then you should be using the same JDK that's used by bazel run. It's not a perfect solution, since you're now using a different resolver, but it should help.

alexeagle commented 4 weeks ago

ooh thanks @shs96c - I imagine you're referring to https://github.com/bazel-contrib/rules_jvm_external/blob/master/docs/api.md#maven_install-resolver ? What are the implications of using maven resolver rather than coursier? I don't want to introduce a new bug.

shs96c commented 4 weeks ago

You'll need to use a lock file --- that's a requirement of the maven resolver. To bootstrap, this can be empty file.

Unless your list of artifacts is very limited, you'll likely end up with a different resolution, but it should still be compatible with your existing code. Once the resolution is done, there is no difference in how the lock file is consumed.