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

Generate config headers from configs.specification #9587

Closed

Conversation

mszabo-wikia
Copy link
Contributor

Since a41bcb1, configuration headers and sources are generated using a Rust tool rather than being hardcoded. Implement the CMake logic necessary to compile the tool and generate these sources. Use a Python script to extract config section names from the specification file instead of hardcoding them into CMake build files to reduce the chance of the CMake build getting out of a sync.

A followup PR will handle connecting the hackc_options target to the Rust libraries that depend on it.

@facebook-github-bot
Copy link
Contributor

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

Copy link
Contributor

@Wilfred Wilfred left a comment

Choose a reason for hiding this comment

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

Generally looks good to me. Would you mind running black to format the Python, so I can merge this?

Since a41bcb1, configuration headers
and sources are generated using a Rust tool rather than being hardcoded.
Implement the CMake logic necessary to compile the tool and generate these sources.
Use a Python script to extract config section names from the
specification file instead of hardcoding them into CMake build files to
reduce the chance of the CMake build getting out of a sync.

A followup PR will handle connecting the hackc_options target to the
Rust libraries that depend on it.
@mszabo-wikia mszabo-wikia force-pushed the generate-config-headers branch from 426a193 to d3aa679 Compare January 28, 2025 19:05
@facebook-github-bot
Copy link
Contributor

@mszabo-wikia has updated the pull request. You must reimport the pull request before landing.

@mszabo-wikia
Copy link
Contributor Author

Done with black 24.10.0. Let me know if I should use a different version!

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in ca8fbd4.

@mszabo-wikia
Copy link
Contributor Author

@Wilfred It looks like the Cargo.toml manifest got lost after the merge—AFAIK this might be because they're generated automatically from the internal buck2 buildfiles. Would it be possible to do that to allow OSS to compile the config generation tool?

Let me know if this requires any restructuring on the CMake side.

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

Successfully merging this pull request may close these issues.

3 participants