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

feat: add rules_graalvm module #836

Closed
wants to merge 2 commits into from

Conversation

sgammon
Copy link
Contributor

@sgammon sgammon commented Aug 14, 2023

Summary

Adds rules_graalvm at the new release version of 0.10.0.

Inaugural features for BCR

  • Build Java into native binaries with native-image
  • Support for Bazel 4-7
  • Support for macOS, Linux, Windows
  • Support for latest versions of GraalVM CE and Oracle GraalVM
  • Hermetic native-image on all OS targets (thank you @fmeum!)
  • Use of GraalVM as a Java toolchain
  • Install and use components via gu
  • Respects conventional Bazel settings like --compilation_mode=opt

Usage

See project docs for the latest installation instructions.

@fmeum fmeum added the presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval label Aug 14, 2023
@sgammon
Copy link
Contributor Author

sgammon commented Aug 14, 2023

@fmeum looks like it needs a repository alias:

(05:30:40) ERROR: error loading package under directory 'graalvm': error loading package '@rules_graalvm~0.9.0//graalvm': Unable to find package for @[unknown repo 'io_bazel_stardoc' requested from @rules_graalvm~0.9.0]//stardoc:stardoc.bzl: The repository '@[unknown repo 'io_bazel_stardoc' requested from @rules_graalvm~0.9.0]' could not be resolved: No repository visible as '@io_bazel_stardoc' from repository '@rules_graalvm~0.9.0'.

I've only applied the @rules_java dependency for users here, so it should not affect users downstream. Can I just push an update to this PR with a fix as a patch, or what do I do?

Thank you for approving.

@fmeum
Copy link
Contributor

fmeum commented Aug 14, 2023

Yes, you can make any changes to this PR you want, modules are only immutable after merge.

I looked into https://github.com/sgammon/rules_graalvm/blob/main/graalvm/BUILD.bazel and would recommend moving the stardoc targets to a dedicated docs package. Users may want to reference the bzl_library targets, but that would fail due to stardoc being a dev dependency only (as it should be).

sgammon added a commit to sgammon/rules_graalvm that referenced this pull request Aug 15, 2023
- fix: move doc targets, recommended in bazelbuild/bazel-central-registry#836
- chore: update module lock
- chore: public visibility for `bzl_library` targets (downstream docs)

(thank you @fmeum!)
sgammon added a commit to sgammon/rules_graalvm that referenced this pull request Aug 15, 2023
- fix: move doc targets, recommended in bazelbuild/bazel-central-registry#836
- chore: update module lock
- chore: public visibility for `bzl_library` targets (downstream docs)

(thank you @fmeum!)
@sgammon
Copy link
Contributor Author

sgammon commented Aug 15, 2023

@fmeum I've fixed the doc targets, but I'm guessing I should issue a new release and return to this PR with a force push on 0.9.1? While modules are immutable, I'd have to re-tag and re-issue the release to change the tarball that is downloaded, right?

I suppose I could avoid re-tagging and only upload a new stable release tarball, with fixed doc targets. That feels like cheating but I suppose it would work, right?

@fmeum
Copy link
Contributor

fmeum commented Aug 16, 2023

You could either close this PR and open a new one or force push over it - modules are only immutable after having been merged.

I suppose I could avoid re-tagging and only upload a new stable release tarball, with fixed doc targets. That feels like cheating but I suppose it would work, right?

While this would work, it's of course more transparent if there is a tag corresponding to that tar ball.

@sgammon sgammon mentioned this pull request Aug 23, 2023
@sgammon sgammon force-pushed the modules/rules_graalvm branch 8 times, most recently from 32b1e4b to a608e67 Compare September 8, 2023 05:48
@sgammon sgammon changed the title feat: add rules_graalvm module feat: add rules_graalvm module (WIP) Sep 8, 2023
@sgammon sgammon force-pushed the modules/rules_graalvm branch 5 times, most recently from 80261a8 to 63a640c Compare September 8, 2023 06:35
- supersedes bazel/bazel-central-registry#865
- supersedes bazel/bazel-central-registry#869
- supersedes bazel/bazel-central-registry#911
- supersedes bazel/bazel-central-registry#912

Signed-off-by: Sam Gammon <sam@elide.ventures>
@sgammon sgammon changed the title feat: add rules_graalvm module (WIP) feat: add rules_graalvm module Sep 8, 2023
@sgammon
Copy link
Contributor Author

sgammon commented Sep 8, 2023

@fmeum this should finally be ready to go 😄

@sgammon sgammon mentioned this pull request Sep 8, 2023
@sgammon
Copy link
Contributor Author

sgammon commented Sep 8, 2023

Ahhh, it looks like the other one was merged instead, auto-sent by the PR bot (#912). That's bad because the integrity value has been updated since that PR -- I was unable to prevent that PR since it was filed by the GitHub App (I had disabled my workflow thinking that was it, but it crept through anyway).

I will publish a fix release immediately, because I figure BCR modules can't be edited after merge @fmeum.

Very sorry about this mess of all these PRs. After this release, it looks like BCR publishing is working smoothly.

@sgammon
Copy link
Contributor Author

sgammon commented Sep 8, 2023

@fmeum (unless there is a way to rollback? i am open to your advice and will prepare the fix release but hold off until i hear)

@fmeum
Copy link
Contributor

fmeum commented Sep 8, 2023

@sgammon If the 0.10.0 release in the BCR is (and always has been) broken, I think that we could make an exception and edit it.

@meteorcloudy What do you think?.

@sgammon
Copy link
Contributor Author

sgammon commented Sep 8, 2023

@fmeum it was the overlap of these two PRs, unfortunately, because I couldn't close the other one. the hash listed on main now is incorrect, but it is correct in this PR.

i can also push a fix release as 0.10.1 and yank 0.10.0, that might be an alternative option? it won't let me build here with an update to a module which exists.

@fmeum
Copy link
Contributor

fmeum commented Sep 8, 2023

@fmeum it was the overlap of these two PRs, unfortunately, because I couldn't close the other one. the hash listed on main now is incorrect, but it is correct in this PR.

i can also push a fix release as 0.10.1 and yank 0.10.0, that might be an alternative option? it won't let me build here with an update to a module which exists.

That's certainly the easiest solution. You can also try submitting a PR that just deletes the module, but yanking is cleaner. Sorry for the inconvenience!

@sgammon
Copy link
Contributor Author

sgammon commented Sep 8, 2023

@fmeum All good, this release was feeling too quiet anyway 😆. I do have control over this PR, so I will push the fix via the PR bot since that is working smoothly now, tag you in it for review, and close this one. In that PR I will also yank this module. That should clean this up nicely and leave only one PR to merge 👍🏻

Thank you for your patience with me!

EDIT: Just kidding, I will file a different PR to yank because I won't have control over that one.

@sgammon sgammon mentioned this pull request Sep 8, 2023
@sgammon sgammon closed this Sep 8, 2023
@meteorcloudy
Copy link
Member

Yanking the broken one sounds fine to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants