Skip to content

Error in subrule: file '@@rules_java~//java/common/rules:android_lint.bzl' cannot use private API #233

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

Closed
gottagofaster236 opened this issue Oct 16, 2024 · 11 comments

Comments

@gottagofaster236
Copy link

gottagofaster236 commented Oct 16, 2024

It is currently impossible to use rules_java >= 8.0.0 with Android.
Was broken by 2c8f7f4
Works in rules_java 7.12.1, does not work in 8.0.0

Reproduction steps:

  1. Clone https://github.com/gottagofaster236/rules_java_reproducer
  2. Run bazel build //...

Output:

ERROR: Traceback (most recent call last):
	File "/private/var/tmp/_bazel_Lev.Leontev/7114f3e2d9f9ded26b3ceaa12ef97052/external/rules_java~/java/common/rules/android_lint.bzl", line 142, column 31, in <toplevel>
		android_lint_subrule = subrule(
Error in subrule: file '@@rules_java~//java/common/rules:android_lint.bzl' cannot use private API
@hvadehra
Copy link
Member

You'll need to use the flag --experimental_rule_extension_api for now. The flag has been flipped for Bazel 8, which will be out next month.

@alexeagle
Copy link

Thanks @hvadehra - however I think it's not a great practice for a Semver-stable ruleset to require experimental features for the LTS version of Bazel. It's not fair to describe an experiment as something required for production use.

Could you instead use @bazel_features to feature-detect on what Bazel supports, and remain compatible?

alexeagle added a commit to aspect-build/aspect-workflows-template that referenced this issue Oct 25, 2024
alexeagle added a commit to aspect-build/aspect-workflows-template that referenced this issue Oct 25, 2024
* chore(deps): update dependency rules_java to v8

* Add required flag

See bazelbuild/rules_java#233

* Update .bazelrc

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Alex Eagle <[email protected]>
@hvadehra
Copy link
Member

I'm not sure how semver applies here. This incompatibility was introduced while moving from v7 to v8. rules_java 8.x is intended for use with Bazel 8 onwards. While it is possible to use with older Bazel versions, it may need some flag flips / patching. Unfortunately, we can't bump the Bazel compatibility version beforehand because it's a chicken-and-egg situation with updating this in Bazel. This problem could go away once Bazel itself no longer bundles rules_java.

That said, this is coming up often enough, that I suppose this warrants a note in the README / release notes.

Could you instead use @bazel_features to feature-detect on what Bazel supports, and remain compatible

Since Bazel performs eager evaluation of all transitively loaded bzl files, the @bazel_features approach only helps for non-toplevel evaluation i.e for code executing during analysis, not loading.

TLDR: For Bazel version N, use rules_java N

@fmeum
Copy link
Contributor

fmeum commented Oct 28, 2024

Since Bazel performs eager evaluation of all transitively loaded bzl files, the @bazel_features approach only helps for non-toplevel evaluation i.e for code executing during analysis, not loading.

That's not strictly true: bazel_features uses a repo rule to generate Starlark code, so if there is a need for this, it would be easy to add conditional loads to it. It can already give you access to conditionally defined globals even in top-level evaluations.

@hvadehra
Copy link
Member

So let us say the requirement is:

# macro.bzl
load(":new_rule.bzl", "my_new_rule")
load(":old_rule.bzl", "my_old_rule")

def macro():
  if bazel_version <= N:
     my_old_rule()
  else:
     my_new_rule()

where loading new_rule.bzl requires Bazel > N (for example, because it declares subrules). How does one achieve this with @bazel_features?

@fmeum
Copy link
Contributor

fmeum commented Oct 28, 2024

You could add a repo rule to rules_java that writes one of the two load statements depending on native.bazel_version into a .bzl file that you then load from at call sites. That's the bazel_features approach, but it doesn't need to go in there if it's specific to rules_java.

But if this is just about subrule, then it would be far easier to set subrule = bazel_features.rules.has_subrule (feature name TBD). As far as I understand, you only need to flip that attribute at the top level, the actual implementation based on subrules should still be loaded fine with older versions of Bazel even if it wouldn't execute without errors.

If you want, I can send a PR, I would just need references to the two versions of the rules.

@hvadehra
Copy link
Member

It's not just the subrule() function, the consuming rule also requires the subrules = ... attribute. So if I'm undestanding correctly, this will require a repo rule in rules_java, and @bazel_features won't help here?

@fmeum
Copy link
Contributor

fmeum commented Oct 28, 2024

Both the call and the subrule attribute could be gated behind a bazel_features feature. To put this in other words, if you can gate the calls and parameters that would cause failures during top-level evaluation behind if <constant> or they are usages of conditionally defined top-level symbols, you don't need conditional loads and bazel_features can help without additional setup.

If you really do need conditional loads (I think you don't), adding your own repo rule would be the easiest solution.

@hvadehra
Copy link
Member

Understood. The repo rule approach is probably better. The alternative will just add clutter to the definitions which would anyways be unusable with older Bazel versions.

@hvadehra
Copy link
Member

Update, rules_java 8.3.0 is available that does not suffer from this problem.

@alexeagle
Copy link

Awesome, thanks so much!

alexeagle added a commit to aspect-build/aspect-workflows-template that referenced this issue Oct 30, 2024
This flag no longer needed, see bazelbuild/rules_java#233 (comment)
alexeagle added a commit to aspect-build/aspect-workflows-template that referenced this issue Nov 3, 2024
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

4 participants