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

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

[Dirk Süsserott]

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".

2. Wouldn't it be useful, if "git rev-parse --show-cdup" (2a) would
return a dot "." instead of nothing if we are already in the topdir?
That would make the steps (2a), (2b), (2c) to a simple "cd $(git
rev-parse --show-cdup)".

(For those who aren't familiar with shell programming: (2b) means: 'if
$cdup is empty, then set it to "."'.)

I know that 2. is a problem, because "git rev-parse" is a plumbing tool.
People likely rely on "--show-dup" to return an empty string. But what
about 1.? I think there's no harm if "-q" would work for
"--is-inside-work-tree" as well.

BTW: I'm sorry that all my sentences start with "I". ;-)

What do you think? If appreciated, I'd like to work on a patch for (1).

Cheers,
     Dirk

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

[Jeff King]

On Fri, Dec 03, 2010 at 10:06:36PM +0100, Dirk Süsserott wrote:

> 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
> ---------------------

Since you are writing shell scripts, you may be interested in doing:

  . git-sh-setup
  require_work_tree
  cd_to_toplevel

> 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".

Yeah, in general rev-parse's "-q" is annoying. It only works with
checking revisions (like "git rev-parse -q --verify HEAD"), and it
probably should apply to a lot more. Patches welcome.

> 2. Wouldn't it be useful, if "git rev-parse --show-cdup" (2a) would
> return a dot "." instead of nothing if we are already in the topdir?
> That would make the steps (2a), (2b), (2c) to a simple "cd $(git
> rev-parse --show-cdup)".

I'm hesitant to change it, as that is a public interface for shell
scripts. However, there is also "--show-toplevel" which will do what you
want in one line (or just cd_to_toplevel as I mentioned above).

> What do you think? If appreciated, I'd like to work on a patch for (1).

Please do.

-Peff

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

[Junio C Hamano]

Dirk Süsserott <newsletter@dirk.my1.cc> 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

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

[Jeff King]

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

> > 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

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

[Dirk Süsserott]

Am 03.12.2010 22:50 schrieb Jeff King:
> On Fri, Dec 03, 2010 at 01:25:36PM -0800, Junio C Hamano wrote:
>
>> 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

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

[Andreas Schwab]

Dirk Süsserott <newsletter@dirk.my1.cc> writes:

> 2. Wouldn't it be useful, if "git rev-parse --show-cdup" (2a) would
> return a dot "." instead of nothing if we are already in the topdir?
> That would make the steps (2a), (2b), (2c) to a simple "cd $(git
> rev-parse --show-cdup)".

The string returned by --show-cdup is to be used as prefix for a
relative path.  If you want to use it standalone just append ".":

cd $(git rev-parse --show-cdup).

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."