Handle deleted folder

[sindresorhus]

If I'm in a git folder on the shell, then delete it outside the shell, I start getting the following messages:

❯ prompt_pure_check_git_arrows:5: bad math expression: operand expected at `> 0 '
prompt_pure_check_git_arrows:6: bad math expression: operand expected at `> 0 '

We should probably handle that case.

[mafredri]

For some reason I can't seem to reproduce this, could you provide reproduction steps? I'll look into handling the comparison in prompt_pure_check_git_arrows() with greater care.

[mafredri]

This change should fix it, but I'd like to reproduce it myself and see if there are other potential problems associated with it.

diff --git a/pure.zsh b/pure.zsh
index 1a02398..dd518f6 100644
--- a/pure.zsh
+++ b/pure.zsh
@@ -52,8 +52,14 @@ prompt_pure_check_git_arrows() {
        command git rev-parse --abbrev-ref @'{u}' &>/dev/null || return

        local arrows=""
-       (( $(command git rev-list --right-only --count HEAD...@'{u}' 2>/dev/null) > 0 )) && arrows='⇣'
-       (( $(command git rev-list --left-only --count HEAD...@'{u}' 2>/dev/null) > 0 )) && arrows+='⇡'
+       local right
+       local left
+
+       right=$(command git rev-list --right-only --count HEAD...@'{u}' 2>/dev/null)
+       left=$(command git rev-list --left-only --count HEAD...@'{u}' 2>/dev/null)
+
+       (( ${right:-0} > 0 )) && arrows='⇣'
+       (( ${left:-0} > 0 )) && arrows+='⇡'
        # output the arrows
        [[ "$arrows" != "" ]] && echo " ${arrows}"
 }

[zmwangx]

I was able to reproduce with a bit of trick.

If we look at prompt_pure_check_git_arrows,

prompt_pure_check_git_arrows() {
    # check if there is an upstream configured for this branch
    command git rev-parse --abbrev-ref @'{u}' &>/dev/null || return

    local arrows=""
    (( $(command git rev-list --right-only --count HEAD...@'{u}' 2>/dev/null) > 0 )) && arrows="${PURE_GIT_DOWN_ARROW:-⇣}"
    (( $(command git rev-list --left-only --count HEAD...@'{u}' 2>/dev/null) > 0 )) && arrows+="${PURE_GIT_UP_ARROW:-⇡}"
    # output the arrows
    [[ "$arrows" != "" ]] && echo " ${arrows}"
}

we would notice that we already have a safeguard in place: the function immediately returns if there's any error checking the upstream of the current branch. So in order to get the errors here, the directory needs to be deleted after the safeguard and before or in the middle of the git rev-list calls. That requires extraordinary timing, and the only hope for reproduction is to have slow git rev-list calls. Therefore, I patched pure.zsh with a sleep call to simulate a slow git rev-list, and also dropped a few line of logs to fine-grain the control of timing. The patch to be applied:

diff --git a/pure.zsh b/pure.zsh
index af4cd24..1515ef9 100644
--- a/pure.zsh
+++ b/pure.zsh
@@ -51,6 +51,10 @@ prompt_pure_check_git_arrows() {
    # check if there is an upstream configured for this branch
    command git rev-parse --abbrev-ref @'{u}' &>/dev/null || return

+   echo "finished parsing upstream branch" >>/tmp/pure.log
+   sleep 5
+   echo "began parsing left/right" >>/tmp/pure.log
+
    local arrows=""
    (( $(command git rev-list --right-only --count HEAD...@'{u}' 2>/dev/null) > 0 )) && arrows="${PURE_GIT_DOWN_ARROW:-⇣}"
    (( $(command git rev-list --left-only --count HEAD...@'{u}' 2>/dev/null) > 0 )) && arrows+="${PURE_GIT_UP_ARROW:-⇡}"

After applying this patch I was able to reproduce the issue by monitoring the log and only removing the directory when the function is sleeping (i.e., inside the simulated slow git rev-list call).

My screencast:

Conclusion: I think the fix proposed in #137 (comment) should fix the problem, and wouldn't have any bad ramifications.

[zmwangx]

I was evaluating the risk of other git commands, and noticed that two other calls are also vulnerable under extremely unfortunate timing.

prompt_pure_async_git_dirty() {
    local untracked_dirty=$1; shift

    # use cd -q to avoid side effects of changing directory, e.g. chpwd hooks
    cd -q "$*"

    if [[ "$untracked_dirty" == "0" ]]; then
        command git diff --no-ext-diff --quiet --exit-code
    else
        test -z "$(command git status --porcelain --ignore-submodules -unormal)"
    fi

    (( $? )) && echo "*"
}

If the directory is removed before or mid-run during the two status commands, then instead of a single * we can get garbage into the preprompt, something like this:

This is because the stderr of prompt_pure_async_git_dirty is also passed to prompt_pure_async_callback as part of the "result" (which is supposed to be either empty or *). This is more of a problem in async.zsh, and I have discussed that in an issue there: mafredri/zsh-async#1. However, for our particular case at hand, it is trivial to mitigate the risk by appending 2>/dev/null to the two git calls.

Other git calls, as far as I can tell, are not vulnerable.

[mafredri]

@zmwangx thanks for investigating this! With this in mind I also think my patch will be sufficient.

I was evaluating the risk of other git commands, and noticed that two other calls are also vulnerable under extremely unfortunate timing.

I was reluctant to hide the error messages from the user as I thought it might be better to know when something went wrong, but I think it's time we didn't cause distress for the users with this. An update is in the works on zsh-async, once that's imported I think we will not need to touch a thing in pure.

[zmwangx]

An update is in the works on zsh-async, once that's imported I think we will not need to touch a thing in pure.

Yeah, we can leave it here for now.