Skip to content

Commit

Permalink
Allow for the specification of the rustfmt path
Browse files Browse the repository at this point in the history
In some situations, rustfmt can't be counted on to be in your PATH.  In
those cases, this adds a format_with(...) function to the builder to
specify the path to rustfmt, which is then passed into the new
format_with function.

A small dance is done here with fmt(...) and format_with(...) to
preserve the public API of the crate, so this, if I understand
correctly, should be an API-compatible change.

I'm not 100% this would be the way I would want to go, but I wanted to
put it up for discussion.  There are more roundabout approaches I could
take to format each file in our build system, but I had deferred from
doing that previously.   The relevant section of a public draft of the
implementation I had put together is here:

https://github.com/bazelbuild/rules_rust/pull/479/files#r583439508

Also open to suggestions as to where this should be tested.  I didn't
see, but may have missed, a good place for it.
  • Loading branch information
David Freese committed Feb 26, 2021
1 parent 61555ff commit bb56617
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 2 deletions.
10 changes: 9 additions & 1 deletion tonic-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,22 @@ pub trait Method {
#[cfg(feature = "rustfmt")]
#[cfg_attr(docsrs, doc(cfg(feature = "rustfmt")))]
pub fn fmt(out_dir: &str) {
// Since fmt is public, preserve the API of fmt by dispatching to a helper function that is
// called within the crate.
format_with("rustfmt", &out_dir);
}

#[cfg(feature = "rustfmt")]
#[cfg_attr(docsrs, doc(cfg(feature = "rustfmt")))]
fn format_with(rustfmt_path: impl AsRef<std::ffi::OsStr>, out_dir: &str) {
let dir = std::fs::read_dir(out_dir).unwrap();

for entry in dir {
let file = entry.unwrap().file_name().into_string().unwrap();
if !file.ends_with(".rs") {
continue;
}
let result = Command::new("rustfmt")
let result = Command::new(rustfmt_path.as_ref())
.arg("--emit")
.arg("files")
.arg("--edition")
Expand Down
20 changes: 19 additions & 1 deletion tonic-build/src/prost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub fn configure() -> Builder {
compile_well_known_types: false,
#[cfg(feature = "rustfmt")]
format: true,
rustfmt_path: None,
emit_package: true,
}
}
Expand Down Expand Up @@ -210,6 +211,8 @@ pub struct Builder {
out_dir: Option<PathBuf>,
#[cfg(feature = "rustfmt")]
format: bool,
#[cfg(feature = "rustfmt")]
rustfmt_path: Option<PathBuf>,
}

impl Builder {
Expand Down Expand Up @@ -239,6 +242,13 @@ impl Builder {
self
}

/// The path that should be used to find rustfmt, if format is enabled.
#[cfg(feature = "rustfmt")]
pub fn format_with(mut self, path: impl AsRef<Path>) -> Self {
self.rustfmt_path = Some(path.as_ref().to_path_buf());
self
}

/// Set the output directory to generate code to.
///
/// Defaults to the `OUT_DIR` environment variable.
Expand Down Expand Up @@ -331,6 +341,11 @@ impl Builder {

#[cfg(feature = "rustfmt")]
let format = self.format;
#[cfg(feature = "rustfmt")]
let rustfmt_path = self
.rustfmt_path
.clone()
.unwrap_or_else(|| "rustfmt".into());

config.out_dir(out_dir.clone());
if let Some(path) = self.file_descriptor_set_path.as_ref() {
Expand All @@ -355,7 +370,10 @@ impl Builder {
#[cfg(feature = "rustfmt")]
{
if format {
super::fmt(out_dir.to_str().expect("Expected utf8 out_dir"));
super::format_with(
rustfmt_path,
out_dir.to_str().expect("Expected utf8 out_dir"),
);
}
}

Expand Down

0 comments on commit bb56617

Please sign in to comment.