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

Implement rule extensions #19507

Open
8 of 9 tasks
comius opened this issue Sep 13, 2023 · 5 comments
Open
8 of 9 tasks

Implement rule extensions #19507

comius opened this issue Sep 13, 2023 · 5 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts type: feature request

Comments

@comius
Copy link
Contributor

comius commented Sep 13, 2023

Checklist:

  • extending from parent
  • calling parent's implementation function (in review)
  • merging public attributes
  • intializers in extended rules
  • limiting rule's extendability
  • stardoc support
  • cfg merging
  • subrules support
  • handle executable

Design document: https://docs.google.com/document/d/1p6z-shWf9sdqo_ep7dcjZCGvqN5r2jsPkJCqHHgfRp4/edit

@comius comius added P2 We'll consider working on this in future. (Assignee optional) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts labels Sep 13, 2023
@comius comius self-assigned this Sep 13, 2023
copybara-service bot pushed a commit that referenced this issue Oct 13, 2023
Add starlarkParent to RuleClass - that's the parent class we need to call.
Add ruleClassUnderEvaluation and superCalled to StarlarkRuleContext. This is the new state.
Add and implement "super" for StarlarkRuleContextApi.

Issue: #19507
PiperOrigin-RevId: 573228317
Change-Id: I35e449cfd31abe6a1801c7dbc4d2016ba532ccb1
@comius comius closed this as completed Nov 14, 2023
@comius comius reopened this Nov 14, 2023
@rbtcollins
Copy link

rbtcollins commented Dec 30, 2023

@comius where/how do you want bugs for this? We've kicked the tires on it a little and found some friction - our use case is decorating language rule foo_test to provide external dependencies - and had issues with both getting the parent executable cleanly, but also dealing with _allowlist_function_transition in the extended rule.

@comius
Copy link
Contributor Author

comius commented Jan 5, 2024

You can open separate bugs or explain more here. I'm aware there might be some problems with executables in executable rules. I'll need to figure this out.

About _allowlist_function_transition. The implicit attribute should be added automatically when a Starlark transition is used. Do you have more specific case where this is causing a problem?

@omar-droubi
Copy link

Hello @comius : Thanks for proposing/implementing this feature.
However, with current Bazel 7.0.2, the rule extension doesn't work for cc_library, cc_binary and all other CC rules. Bazel aborts with "Private API Usage" error.

Is this by design ? or is it caused because CC rules are still part of the builtins of Bazel and haven't moved to rules_cc ?

I can see that CC Rules are now in Starlark with some reliance on native rules.

@comius
Copy link
Contributor Author

comius commented Feb 16, 2024

No, it’s unintentional. C++ rules should be extendable in current form already. I’ll have a look.

@timothyg-stripe
Copy link
Contributor

@comius I was looking into creating a rule that extends java_library, which I thought would be a supported use case given the number of examples in the design doc that use java_library. Unfortunately, I found that this is not possible (at least on Bazel 7.0.1 with --experimental_rule_extension_api=true):

Error in rule: The rule 'java_library' is not extendable. Only Starlark rules not using deprecated features (like implicit outputs, output to genfiles) may be extended. Special rules like analysis tests or rules using build_settings cannot be extended.

It looks like java_library isn't extendable because it has two implicit outputs. Do you have any plans to support extending rules with implicit outputs, or else remove them entirely from java_library? We'd rather not resort to --experimental_builtins_bzl_path, as the ability to piggy-back off of native rules w/o maintaining our fork of @_builtins is a primary reason why we'd like to use rule extensions in the first place.

copybara-service bot pushed a commit that referenced this issue Jan 2, 2025
`cc_binary` has an initializer that may set a private attribute, which is only permitted for built-in rules. The allowlist check always used the outermost rule class, thus failing if `cc_binary` is extended by a non-built-in rule. This is fixed by checking the rule class that actually declares the initializer.

Work towards #19507 (comment)

Closes #24778.

PiperOrigin-RevId: 711361632
Change-Id: I3c0b6e1e6c50fd1af9f1dc635052d0054114ee2d
bazel-io pushed a commit to bazel-io/bazel that referenced this issue Jan 2, 2025
`cc_binary` has an initializer that may set a private attribute, which is only permitted for built-in rules. The allowlist check always used the outermost rule class, thus failing if `cc_binary` is extended by a non-built-in rule. This is fixed by checking the rule class that actually declares the initializer.

Work towards bazelbuild#19507 (comment)

Closes bazelbuild#24778.

PiperOrigin-RevId: 711361632
Change-Id: I3c0b6e1e6c50fd1af9f1dc635052d0054114ee2d
bazel-io pushed a commit to bazel-io/bazel that referenced this issue Jan 2, 2025
`cc_binary` has an initializer that may set a private attribute, which is only permitted for built-in rules. The allowlist check always used the outermost rule class, thus failing if `cc_binary` is extended by a non-built-in rule. This is fixed by checking the rule class that actually declares the initializer.

Work towards bazelbuild#19507 (comment)

Closes bazelbuild#24778.

PiperOrigin-RevId: 711361632
Change-Id: I3c0b6e1e6c50fd1af9f1dc635052d0054114ee2d
github-merge-queue bot pushed a commit that referenced this issue Jan 8, 2025
`cc_binary` has an initializer that may set a private attribute, which
is only permitted for built-in rules. The allowlist check always used
the outermost rule class, thus failing if `cc_binary` is extended by a
non-built-in rule. This is fixed by checking the rule class that
actually declares the initializer.

Work towards
#19507 (comment)

Closes #24778.

PiperOrigin-RevId: 711361632
Change-Id: I3c0b6e1e6c50fd1af9f1dc635052d0054114ee2d

Commit
7093088

Co-authored-by: Fabian Meumertzheim <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-API API for writing rules/aspects: providers, runfiles, actions, artifacts type: feature request
Projects
None yet
Development

No branches or pull requests

4 participants