-
Notifications
You must be signed in to change notification settings - Fork 522
Read default annotation values from package's Cargo.toml metadata #2124
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
Conversation
crate_universe/src/config.rs
Outdated
| /// additive_build_file_contents = """ | ||
| /// ... | ||
| /// """ |
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.
I considered a more TOML-native form for additive_build_file_content, but just using multiline string literal containing Starlark seems all right.
Support for the structured form can be introduced separately if desired.
[package.metadata.bazel]
extra_aliased_targets = { cxx_cc = "cxx_cc" }
gen_build_script = false
[[package.metadata.bazel.additive_build_file_content]]
[package.metadata.bazel.additive_build_file_content.cc_library]
name = "cxx_cc"
srcs = ["src/cxx.cc"]
hdrs = ["include/cxx.h"]
include_prefix = "rust"
includes = ["include"]
linkstatic = true
strip_include_prefix = "include"
visibility = ["//visibility:public"]27413f3 to
4ae5e42
Compare
4ae5e42 to
cf1b751
Compare
UebelAndre
left a comment
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.
This is a pretty awesome change! Could you also add a unit test for this? I'm thinking a Cargo.toml file somewhere in crate_universe/test_data and have a test that reads the [package.metadata.bazel] section?
|
I made a couple attempts at a test, but I haven't been able to figure out how to get crate_universe to pay attention to local Here is what I have so far: affc1b5. |
| .sum(); | ||
|
|
||
| if !extras.is_empty() { | ||
| crate_extra.apply_defaults_from_package_metadata(&pkg.metadata); |
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.
I was thinking that the testing would just be in this file and cover the changes around here. Not necessarily a brand new test target. I think that'd get sufficient coverage of the functionality, right?
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.
Ah, I get it now. Good call. I have added that test.
UebelAndre
left a comment
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.
Fantastic! Thank you so much!
…zelbuild#2124) This PR allows Cargo package authors to specify metadata that replaces the need for crate_universe users to write `annotations`. Crate_universe will pick up default values for various `annotations` fields from the metadata provided by the package author. It still remains possible for the crate_universe user to write their own `annotations` for the crate anyway, in which case these override the default values distributed by the package author. For example the `cxx` crate contains some C++ source code that must get linked by the user. To accomplish this, all users of the crate from Bazel would _previously_ have needed to fill in the following annotation: ```bzl crates_repository( name = "...", annotations = { "cxx": [crate.annotation( additive_build_file_content = """ cc_library( name = "cxx_cc", srcs = ["src/cxx.cc"], hdrs = ["include/cxx.h"], include_prefix = "rust", includes = ["include"], linkstatic = True, strip_include_prefix = "include", visibility = ["//visibility:public"], ) """, extra_aliased_targets = {"cxx_cc": "cxx_cc"}, gen_build_script = False, )], }, ... ) ``` Now `cxx` ships this annotation as part of [the package metadata in its Cargo.toml](https://github.com/dtolnay/cxx/blob/1.0.105/Cargo.toml#L50-L64), and Bazel users can pull in `cxx` without handwriting their own `crate.annotation`. ```toml [package.metadata.bazel] additive_build_file_content = """ cc_library( name = "cxx_cc", srcs = ["src/cxx.cc"], hdrs = ["include/cxx.h"], include_prefix = "rust", includes = ["include"], linkstatic = True, strip_include_prefix = "include", visibility = ["//visibility:public"], ) """ extra_aliased_targets = { cxx_cc = "cxx_cc" } gen_build_script = false ```
This PR allows Cargo package authors to specify metadata that replaces the need for crate_universe users to write
annotations.Crate_universe will pick up default values for various
annotationsfields from the metadata provided by the package author. It still remains possible for the crate_universe user to write their ownannotationsfor the crate anyway, in which case these override the default values distributed by the package author.For example the
cxxcrate contains some C++ source code that must get linked by the user. To accomplish this, all users of the crate from Bazel would previously have needed to fill in the following annotation:Now
cxxships this annotation as part of the package metadata in its Cargo.toml, and Bazel users can pull incxxwithout handwriting their owncrate.annotation.