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

false positive which("java") check on macOS 12.6 #770

Open
dpoluyanov opened this issue Oct 13, 2022 · 1 comment
Open

false positive which("java") check on macOS 12.6 #770

dpoluyanov opened this issue Oct 13, 2022 · 1 comment

Comments

@dpoluyanov
Copy link

dpoluyanov commented Oct 13, 2022

Hi,
I investigated an issue with coursier fetch on MacBook hosts without installed java and discovered that it fails with:

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 ...

$ 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 --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

@dpoluyanov
Copy link
Author

Still doesn't help overall because of _sha256_hasher and _list_packages jars, but at-least one step forward

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

No branches or pull requests

1 participant