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.

Sign up for GitHub

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

Add a feature to make which dependency optional #1615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f64287d
Add which-rustfmt feature
lopopolo Sep 5, 2019
b97192b
Do not use which if which-rustfmt feature is disabled
lopopolo Sep 5, 2019
af4eebd
Return Error if no rustfmt path given and which-rustfmt feature is di…
lopopolo Sep 16, 2019
258940a
Test bindgen with no default features
lopopolo Sep 16, 2019
ebd8d4d
lint ci/script.sh
lopopolo Sep 16, 2019
e627649
Set RUSTFMT for nofeatures tests
lopopolo Sep 16, 2019
c22fcf3
Add rustfmt to nightly toolchain
lopopolo Sep 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 4 additions & 2 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
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
21 changes: 17 additions & 4 deletions ci/script.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,31 @@ 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"
rustup component add --toolchain nightly rustfmt
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")
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
./ci/assert-no-diff.sh
;;

"expectations")
cd ./tests/expectations
cargo test $BINDGEN_PROFILE
cargo test "$BINDGEN_PROFILE"
;;

"misc")
Expand All @@ -43,6 +55,7 @@ case "$BINDGEN_JOB" in
# TODO: Actually run quickchecks once `bindgen` is reliable enough.
cargo test
;;

*)
echo "Error! Unknown \$BINDGEN_JOB: '$BINDGEN_JOB'"
exit 1
Expand Down
4 changes: 4 additions & 0 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
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 @@ -1908,10 +1909,13 @@ impl Bindings {
if let Ok(rustfmt) = env::var("RUSTFMT") {
return Ok(Cow::Owned(rustfmt.into()));
}
#[cfg(feature = "which-rustfmt")]
match which::which("rustfmt") {
Ok(p) => Ok(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.

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
Expand Down