Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
May 6, 2025
Merged

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

merged 10 commits into from
May 6, 2025

Conversation

kbsteere
Copy link
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
Copy link

ti-chi-bot bot commented Mar 31, 2025

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

@kbsteere
updated protobuf version to be a minimum of 3.7.2 to address GHSA-2gh…
a02a728 
…3-rmm4-6rq5. Added linter allows so build and tests pass

Signed-off-by: Kyle Steere <[email protected]>
@kbsteere kbsteere force-pushed the bump-protobuf-version-3.x branch from 5008794 to a02a728 Compare March 31, 2025 22:17
@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
Copy link
Contributor Author

kbsteere commented Apr 1, 2025 •
edited
Loading

@YangKeao @Xuanwo can you review these changes?

@YangKeao YangKeao self-requested a review April 8, 2025 07:47
YangKeao
YangKeao reviewed Apr 8, 2025
View reviewed changes
@kbsteere
Copy link
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.

@kbsteere
added .pure implementation
c6e01b6 
removed commented lines from previous commit
commented out profile.encode due to testing failures

Signed-off-by: Kyle Steere <[email protected]>
@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
Contributor Author

@YangKeao need anything else from me?

@YangKeao
Copy link
Member

YangKeao commented Apr 28, 2025 •
edited
Loading

@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

@kbsteere
Copy link
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

YangKeao
YangKeao reviewed Apr 29, 2025
View reviewed changes
@kbsteere kbsteere requested a review from YangKeao April 30, 2025 15:43
@kbsteere
Copy link
Contributor Author

kbsteere commented May 1, 2025

Hey @YangKeao need approval for workflow run.

@YangKeao
upgrade MSRV to 1.74.0
7fe40e4 
Signed-off-by: Yang Keao <[email protected]>
@YangKeao
Copy link
Member

YangKeao commented May 6, 2025

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.

@YangKeao YangKeao force-pushed the bump-protobuf-version-3.x branch from 070052f to 30c0d0c Compare May 6, 2025 09:15
YangKeao
YangKeao approved these changes May 6, 2025
View reviewed changes
Copy link
Member

@YangKeao YangKeao left a comment

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
45 of 46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YangKeao YangKeao YangKeao approved these changes

Assignees
No one assigned
Labels
dco-signoff: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kbsteere @YangKeao