Re: [PATCH] -C/--chdir command line option

From: Jeff King <peff@peff.net>
Date: Oct 19, 2008, 13:17 UTC
Message-ID: <20081019131745.GA8643@coredump.intra.peff.net>
In-Reply-To: <20081019000227.GA9423@charybdis.dreamhost.com>

On Sat, Oct 18, 2008 at 05:02:27PM -0700, Maciej Pasternacki wrote:

Show 10 quoted lines

> In my project cl-librarian, which is a layer built upon different
> version control systems, I need to run git pull, but start it from
> outside of work tree; however, pull needs to be in work tree (as in
> getcwd()) in order to update said work tree.  --git-dir and
> --work-tree options don't work; I discussed it on #git yesterday,
> and it turned out that this issue is nontrivial:
> [...]
> the best workaround that occured to me was adding -C/--chdir option to
> main git binary, like one that Make accepts.  This fixed my problem, I
> paste the resulting patch below:

I'm not completely opposed to -C, as it does match some other tools, but it does seem superfluous with --git-dir and --work-tree. Which makes me feel like we are just papering over a bug instead of actually fixing it (and to be honest, I always wondered what the point of "make -C" was in the face of "cd dir && make").

I'm not sure if the actual problem is related to the oft-discussed, unresolved work-tree startup woes, or is something much simpler to fix. I'll try to look closer later today.

A few comments on your patch:

Show 6 quoted lines

> From 4461cd1be99772c93a83bcdfe6e6a86e71abaa35 Mon Sep 17 00:00:00 2001
> From: Maciej Pasternacki <maciej@pasternacki.net>
> Date: Sun, 19 Oct 2008 01:33:49 +0200
> Subject: [PATCH] Add command line option --chdir/-C to allow setting git process work directory.
> 
> Signed-off-by: Maciej Pasternacki <maciej@pasternacki.net>

Please follow the usual list conventions; this stuff should be the actual headers of your mail, not in the body of the message. And some of the justification you gave above should be part of the commit message.

> +		} else if (!strcmp(cmd, "-C") || !strcmp(cmd, "--chdir")) {
> +                        chdir((*argv)[1]);

No error detection and reporting? I think I would be quite happy for "git -C /some/path clean -dx" to bail out when it saw that it couldn't change directory instead of just deleting the contents of wherever I happened to be.

Also, the envchanged flag should probably be set, as for the git-dir and work-tree options.

-Peff