Good day.

I had found that FreeBSD's /bin/sh refuses to work with git 1.5.3.2.
correctly: no flags are recognized.  The details and fix are below.
I don't currently know if the Bash's behaviour is POSIXly correct
or the 'case' statement semantics is not very well defined.   But
the following patch fixes the things for the FreeBSD.

Here we go.

-----

Option parsing in the Git shell scripts uses the construct 'while
case "$#" in 0) break ;; esac; do ... done'.  This is neat, because
it needs no external commands invocation.  But in the case when
/bin/sh is not GNU Bash (for example, on FreeBSD) this cycle will
not be executed at all.

The fix is to add the case branch '*) : ;;'.  It also needs no
external commands invocation and it does its work, because ':'
always returns zero.

Signed-off-by: Eygene Ryabinkin <rea-git@codelabs.ru>
---
 git-am.sh                  |    2 +-
 git-clean.sh               |    2 +-
 git-commit.sh              |    2 +-
 git-fetch.sh               |    2 +-
 git-filter-branch.sh       |    2 +-
 git-instaweb.sh            |    2 +-
 git-ls-remote.sh           |    2 +-
 git-merge.sh               |    2 +-
 git-mergetool.sh           |    2 +-
 git-pull.sh                |    2 +-
 git-quiltimport.sh         |    2 +-
 git-rebase--interactive.sh |    2 +-
 git-rebase.sh              |    2 +-
 git-repack.sh              |    2 +-
 git-reset.sh               |    2 +-
 git-submodule.sh           |    2 +-
 16 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 6809aa0..0bd8d34 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -109,7 +109,7 @@ dotest=.dotest sign= utf8=t keep= skip= interactive= resolved= binary=
 resolvemsg= resume=
 git_apply_opt=
 
-while case "$#" in 0) break;; esac
+while case "$#" in 0) break;; *) : ;; esac
 do
 	case "$1" in
 	-d=*|--d=*|--do=*|--dot=*|--dote=*|--dotes=*|--dotest=*)
diff --git a/git-clean.sh b/git-clean.sh
index a5cfd9f..1fac731 100755
--- ...
[ message continues ]

I do not doubt that "while case $# in 0) break ;; esac" does not
work for your shell.  But I think the above comment is grossly
misleading.

Don't mention bash there.  You sound as if you are blaming
bashism, but the thing is, your shell is simply broken.

You have other choices than bash on BSD don't you?

My quick test shows that ksh, pdksh and dash seem to work
correctly.  This idiom is what I picked up around late 80's from
somebody, and kept using on many variants of Unices.  I would
find quite surprising that something that claims to be a shell
does not work correctly.  Even /bin/sh that comes with Solaris
seems to work correctly, which should tell you something.

OpenBSD's /bin/sh seems to be Ok; I do not know whose shell they
use, but it seems to be hard-linked to /bin/ksh which is pdksh.
-

[ message continues ]

Junio, good day.


OK, you're right.  Especially if /bin/sh from Solaris and OpenBSD
are working and they are not Bash.  But I would not tell that
the shell is broken now -- I had not seen the POSIX specification.

Did not understand the question, sorry.  The thing is that
FreeBSD has /bin/sh that is derived from the original Berkeley
shell.  And it is desirable to have it working with Git
script, since I don't want to make bash (or whatever shell

OK, I think I need to find out why FreeBSD's /bin/sh behaves
like this, because the test you propose on your next message
works.  See below.

By the way, my FreeBSD is 7-CURRENT, but I'll test on 6-STABLE
and perhaps on 4-STABLE on Monday.


It says 'case returned ok', so I will try to understand why it
works here and does not work in the 'while' construct.

Thanks for the pointer!
-- 
Eygene
-

[ message continues ]

I vaguely recall somebody else had exactly this issue and he
concluded that the shell was busted.  I do not recall the
details of the story but interestingly, if he did something that
accesses "$#" before the problematic "while case $# in ..." the
shell behaved for him in his experiments.

Just to make sure you do not misunderstand me, I am not trying
to be difficult.  I am trying to assess (1) if it is sensible to
support that broken shell, and (2) if so what the exact breakage
is, especially because as the above shows the breakage does not
look like what your "fix" literally suggests, and what is
involved in working it around.

Also by my comment about "/bin/sh and bash not being the only
shells available on FreeBSD", I did not mean that you should
change your /bin/sh.  You can build git with SHELL_PATH make
varilable pointing at a non-broken shell, which does not have to
be installed as /bin/sh.

-

[ message continues ]

If one's installing from the ports tree, then the port should depend on a
non-broken shell and set SHELL_PATH. And as for installing by hand, just print
out a warning that SHELL_PATH points to a broken shell and be done with it.
This is a FreeBSD bug, not a git one.

I had been meaning to write up a bug about this using a small test case, but I
couldn't reproduce it.


Adam
-

[ message continues ]

I have always been assuming it to be the case (this construct is
not my invention but is an old school idiom I just inherited
from my mentor) and never looked at the spec recently, but I
re-read it just to make sure.  The answer is yes.

Visit http://www.opengroup.org/onlinepubs/000095399/ and follow
"Shell and Utilities volume (XCU)" and then "Case conditional
construct".

    Exit Status

    The exit status of case shall be zero if no patterns are
    matched. Otherwise, the exit status shall be the exit status of
    the last command executed in the compound-list.

So, as David suggests, if

        false
        case Ultra in
        Super) false ;;
        Hyper) true ;;
        esac && echo case returned ok

does not say "case returned ok", then the shell has a bit of
problem.
-

[ message continues ]

Junio, *, good day.



Correct: the current /bin/sh for FreeBSD does not set zero exit
code if no case patterns were matched.  So, I apologize for my quick
decision on the non-brokenness of the /bin/sh -- it is broken.

I had fixed the shell and filed the problem report.  May be the
change will be incorporated into the future release of FreeBSD.
Meanwhile, I had added workarounds to the other places Junio mentioned
in his follow-up and will try to push this patch to the FreeBSD
port of Git.  The explanation had been changed too ;))

Thanks to all people who helped me to realize what is wrong and where!
-- 
Eygene
-

[ message continues ]

Independent of that: would you mind a patch replacing that idiom with

while : do case xxx) break; esac

instead?  I find breaking out of the condition rather than the body
awkward, and I find a non-matching case statement, POSIX or not, quite
unobvious in the place of a true while condition.

It is a bit too much of cleverness for my taste.  Never mind that the
current FreeBSD shell does not understand it due to being buggy: I
find that this is not very readable to the human reader either without
a double take.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum
-

[ message continues ]

I am assuming that this works around _a_ bug in that /bin/sh; I
would make sure I understand the nature of the bug.  Is it Ok to
understand that with that shell, after this construct runs:

	case <some word> in
        <case arm #1>)
        	something ;;
	<case arm #2>)
        	something else ;;
	esac

the status from the whole case statement is false, when <some word>
does not match any of the glob patterns listed in any of the case arm?

That is, what does the shell say if you do this?

	case Ultra in
        Super)
        	false ;;
	Hyper)
        	true ;;
	esac &&
        echo case returned ok

The reason I ask is because

	while case $# in 0) ... esac
        do
        	...
	done

is not the only place the status from "case" itself matters in
our scripts.  There are places that do

	something &&
	case ... in
        ...
        esac &&
        something else

and we would need to add no-op match-everything arm to all of
such case statements in our scripts.

Besides test scripts, there is one in git-ls-remote.sh which you
seem to have missed.
-

[ message continues ]