Re: Q about git rev-parse {--is-inside-work-tree, --show-cdup}

Dirk Süsserott [off-list ref] writes:

Hi together,

I wrote some shell scripts that do sth with my git repositories. I place
my scripts in my ~/bin folder (not in the repo). So the first step in my
scripts is always to check whether they get called from whithin a git
repo and bail out if they don't.


I do this like so:

---------------------
if [ "$(git rev-parse --is-inside-work-tree)" = "true" ] # (1)
then
     here=$(pwd)
     cdup=$(git rev-parse --show-cdup);    # (2a)
     cdup=${cdup:-"."}                     # (2b)
     cd $cdup                              # (2c)

     [do sth useful from the topdir]

     cd $here
     exit 0;
else
     echo "Not inside a git working tree."
     exit 1;
fi
---------------------

I have two questions:

1. Wouldn't it be useful, if "git rev-parse" (1) had an option "-q" that
simply indicates whether "--is-inside-work-tree" is true by means of the
return code? Actually it has an option "-q" but that doesn't work with
"--is-inside-work-tree".

That would break existing scripts that expect "-q" to squelch only the
error output, no?  I think the risk of breaking existing scripts that
other people wrote over time that you (and I) haven't seen outweighs any
benefit (i.e. "if test $(rev-parse...) = true" vs "if rev-parse...") you
are seeing here.

Another thing to consider is what the script should do when rev-parse
detects an error.  Do we know that all scripts want to behave exactly the
same way in two cases: (1) when run outside the work tree; and (2) when
they cannot determine if they were run from inside or outside?  I don't
think so.

In your example, you are only interested to work inside work tree, and you
may find "if rev-parse... then do this interesting thing else fail fi"
sufficient, but a script by somebody else may want to make sure that it is
not run inside any git controlled working tree, and it cannot say "if
rev-parse then punt else do this big thing fi" is not an appropriate way
to write it.

So in that sense, "if rev-parse ..." is not a huge improvement to begin
with, for people who want to write their script strictly.  They would
probably need to say something like:

	ans=$(git rev-parse ...) || {
		do the error thing
                exit 1
	}
       	case "$ans" in
        true)
		do the inside-work-tree thing ;;
	false)
		do the outside-work-tree thing ;;
	*)
        	do the oops thing;;
	esac

On Fri, Dec 03, 2010 at 01:25:36PM -0800, Junio C Hamano wrote:

quoted

1. Wouldn't it be useful, if "git rev-parse" (1) had an option "-q" that
simply indicates whether "--is-inside-work-tree" is true by means of the
return code? Actually it has an option "-q" but that doesn't work with
"--is-inside-work-tree".

That would break existing scripts that expect "-q" to squelch only the
error output, no?  I think the risk of breaking existing scripts that
other people wrote over time that you (and I) haven't seen outweighs any
benefit (i.e. "if test $(rev-parse...) = true" vs "if rev-parse...") you
are seeing here.

Right now "-q" doesn't do _anything_ for --is-inside-work-tree, AFAICT.
It is a useless no-op. So I don't know if we are breaking anybody. What
does somebody doing "git rev-parse -q --is-inside-work-tree" expect to
happen?

I don't see why they would expect it to suppress error output. Usually
"-q" is about "suppress non-essential output, but keep errors coming".
If you wanted to suppress errors, you would use "2>/dev/null".

That being said, in my original reply I only half-thought about Dirk's
problem, and considered more the number of times "git rev-parse -q" has
annoyed me in the past by doing nothing[1], and just assumed this was
another such case. It really isn't that hard to just check $(git
rev-parse) in this instance.

-Peff

[1] I wish I could remember my exact case. It's something that I
remember coming up no more than once every month or two, but that annoys
me every time, because it doesn't do what I expect.

-Peff

Am 03.12.2010 22:50 schrieb Jeff King:

On Fri, Dec 03, 2010 at 01:25:36PM -0800, Junio C Hamano wrote:

quoted

That would break existing scripts that expect "-q" to squelch only the
error output, no?  I think the risk of breaking existing scripts that
other people wrote over time that you (and I) haven't seen outweighs any
benefit (i.e. "if test $(rev-parse...) = true" vs "if rev-parse...") you
are seeing here.

Right now "-q" doesn't do _anything_ for --is-inside-work-tree, AFAICT.
It is a useless no-op. So I don't know if we are breaking anybody. What
does somebody doing "git rev-parse -q --is-inside-work-tree" expect to
happen?

Peff, Junio, thanks for your answers.

I already had a suspicion that changing plumbing tools like
rev-parse was a bad idea. You confirmed that. However, I still
think it's a little bug that "--show-dup" returns an empty string
instead of a dot when already in topdir. But it's too late to
change that, I guess. I didn't know about Peff's suggestions
"git-sh-setup" and "--show-toplevel". They may help me.

@Peff, right, it's annoying that -q sometimes works and
sometimes doesn't. To my opinion switches like --quiet and
--verbose should _always_ work.

     Dirk