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

Publish claro-lang@0.1.506 #1901

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

JasonSteving99
Copy link
Contributor

Push latest version. I've validated this locally using the --registry=https://raw.githubusercontent.com/JasonSteving99/bazel-central-registry/main and I'm able to build against this module version in a test project.

fmeum
fmeum previously approved these changes Apr 29, 2024
@bazel-io bazel-io dismissed fmeum’s stale review April 29, 2024 16:09

Require module maintainers' approval for newly pushed changes.

@JasonSteving99
Copy link
Contributor Author

@fmeum Presubmits were previously failing because it didn't like the Bazel version 6.4.x so I've changed it to 6.x.

@fmeum fmeum added the presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval label Apr 29, 2024
fmeum
fmeum previously approved these changes Apr 29, 2024
@fmeum fmeum enabled auto-merge (squash) April 29, 2024 19:36
auto-merge was automatically disabled April 29, 2024 21:33

Head branch was pushed to by a user without write access

@bazel-io bazel-io dismissed fmeum’s stale review April 29, 2024 21:34

Require module maintainers' approval for newly pushed changes.

@JasonSteving99
Copy link
Contributor Author

Presubmits didn't like the path to the test module being //examples/bzlmod/ so I've updated it to examples/bzlmod/

@JasonSteving99
Copy link
Contributor Author

@fmeum I'm super confused by these presubmit errors for some reason complaining about -jar external/rules_java~5.5.1~toolchains~remote_java_tools/java_tools/turbine_direct_binary_deploy.jar throwing this exception... java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 1442.

Is there something I don't know about needing an explicit dep on rules_java in Bazel now? I've been under the impression that Bazel's Java support was builtin. After some searching, I found bazelbuild/bazel#18743 that shows people fixing similar Turbine issues by upgrading their rules_java dep? I'm not running into this issue on mac locally and I'm not using rules_java to get java_library or java_binary rules anywhere (e.g.)...

Do you think I need to do a big overarching refactor to update to use rules_java now? Or is there something else I'm missing? Primarily I'm confused why this isn't happening to me locally, but this is an issue here in these presubmits.

@fmeum
Copy link
Contributor

fmeum commented Apr 30, 2024

You have to update rules_java past a version with a fix for this bug, but you don't need to load any symbols from it. Just adding a bazel_dep for it should be good enough and also ensure that the consumers of your module get a version with the fixes required to build it.

@JasonSteving99
Copy link
Contributor Author

Oh, very interesting, thanks for the info. So something like bazel_dep(name = "rules_java", version = "7.5.0") will implicitly update all my references to java_* without needing to add something like load("@rules_java//java:defs.bzl", "java_library") to BUILD files? That's surprising, but hey if that's the case, I'll be happy it's just a one-liner fix.

JasonSteving99 added a commit to JasonSteving99/claro-lang that referenced this pull request May 1, 2024
Unfortunately, the presubmits for the latest PR to Bzlmod failed with some
mysterious Turbine error that's apparently been addressed by the latest
rules_java. It turns out that although Java support is built into Bazel,
you can separately update the java_* rule implementations without updating
to a later Bazel version by placing a Module dep on rules_java. This is new
info, but good to know. I chose rules_java@7.2.0 b/c apparently that's the
latest version that actually supports Bazel 6.4.0 which Claro currently targets.

- bazelbuild/bazel-central-registry#1901
- https://buildkite.com/bazel/bcr-presubmit/builds/5034#018f2bc7-7567-42c9-95df-814fc11b561c
- bazelbuild/rules_java#159
@JasonSteving99
Copy link
Contributor Author

@fmeum More presubmit failures when things are building locally. I get the feeling this is because the presubmits don't appear to actually honor my project's .bazelrc file? In order to add the rules_java dep we discussed previously, I needed to add some flags to configure the Java version correctly (--java_language_version=11 and --tool_java_language_version=11), and this isn't reflecting in the actual presubmit build during the "Verify Build Targets" step.

Notice that during the "Run Test Module" step, the flags are configured correctly: https://buildkite.com/bazel/bcr-presubmit/builds/5072#018f31dd-ed62-4435-a17b-71522fff182c/420-431

But during the "Verify Build Targets" step, the flags are missing: https://buildkite.com/bazel/bcr-presubmit/builds/5072#018f31dd-ed5f-4c12-b27a-6fb62f2a16a1/260-269

After tracking down this bcr_presubmit.py script, it looks to me like this difference in build flags is because the test module is setup to read from my test module's .bazelrc and append to it whereas the "Verify Build Targets" step is setup to simply create a totally new .bazelrc ignoring my project's config

Is there anything to be done about this? It's very possible that this misconfiguration could be the cause of the latest failures.

@fmeum
Copy link
Contributor

fmeum commented May 1, 2024

You can use the build_flags and test_flags fields in presubmit.yaml to do what you would otherwise do in .bazelrc. Could you try that?

Since any flag needed here is something that your users would also have to specify on their command-line, it's arguably better to force authors to spell them out explicitly here instead of automatically sourcing the .bazelrc.

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

2 participants