Accept [package.metadata.raze], superseding [raze]
#274
Conversation
Cargo sets aside the [`[package.metadata]`][md] namespace for use by third-party tools like `cargo-raze`. By specifying configuration under `[package.metadata.raze]` rather than `[raze]`, we avoid an “unused manifest key: raze” warning on every Cargo command. This patch teaches `cargo-raze` to accept either `[raze]` or the new `[package.metadata.raze]`. In case of conflict, it prints a warning and uses the value of `[package.metadata.raze]`. In the future, we can remove the `[raze]` key entirely if desired. Fixes google#273. Changes outside of `impl/` generated by: ``` git ls-files -z ':!impl' | xargs -0 sed -i 's/\[raze/[package.metadata.raze/' ``` [md]: https://doc.rust-lang.org/cargo/reference/manifest.html#the-metadata-table Test Plan: With this patch, running `./smoke-test.sh` passes both before and after updating all the examples with the above `sed` command. Before the `sed` update, the smoke test prints the deprecation warning, as desired. wchargin-branch: toml-package-metadata-raze wchargin-source: 4932fedcc808b17506aadf4b69f6aa78850fe5c6
wchargin-branch: toml-package-metadata-raze wchargin-source: f2b7fb6fb07bd1f478aa1e8c14f7d623f86f69b3
| @@ -370,7 +381,35 @@ pub fn load_settings<T: AsRef<Path>>(cargo_toml_path: T) -> Result<RazeSettings, | |||
| return Err(RazeError::Generic(err.to_string())); | |||
| } | |||
| let mut settings = match toml::from_str::<CargoToml>(&toml_contents) { | |||
There was a problem hiding this comment.
Do you know if package.metadata.raze settings can be parsed from cargo metadata?
Specifically, could something like the CargoMetadataFetcher be used to load these settings more holistically?
There was a problem hiding this comment.
Indeed, they can. It looks like this:
$ cargo metadata --format-version 1 | jq '.. | .raze? | select(.)'
{
"genmode": "Remote",
"workspace_path": "//third_party/rust",
"crates": {
"crc": {
"1.8.1": {
"gen_buildrs": true
}
},
"indexmap": {
"1.6.0": {
"additional_flags": [
"--cfg=has_std"
]
}
},
"proc-macro2": {
"1.0.24": {
"additional_flags": [
"--cfg=use_proc_macro",
"--cfg=wrap_proc_macro"
]
}
},
"prost-build": {
"0.6.1": {
"gen_buildrs": true
}
},
"thiserror": {
"1.0.21": {
"gen_buildrs": true
}
}
}
}The path is .packages[].metadata.raze, so you’d have to pluck it from
the right package.
I haven’t looked at CargoMetadataFetcher. If you’d prefer that
approach, I can do so (but no promises on when I’ll get around to it).
Note that [raze] does not show up in cargo metadata, so as long as
you want to keep both around you would need two parsing paths. So
I might propose a plan of (a) parse both from Cargo.toml, (b) wait
until comfortable dropping compat, (c) remove CargoToml::raze,
(d) migrate to parsing from cargo metadata now that that’s easier.
There was a problem hiding this comment.
I was more trying to find out if this information could be parsed all from cargo metadata.
There was a problem hiding this comment.
Looks like the [package.metadata.raze] info can; the [raze] info can’t.
|
@acmcarther could you do a release for this change? |
|
Thanks for the reviews! |
It would be nice to land #127, then I'd be fine releasing 0.7.0 to include this. |
|
Released in 0.7.0 |
Summary:
Version 0.7.0 of `cargo-raze` was just released. Some visible changes:
- can now set `[package.metadata.raze]` instead of `[raze]` to avoid a
Cargo warning: <google/cargo-raze#274>
- build files have been renamed and restructured to be more compliant
with Buildifier: <google/cargo-raze#260>
If you have an old version installed, run `cargo install cargo-raze` to
update to the most recent version.
Changes to `third_party/rust/` generated with:
```
(rm -rf third_party/rust/ && cd tensorboard/data/server && cargo raze)
```
The changes to `Cargo.toml` and `third_party/rust/` are independent.
Test Plan:
Run `(cd tensorboard/data/server && cargo test)` and note that the tests
run without an “unused manifest key: raze” warning. Note further that
everything still builds with Bazel.
wchargin-branch: rust-regen-raze-0.7.0
wchargin-source: c73530a2f7948b4842617ff686f645e66e917d0d
Summary:
Version 0.7.0 of `cargo-raze` was just released. Some visible changes:
- can now set `[package.metadata.raze]` instead of `[raze]` to avoid a
Cargo warning: <google/cargo-raze#274>
- build files have been renamed and restructured to be more compliant
with Buildifier: <google/cargo-raze#260>
If you have an old version installed, run `cargo install cargo-raze` to
update to the most recent version.
Changes to `third_party/rust/` generated with:
```
(rm -rf third_party/rust/ && cd tensorboard/data/server && cargo raze)
```
The changes to `Cargo.toml` and `third_party/rust/` are independent.
Test Plan:
Run `(cd tensorboard/data/server && cargo test)` and note that the tests
run without an “unused manifest key: raze” warning. Note further that
everything still builds with Bazel.
wchargin-branch: rust-regen-raze-0.7.0
Cargo sets aside the
[package.metadata]namespace for use bythird-party tools like
cargo-raze. By specifying configuration under[package.metadata.raze]rather than[raze], we avoid an “unusedmanifest key: raze” warning on every Cargo command.
This patch teaches
cargo-razeto accept either[raze]or the new[package.metadata.raze]. In case of conflict, it prints a warning anduses the value of
[package.metadata.raze]. In the future, we canremove the
[raze]key entirely if desired.Fixes #273.
Changes outside of
impl/generated by:Test Plan:
With this patch, running
./smoke-test.shpasses both before and afterupdating all the examples with the above
sedcommand. Before thesedupdate, the smoke test prints the deprecation warning, as desired.
wchargin-branch: toml-package-metadata-raze