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

Add a feature to make which dependency optional #1615

Merged
merged 7 commits into from Sep 17, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions Cargo.toml
Expand Up @@ -54,7 +54,7 @@ lazy_static = "1"
peeking_take_while = "0.1.2"
quote = { version = "1", default-features = false }
regex = "1.0"
which = ">=1.0, <3.0"
which = { version = ">=1.0, <3.0", optional = true }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about just leaving this change, then the feature will be automatically called which (you get a feature for each optional dependency).

shlex = "0.1"
fxhash = "0.2"
# New validation in 0.3.6 breaks bindgen-integration:
Expand All @@ -70,9 +70,11 @@ optional = true
version = "0.4"

[features]
default = ["logging", "clap"]
default = ["logging", "clap", "which-rustfmt"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So then you replace which-rustfmt by which here...

logging = ["env_logger", "log"]
static = []
# Dynamically discover a `rustfmt` binary using the `which` crate
which-rustfmt = ["which"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And remove this line.


# These features only exist for CI testing -- don't use them if you're not hacking
# on bindgen!
Expand Down
18 changes: 13 additions & 5 deletions src/lib.rs
Expand Up @@ -32,6 +32,7 @@ extern crate quote;
extern crate proc_macro2;
extern crate regex;
extern crate shlex;
#[cfg(feature = "which-rustfmt")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And s/which-rustfmt/which here.

extern crate which;

#[cfg(feature = "logging")]
Expand Down Expand Up @@ -1900,18 +1901,21 @@ impl Bindings {
}

/// Gets the rustfmt path to rustfmt the generated bindings.
fn rustfmt_path<'a>(&'a self) -> io::Result<Cow<'a, PathBuf>> {
fn rustfmt_path<'a>(&'a self) -> io::Result<Option<Cow<'a, PathBuf>>> {
debug_assert!(self.options.rustfmt_bindings);
if let Some(ref p) = self.options.rustfmt_path {
return Ok(Cow::Borrowed(p));
return Ok(Some(Cow::Borrowed(p)));
}
if let Ok(rustfmt) = env::var("RUSTFMT") {
return Ok(Cow::Owned(rustfmt.into()));
return Ok(Some(Cow::Owned(rustfmt.into())));
}
#[cfg(feature = "which-rustfmt")]
match which::which("rustfmt") {
Ok(p) => Ok(Cow::Owned(p)),
Ok(p) => Ok(Some(Cow::Owned(p))),
Err(e) => Err(io::Error::new(io::ErrorKind::Other, format!("{}", e))),
}
#[cfg(not(feature = "which-rustfmt"))]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just return an error here, like Err(io::Error::new(io::ErrorKind::Other, "which wasn't enabled, and no rustfmt binary specified"))), then the patch becomes much simpler, wdyt?

The error is not fatal so you'd still generate bindings.

Ok(None)
}

/// Checks if rustfmt_bindings is set and runs rustfmt on the string
Expand All @@ -1926,7 +1930,11 @@ impl Bindings {
return Ok(Cow::Borrowed(source));
}

let rustfmt = self.rustfmt_path()?;
let rustfmt = if let Some(rustfmt) = self.rustfmt_path()? {
rustfmt
} else {
return Ok(Cow::Borrowed(source));
};
let mut cmd = Command::new(&*rustfmt);

cmd
Expand Down