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

Propagate the testonly= attribute in the pybind_extension macro. #81

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

apronchenkov
Copy link
Contributor

Motivation: Enable test-only pybind_extensions to depend on test-only targets.

Note: Tested manually, using git_override.

Motivation: Enable test-only pybind_extensions to depend on test-only targets.
@junyer
Copy link
Contributor

junyer commented Mar 28, 2024

Thanks for the PR! Just before I merge it, I want to fill the gap in my Starlark knowledge: How is kwargs actually propagating through the implementation? Is it happening implicitly or magically via the rule context somehow? Do you know, @rickeylev?

@rickeylev
Copy link
Collaborator

how is kwargs propagating

So recall there are two phases: first loading phase and then analysis phase.

Loading phase is when macros run. Macros are the more "regular" looking functions that take arbitrary args, including **kwargs. The propagation of common args (testsonly, compatible_with, etc) to other function calls must be done manually. These common args can only be propagating during loading phase. When loading phase ends, the graph of targets has been decided/finalized/fixed.

When analysis phase runs, that's when the functions that have the ctx arg are run. The ctx object contains all the target-specific state. It doesn't magically propagate anywhere; if you have a helper function, then you have to manually pass it to that helper function.

HTH

@rickeylev rickeylev merged commit c3cfc54 into pybind:master Mar 28, 2024
@junyer
Copy link
Contributor

junyer commented Mar 28, 2024

Sorry, what I meant was that testonly, visibility et al. aren't explicitly named anywhere, so... how do they end up where they need to be such that this PR does what it does? :)

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

Successfully merging this pull request may close these issues.

3 participants