Skip to content

Commit

Permalink
Add option to prost-build to skip the run of protoc (#442)
Browse files Browse the repository at this point in the history
* 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 <luciofranco14@gmail.com>
  • Loading branch information
David Freese and LucioFranco authored Sep 17, 2021
1 parent 859f243 commit 3f2e465
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 28 deletions.
86 changes: 58 additions & 28 deletions prost-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ pub struct Config {
default_package_filename: String,
protoc_args: Vec<OsString>,
disable_comments: PathMap<()>,
skip_protoc_run: bool,
include_file: Option<PathBuf>,
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)?;
Expand Down Expand Up @@ -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,
}
}
Expand Down
8 changes: 8 additions & 0 deletions tests/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

0 comments on commit 3f2e465

Please sign in to comment.