Re: git add -A fails in empty repository since 1.8.5

[Junio C Hamano <gitster@pobox.com>]

Duy Nguyen <pclouds@gmail.com> writes:

> On Wed, Dec 18, 2013 at 3:44 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
>> FWIW, git-bisect points to 84b8b5d (that is $gmane/230349).
>>
>> On Wed, Dec 18, 2013 at 9:06 AM, Thomas Ferris Nicolaisen
>> <tfnico@gmail.com> wrote:
>>> This was discussed on the Git user list recently [1].
>>>
>>> #in a repo with no files
>>>> git add -A
>>> fatal: pathspec '.' did not match any files
>>>
>>> The same goes for git add . (and -u).
>>>
>>> Whereas I think some warning feedback is useful, we are curious
>>> whether this is an intentional change or not.
>
> I was not aware of this case when I made the change. It's caused by
> this change that removes pathspec.raw[i][0] check in builtin/add.c in
> 84b8b5d .
>
> -               for (i = 0; pathspec.raw[i]; i++) {
> -                       if (!seen[i] && pathspec.raw[i][0]
> -                           && !file_exists(pathspec.raw[i])) {
> +               for (i = 0; i < pathspec.nr; i++) {
> +                       const char *path = pathspec.items[i].match;
> +                       if (!seen[i] && !file_exists(path)) {

Isn't that pathspec.raw[i][0] check merely an attempt to work around
the combination of

 (1) "the current directory" pathspec "." is sanitized down to an
     empty string by the pathspec code; and

 (2) even though file_exists() is willing to say "yes" to a non-file
     (namely, a directory), it is not prepared to take an empty
     string resulting from (1) to mean "the directory .".

> Adding it back requires some thinking because "path" in the new code
> could be something magic..

Ehh, why?  Shouldn't "something magic" that did _not_ match
(i.e. not in seen[]) diagnosed as such?

I am wondering why we even need !file_exists(path) check there in
the first place.  We run fill_directory() and then let
prune_directory() report which pathspec did not have any match via
the seen[] array.  We also match pathspec against the index to see
if there are pathspec that does not match anything.  So at that
point of the codeflow, we ought to be able to make sure that seen[]
is the _only_ thing we need to consult to see if there are any
pathspec elements that did not match.

Stepping back even further, I wonder if this "yes, I found a
matching entity and know this is not an end-user typo" bit actually
should be _in_ "struct pathspec".  Traditionally we implemented that
bit as a separate seen[] array parallel to "const char **pathspec"
array, but that was merely because we only had the list of strings.
Now we express a pathspec as a list of "struct pathspec" elements,
I think seen[] can and should become part of the pathspec.  Am I
missing something?


> and the new behavior makes sense, so I'm
> inclined to keep it as is, unless people have other opinions.
>
>>>
>>> [1] https://groups.google.com/d/topic/git-users/Qs4YSPhTsqE/discussion
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe git" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html