Re: [PATCH] -C/--chdir command line option
- From
Maciej Pasternacki <maciej@pasternacki.net>
- Date
- Oct 19, 2008, 13:47 UTC
- Message-ID
- <86685067-021C-4DC5-A462-AA6834208BDE@pasternacki.net>
- In-Reply-To
- <20081019131745.GA8643@coredump.intra.peff.net>
On 2008-10-19, at 15:17, Jeff King wrote:
Show 23 quoted lines
> On Sat, Oct 18, 2008 at 05:02:27PM -0700, Maciej Pasternacki wrote: > >> 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").
The point of make -C is exactly my usage pattern: invoking make from external program. I have seen, at least in Lisp (external-program) and Python (os.spawn, subprocess), libraries to safely and portably execute external program by giving literal list of arguments (an easy to use wrapper to fork()+exec(); I don't know if such feature exists for C). This allows me not to worry about quoting strange characters in pathnames (think injections, if just making it possible to use "&" character in directory name is not good enough a reason). The equivalent of "cd dir && make" would be to use system() or invoke sh - c; both would require shell-quoting the directory pathname, and would carry portability issues -- how do I know that shell on target system is actually called "sh" and it supports "&&"? At least Windows systems would be problematic.
The -C option allows me to just specify the directory as it is, and child process, after the fork, will take care of the chdir(). No quoting, no portability problems, injection-safety included.
As for -C being superfluous: --git-dir and --work-tree seem to support weird usage patterns (like work tree separate from git-dir), but it seems to me that --work-tree could be just made a synonym of -C, and it would behave as expected without creating chicken-and-egg problem that doener on #git found (which I don't really understand, but I also can't see what --work-tree allows that -C would not -- except creating more fragility). I won't push on --work-tree and -C being functionally synonymous, since I don't know git well enough, it's just my impression at the moment.
> 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.
ACK. That was how I understood instructions linked from git.or.cz, I'll paste message correctly for next version of the patch.
Show 7 quoted lines
>> + } 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.Oops. Looks like I'm spoiled by high-level languages that would automatically catch and display an error. I'll prepare patch v2 today.
> Also, the envchanged flag should probably be set, as for the git-dir > and > work-tree options.
OK. I thought it means literally environment change, as in setenv().
Thank you for your comments, off to make v2 patch, Maciej.
-- -><- Maciej Pasternacki -><- http://www.pasternacki.net/ -><-