-
Notifications
You must be signed in to change notification settings - Fork 483
Support crate features #37
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
Comments
+@dslomov @gregestren @laurentlb It is unclear to me which Bazel/Skylark abstractions this would map best to. The way features propagate across the dependency graph in Cargo makes it seem that aspects may be the way to go, but maybe some of Bazel's configuration features may help as well. Thoughts? |
I'm not sure we should support any special behavior for them at this time. For the purposes of a cargo integration, we can let cargo resolve them statically as part of a BUILD file generator. It turns out that feature flags have a lot of nuance, not all of which is intentional or good. One part of that is how flags are additive globally within a cargo project-- something it seems not a lot of folks are even aware of. One important caveat is dependencies based on the target itself. I'd suggest we make the target triple explicit as part of the BUILD file generation step (and default to whatever cargo infers), as it doesn't require any special support on our end at this time. EDIT: clarified the last part. |
I've been toying with this for a while, and I'm thinking the best immediate approach might be to have a series of "blessed configurations". We could then use the To motivate this, Cargo's current feature/dependency enabling system allows you to combine an arbitrary combination of system properties to enable a feature or dependency. The system properties in play look like (from my platform):
I'd suggest we map down to a series of "artificial configurations". One representative of "linux", one representative of "windows", and one representative of "darwin", and use those with cargo's feature resolution machinery to bucket up what's available to Cargo. We could add additional (or maybe facilitate user defined configurations) at some later point. All of that said, this is heavily dependent on how we decide to integrate with cargo in #2 |
This makes `sync` a Cargo feature flag and removes it from the C++ FFI. For now, this has the result of breaking Pedro, but fortunately the Pedro sync support is pre-alpha and that project should just be able to turn the feature back on. (See below.) **Bazel support:** rules_rust supports a [crate_features](https://bazelbuild.github.io/rules_rust/rust.html#rust_static_library) attribute on rust targets, and I've confirmed it works with this. It doesn't seem to be able to affect the dependency graph, though ([follow this issue](bazelbuild/rules_rust#37)) and it's probably not configurable without editing the BUILD file. More research is needed. **The C++ FFI** probably can't be monolithic like it is today - we'll have to think of a way to split it up, or possibly push more of the FFI into the agent projects themselves.
Cargo supports Features, which, according to the documentation is used for the following:
(1) is currently supported via the
crate_features
attribute, which sets flags to enable different conditional compilation features viacfg
. More discussion and design work is needed in order to support (2) and (3).The text was updated successfully, but these errors were encountered: