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

babashka.process crashes when there are certain GraalVM classes on the classpath #76

Closed
hansbugge opened this issue Jul 4, 2022 · 12 comments

Comments

@hansbugge
Copy link
Contributor

When there are certain GraalVM classes on the classpath babashka.process crashes.

I'm guessing that this is because babashka.process wrongly thinks that it is running in a native GraalVM environment. I noticed it when running nextjournal/clerk which depends on org.graalvm.js/js via io.github.nextjournal/markdown.

$ clojure -Sdeps '{:deps {io.github.nextjournal/clerk {:mvn/version "0.8.451"} babashka/process {:mvn/version "0.1.4"}}}'
Clojure 1.11.1
user=> (require 'babashka.process)
Syntax error (IllegalArgumentException) compiling . at (babashka/process.cljc:528:10).
No matching method exec found taking 3 args for class org.graalvm.nativeimage.ProcessProperties
user=> (boolean (resolve 'org.graalvm.nativeimage.ProcessProperties))
true
user=> (System/getProperty "java.vm.version")
"17.0.3+7"
user=> (System/getProperty "java.vm.name")
"OpenJDK 64-Bit Server VM"
@borkdude
Copy link
Contributor

borkdude commented Jul 4, 2022

@hansbugge So you are not running in GraalVM, but there is org.graalvm.nativeimage.ProcessProperties on the classpath? Which library is bringing that in?

@borkdude
Copy link
Contributor

borkdude commented Jul 4, 2022

Btw, the library should work both in a non-Graal and Graal JVM

@hansbugge
Copy link
Contributor Author

hansbugge commented Jul 4, 2022

Yes indeed, it's on the classpath, and I'm on a regular OpenJDK 17. I don't know why, but it's pulled in by Clerk via a transitive dependency on org.graalvm.js/js as far as I can tell.

I can reproduce the error with the commands I put in the top comment.

By the way, my workaround for now is to pull in babashka/process {:mvn/version "0.1.1"} which doesn't have the if-graal macro.

@borkdude
Copy link
Contributor

borkdude commented Jul 4, 2022

What I don't understand is that you can resolve org.graalvm.nativeimage.ProcessProperties but that org.graalvm.nativeimage.ProcessProperties/exec with the overload of 2 args isn't found.

@borkdude
Copy link
Contributor

borkdude commented Jul 4, 2022

I think I understand now. The third argument to exec was added in a later version of GraalVM. So it's likely that if clerk or you would upgrade the graaljs version, you would not have this error.

@hansbugge
Copy link
Contributor Author

Well, the overload with 2 args does exist, but not the one with 3.

In the same REPL as above:

user=> (require '[clojure.reflect :as r])
nil
user=> (-> org.graalvm.nativeimage.ProcessProperties r/reflect :members (->> (filter #(= 'exec (:name %)))) clojure.pprint/pprint)
({:name exec,
  :return-type void,
  :declaring-class org.graalvm.nativeimage.ProcessProperties,
  :parameter-types [java.nio.file.Path java.lang.String<>],
  :exception-types [],
  :flags #{:varargs :public :static}})

@hansbugge
Copy link
Contributor Author

You're right, that fixed it!

I gave an explicit dependency on org.graalvm.js/js {:mvn/version "22.1.0"} which overrides version 21.3.0 from Clerk.

clj -Sdeps '{:deps {io.github.nextjournal/clerk {:mvn/version "0.8.451"} babashka/process {:mvn/version "0.1.4"} org.graalvm.js/js {:mvn/version "22.1.0"}}}'
Clojure 1.11.1
user=> (require 'babashka.process)
nil

@borkdude
Copy link
Contributor

borkdude commented Jul 4, 2022

cc @mk

@hansbugge
Copy link
Contributor Author

That was quick. Thanks for your help!

borkdude added a commit that referenced this issue Jul 4, 2022
@borkdude
Copy link
Contributor

borkdude commented Jul 4, 2022

0.1.7 released

@mk
Copy link

mk commented Jul 4, 2022

@borkdude thanks for the mention. Is there anything actionable to do in Clerk? We actually rolled back an upgrade of graal-js after folks noticed that we dropped Java 8 compatibility nextjournal/clerk#178.

@borkdude
Copy link
Contributor

borkdude commented Jul 4, 2022

I guess not, just a FYI.

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

3 participants