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: typo in gu post-install actions #99

Merged
merged 2 commits into from
Sep 4, 2023

Conversation

mantasindrasius
Copy link
Contributor

@mantasindrasius mantasindrasius commented Sep 3, 2023

I couldn't find a proper example as well as I didn't manage to make the graalvm working due to following issues:L

  1. There seems to be a typo bug in graalvm_bindist.bzl.
  2. After fixing the bug and installing js, I'm still getting the error below, so I'd like @sgammon 's help to make a working example:
Exception in thread "main" java.lang.IllegalArgumentException: A language with id 'js' is not installed. Installed languages are: [].
	at org.graalvm.truffle/com.oracle.truffle.polyglot.PolyglotEngineException.illegalArgument(PolyglotEngineException.java:131)
	at org.graalvm.truffle/com.oracle.truffle.polyglot.PolyglotEngineImpl.throwNotInstalled(PolyglotEngineImpl.java:1109)
	at org.graalvm.truffle/com.oracle.truffle.polyglot.PolyglotEngineImpl.requirePublicLanguage(PolyglotEngineImpl.java:1116)
	at org.graalvm.truffle/com.oracle.truffle.polyglot.PolyglotContextImpl.requirePublicLanguage(PolyglotContextImpl.java:1491)
	at org.graalvm.truffle/com.oracle.truffle.polyglot.PolyglotContextImpl.lookupLanguageContext(PolyglotContextImpl.java:1453)
	at org.graalvm.truffle/com.oracle.truffle.polyglot.PolyglotContextImpl.eval(PolyglotContextImpl.java:1462)
	at org.graalvm.truffle/com.oracle.truffle.polyglot.PolyglotContextDispatch.eval(PolyglotContextDispatch.java:63)
	at org.graalvm.sdk/org.graalvm.polyglot.Context.eval(Context.java:399)
	at org.graalvm.sdk/org.graalvm.polyglot.Context.eval(Context.java:425)
	at Main.main(Main.java:14)

@sgammon

This comment was marked as outdated.

@sgammon
Copy link
Owner

sgammon commented Sep 3, 2023

@mantasindrasius I've discovered the bug, which I filed as #100. I'm pushing a fix and then I will return to this PR to amend the test. Thanks again or reporting. It looks like transitive component dependencies were installed, but not the actual direct dependencies requested by the user (oops).

sgammon added a commit that referenced this pull request Sep 3, 2023
- fix: add requested component to list of effective components
  installed via bindist

Fixes and closes #100. Thanks to
@mantasindrasius for reporting!

Relates-To: #100
Relates-To: #99
Signed-off-by: Sam Gammon <sam@elide.ventures>
sgammon added a commit that referenced this pull request Sep 3, 2023
- fix: add requested component to list of effective components
  installed via bindist

Fixes and closes #100. Thanks to
@mantasindrasius for reporting!

Relates-To: #100
Relates-To: #99
Signed-off-by: Sam Gammon <sam@elide.ventures>
@sgammon sgammon changed the title Example with components usage and a startswith method typo fix fix: typo in gu post-install actions Sep 3, 2023
@sgammon sgammon mentioned this pull request Sep 3, 2023
@sgammon sgammon changed the base branch from main to release/0.10.x September 3, 2023 23:09
@@ -1 +1,3 @@
build --enable_bzlmod
build --java_runtime_version=graalvm_20
build --tool_java_runtime_version=graalvm_20
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is how you use the GraalVM JVM as your Java Toolchain in Bazel, which you must do so that it can find the GraalVM org.graalvm.polyglot.Context type.

the other way to do this is to install and build against org.graalvm.sdk:graal-sdk from Maven, which provides these symbols on non-GraalVM runtimes.

@@ -7,7 +7,7 @@ module(

bazel_dep(
name = "rules_java",
version = "6.5.0",
version = "6.4.0",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bazel 6 needs rules_java 6.4.0 or previous

Comment on lines +23 to +25
extra_args = [
"--language:js",
],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enabling the component in the native_image: the components-ce sample, which was on the release/0.10.x branch, has taken on this test.

Comment on lines +9 to +15
OutputStream myOut = new BufferedOutputStream(System.out);
Context context = Context.newBuilder("js")
.out(myOut)
.allowAllAccess(true)
.build();
context.eval("js", "42");
context.close();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've kept your test code, but moved it to components-ce, where we are already testing components.

@sgammon sgammon marked this pull request as ready for review September 3, 2023 23:12
@sgammon sgammon self-assigned this Sep 3, 2023
@sgammon sgammon added bug Something isn't working components Issues relating to GraalVM components labels Sep 3, 2023
@sgammon sgammon added this to the 1.0.0 milestone Sep 3, 2023
@sgammon sgammon linked an issue Sep 3, 2023 that may be closed by this pull request
sgammon added a commit that referenced this pull request Sep 3, 2023
- fix: add requested component to list of effective components
  installed via bindist

Fixes and closes #100. Thanks to
@mantasindrasius for reporting!

Relates-To: #100
Relates-To: #99
Signed-off-by: Sam Gammon <sam@elide.ventures>
@sgammon
Copy link
Owner

sgammon commented Sep 3, 2023

this will ship with #80 instead of being merged directly into main, because it now depends on the 0.10.x (the components-ce test is included in that branch).

update: actually, i can split the test and fix commits so i can cherry-pick the fix on to main safely, which is what I will do.

@sgammon sgammon force-pushed the install-typo-fix branch 2 times, most recently from 5c9350a to 035b51b Compare September 3, 2023 23:21
sgammon and others added 2 commits September 3, 2023 16:22
- fix: `startsWith` should be `startswith`
- test: update `components-ce` test to include component usage

Thank you to @mantasindrasius!

Co-authored-by: Mantas Indrašius <mantasi@wix.com>
Signed-off-by: Sam Gammon <sam@elide.ventures>
- test: add code to `components-ce` test to ensure `js`
  component is installed and usable

Co-authored-by: Mantas Indrašius <mantasi@wix.com>
Signed-off-by: Sam Gammon <sam@elide.ventures>
@sonarcloud
Copy link

sonarcloud bot commented Sep 3, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sgammon
Copy link
Owner

sgammon commented Sep 3, 2023

@mantasindrasius We're good to go here.

Here's what you should do:

graalvm_repository(
    name = "graalvm",
    components = [
        "js",  # you only need this, not the `setup_action`
    ],
    distribution = "oracle",
    java_version = "20",
    version = "20.0.2",
)

# ...

register_toolchains("@graalvm//:jvm")  # registers the Java Toolchain
register_toolchains("@graalvm//:sdk")  # registers the native-image toolchain

In your .bazelrc:

build --java_runtime_version=graalvm_20
build --tool_java_runtime_version=graalvm_20

Then, in your BUILD.bazel file:

native_image(
    name = "main-native",
    main_class = "Main",
    deps = [":java"],
    extra_args = [
        "--language:js",  # add this to build `js` into your `native-image`
    ],
)

Note that there is no need to provide the native_image_tool attribute, since in this case it is using Toolchains to resolve native-image.

This will build JS into your native image target; it also sets up your Java toolchains so you can build against the GraalVM SDK types. Once this PR is merged, you will want to use the release/0.10.x branch, which has the latest fixes and toolchain support. You can consult the components-ce sample if you want to see how it works. I've manually tested that sample against Bazel 6.3.2.

@sgammon sgammon merged commit 9fd60a7 into sgammon:release/0.10.x Sep 4, 2023
56 checks passed
@mantasindrasius
Copy link
Contributor Author

Amazing. Thanks for responsiveness!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working components Issues relating to GraalVM components
Projects
Development

Successfully merging this pull request may close these issues.

Typo in the gu post-install actions
2 participants