search

rust-lang / rust-installer

The Bourne shell installer used by Rust and Cargo
Apache License 2.0
60 stars 68 forks source link

Error with bash when CDPATH is used #31

Closed csw closed 9 years ago

csw commented 9 years ago

When attempting to install multirust, I got an odd sequence of errors from gen-installer.sh, starting with:

cat: /var/folders/m5/fbv0p869151632cb1fcbv69h0000gn/T/rustup-tmp-install.sDLfqclz/mult
irust/src/rust-installer
/var/folders/m5/fbv0p869151632cb1fcbv69h0000gn/T/rustup-tmp-install.sDLfqclz/multirust
/src/rust-installer/rust-installer-version: No such file or directory

I use Mac OS X, where /bin/sh is bash, and I have CDPATH set to a value including .. It turns out that the problem is with this shell script construction: $(cd $(dirname "$0") && pwd). (Really, any sort of $(cd $foo && pwd).) When dirname "$0" does not start with . or /, and CDPATH includes ., bash's cd builtin echoes the selected directory to stdout. (Arguably this is a bash bug, since this behavior makes no sense for non-interactive use...) Multirust triggers this by invoking gen-installer as src/rust-installer/gen-installer.sh.

This could be fixed by redirecting the output of cd to /dev/null ($(cd $(dirname "$0") >/dev/null && pwd)), or by adding unset CDPATH to the shell scripts. I'm not sure which would be preferable, stylistically, but I'd be happy to put a pull request together.

brson commented 9 years ago

Awesome analysis, thanks. I think that line exists in most of our shell scripts and I saw a similar patch go by recently for one. Let's go with redirecting cd to /dev/null.

brson commented 9 years ago

With a comment explaining why!

brson commented 9 years ago

Maybe unsetting CDPATH is the right thing to do. I don't see how letting the environment mess with our cd invocations can do anything but break.