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

[8.0.0] Symbolic macro attribute inheritance #24280

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

bazel-io
Copy link
Member

Allows the following syntax:

my_cc_library = macro(
    # inherit all attributes from a given rule or macro symbol (or "common" for
    # the set of common Starlark rule attributes)
    inherit_attrs = native.cc_library,
    attrs = {
        # remove cxxopts from inherited attrs list
        "cxxopts": None,
        # override the copts attribute inherited from native.cc_library
        "copts": attr.string_list(default = ["-D_FOO"]),
    },
    ...
)

Fixes #24066

Tricky parts:

  • Some common rule attributes ("testonly", "size", etc.) have computed defaults;
    native rule "license" and "distribs" attrs have defaults which fail type check if
    lifted. The only sane way of handling such attrs, if the attribute is unset in
    the macro function call, to pass the value as None to the implementation function
    so that the implementation function will pass None to the rule function, which
    will thus set the default appropriately.
  • We need RuleFunctionApi and MacroFunctionApi interfaces (yet more interfaces,
    alas, or we get a circular dependency) to express the type of inherit_attrs param
  • Iterating over all native rules (for testing inheritance from them) is tricky: we
    need to look at builtins (since the ConfiguredRuleClassProvider may have stubs for
    java rules overridden by builtins). This is already correctly handled by
    bazel info build-language, so let's move the logic into a utility class.

RELNOTES: Add inherit_attrs param to macro() to allow symbolic macros to
inherit attributes from rules or other symbolic macros.
PiperOrigin-RevId: 694154352
Change-Id: I849a7f16b4da8eb2829cdbc6a131d85a28bc4740

Commit eec5305

Allows the following syntax:

```
my_cc_library = macro(
    # inherit all attributes from a given rule or macro symbol (or "common" for
    # the set of common Starlark rule attributes)
    inherit_attrs = native.cc_library,
    attrs = {
        # remove cxxopts from inherited attrs list
        "cxxopts": None,
        # override the copts attribute inherited from native.cc_library
        "copts": attr.string_list(default = ["-D_FOO"]),
    },
    ...
)
```

Fixes bazelbuild#24066

Tricky parts:
* Some common rule attributes ("testonly", "size", etc.) have computed defaults;
  native rule "license" and "distribs" attrs have defaults which fail type check if
  lifted. The only sane way of handling such attrs, if the attribute is unset in
  the macro function call, to pass the value as None to the implementation function
  so that the implementation function will pass None to the rule function, which
  will thus set the default appropriately.
* We need RuleFunctionApi and MacroFunctionApi interfaces (yet more interfaces,
  alas, or we get a circular dependency) to express the type of inherit_attrs param
* Iterating over all native rules (for testing inheritance from them) is tricky: we
  need to look at builtins (since the ConfiguredRuleClassProvider may have stubs for
  java rules overridden by builtins). This is already correctly handled by
  `bazel info build-language`, so let's move the logic into a utility class.

RELNOTES: Add inherit_attrs param to macro() to allow symbolic macros to
inherit attributes from rules or other symbolic macros.
PiperOrigin-RevId: 694154352
Change-Id: I849a7f16b4da8eb2829cdbc6a131d85a28bc4740
@bazel-io bazel-io added the awaiting-review PR is awaiting review from an assigned reviewer label Nov 11, 2024
@bazel-io bazel-io requested a review from a team as a code owner November 11, 2024 21:17
@bazel-io bazel-io added the team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob label Nov 11, 2024
@tetromino
Copy link
Contributor

@Wyverald - could you please add this to the merge queue? @comius approved it for cherry-picking into 8.0

@iancha1992 iancha1992 added this pull request to the merge queue Nov 11, 2024
Merged via the queue into bazelbuild:release-8.0.0 with commit f91b9b9 Nov 11, 2024
46 checks passed
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Nov 11, 2024
@Wyverald Wyverald deleted the cp24066-8.0.0 branch November 13, 2024 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants