Re: [PATCH] Teach git to change to a given directory using -C option

From: Jeff King <peff@peff.net>
Date: Apr 19, 2013, 16:12 UTC
Message-ID: <20130419161250.GC14263@sigill.intra.peff.net>
In-Reply-To: <1366374108-23725-1-git-send-email-ayiehere@gmail.com>

On Fri, Apr 19, 2013 at 08:21:48PM +0800, Nazri Ramliy wrote:

Show 16 quoted lines

> This is similar in spirit to to "make -C dir ..." and "tar -C dir ...".
> 
> Signed-off-by: Nazri Ramliy <ayiehere@gmail.com>
> ---
> Often I find myself needing to find out quickly the status of a repository that
> is not in my currenct working directory, like this:
> 
>          $ (cd ~/foo; git log -1)
> 
> With this patch now i can simply do:
> 
>          $ git -C ~/.zsh log -1 
> 
> That's just one example. I think those who are familiar with the -C arguments
> to "make" and "tar" commands would get the "handiness" of having this option in
> git.

This motivation should probably go into the commit message.

I think it's worth pausing for a moment and considering if we can do this already with existing features.

You can _almost_ do this with "git --git-dir". But it expects the actual git directory, not a starting point for finding the git directory. And it remains in your same working dir. So with a bare repository, these two are equivalent:

  $ git --git-dir=/path/to/foo.git ...
  $ git -C /path/to/foo.git ...

But with a non-bare repo, this does not work:

  $ git --git-dir=/path/to/non-bare ...

You must instead say:

  $ git --git-dir=/path/to/non-bare/.git ...

and even then, I think it will treat your current directory as the working tree, not /path/to/non-bare.

So I think "-C" is a worthwhile addition compared to just "--git-dir".

It is redundant with "(cd foo && git ...)" in the shell, as you note, but sometimes it is more convenient to use "-C" (especially if you are exec-ing git from another program and want to avoid the shell entirely for quoting reasons).

Show 11 quoted lines

> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 6a875f2..20bba86 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -379,6 +379,9 @@ displayed. See linkgit:git-help[1] for more information,
>  because `git --help ...` is converted internally into `git
>  help ...`.
>  
> +-C <directory>::
> +	Change to given directory before doing anything else.
> +

It might make sense to clarify this as "...anything else, including determining the location of the git repository directory". If you think hard about it, doing anything else would not really make much sense, but spelling it out makes it clear what the option can be used for.

> +		if (!prefixcmp(cmd, "-C")) {

Should this be strcmp? You do not seem to handle "-Cfoo" below.

> +			if (*argc < 2) {
> +				fprintf(stderr, "No directory given for -C.\n" );
> +				usage(git_usage_string);
> +			}

I know you are copying this from the other options in the same function, but I wonder if they should all be calling "error()" (and dropping the terminating ".") to better match our usual error messages.

> +			if (chdir((*argv)[1]))
> +				die_errno("Cannot change to '%s'", (*argv)[1]);
> +			(*argv)++;
> +			(*argc)--;

You would want to set "*envchanged = 1" here. The intent of that flag is that git would need to throw away things it has looked up already (like the git dir) in order to correctly utilize the options (and since we haven't implemented that "throw away" step, it just complains and dies).

I didn't try it, but I suspect your patch would be broken with:

  $ git config alias.logfoo '-C /path/to/foo log'
  $ cd /some/other/repo
  $ git logfoo

It would still use /some/other/repo as a $GIT_DIR, having looked it up before processing the "-C".

-Peff