bazel-contrib / bazel-contrib/rules_jvm_external
false positive which("java") check on macOS 12.6
- Dominant language
- Java
- Stars
- 373
- Forks
- 301
- Avg merge
- 7d 17h
- Merged PRs (30d)
- 3
Description
Hi,
I investigated an issue with coursier fetch on MacBook hosts without installed `java` and discovered that it fails with:
```bash
ERROR: /Users/d.poluyanov/crpt/workspace/WORKSPACE:68:26: fetching coursier_fetch rule //external:unpinned_maven: Traceback (most recent call last):
File "/private/var/tmp/_bazel_d.poluyanov/1794b0d0fe278908fc52615a228a9574/external/rules_jvm_external/coursier.bzl", line 879, column 13, in _coursier_fetch_impl
fail("Unable to run coursier: " + hasher_exec_result.stderr)
Error in fail: Unable to run coursier: The operation couldn’t be completed. Unable to locate a Java Runtime.
```
So digging around the code, I've found that it relies either on os.env.JAVA_HOME or on location of java (via `repository_ctx.which("java")`), and latter always resolved to `/usr/bin/java` (at least on clean macOS 12.6).
`/usr/bin/java` on those hosts returns an output in stderr an message with link to java site to download JDK, so check only on presence on `java` is not enough for launch coursier with `/usr/bin/java -jar ...`
```bash
$ which java
/usr/bin/java
$ /usr/bin/java -version
The operation couldn’t be completed. Unable to locate a Java Runtime.
Please visit http://www.java.com for information on installing Java.
```
And this `/usr/bin/java` couldn't be deleted even with sudo rights.
I've modified `_java_path` not only to check presence of `java` in PATH, but also for check exit code for `java -version`, to be sure that it is real java binary, and not a wrapper.
```diff
diff --git a/coursier.bzl b/coursier.bzl
index 1a1a64f..87d4f68 100644
--- a/coursier.bzl
+++ b/coursier.bzl
@@ -208,7 +208,12 @@ def _java_path(repository_ctx):
if java_home != None:
return repository_ctx.path(java_home + "/bin/java")
elif repository_ctx.which("java") != None:
- return repository_ctx.which("java")
+ result = repository_ctx.execute(
+ [repository_ctx.which("java"), "-version"],
+ quiet = True,
+ )
+ if result.return_code == 0:
+ return repository_ctx.which("java")
return None
# Generate the base `coursier` command depending on the OS, JAVA_HOME or the
```
So, will be happy if this patch will be accepted to upstream
Contributor guide
Research direction
Start in coursier.bzl at _java_path and inspect how repository_ctx.which("java") is used during Java discovery. Reproduce the macOS 12.6 case or an equivalent environment, then verify that coursier fetch rejects the nonfunctional /usr/bin/java wrapper while accepting a working Java executable.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- build-system
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100