From 3f2e4653326ec92d50e82c4f271400babd553101 Mon Sep 17 00:00:00 2001 From: David Freese Date: Fri, 17 Sep 2021 08:53:18 -0700 Subject: [PATCH] Add option to prost-build to skip the run of protoc (#442) * Add option to prost-build to skip the run of protoc Our build system ends up generating all of the relevant protobuf files already, so this ends up being a redundant step for us. This allows the running of the protoc step to be skipped. There are some API concerns with this. The inputs to compile_protos are now ignored if that flag is set. I didn't know to balance that with Config::file_descriptor_set_path, which effectively allows for an input of a filename into the config. I could create a separate entry, similar to compile_protos, that takes either: 1. Nothing and uses the config function input. This, however, can be left unspecified and error out, which isn't super user-friendly. 2. A path, but this would now ignore the existing config input function if it was passed in. I'm happy to put up a change that does that, but it wasn't clear either was great, so I ended up going with the simplest approach. * fix typo and build failure Co-authored-by: Lucio Franco --- prost-build/src/lib.rs | 86 ++++++++++++++++++++++++++++-------------- tests/src/build.rs | 8 ++++ 2 files changed, 66 insertions(+), 28 deletions(-) diff --git a/prost-build/src/lib.rs b/prost-build/src/lib.rs index 3a6074ee4..9607e4276 100644 --- a/prost-build/src/lib.rs +++ b/prost-build/src/lib.rs @@ -233,6 +233,7 @@ pub struct Config { default_package_filename: String, protoc_args: Vec, disable_comments: PathMap<()>, + skip_protoc_run: bool, include_file: Option, } @@ -637,6 +638,25 @@ impl Config { self } + /// In combination with with `file_descriptor_set_path`, this can be used to provide a file + /// descriptor set as an input file, rather than having prost-build generate the file by calling + /// protoc. Prost-build does require that the descriptor set was generated with + /// --include_source_info. + /// + /// In `build.rs`: + /// + /// ```rust + /// # let mut config = prost_build::Config::new(); + /// config.file_descriptor_set_path("path/from/build/system") + /// .skip_protoc_run() + /// .compile_protos(&["src/items.proto"], &["src/"]); + /// ``` + /// + pub fn skip_protoc_run(&mut self) -> &mut Self { + self.skip_protoc_run = true; + self + } + /// Configures the code generator to not strip the enum name from variant names. /// /// Protobuf enum definitions commonly include the enum name as a prefix of every variant name. @@ -731,6 +751,8 @@ impl Config { /// specify non-default code generation options. See that function for more information about /// the arguments and generated outputs. /// + /// The `protos` and `includes` arguments are ignored if `skip_protoc_run` is specified. + /// /// # Example `build.rs` /// /// ```rust,no_run @@ -766,48 +788,55 @@ impl Config { // [1]: http://doc.crates.io/build-script.html#outputs-of-the-build-script let tmp; - let file_descriptor_set_path = match self.file_descriptor_set_path.clone() { - Some(file_descriptor_set_path) => file_descriptor_set_path, - None => { - tmp = tempfile::Builder::new().prefix("prost-build").tempdir()?; - tmp.path().join("prost-descriptor-set") + let file_descriptor_set_path = if let Some(path) = &self.file_descriptor_set_path { + path.clone() + } else { + if self.skip_protoc_run { + return Err(Error::new( + ErrorKind::Other, + "file_descriptor_set_path is required with skip_protoc_run", + )); } + tmp = tempfile::Builder::new().prefix("prost-build").tempdir()?; + tmp.path().join("prost-descriptor-set") }; - let mut cmd = Command::new(protoc()); - cmd.arg("--include_imports") - .arg("--include_source_info") - .arg("-o") - .arg(&file_descriptor_set_path); + if !self.skip_protoc_run { + let mut cmd = Command::new(protoc()); + cmd.arg("--include_imports") + .arg("--include_source_info") + .arg("-o") + .arg(&file_descriptor_set_path); - for include in includes { - cmd.arg("-I").arg(include.as_ref()); - } + for include in includes { + cmd.arg("-I").arg(include.as_ref()); + } - // Set the protoc include after the user includes in case the user wants to - // override one of the built-in .protos. - cmd.arg("-I").arg(protoc_include()); + // Set the protoc include after the user includes in case the user wants to + // override one of the built-in .protos. + cmd.arg("-I").arg(protoc_include()); - for arg in &self.protoc_args { - cmd.arg(arg); - } + for arg in &self.protoc_args { + cmd.arg(arg); + } - for proto in protos { - cmd.arg(proto.as_ref()); - } + for proto in protos { + cmd.arg(proto.as_ref()); + } - let output = cmd.output().map_err(|error| { + let output = cmd.output().map_err(|error| { Error::new( error.kind(), format!("failed to invoke protoc (hint: https://docs.rs/prost-build/#sourcing-protoc): {}", error), ) })?; - if !output.status.success() { - return Err(Error::new( - ErrorKind::Other, - format!("protoc failed: {}", String::from_utf8_lossy(&output.stderr)), - )); + if !output.status.success() { + return Err(Error::new( + ErrorKind::Other, + format!("protoc failed: {}", String::from_utf8_lossy(&output.stderr)), + )); + } } let buf = fs::read(file_descriptor_set_path)?; @@ -976,6 +1005,7 @@ impl default::Default for Config { default_package_filename: "_".to_string(), protoc_args: Vec::new(), disable_comments: PathMap::default(), + skip_protoc_run: false, include_file: None, } } diff --git a/tests/src/build.rs b/tests/src/build.rs index bd847d494..522768b6a 100644 --- a/tests/src/build.rs +++ b/tests/src/build.rs @@ -173,4 +173,12 @@ fn main() { &[src.join("packages")], ) .unwrap(); + + // Run the last command again, while skipping the protoc run. Since file_descriptor_set_path + // has been set, it will already exist, and should produce the same result. The inputs are also + // ignored, so provide fake input. + config + .skip_protoc_run() + .compile_protos(&[] as &[&str], &[] as &[&str]) + .unwrap(); }