git diff fails with bad revision '' on OSX

[keatsfonam]

Check list

• [X] I have read through the README
• [X] I have the latest version of forgit
• [X] I have searched through the existing issues

Environment info

• OS
  • [ ] Linux
  • [X] Mac OS X
  • [ ] Windows
  • [ ] Others:
• Shell
  • [ ] bash
  • [x] zsh
  • [ ] fish

Problem / Steps to reproduce

versions:

git version 2.44.0
zsh 5.9 (x86_64-apple-darwin23.0)

Commit ab1116da0e11b90bdcd54f33f172d96bc7fbd392 broke git diff for my OSX system:

❯ bin/git-forgit diff
fatal: bad revision ''

Bad:

❯ git checkout ab1116da0e11b90bdcd54f33f172d96bc7fbd392
HEAD is now at ab1116d Refactor: Replace deferred git commands with regular git commands

❯ bin/git-forgit diff
fatal: bad revision ''

Previous commit (61db097badc66dd9f7ee78f5ae4731aeb0cba449):

❯ git checkout 61db097badc66dd9f7ee78f5ae4731aeb0cba449
Previous HEAD position was ab1116d Refactor: Replace deferred git commands with regular git commands
HEAD is now at 61db097 Refactor: Replace _forgit_emojify deferred code variable with a function
❯ bin/git-forgit diff

(no error)

Other commands seem to work I tested with bash GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin23) and it is also broken

[sandr01d]

@cjappl could you take a look, I'm unable to reproduce this on Linux. @keatsfonam is $FORGIT_DIFF_GIT_OPTS set in your environment?

[keatsfonam]

FORGIT_DIFF_GIT_OPTS is not set

OSX version is 14.4 (23E214)

[keatsfonam]

Issue seems to arise from this line https://github.com/wfxr/forgit/commit/ab1116da0e11b90bdcd54f33f172d96bc7fbd392#diff-e7404556b00c1c7af95effe409a0f05ded287236878c22735194ee0bc0b98d11R231

when the commits array is empty it is evaluated differently compared to with the string

# old behavior
❯ git diff --name-status $commits --
M       bin/git-forgit

# new behavior
❯ git diff --name-status "${commits[@]}" --
fatal: bad revision ''

[sandr01d]

Issue seems to arise from this line ab1116d#diff-e7404556b00c1c7af95effe409a0f05ded287236878c22735194ee0bc0b98d11R231

when the commits array is empty it is evaluated differently compared to with the string

# old behavior
❯ git diff --name-status $commits --
M       bin/git-forgit

# new behavior
❯ git diff --name-status "${commits[@]}" --
fatal: bad revision ''

Thanks @keatsfonam, very helpful! Does the following work for you or do you receive an error aswell:

git diff --name-status ${commits:+"${commits[@]}"} --

This appears to be an issue with the old bash version that OSX is using:

https://stackoverflow.com/questions/7577052/bash-empty-array-expansion-with-set-u

[keatsfonam]

It works:

❯ git diff --name-status ${commits:+"${commits[@]}"} --
M       bin/git-forgit

And looks like bash 3.2 is fairly ancient. I tested with 5.2 and master works properly as expected. Ideally it would support OSX out of the box

Thanks

[sandr01d]

And looks like bash 3.2 is fairly ancient. I tested with 5.2 and master works properly as expected.

It is, unfortunately we can't expect Apple to update Bash any time soon. Probably not at all.

Ideally it would support OSX out of the box

Absolutely, we're fixing this. I've created #375. Could you give it a try and let me know if it works for you?

[keatsfonam]

@sandr01d I tested out the PR. It is actually still broken for me

bash-3.2$ bin/git-forgit diff
fatal: bad revision ''

this is what the string looks like when echoed:

git diff --name-status  --

executing that directly into the shell works

this part now works directly in the shell without the vars set

bash-3.2$ git diff --name-status "${_forgit_diff_git_opts[@]}" ${commits[@]+"${commits[@]}"} -- "${files[@]}"
M       bin/git-forgit

It only seems to fail when running the script. I'm trying to see what the issue is. I had some success removing whitespace next to the commits

[keatsfonam]

To reproduce:

docker run --entrypoint "/usr/local/bin/bash" -it bash:3.2 -c "apk add git fzf && git clone https://github.com/sandr01d/forgit.git -b fix-gd-macos && cd forgit && bin/git-forgit diff"

....

fatal: bad revision ''