Skip to content

Protobuf 3.7.2 bump and updated build.rs to fix GHSA-2gh3-rmm4-6rq5, linter allows added. - #264

Merged
YangKeao merged 10 commits into
tikv:masterfrom
kbsteere:bump-protobuf-version-3.x
May 6, 2025
Merged

Protobuf 3.7.2 bump and updated build.rs to fix GHSA-2gh3-rmm4-6rq5, linter allows added. #264
YangKeao merged 10 commits into
tikv:masterfrom
kbsteere:bump-protobuf-version-3.x

Conversation

@kbsteere

Copy link
Copy Markdown
Contributor
  • updated protobuf version to be a minimum of 3.7.2 to address GHSA-2gh3-rmm4-6rq5.
  • Added linter allows so build and tests pass

@ti-chi-bot

ti-chi-bot Bot commented Mar 31, 2025

Copy link
Copy Markdown

Welcome @kbsteere! It looks like this is your first PR to tikv/pprof-rs 🎉

…3-rmm4-6rq5. Added linter allows so build and tests pass

Signed-off-by: Kyle Steere <kyle.steere@chainguard.dev>
Signed-off-by: Kyle Steere <kyle.steere@chainguard.dev>
…pure

Signed-off-by: Kyle Steere <kyle.steere@chainguard.dev>
@kbsteere kbsteere changed the title Protobuf 3.7.2 bump to fix GHSA-2gh3-rmm4-6rq5, linter allows added. Protobuf 3.7.2 bump and updated build.rs to fix GHSA-2gh3-rmm4-6rq5, linter allows added. Mar 31, 2025
@kbsteere

kbsteere commented Apr 1, 2025

Copy link
Copy Markdown
Contributor Author

@YangKeao @Xuanwo can you review these changes?

@YangKeao
YangKeao self-requested a review April 8, 2025 07:47
Comment thread build.rs Outdated
Comment thread examples/backtrace_while_sampling.rs Outdated
Comment thread examples/profile_proto_with_prost.rs
@kbsteere

Copy link
Copy Markdown
Contributor Author

@YangKeao fixed all the comments. I did have issues with cargo test. This line gave me issues. Seems like the protos don't have an encode for profile.

removed commented lines from previous commit
commented out profile.encode due to testing failures

Signed-off-by: Kyle Steere <kyle.steere@chainguard.dev>
@kbsteere
kbsteere force-pushed the bump-protobuf-version-3.x branch from 8fc0b7a to c6e01b6 Compare April 11, 2025 13:17
@kbsteere
kbsteere requested a review from YangKeao April 17, 2025 19:41
@kbsteere

Copy link
Copy Markdown
Contributor Author

@YangKeao need anything else from me?

@YangKeao

YangKeao commented Apr 28, 2025

Copy link
Copy Markdown
Member

@kbsteere Thanks for your contribution! This PR looks good 👍 ! It can be merged once it passed the CI.

The failure is caused by stepancheg/rust-protobuf#746. The newer version of rust-protobuf cannot be compiled with rustc < 1.71.0. Could you help to update the MSRV to 1.71.0?

Here is an example to update the MSRV: https://github.com/tikv/pprof-rs/pull/204/files. It mainly changed three things:

  1. The README.md.
  2. CI configuration. I think you'll only need to modify the toolchains from 1.66.0 to 1.71.0.
  3. Cargo.toml

Signed-off-by: Kyle Steere <kyle.steere@chainguard.dev>
Signed-off-by: Kyle Steere <kyle.steere@chainguard.dev>
@kbsteere

Copy link
Copy Markdown
Contributor Author

Fixed errors with cargo clippy warrnings. This will build but it's still going to fail for all the machines that run older versions of rust. @YangKeao

Comment thread .github/workflows/rust.yml Outdated
Signed-off-by: Kyle Steere <kyle.steere@chainguard.dev>
@kbsteere
kbsteere requested a review from YangKeao April 30, 2025 15:43
@kbsteere

kbsteere commented May 1, 2025

Copy link
Copy Markdown
Contributor Author

Hey @YangKeao need approval for workflow run.

Signed-off-by: Yang Keao <yangkeao@chunibyo.icu>
@YangKeao

YangKeao commented May 6, 2025

Copy link
Copy Markdown
Member

Hey @YangKeao need approval for workflow run.

Sorry. I misunderstood the mentioned issue, and the 1.71.0 is not enough. It should be upgraded to 1.74.0 (I guess it's caused by https://rust-lang.github.io/rfcs/2145-type-privacy.html). I've pushed a new commit to your branch and hope it'll pass.

Signed-off-by: Yang Keao <yangkeao@chunibyo.icu>
@YangKeao
YangKeao force-pushed the bump-protobuf-version-3.x branch from 070052f to 30c0d0c Compare May 6, 2025 09:15

@YangKeao YangKeao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Good job! Thanks for your contribution again 🍻 . I'll bump the version and publish a new version later.

@YangKeao
YangKeao merged commit 3d4e696 into tikv:master May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants