feat(build): Add option to emit rerun-if-changed instructions #1021
Conversation
Currently, tonic-build does not emit cargo:rerun-if-changed instructions. This will cause the build script to be rerun if any file changes in the package. This commit implements an option on the Builder called emit_rerun_if_changed that, when enabled, will emit the cargo:rerun-if-changed instructions for all of the proto file paths and include directories. Fixes: hyperium#1020 Refs: hyperium#1019
| @@ -27,6 +27,7 @@ pub fn configure() -> Builder { | |||
| emit_package: true, | |||
| protoc_args: Vec::new(), | |||
| include_file: None, | |||
| emit_rerun_if_changed: std::env::var_os("CARGO").is_some(), | |||
There was a problem hiding this comment.
Not sure if run-time detection of whether the code is run as a build script is
- right, and
- necessary
Setting it to true may constitute a breaking change if people are using tonic_build outside of build scripts.
We could also set it to false in this version of tonic_build and make it true or automatically detected in the next major release.
There was a problem hiding this comment.
I think its fine to set this for a build script. Do you know what other projects with similar features use? I don't mind auto enabling this in this release since I don't really think its a breaking style change etc.
There was a problem hiding this comment.
I skimmed through the cargo reference but did not find anything else. I also failed to find results for how to detect Rust code is being run from a build script through Google. I hope this does a good enough job.
tonic-build/src/prost.rs
Outdated
| /// Enable or disable emitting the rerun-if-changed cargo instruction | ||
| /// | ||
| /// If set, emits rerun-if-changed instructions so that Cargo understands when to rerun the build script. |
There was a problem hiding this comment.
Is this the only place I need to document this option?
There was a problem hiding this comment.
Yeah I think this is fine, I would also add info about the default setting that if you're in a build script this will be enabled and outside of a build script it won't
tonic-build/src/prost.rs
Outdated
| /// Enable or disable emitting the rerun-if-changed cargo instruction | ||
| /// | ||
| /// If set, emits rerun-if-changed instructions so that Cargo understands when to rerun the build script. |
There was a problem hiding this comment.
Yeah I think this is fine, I would also add info about the default setting that if you're in a build script this will be enabled and outside of a build script it won't
| @@ -27,6 +27,7 @@ pub fn configure() -> Builder { | |||
| emit_package: true, | |||
| protoc_args: Vec::new(), | |||
| include_file: None, | |||
| emit_rerun_if_changed: std::env::var_os("CARGO").is_some(), | |||
There was a problem hiding this comment.
I think its fine to set this for a build script. Do you know what other projects with similar features use? I don't mind auto enabling this in this release since I don't really think its a breaking style change etc.
|
@mickvangelderen just need you to run a fmt pass |
That was sloppy, sorry. |
| @@ -27,6 +27,7 @@ pub fn configure() -> Builder { | |||
| emit_package: true, | |||
| protoc_args: Vec::new(), | |||
| include_file: None, | |||
| emit_rerun_if_changed: std::env::var_os("CARGO").is_some(), | |||
LucioFranco
changed the title
|
and no problem! its easy to miss the formatting, I usually just have it auto fmt on save. |
|
@LucioFranco thanks for the help getting this in! |
Fixes: #1020
Refs: #1019
Motivation
Currently, tonic-build does not emit cargo:rerun-if-changed
instructions. This will cause the build script to be rerun if any file
changes in the package.
Solution
This PR implements an option on the Builder called
emit_rerun_if_changed that, when enabled, will emit the
cargo:rerun-if-changed instructions for all of the proto file paths
and include directories.