Add protobuf with corrected cmake build
ClosedPublic
Actions

Authored by • jon on Apr 27 2022, 2:33 PM.

Details

Reviewers

varun
• max
ashoat

Commits

rCOMMa8aa1e71d35c: Add protobuf with corrected cmake build

Summary

Unfortunately the protobuf 3.15 available in nix is using
an autotool build, which doesn't install the required protobuf-config.cmak
required by grpc to find the related package.

Upstreamed PR: https://github.com/NixOS/nixpkgs/pull/169497

Depends on D3793

Test Plan

nix build .#protobuf_3_15_cmake

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• jon created this revision.Apr 27 2022, 2:33 PM

Herald added subscribers: • abosh, • karol, atul and 3 others. · View Herald TranscriptApr 27 2022, 2:33 PM

Harbormaster completed remote builds in B8622: Diff 12011.Apr 27 2022, 2:40 PM

• jon requested review of this revision.Apr 27 2022, 2:40 PM

Fix build for mac

• jon added a child revision: D3867: [services] Export comm's grpc generated files as cmake project.Apr 27 2022, 5:00 PM

• jon added a reviewer: • max.Apr 27 2022, 5:01 PM

Harbormaster completed remote builds in B8625: Diff 12015.Apr 27 2022, 5:02 PM

• jon added a child revision: D3874: Update nix dev shell.Apr 28 2022, 9:26 PM

varun requested changes to this revision.May 2 2022, 12:42 PM

varun added inline comments.

nix/cmake-install-path.patch
1–32	should this file be in this diff?

This revision now requires changes to proceed.May 2 2022, 12:42 PM

varun added inline comments.May 2 2022, 12:43 PM

nix/cmake-install-path.patch
1–32	ah wait i see how it's used now

• jon marked 2 inline comments as done.May 2 2022, 12:55 PM

• jon added inline comments.

nix/cmake-install-path.patch
1–32	Yep, used in `patches` of the other file

• jon requested review of this revision.May 2 2022, 12:56 PM

• jon marked an inline comment as done.

I see in your test plan that nix build .#protobuf_3_15_cmake works, but were you able to link the build output successfully? i.e. did you try building one of the services that depends on that package?

This revision now requires changes to proceed.May 11 2022, 11:00 AM

I don't think I was able to test it with nix develop and doing an impure workflow. Nix packaging is to come.

• jon requested review of this revision.May 11 2022, 1:37 PM

Remove cross compilation logic, likely unneeded

varun accepted this revision.May 12 2022, 9:12 AM

This revision is now accepted and ready to land.May 12 2022, 9:12 AM

Herald added a reviewer: ashoat. · View Herald TranscriptMay 12 2022, 9:12 AM

This revision now requires review to proceed.May 12 2022, 9:12 AM

Harbormaster completed remote builds in B9038: Diff 12581.May 12 2022, 9:14 AM

Makes sense to me!

This revision is now accepted and ready to land.May 12 2022, 12:54 PM

Closed by commit rCOMMa8aa1e71d35c: Add protobuf with corrected cmake build (authored by • jon). · Explain WhyMay 16 2022, 9:04 AM