Skip to content

python_toolchain: set and use preprocessor flags for extensions #1080

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

python_toolchain: set and use preprocessor flags for extensions #1080

wants to merge 1 commit into from

Conversation

@zsol
Copy link
Contributor

@zsol zsol commented Aug 15, 2025

This PR:

  • adds a new extension_preprocessor_flags field to PythonToolchainInfo
  • starts using it from the cxx_python_extension rule
  • wires the new field up from remote_python_toolchain
  • moves the python_cextension targets from under with_prelude/cpp to with_prelude/python
  • enables them so CI starts covering these
  • removes the main() function from the extension cpp file

At the end of this all, remote_python_toolchain (and so the demo toolchains) is now capable of building Python C extensions without having Python installed on the host at all.

Test plan:

buck2\examples\with_prelude  "python_toolchain: set a…" [8 +22-58]
❯ ..\..\target\release\buck2 run python/use_cextension:cext
Build ID: fe1005ad-8885-47c3-9d70-830a65addf98
Network: Up: 0B  Down: 0B
Command: run.
Time elapsed: 0.0s
BUILD SUCCEEDED - starting your binary
hello from cpython extension from cpp toolchain

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 15, 2025
@facebook-github-bot
Copy link
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this in D80338151. (Because this pull request was imported automatically, there will not be any future comments.)

@zsol zsol force-pushed the push-ttokskysrrzq branch 2 times, most recently from ea553fc to 3dd63e7 Compare August 15, 2025 14:55
@meta-cla
Copy link

meta-cla bot commented Aug 15, 2025

Hi @zsol!

Thank you for your pull request.

We require contributors to sign our Contributor License Agreement, and yours needs attention.

You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@zsol zsol force-pushed the push-ttokskysrrzq branch from 3dd63e7 to 825395a Compare August 16, 2025 10:25
@zsol zsol force-pushed the push-ttokskysrrzq branch from 825395a to 2841b47 Compare August 16, 2025 14:26
@zsol
Copy link
Contributor Author

zsol commented Aug 27, 2025

@itamaro wdyt?

itamaro
itamaro reviewed Sep 7, 2025
View reviewed changes
Copy link
Contributor

@itamaro itamaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

one naming nit -- I guess extension_preprocessor_flags is technically accurate, but perhaps it would be clearer to users to call this extension_compiler_flags? or even cxx_extension_compiler_flags, since this wouldn't make sense for, e.g., a rust python extension?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@itamaro itamaro itamaro left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zsol @facebook-github-bot @itamaro