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

Allow cc_binary with dynamic_deps to be extended #24778

Closed
wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Dec 20, 2024

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)

`cc_binary` has an initializer that sets 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.
@fmeum fmeum requested a review from a team as a code owner December 20, 2024 11:10
@fmeum fmeum requested review from katre and comius and removed request for a team and katre December 20, 2024 11:10
@github-actions github-actions bot added team-Configurability platforms, toolchains, cquery, select(), config transitions awaiting-review PR is awaiting review from an assigned reviewer labels Dec 20, 2024
@fmeum fmeum changed the title Allow cc_binary to be extended Allow cc_binary with dynamic_deps to be extended Dec 20, 2024
@comius comius added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Dec 20, 2024
@fmeum
Copy link
Collaborator Author

fmeum commented Dec 20, 2024

@bazel-io fork 8.0.1

@fmeum
Copy link
Collaborator Author

fmeum commented Dec 20, 2024

@bazel-io fork 7.5.0

@copybara-service copybara-service bot closed this in 7093088 Jan 2, 2025
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jan 2, 2025
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request 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 pull request 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
@fmeum fmeum deleted the 19507-fix-cc-binary-parent branch January 6, 2025 10:31
github-merge-queue bot pushed a commit that referenced this pull request 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
team-Configurability platforms, toolchains, cquery, select(), config transitions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants