MichaelAquilina/zsh-autoswitch-virtualenv

stat: bad file descriptor on execution of check_venv on OS X Mojave

madsenwattiq opened this issue · 11 comments

Issue Details

Please provide the following details when opening an issue:

Operating System (uname -a)

Darwin lewontinmadsenlaborg.local 18.6.0 Darwin Kernel Version 18.6.0: Thu Apr 25 23:16:27 PDT 2019; root:xnu-4903.261.4~2/RELEASE_X86_64 x86_64

zsh version (zsh --version)

zsh 5.5.1 (x86_64-apple-darwin17.5.0)

autoswitch-virtualenv version

1.14.0

How is zsh-autoswitch-virtualenv installed?

  • zplug
  • oh-my-zsh
  • Antigen
  • Other (please specify)

Steps to reproduce the issue

check_venv fails because "stat -f %u "$venv_path"" fails.

Simply doing "stat -f %u .venv" at command line gives:

stat: bad file descriptor

whereas "stat .venv" gives full output. Something is apparently different about the "stat" version syntax on newish (Mojave?) versions of OS X.

Hi @madsenibis
Could you check what the following command gives you?

stat -c %u .venv

On the current version of OS X, I get:

stat: bad option: -c

what about stat --version?

Hmm. Weird.

stat: no files given

Ah! I found the problem. If you're using ZSH as your shell, stat becomes a built in shell command with decidedly non-standard options. The original stat -f %u .venv works fine if I use the path to the actual Unix executable /usr/bin/stat. How about I give you a PR which figures out how stat should be called?

@madsenibis go for it :)

If possible please provide a test for it

Still thinking about the approach here -- it turns out that what is loading the zsh/stat plugin (and thus causing check_venv to fail given the different syntax), is a Kubernetes prompt-setting script. So it seems like perhaps the best generic approach would be to:
(a) check if zsh/stat is loaded before really starting the bulk of check_venv
(b) if so, call stat with an absolute path

would that be an acceptable approach, do you think?

rnc commented

@madsenibis @MichaelAquilina It already uses an explicit path for rm (/bin/rm) so my vote would be to keep it simple and just immediately use an explicit path ? On my Fedora system /bin is a symbolic link to /usr/bin - is /usr/bin is standard cross-platform location for stat?

What I'm testing is something where we determine the path for "stat" after determining if what's in the "path" is a shell builtin function first, which would find the executable non-shell version regardless of whether /bin or /usr/bin are separate or a symlink.

function _get_stat_path() {
    # default case is to use what's on the path
    local stat_executable="stat"

    # determine what's on the path
    local path_to_stat=`which stat`

    # if Unix stat is bypassed for zsh/stat module, ensure use of Unix stat
    if [[ $$path_to_stat =~ "built-in" ]];
    then
        stat_executable=`whence -p stat`
    fi

    printf "%s" "$stat_executable"
}

Would that work?

@madsenibis @MichaelAquilina It already uses an explicit path for rm (/bin/rm) so my vote would be to keep it simple and just immediately use an explicit path ? On my Fedora system /bin is a symbolic link to /usr/bin - is /usr/bin is standard cross-platform location for stat?

I would prefer this solution if we are confident this will be consistent across systems.

What I'm testing is something where we determine the path for "stat" after determining if what's in the "path" is a shell builtin function first, which would find the executable non-shell version regardless of whether /bin or /usr/bin are separate or a symlink.

Alternatively this would work, but I would change the function so that it works to execute directly

function _stat() {
    # all the code you wrote stores the result in $stat_path local variable
    "$stat_path" $@
}

@madsenibis would you mind testing out #121 to see if it works for you?

This works, thank you!