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

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

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

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

  . git-sh-setup
  require_work_tree
  cd_to_toplevel

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.

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

Please do.

-Peff

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

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:

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:

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

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

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.