search

rust-lang / rust-bindgen

Automatically generates Rust FFI bindings to C (and some C++) libraries.
https://rust-lang.github.io/rust-bindgen/
BSD 3-Clause "New" or "Revised" License
4.23k stars 679 forks source link

Remove which dependency #2809

Closed rukai closed 2 months ago

rukai commented 2 months ago

The which dependency was introduced alongside the ability for bindgen to call rustfmt: https://github.com/rust-lang/rust-bindgen/pull/905

Theres been some dancing around the need for this dependency, with a feature added to disable it back in 2019 https://github.com/rust-lang/rust-bindgen/pull/1615

But I dont believe its actually required at all and in this PR I propose that we can just remove it entirely.

My understanding is that which will check the PATH for any binaries matching the passed name. However if we were to just attempt to execute that name directly the OS would perform the exact same operation, checking the PATH for any binaries matching the name. So we should just directly execute rustfmt, unless its overridden by the RUSTFMT env var.

If my understanding is incorrect, lets instead document why which is needed.

pvdrz commented 2 months ago

I agree with you, I don't see why we need which considering that Command already does the whole $PATH resolution itself. It could be just there for legacy reasons but there could be some reason why people still need it so I'm pinging @emilio in case he knows why.