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

[7.0.0] Experimental extend rule and subrule functionality #20429

Merged
merged 10 commits into from
Dec 4, 2023

Conversation

comius
Copy link
Contributor

@comius comius commented Dec 4, 2023

Following commits were cleanly cherry-picked:

e4c7cb6 [email protected] Support incoming configuration transition in extended rules
faa2f3d [email protected] Support subrules in extended rules
d70d198 [email protected] Add an escape hatch for any type attributes in the initializer
c00c272 [email protected] Cleanup incoming configuration transitions on Starlark rules
82a469f [email protected] Copy and lift labels in arguments of initializers
6733c18 [email protected] Remove unused OBJECT_LIST type
e7a3dab [email protected] Remove unused FILESET_ENTRY label class
c959990 [email protected] Implement type checks and label conversion for Starlark values
c4f4073 [email protected] Move convertFromBuildLangType into BuildType class

Additional commit "Define function_transition_allowlist in StarlarkRuleClassFunctionsTest" just fixes the mocking of the the tests.

This is preparation to check types in initializers and potentially also in symbolic macros.

The change is no-op.

PiperOrigin-RevId: 582181640
Change-Id: I4b0f9b017650c614c1cba4958cd1aac80684fd57
Check the types, copy all values into immutable Starlark types and lift strings into Labels.

PiperOrigin-RevId: 582244528
Change-Id: I309bedcfe9a5fec50d997c900f209c63e7f11f38
PiperOrigin-RevId: 582265138
Change-Id: Ief031387ae0a13c0be5c3dfb01f52198629dc9d1
IMO it's better removed before somebody gets an idea to use it.

PiperOrigin-RevId: 582309747
Change-Id: I45a9ae26cd36d09c26819eaf97e9690e84b15166
The arguments are copied into immutable Starlark values. They are type checked.  Strings are converted to labels (in the right context).

I was worried copying would cause memory regression, but it doesn't. (There's initializer on cc_binary).

There's a potential optimisation to use Starlark types everywhere. StarlarkList, Dict are already as good as ImmutableList and ImmutableMap (I think). We'd need to converge Selectors to a single type. Then the convert operation becomes the same as copyAndLiftStarlarkValue. And it could be optimised to a no-op when called twice.

PiperOrigin-RevId: 582324735
Change-Id: I64ee4f70fa12f9818d57474ba33d085aeed347ad
Remove PatchTransition and TransitionFactory. Those don't seem to be possible inputs in Starlark, because they don't implement StarlarkValue, with exception of ExecTransitionFactory. Using exec as an incoming transition seems like a really weird use case, and it's probably better not to support it, than making Bazel code more complex.

There seem to be no uses of exec incoming transition on github: https://github.com/search?q=%22cfg+%3D+config%22+language%3AStarlark&type=repositories&ref=advsearch

Remove RuleClass.cfg(PatchTransition). It was only used in tests. Inline the uses.
Add Nullable to RuleClass.transitionFactory.

This is preparation to support cfg on extended rules.

RELNOTES[INC]: Incoming transitions on rules can't be set to "exec" transition.

PiperOrigin-RevId: 582969396
Change-Id: I1e6391d17671fdfa60e1945561396107172c6a4a
Example are of such attributes are tristate. They are not supported in Starlark and while Starlarkfiying the rules, we didn't do a cleanup. Instead we used wrapper macros.

PiperOrigin-RevId: 583939506
Change-Id: I1b87e2bef4aa46d5660cfa43fbb40a9ac053be30
Everything that current subrule implementation adds to the rule class (attributes, toolchain, configuration fragments), gets into the child's ruleclass by the way of inheritance.

Checks against currently executing rule class if it has the right subrule.

PiperOrigin-RevId: 584028970
Change-Id: I1eace964cf23a4144eece12aa3dfbd1be6aadd4b
PiperOrigin-RevId: 587721322
Change-Id: I70736474f8cd59e9e21a255fce6420dfc725b139
@comius comius requested a review from hvadehra December 4, 2023 17:27
@comius comius requested a review from a team as a code owner December 4, 2023 17:27
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Configurability platforms, toolchains, cquery, select(), config transitions labels Dec 4, 2023
@keertk keertk added this to the 7.0.0 release blockers milestone Dec 4, 2023
@keertk keertk added the soft-release-blocker Soft release blockers that are nice to have, but shouldn't block the release if it's the last one. label Dec 4, 2023
@comius comius requested a review from Wyverald December 4, 2023 18:02
@keertk keertk enabled auto-merge (squash) December 4, 2023 18:39
@keertk keertk merged commit e35c29f into bazelbuild:release-7.0.0 Dec 4, 2023
25 checks passed
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
soft-release-blocker Soft release blockers that are nice to have, but shouldn't block the release if it's the last one. team-Configurability platforms, toolchains, cquery, select(), config transitions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants