From f64287d557fd7a8bfe7b22bcac27bd9c8fc2003b Mon Sep 17 00:00:00 2001 From: Ryan Lopopolo Date: Thu, 5 Sep 2019 15:28:59 -0700 Subject: [PATCH 1/7] Add which-rustfmt feature This feature controls whether bindgen will use the which crate to detect the rustfmt binary. This makes which an optional dependency. which-rustfmt is a default feature which makes this change backward compatible. --- Cargo.toml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2f98a0ea99..232d9c3596 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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 } shlex = "0.1" fxhash = "0.2" # New validation in 0.3.6 breaks bindgen-integration: @@ -70,9 +70,11 @@ optional = true version = "0.4" [features] -default = ["logging", "clap"] +default = ["logging", "clap", "which-rustfmt"] logging = ["env_logger", "log"] static = [] +# Dynamically discover a `rustfmt` binary using the `which` crate +which-rustfmt = ["which"] # These features only exist for CI testing -- don't use them if you're not hacking # on bindgen! From b97192b3e442b0a910489529889a03f950cbf53c Mon Sep 17 00:00:00 2001 From: Ryan Lopopolo Date: Thu, 5 Sep 2019 15:41:42 -0700 Subject: [PATCH 2/7] Do not use which if which-rustfmt feature is disabled This commit changes the API of rustfmt_path to return Result>>. Ok(None) is returned in the case where which is disabled and no rustfmt command is supplied explicitly either via configuration or env variable. Downstream code checks for the presence of None to directly return the source without emitting an error. --- src/lib.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b59c9271c7..d7aa537ea1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -32,6 +32,7 @@ extern crate quote; extern crate proc_macro2; extern crate regex; extern crate shlex; +#[cfg(feature = "which-rustfmt")] extern crate which; #[cfg(feature = "logging")] @@ -1900,18 +1901,21 @@ impl Bindings { } /// Gets the rustfmt path to rustfmt the generated bindings. - fn rustfmt_path<'a>(&'a self) -> io::Result> { + fn rustfmt_path<'a>(&'a self) -> io::Result>> { 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"))] + Ok(None) } /// Checks if rustfmt_bindings is set and runs rustfmt on the string @@ -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 From af4eebdacd8c3a665196c744df097b638bcd4df3 Mon Sep 17 00:00:00 2001 From: Ryan Lopopolo Date: Sun, 15 Sep 2019 18:23:12 -0700 Subject: [PATCH 3/7] Return Error if no rustfmt path given and which-rustfmt feature is disabled --- src/lib.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index d7aa537ea1..722e53e9e5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1901,21 +1901,21 @@ impl Bindings { } /// Gets the rustfmt path to rustfmt the generated bindings. - fn rustfmt_path<'a>(&'a self) -> io::Result>> { + fn rustfmt_path<'a>(&'a self) -> io::Result> { debug_assert!(self.options.rustfmt_bindings); if let Some(ref p) = self.options.rustfmt_path { - return Ok(Some(Cow::Borrowed(p))); + return Ok(Cow::Borrowed(p)); } if let Ok(rustfmt) = env::var("RUSTFMT") { - return Ok(Some(Cow::Owned(rustfmt.into()))); + return Ok(Cow::Owned(rustfmt.into())); } #[cfg(feature = "which-rustfmt")] match which::which("rustfmt") { - Ok(p) => Ok(Some(Cow::Owned(p))), + Ok(p) => Ok(Cow::Owned(p)), Err(e) => Err(io::Error::new(io::ErrorKind::Other, format!("{}", e))), } #[cfg(not(feature = "which-rustfmt"))] - Ok(None) + Err(io::Error::new(io::ErrorKind::Other, "which wasn't enabled, and no rustfmt binary specified")) } /// Checks if rustfmt_bindings is set and runs rustfmt on the string @@ -1930,11 +1930,7 @@ impl Bindings { return Ok(Cow::Borrowed(source)); } - let rustfmt = if let Some(rustfmt) = self.rustfmt_path()? { - rustfmt - } else { - return Ok(Cow::Borrowed(source)); - }; + let rustfmt = self.rustfmt_path()?; let mut cmd = Command::new(&*rustfmt); cmd From 258940a8539897361a128fd0573cb7be154176f2 Mon Sep 17 00:00:00 2001 From: Ryan Lopopolo Date: Sun, 15 Sep 2019 18:33:54 -0700 Subject: [PATCH 4/7] Test bindgen with no default features --- .travis.yml | 8 ++++++++ ci/script.sh | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/.travis.yml b/.travis.yml index d8bc595d79..36e10e1a91 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,20 +14,28 @@ env: - LLVM_VERSION="3.8" BINDGEN_JOB="test" BINDGEN_PROFILE="--release" - LLVM_VERSION="3.8" BINDGEN_JOB="integration" BINDGEN_PROFILE= - LLVM_VERSION="3.8" BINDGEN_JOB="integration" BINDGEN_PROFILE="--release" + - LLVM_VERSION="3.8" BINDGEN_JOB="nofeatures" BINDGEN_PROFILE= + - LLVM_VERSION="3.8" BINDGEN_JOB="nofeatures" BINDGEN_PROFILE="--release" - LLVM_VERSION="3.9" BINDGEN_JOB="test" BINDGEN_PROFILE= - LLVM_VERSION="3.9" BINDGEN_JOB="test" BINDGEN_PROFILE="--release" - LLVM_VERSION="3.9" BINDGEN_JOB="integration" BINDGEN_PROFILE= - LLVM_VERSION="3.9" BINDGEN_JOB="integration" BINDGEN_PROFILE="--release" + - LLVM_VERSION="3.9" BINDGEN_JOB="nofeatures" BINDGEN_PROFILE= + - LLVM_VERSION="3.9" BINDGEN_JOB="nofeatures" BINDGEN_PROFILE="--release" - LLVM_VERSION="4.0" BINDGEN_JOB="test" BINDGEN_PROFILE= - LLVM_VERSION="4.0" BINDGEN_JOB="test" BINDGEN_PROFILE="--release" - LLVM_VERSION="4.0" BINDGEN_JOB="integration" BINDGEN_PROFILE= - LLVM_VERSION="4.0" BINDGEN_JOB="integration" BINDGEN_PROFILE="--release" + - LLVM_VERSION="4.0" BINDGEN_JOB="nofeatures" BINDGEN_PROFILE= + - LLVM_VERSION="4.0" BINDGEN_JOB="nofeatures" BINDGEN_PROFILE="--release" - LLVM_VERSION="5.0" BINDGEN_JOB="test" BINDGEN_PROFILE= - LLVM_VERSION="5.0" BINDGEN_JOB="test" BINDGEN_PROFILE="--release" - LLVM_VERSION="5.0" BINDGEN_JOB="test" BINDGEN_PROFILE= BINDGEN_FEATURES="testing_only_extra_assertions" - LLVM_VERSION="5.0" BINDGEN_JOB="test" BINDGEN_PROFILE="--release" BINDGEN_FEATURES="testing_only_extra_assertions" - LLVM_VERSION="5.0" BINDGEN_JOB="integration" BINDGEN_PROFILE= - LLVM_VERSION="5.0" BINDGEN_JOB="integration" BINDGEN_PROFILE="--release" + - LLVM_VERSION="5.0" BINDGEN_JOB="nofeatures" BINDGEN_PROFILE= + - LLVM_VERSION="5.0" BINDGEN_JOB="nofeatures" BINDGEN_PROFILE="--release" - LLVM_VERSION="5.0" BINDGEN_JOB="expectations" BINDGEN_PROFILE= - LLVM_VERSION="5.0" BINDGEN_JOB="expectations" BINDGEN_PROFILE="--release" - LLVM_VERSION="5.0" BINDGEN_JOB="misc" diff --git a/ci/script.sh b/ci/script.sh index 880896cd72..eeed674fa4 100755 --- a/ci/script.sh +++ b/ci/script.sh @@ -43,6 +43,10 @@ case "$BINDGEN_JOB" in # TODO: Actually run quickchecks once `bindgen` is reliable enough. cargo test ;; + "nofeatures") + cargo test $BINDGEN_PROFILE --no-default-features + ./ci/assert-no-diff.sh + ;; *) echo "Error! Unknown \$BINDGEN_JOB: '$BINDGEN_JOB'" exit 1 From ebd8d4d78acb94eb9ffe936ed652a43726a992ec Mon Sep 17 00:00:00 2001 From: Ryan Lopopolo Date: Sun, 15 Sep 2019 18:36:00 -0700 Subject: [PATCH 5/7] lint ci/script.sh --- ci/script.sh | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/ci/script.sh b/ci/script.sh index eeed674fa4..6bf659605b 100755 --- a/ci/script.sh +++ b/ci/script.sh @@ -14,19 +14,25 @@ case "$BINDGEN_JOB" in # Need rustfmt to compare the test expectations. rustup update nightly rustup component add rustfmt - export RUSTFMT="$(rustup which rustfmt)" - cargo test $BINDGEN_PROFILE --features "$BINDGEN_FEATURES" + RUSTFMT="$(rustup which rustfmt)" + export RUSTFMT + cargo test "$BINDGEN_PROFILE" --features "$BINDGEN_FEATURES" ./ci/assert-no-diff.sh ;; "integration") cd ./bindgen-integration - cargo test $BINDGEN_PROFILE --features "$BINDGEN_FEATURES" + cargo test "$BINDGEN_PROFILE" --features "$BINDGEN_FEATURES" + ;; + + "nofeatures") + cargo test "$BINDGEN_PROFILE" --no-default-features + ./ci/assert-no-diff.sh ;; "expectations") cd ./tests/expectations - cargo test $BINDGEN_PROFILE + cargo test "$BINDGEN_PROFILE" ;; "misc") @@ -43,10 +49,7 @@ case "$BINDGEN_JOB" in # TODO: Actually run quickchecks once `bindgen` is reliable enough. cargo test ;; - "nofeatures") - cargo test $BINDGEN_PROFILE --no-default-features - ./ci/assert-no-diff.sh - ;; + *) echo "Error! Unknown \$BINDGEN_JOB: '$BINDGEN_JOB'" exit 1 From e627649839ab7b90fc3e7c6c7a704d880ad425e8 Mon Sep 17 00:00:00 2001 From: Ryan Lopopolo Date: Sun, 15 Sep 2019 19:19:12 -0700 Subject: [PATCH 6/7] Set RUSTFMT for nofeatures tests --- ci/script.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ci/script.sh b/ci/script.sh index 6bf659605b..8c5d5de7b7 100755 --- a/ci/script.sh +++ b/ci/script.sh @@ -26,6 +26,10 @@ case "$BINDGEN_JOB" in ;; "nofeatures") + rustup update nightly + rustup component add rustfmt + RUSTFMT="$(rustup which rustfmt)" + export RUSTFMT cargo test "$BINDGEN_PROFILE" --no-default-features ./ci/assert-no-diff.sh ;; From c22fcf377003de2fe4480b1510ce903a133cc430 Mon Sep 17 00:00:00 2001 From: Ryan Lopopolo Date: Sun, 15 Sep 2019 20:19:57 -0700 Subject: [PATCH 7/7] Add rustfmt to nightly toolchain --- ci/script.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/script.sh b/ci/script.sh index 8c5d5de7b7..7003618a56 100755 --- a/ci/script.sh +++ b/ci/script.sh @@ -14,6 +14,7 @@ case "$BINDGEN_JOB" in # Need rustfmt to compare the test expectations. rustup update nightly rustup component add rustfmt + rustup component add --toolchain nightly rustfmt RUSTFMT="$(rustup which rustfmt)" export RUSTFMT cargo test "$BINDGEN_PROFILE" --features "$BINDGEN_FEATURES" @@ -28,6 +29,7 @@ case "$BINDGEN_JOB" in "nofeatures") rustup update nightly rustup component add rustfmt + rustup component add --toolchain nightly rustfmt RUSTFMT="$(rustup which rustfmt)" export RUSTFMT cargo test "$BINDGEN_PROFILE" --no-default-features