Re: [PATCH 0/3] clone --local fixes

From: Jeff King <peff@peff.net>
Date: May 28, 2012, 05:36 UTC
Message-ID: <20120528053602.GA11174@sigill.intra.peff.net>
In-Reply-To: <7vsjemp20j.fsf@alter.siamese.dyndns.org>

On Sat, May 26, 2012 at 11:32:44PM -0700, Junio C Hamano wrote:

Show 5 quoted lines

> So your example "file:///" should *not* work even if --local is given,
> unless you happen to have a directory called "file:" in your current
> directory and it has path/to/repo subdirectory, which is a git repository.
> Specifically, the repository at /path/to/repo is not what the command line
> is naming.

I think it depends on the definition of "--local". If it means "when we are cloning without a URL, turn on the local optimizations", then yes, "file://" should not work. If it means "turn on local optimizations if this destination supports it", then it should.

The current behavior is ambiguous as to whether it is the first case, or whether it is the second, and it was simply buggy. The history you gave argues that the original intent was the former. But to me that is much less important than what is useful and least surprising to users.

There are basically three sane behaviors for "git clone --local file://":

  1. --local is silently ignored, because it means "if we are local,
     turn on optimizations" (though it has a horrible name, in that case)

  2. it's an error; you should not use --local with a URL

  3. it should use local optimizations (because file:// is local)

Is there a compelling reason not to do (3)? It seems to be the friendliest and least surprising to me.

I'll admit I don't care too much about this use case. People don't tend to type "file://..." when they could type the simpler thing, so I doubt anyone is hurting. I just don't see a reason not to make it work, besides the usual "it is a non-zero amount of code".

> I think we probably should stop advertising --local in the documentation
> and help text.

I kind of agree, and that was what I was going to do originally (instead of making it work). But I do think that "--no-local" (from my patch 3) is actually useful in practice, so I'd rather not drop the option from the documentation entirely.

-Peff