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

#1021 seems to induce cargo recompilations #1059

Closed
cpg314 opened this issue Aug 16, 2022 · 4 comments
Closed

#1021 seems to induce cargo recompilations #1059

cpg314 opened this issue Aug 16, 2022 · 4 comments

Comments

@cpg314
Copy link

cpg314 commented Aug 16, 2022

Bug Report

Version

tonic 0.8.0

Platform

Linux 5.13.0-40-generic #45~20.04.1-Ubuntu SMP Mon Apr 4 09:38:31 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Description

After updating tonic from 0.6 to 0.8, we noticed that our workspace crate containing tonic/prost-generated code (through a build script) was being repeatedly rebuilt by Cargo, even though neither the protobufs nor the build scripts had changed.
This happened in our CI pipeline (using the official Rust 1.63 docker images), but not on the developer's machines.

This had catastrophic consequences, as all other crates in the workspace depended on that one and had to be rebuilt repeatedly.

Investigating with rust-lang/cargo#2904 (CARGO_LOG=cargo::core::compiler::fingerprint=trace) showed that the crate was marked as stale due to the .proto file being more recent than the output file (containing the build script output and used by Cargo to determine whether the build script output is stale), by about 2 seconds.

The problem disappears when setting .emit_rerun_if_changed(false), from the default of true introduced by #1021.

We have not verified this, but one possibility is that the protoc version included in Debian's protobuf-compiler actually changes the modification time of the .proto file when called during the build script, directly invalidating the output.

@LucioFranco
Copy link
Member

Oh wow okay, maybe it makes sense to open an issue with the protobuf-compiler version included with debian? Or in your CI providing your own binaries for it? I don't feel like there is really anything actionable from the prost/tonic side of things since there is already a config option for this.

@cpg314
Copy link
Author

cpg314 commented Aug 16, 2022

Yeah, I've opened an issue here first so that the next person doesn't have to spend too much time figuring out this behaviour (new from 0.8.0 IIUC) and the emit_rerun_if_changed(false) solution. Once I have more time, I will look more carefully to confirm or inform the hypothesis on the .proto files getting their modification time changed.

@matze
Copy link
Contributor

matze commented Aug 22, 2022

Interestingly, we had a similar issue when moving from 0.7 to 0.8. However, the root cause was slightly different. We use glob::glob to find all protos in a proto subdirectory. However, we did not want the proto to be part of the package name so we stripped the prefix from the path names. Now, for some weird reason it always compiled just fine (both 0.7 and 0.8) perhaps because the includes parameter contained the proto directory as well. However, with 0.8 cargo noticed that none of those protos actually exist and then determined to recompile again and again. Removing the prefix stripping fixed it then.

@LucioFranco
Copy link
Member

I believe we can close this issue since it doesn't seem like a tonic issue but external factors combining with the new changes.

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