Skip to content
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.

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

prost-build occasionally fails to compile protobuf C++ #653

Closed
benesch opened this issue May 21, 2022 · 4 comments
Closed

prost-build occasionally fails to compile protobuf C++ #653

benesch opened this issue May 21, 2022 · 4 comments

Comments

@benesch
Copy link

benesch commented May 21, 2022

In v0.10, we're seeing prost-build occasionally fail to compile protobuf from source. We've now seen this in two projects with entirely different build systems (MaterializeInc/materialize and our internal cloud codebase), which makes me think it's not a bug in our build system. The only commonality is in that in both cases we're compiling inside of Docker. Here's two representative examples:

I have a few separate thoughts here!

  • Is CMake even supported for building protobuf on Unix machines? The docs seem to suggest that the CMake build system exists to support compilation on Windows.
  • In our protobuf-src crate, we use the autotools build system instead, because it seems to be the officially supported way to compile protobuf on Unix. That was stable in CI for us for several months, when we'd forked prost to use protobuf-src.
  • If you don't want to switch to the autotools system, is there some way to turn off compilation of the unit tests? That's what seems to fail most readily.
  • It'd be great to eventually have prost-build use protobuf-src rather than managing its own protoc compilation. That way projects that use protoc outside of prost only need to compile protobuf from source once. We'd be happy to transfer ownership of protobuf-src to Tokio if that's a supply chain concern.
@LucioFranco
Copy link
Member

Right, I will say I am not a c++ build tool expert and it could totally be that cmake has some issues on non-windows builds. Though I don't know why 😆

In our protobuf-src crate, we use the autotools build system instead, because it seems to be the officially supported way to compile protobuf on Unix. That was stable in CI for us for several months, when we'd forked prost to use protobuf-src.

The reason I didn't go with protobuf-src is because I wanted to control the version of protobuf being included and not depend on a separately owned crate that is quite critical to the build process. I would be happy to give you access/take ownership of the protobuf-src crate and move it into here. Let me know if you want to coordinate here.

f you don't want to switch to the autotools system, is there some way to turn off compilation of the unit tests? That's what seems to fail most readily.

Would #620 work for your use case instead of autotools? Or is this a case where we want to support different ways to fetch protoc.

#624 mentions to make prost-build more modular with respect to how we fetch protoc.

Is there some way to turn off compilation of the unit tests?

Which unit tests are you referring too?

It'd be great to eventually have prost-build use protobuf-src rather than managing its own protoc compilation. That way projects that use protoc outside of prost only need to compile protobuf from source once. We'd be happy to transfer ownership of protobuf-src to Tokio if that's a supply chain concern.

Yeah, lets do it, would yall be interested in helping maintain it at all?

@ricnewton
Copy link
Contributor

FYI you can turn off unit tests like this: master...ricnewton:master

I can raise a PR but was waiting for outcome of #620

@benesch
Copy link
Author

benesch commented May 24, 2022

The reason I didn't go with protobuf-src is because I wanted to control the version of protobuf being included and not depend on a separately owned crate that is quite critical to the build process. I would be happy to give you access/take ownership of the protobuf-src crate and move it into here. Let me know if you want to coordinate here.

Sure thing! I added you as an individual owner of the crate on crates.io. I tried to add the tokio-rs/prost-core team, but since I don't have access to the team I don't think I can!

Yeah, lets do it, would yall be interested in helping maintain it at all?

Absolutely!

Which unit tests are you referring too?

Ah, the unit tests in protobuf itself—exactly the one @ricnewton has demonstrated disabling here: master...ricnewton:master.

Would #620 work for your use case instead of autotools? Or is this a case where we want to support different ways to fetch protoc.

Just posted on #620! That approach scares me, to be quite honest! I've been burned in the past a few times by downstream software reimplementing upstream build systems. Would love to come up with some way of modularizing prost-build, so that @Jake-Shadle can plug in his protoc build implementation at @EmbarkStudios and we can plug in ours at @MaterializeInc.

@LucioFranco
Copy link
Member

#657 fixes this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants