-
-
Notifications
You must be signed in to change notification settings - Fork 103
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
Add support for feature selection #108
Conversation
This patch builds on the parent commit, but changes the API to use a target property. Since this approach requires CMake >= 3.19, the rust2cpp test that received features was copied into the new features test and reverted from the old test, so that it can still be run with older cmake versions.
* Make sure to use a space separator for the cargo command line * Join the target property list to the expected comma delimiter for the generator
And $<IF> without the else part can be rewritten as plain $<>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
I think it might still make sense to be able to do
corrosion_import_crate(MANIFEST_PATH rust/Cargo.toml FEATURES myfeature)
in the common case, don't you think?
README.md
Outdated
@@ -152,6 +152,10 @@ The pairs can also be generator expressions, as they will be evaluated using $<G | |||
For example this may be useful if the build scripts of crates look for environment variables. | |||
This feature requires CMake >= 3.19.0. | |||
|
|||
Cargo crates sometimes offer features, which traditionally can be specified with `cargo build` on the | |||
command line as opt-in. You can select the features to use for a crate imported with Corrosion by | |||
setting the `CORROSION_FEATURES` target list property on the targets created by `corrosion_import_crate`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the README would benefit from an example
Hm, that's true. So |
Reference the second function that's guarded by the second feature, otherwise we don't know if both were passed.
yeah |
This restores what 79e498d added initially, the most convenience API for the common case of importing just one crate.
The target property is empty, if it does not exist. If it is not empty, then map it to the correct value (--foo) or the empty string.
This merge requests is based on the commit in #99 but changes the code to use a target property.