Skip to content

Remove generated code from shim-protos (and fix CI) #98

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Sep 29, 2022

Conversation

mxpv
Copy link
Member

@mxpv mxpv commented Sep 27, 2022

While working on #97 I found an interesting issue. Since we currently commit all generated code, we should pin protobuf version to be exactly 3.1.0. Otherwise, whenever Protobuf crate gets updated, shim-protos will be broken with something like:

--> crates/shim-protos/src/cgroups/gogo.rs:26:49
[413](https://github.com/containerd/rust-extensions/actions/runs/3129846807/jobs/5079425017#step:4:414)
   |
[414](https://github.com/containerd/rust-extensions/actions/runs/3129846807/jobs/5079425017#step:4:415)
26 | const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_3_1_0;
[415](https://github.com/containerd/rust-extensions/actions/runs/3129846807/jobs/5079425017#step:4:416)
   |                                                 ^^^^^^^^^^^^^ help: a constant with a similar name exists: `VERSION_3_2_0`

This is because, according to semver, when we define protobuf = "3.1.0" in Cargo.toml, cargo may use any newer version up to < 2.0.0.

Another approach, more preferable in Rust, is to exclude any generated code from repo, and use include! macro instead. See bindgen example: https://rust-lang.github.io/rust-bindgen/tutorial-4.html

The only thing to keep in mind about include! macro is that it does not support any form of #![allow(XYZ)] attributes, so we have to remote those to build time (and define at crate level) - rust-lang/rfcs#752

Signed-off-by: Maksym Pavlenko pavlenko.maksym@gmail.com

Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
@mxpv mxpv changed the title Remove generated code from shim-protos Remove generated code from shim-protos (and fix CI) Sep 27, 2022
@mxpv mxpv requested review from Burning1020 and fuweid September 27, 2022 17:07
Copy link
Member

@Burning1020 Burning1020 left a comment

Choose a reason for hiding this comment

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

LGTM

@mxpv mxpv merged commit 0355f05 into containerd:main Sep 29, 2022
@mxpv mxpv deleted the codegen branch September 29, 2022 16:54
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants