Re: [PATCH/alternative/raw and rough] setup.c: denote repo wide pathspecs by ':'

From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Mar 2, 2011, 17:31 UTC
Message-ID: <4D6E7EF0.5040106@drmicha.warpmail.net>
In-Reply-To: <7vhbblcvl7.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 02.03.2011 17:53:

Show 14 quoted lines

> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> [*] I keep forgetting that some people may have files whose names begin
>> with ":". They are ambiguous now already with "treeish pathspec"
>> commands, but not with "pathspec" commands. The latter would change.
> 
> Just to make sure I understand that they have easy workarounds:
> 
>  - If you have a path foo/:bar, you can say
> 
>    git log master -- foo/:bar
> 
>    because ':' signals the magic and gets stripped only when it is at the
>    beginning (i.e. not affecting foo/:bar); and

Yes.

Show 8 quoted lines

> 
>  - For :boz at the root level, you can say
> 
>    git log master -- '\:boz'
> 
>    because the backslash in '\:boz' makes the colon not at the beginning and
>    the glob match sees '\:boz' and then matches '\:' with literal ':' at the
>    beginning of the pathname ":boz".

Yes. Due to the shell escaping, a shorter way is

git log master -- ::boz

:)

Show 14 quoted lines

> 
> In very old times, git used to work only from the top-level of the working
> tree.
> 
> The way we give an illusion that a command is restricted within the
> current working directory was by learning the "prefix" returned by
> setup_git_directory() while it chdir(2)'s up to the root level of the
> working tree, and then by limiting the operation to the pathspec given
> from the command line (each of whose elements prefixed by "prefix" by
> calling get_pathspec()).
> 
> Your ':'-prefix trick will naturally work very well with this arrangement.
> Instead of prefixing the "prefix", you would just strip ':' from the front
> for such a magic pathspec element, and that should be all that is necessary.

and pretend prefix == NULL, exactly.

Show 11 quoted lines

> 
> There is a small worry, though.  Some codepaths have tricks that take
> advantage of the knowledge of the current behaviour that the resulting
> pathspec elements all refer to subtree under the "prefix", and try to
> optimize their tree traversal.  I think dir.c:fill_directory()'s use of
> common_prefix() is safe (it recomputes what is common based on the result
> of get_pathspec(), not blindly using the original "prefix"), but we need
> to make sure there isn't a codepath that blindly believes that the
> original "prefix" defines the extent of the operation.  Anything that
> understands "../" to step outside the cwd should be already safe, so I
> hopefully am being worried too much.

Except for rerere, I've tried all callers, and all work. ls-tree is a bit strange, but that was true already for rev:path. I think it's OK if ls-tree does not grok this, but I'll have another look.

Show 6 quoted lines

> 
> Earlier, the list consensus was that if we were to aim for uniformity, we
> should make everything relative to the root of the working tree when there
> is no pathspec by default, because you can always give a single '.' to
> restrict the extent of the operation to the cwd, but you cannot extend the
> extent of the operation without tediously counting "../".

Hadn't we decided there were exceptions (e.g. grep), and there weren't that many suggested changes (to repo-wide) left?

>  Would this ':'
> trick affect that argument?  If a command is relative to the cwd with no
> pathspec, you can now give a single ':' to affect the whole tree.

In my view yes. I would even say: If we don't change every single command to repo-wide default there is no need to change (and break things) if we have an easy one-character way of saying "repo-wide".

Show 12 quoted lines

> 
> As I wrote in my response to Jeff in
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/133570/focus=133874
> 
> I always thought that it would be the best solution that makes the choice
> of the default irrelevant, and this ":" trick certainly feels like this is
> that solution (I also think having a good default matters).
> 
> And we can start thinking about deprecating --full-tree option, no?  I
> like that, too ;-).
>

:)

Michael