git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* git add -A fails in empty repository since 1.8.5
@ 2013-12-18  8:06 Thomas Ferris Nicolaisen
  2013-12-18  8:44 ` Antoine Pelisse
  2013-12-23  9:02 ` [PATCH] add: don't complain when adding empty project root Nguyễn Thái Ngọc Duy
  0 siblings, 2 replies; 18+ messages in thread
From: Thomas Ferris Nicolaisen @ 2013-12-18  8:06 UTC (permalink / raw)
  To: git@vger.kernel.org

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.

[1] https://groups.google.com/d/topic/git-users/Qs4YSPhTsqE/discussion

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: git add -A fails in empty repository since 1.8.5
  2013-12-18  8:06 git add -A fails in empty repository since 1.8.5 Thomas Ferris Nicolaisen
@ 2013-12-18  8:44 ` Antoine Pelisse
  2013-12-18 11:59   ` Duy Nguyen
  2013-12-23  9:02 ` [PATCH] add: don't complain when adding empty project root Nguyễn Thái Ngọc Duy
  1 sibling, 1 reply; 18+ messages in thread
From: Antoine Pelisse @ 2013-12-18  8:44 UTC (permalink / raw)
  To: Thomas Ferris Nicolaisen, Nguyễn Thái Ngọc Duy
  Cc: git@vger.kernel.org

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.
>
> [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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: git add -A fails in empty repository since 1.8.5
  2013-12-18  8:44 ` Antoine Pelisse
@ 2013-12-18 11:59   ` Duy Nguyen
  2013-12-18 18:54     ` Junio C Hamano
  2013-12-18 20:57     ` Junio C Hamano
  0 siblings, 2 replies; 18+ messages in thread
From: Duy Nguyen @ 2013-12-18 11:59 UTC (permalink / raw)
  To: Antoine Pelisse; +Cc: Thomas Ferris Nicolaisen, git@vger.kernel.org

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)) {

Adding it back requires some thinking because "path" in the new code
could be something magic.. 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



-- 
Duy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: git add -A fails in empty repository since 1.8.5
  2013-12-18 11:59   ` Duy Nguyen
@ 2013-12-18 18:54     ` Junio C Hamano
  2013-12-18 19:24       ` Matthieu Moy
  2013-12-18 20:57     ` Junio C Hamano
  1 sibling, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2013-12-18 18:54 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Antoine Pelisse, Thomas Ferris Nicolaisen, git@vger.kernel.org

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.

The logic to produce that error message is primarily to catch a typo
like:

	$ git add Nakefile

when the user meant to say Makefile.

It could be argued that a "git add [<any option>] .", with an
explicit "." given by the end-user, that is run in an empty
directory may be an error worth reporting.  Just like it is likely
for the user to have wanted to add some other file when he typed
Nakefile and it is not good to silently decide "ah, nothing matches
the pathspec, so not adding anything is the right thing to do" in
such a case, it is plausible that the user thought that he was in
some other directory he wanted to add its contents to the index when
he gave us the explicit ".", while he was in fact in a wrong
directory, and it is not good to silently decide "nothing there to
add so I won't do anything" without any indication of an error.

We should *not* error out "git add [<any option>]" without any
end-user pathspecs, especially with that error message, on the other
hand.

> 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)) {
>
> Adding it back requires some thinking because "path" in the new code
> could be something magic.. 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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: git add -A fails in empty repository since 1.8.5
  2013-12-18 18:54     ` Junio C Hamano
@ 2013-12-18 19:24       ` Matthieu Moy
  2013-12-18 19:56         ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Matthieu Moy @ 2013-12-18 19:24 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Duy Nguyen, Antoine Pelisse, Thomas Ferris Nicolaisen,
	git@vger.kernel.org

Junio C Hamano <gitster@pobox.com> writes:

> It could be argued that a "git add [<any option>] .", with an
> explicit "." given by the end-user, that is run in an empty
> directory may be an error worth reporting.

But what we have right now is really weird:

# setup repo with one empty dir:
$ rm -fr test
$ git init test
Initialized empty Git repository in /tmp/test/.git/
$ cd test
$ mkdir foo

$ git add .
fatal: pathspec '.' did not match any files

=> The one we're discussing.

$ git add foo

=> No error when an empty directory other than . is given.

$ cd foo
$ git add .

=> No error either for "git add ." when not at the root of the repo.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: git add -A fails in empty repository since 1.8.5
  2013-12-18 19:24       ` Matthieu Moy
@ 2013-12-18 19:56         ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2013-12-18 19:56 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Duy Nguyen, Antoine Pelisse, Thomas Ferris Nicolaisen,
	git@vger.kernel.org

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> It could be argued that a "git add [<any option>] .", with an
>> explicit "." given by the end-user, that is run in an empty
>> directory may be an error worth reporting.
>
> But what we have right now is really weird:

I know.  That is why I said "It _could_ be argued".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: git add -A fails in empty repository since 1.8.5
  2013-12-18 11:59   ` Duy Nguyen
  2013-12-18 18:54     ` Junio C Hamano
@ 2013-12-18 20:57     ` Junio C Hamano
  2013-12-19  0:49       ` Duy Nguyen
  1 sibling, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2013-12-18 20:57 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Antoine Pelisse, Thomas Ferris Nicolaisen, git@vger.kernel.org

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: git add -A fails in empty repository since 1.8.5
  2013-12-18 20:57     ` Junio C Hamano
@ 2013-12-19  0:49       ` Duy Nguyen
  0 siblings, 0 replies; 18+ messages in thread
From: Duy Nguyen @ 2013-12-19  0:49 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Antoine Pelisse, Thomas Ferris Nicolaisen, git@vger.kernel.org

On Thu, Dec 19, 2013 at 3:57 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 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 .".

Yeah, and it was added so intentionally in 07d7bed (add: don't
complain when adding empty project root - 2009-04-28). So this is a
regression.

>> 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.

See e96980e (builtin-add: simplify (and increase accuracy of) exclude
handling - 2007-06-12). It has something to do with directory check
originally, then we don't care about S_ISDIR() any more and turn it to
file_exists(). Maybe it's safe to remove it now. Need to check
fill_directory() again..

> 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?

Yes it probably better belongs to struct pathspec. Turning it into 1
flag would simplify seen[] memory management too.

>
>
>> 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



-- 
Duy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] add: don't complain when adding empty project root
  2013-12-18  8:06 git add -A fails in empty repository since 1.8.5 Thomas Ferris Nicolaisen
  2013-12-18  8:44 ` Antoine Pelisse
@ 2013-12-23  9:02 ` Nguyễn Thái Ngọc Duy
  2013-12-23 17:48   ` Torsten Bögershausen
  2013-12-26 17:25   ` Jonathan Nieder
  1 sibling, 2 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2013-12-23  9:02 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, tfnico, Nguyễn Thái Ngọc Duy

This behavior was added in 07d7bed (add: don't complain when adding
empty project root - 2009-04-28) then broken by 84b8b5d (remove
match_pathspec() in favor of match_pathspec_depth() -
2013-07-14). Reinstate it.

Noticed-by: Thomas Ferris Nicolaisen <tfnico@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/add.c  | 2 +-
 t/t3700-add.sh | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/builtin/add.c b/builtin/add.c
index 226f758..fbd3f3a 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -544,7 +544,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 		for (i = 0; i < pathspec.nr; i++) {
 			const char *path = pathspec.items[i].match;
-			if (!seen[i] &&
+			if (!seen[i] && pathspec.items[i].match[0] &&
 			    ((pathspec.items[i].magic &
 			      (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||
 			     !file_exists(path))) {
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index aab86e8..1535d8f 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -307,4 +307,8 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out
 	test_i18ncmp expect.err actual.err
 '
 
+test_expect_success 'git add -A on empty repo does not error out' '
+	git init empty && ( cd empty && git add -A . )
+'
+
 test_done
-- 
1.8.5.2.240.g8478abd

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] add: don't complain when adding empty project root
  2013-12-23  9:02 ` [PATCH] add: don't complain when adding empty project root Nguyễn Thái Ngọc Duy
@ 2013-12-23 17:48   ` Torsten Bögershausen
  2013-12-23 23:46     ` Duy Nguyen
  2013-12-26 17:25   ` Jonathan Nieder
  1 sibling, 1 reply; 18+ messages in thread
From: Torsten Bögershausen @ 2013-12-23 17:48 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy, git; +Cc: Junio C Hamano, tfnico

On 2013-12-23 10.02, Nguyễn Thái Ngọc Duy wrote:
> This behavior was added in 07d7bed (add: don't complain when adding
> empty project root - 2009-04-28) then broken by 84b8b5d (remove
> match_pathspec() in favor of match_pathspec_depth() -
> 2013-07-14). Reinstate it.
> 
> Noticed-by: Thomas Ferris Nicolaisen <tfnico@gmail.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/add.c  | 2 +-
>  t/t3700-add.sh | 4 ++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/add.c b/builtin/add.c
> index 226f758..fbd3f3a 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -544,7 +544,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  
>  		for (i = 0; i < pathspec.nr; i++) {
>  			const char *path = pathspec.items[i].match;
> -			if (!seen[i] &&
> +			if (!seen[i] && pathspec.items[i].match[0] &&
>  			    ((pathspec.items[i].magic &
>  			      (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||
>  			     !file_exists(path))) {
> diff --git a/t/t3700-add.sh b/t/t3700-add.sh
> index aab86e8..1535d8f 100755
> --- a/t/t3700-add.sh
> +++ b/t/t3700-add.sh
> @@ -307,4 +307,8 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out
>  	test_i18ncmp expect.err actual.err
>  '
>  
> +test_expect_success 'git add -A on empty repo does not error out' '
> +	git init empty && ( cd empty && git add -A . )
> +'
> +
>  test_done
> 
I am (a little bit) confused.

This is what git does:
 rm -rf test && mkdir test && cd test && git init && touch A && mkdir D && cd D && touch B && git add . && git status
Initialized empty Git repository in /Users/tb/test/test/.git/
On branch master

Initial commit

Changes to be committed:
  (use "git rm --cached <file>..." to unstage)

        new file:   B

Untracked files:
  (use "git add <file>..." to include in what will be committed)

        ../A
----------------
And the behaviour is in line with
https://www.kernel.org/pub/software/scm/git/docs/git-add.html

"." stands for the current directory somewhere in the worktree,
not only the "project root".
-----------------

Could it make sense to mention that replace
[PATCH] add: don't complain when adding empty project root
with
[PATCH] add: don't complain when adding empty directory.

(and similar in the commit message)
/Torsten

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] add: don't complain when adding empty project root
  2013-12-23 17:48   ` Torsten Bögershausen
@ 2013-12-23 23:46     ` Duy Nguyen
  2013-12-24 21:48       ` Torsten Bögershausen
  0 siblings, 1 reply; 18+ messages in thread
From: Duy Nguyen @ 2013-12-23 23:46 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Git Mailing List, Junio C Hamano, Thomas Ferris Nicolaisen

On Tue, Dec 24, 2013 at 12:48 AM, Torsten Bögershausen <tboegi@web.de> wrote:
>> +test_expect_success 'git add -A on empty repo does not error out' '
>> +     git init empty && ( cd empty && git add -A . )
>> +'
>> +
>>  test_done
>>
> I am (a little bit) confused.
>
> This is what git does:
>  rm -rf test && mkdir test && cd test && git init && touch A && mkdir D && cd D && touch B && git add . && git status
> Initialized empty Git repository in /Users/tb/test/test/.git/
> On branch master
>
> Initial commit
>
> Changes to be committed:
>   (use "git rm --cached <file>..." to unstage)
>
>         new file:   B
>
> Untracked files:
>   (use "git add <file>..." to include in what will be committed)
>
>         ../A
> ----------------
> And the behaviour is in line with
> https://www.kernel.org/pub/software/scm/git/docs/git-add.html
>
> "." stands for the current directory somewhere in the worktree,
> not only the "project root".

Yes, except in this case "." is project root because current dir is. I
could have done "git add -A" (without the dot) like reported, but that
will be deprecated soon. Another way to make it clear about project
root is use "git add -A :/". I'll send an update if it makes it
clearer.

> -----------------
>
> Could it make sense to mention that replace
> [PATCH] add: don't complain when adding empty project root
> with
> [PATCH] add: don't complain when adding empty directory.

We don't complain about adding an empty directory before or after this patch.

>
> (and similar in the commit message)
> /Torsten
>



-- 
Duy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] add: don't complain when adding empty project root
  2013-12-23 23:46     ` Duy Nguyen
@ 2013-12-24 21:48       ` Torsten Bögershausen
  2013-12-24 23:49         ` Duy Nguyen
  0 siblings, 1 reply; 18+ messages in thread
From: Torsten Bögershausen @ 2013-12-24 21:48 UTC (permalink / raw)
  To: Duy Nguyen, Torsten Bögershausen
  Cc: Git Mailing List, Junio C Hamano, Thomas Ferris Nicolaisen

On 2013-12-24 00.46, Duy Nguyen wrote:
>
[snip]
> We don't complain about adding an empty directory before or after this patch.
Ok, thanks for the explanation.

I think that
https://www.kernel.org/pub/software/scm/git/docs/git-add.html
could deserve an update.

My understanding is that <filepattern> is related to $GIT_DIR,
but "." is the current directory. 

I will be happy to write a patch,
(or to review one, whatever comes first)
/Torsten
 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] add: don't complain when adding empty project root
  2013-12-24 21:48       ` Torsten Bögershausen
@ 2013-12-24 23:49         ` Duy Nguyen
  2014-01-30 11:39           ` Torsten Bögershausen
  0 siblings, 1 reply; 18+ messages in thread
From: Duy Nguyen @ 2013-12-24 23:49 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Git Mailing List, Junio C Hamano, Thomas Ferris Nicolaisen

On Wed, Dec 25, 2013 at 4:48 AM, Torsten Bögershausen <tboegi@web.de> wrote:
> On 2013-12-24 00.46, Duy Nguyen wrote:
>>
> [snip]
>> We don't complain about adding an empty directory before or after this patch.
> Ok, thanks for the explanation.
>
> I think that
> https://www.kernel.org/pub/software/scm/git/docs/git-add.html
> could deserve an update.
>
> My understanding is that <filepattern> is related to $GIT_DIR,
> but "." is the current directory.
>
> I will be happy to write a patch,
> (or to review one, whatever comes first)
> /Torsten

filepattern is related to current directory too (e.g. "*.sh" from "t"
won't cover git-rebase.sh, ":/*.sh" does). Yes a patch to update
git-add.txt to use the term "pathspec" instead of "filepattern" would
be nice. A pointer to pathspec glossary could help discover
case-insensitive matching, negative matching..
-- 
Duy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] add: don't complain when adding empty project root
  2013-12-23  9:02 ` [PATCH] add: don't complain when adding empty project root Nguyễn Thái Ngọc Duy
  2013-12-23 17:48   ` Torsten Bögershausen
@ 2013-12-26 17:25   ` Jonathan Nieder
  2013-12-26 18:54     ` Junio C Hamano
  1 sibling, 1 reply; 18+ messages in thread
From: Jonathan Nieder @ 2013-12-26 17:25 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, tfnico

Hi,

Nguyễn Thái Ngọc Duy wrote:

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Thanks.

[...]
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -544,7 +544,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  
>  		for (i = 0; i < pathspec.nr; i++) {
>  			const char *path = pathspec.items[i].match;
> -			if (!seen[i] &&
> +			if (!seen[i] && pathspec.items[i].match[0] &&
>  			    ((pathspec.items[i].magic &
>  			      (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||
>  			     !file_exists(path))) {

Nit: in this loop there's already the synonym 'path' for item.match,
so perhaps

			if (!seen[i] && path[0] && ...)

would be clearer.

Should "git add --refresh ." get the same treatment?

> --- a/t/t3700-add.sh
> +++ b/t/t3700-add.sh
> @@ -307,4 +307,8 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out
>  	test_i18ncmp expect.err actual.err
>  '
>  
> +test_expect_success 'git add -A on empty repo does not error out' '
> +	git init empty && ( cd empty && git add -A . )
> +'

Adding a test at the end like this means the tests come in chronological
order instead of logical order and simultaneous patches to the same
test script become more likely to conflict.

How about something like the following, for squashing in?

With or without the tweaks below,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

diff --git i/builtin/add.c w/builtin/add.c
index fbd3f3a..d7e3e44 100644
--- i/builtin/add.c
+++ w/builtin/add.c
@@ -544,7 +544,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 		for (i = 0; i < pathspec.nr; i++) {
 			const char *path = pathspec.items[i].match;
-			if (!seen[i] && pathspec.items[i].match[0] &&
+			if (!seen[i] && path[0] &&
 			    ((pathspec.items[i].magic &
 			      (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||
 			     !file_exists(path))) {
diff --git i/t/t3700-add.sh w/t/t3700-add.sh
index 1535d8f..fe274e2 100755
--- i/t/t3700-add.sh
+++ w/t/t3700-add.sh
@@ -272,6 +272,25 @@ test_expect_success '"add non-existent" should fail' '
 	! (git ls-files | grep "non-existent")
 '
 
+test_expect_success 'git add -A on empty repo does not error out' '
+	rm -fr empty &&
+	git init empty &&
+	(
+		cd empty &&
+		git add -A . &&
+		git add -A
+	)
+'
+
+test_expect_success '"git add ." in empty repo' '
+	rm -fr empty &&
+	git init empty &&
+	(
+		cd empty &&
+		git add .
+	)
+'
+
 test_expect_success 'git add --dry-run of existing changed file' "
 	echo new >>track-this &&
 	git add --dry-run track-this >actual 2>&1 &&
@@ -307,8 +326,4 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out
 	test_i18ncmp expect.err actual.err
 '
 
-test_expect_success 'git add -A on empty repo does not error out' '
-	git init empty && ( cd empty && git add -A . )
-'
-
 test_done

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] add: don't complain when adding empty project root
  2013-12-26 17:25   ` Jonathan Nieder
@ 2013-12-26 18:54     ` Junio C Hamano
  2013-12-26 19:24       ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2013-12-26 18:54 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Nguyễn Thái Ngọc Duy, git, tfnico

Jonathan Nieder <jrnieder@gmail.com> writes:

> How about something like the following, for squashing in?
>
> With or without the tweaks below,
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks, both.

Regarding "git add --refresh" (no other arguments), it would say
"Nothing specified, nothing added.", and that is unrelated to the
breakage reported and fixed in this thread, I think.  It is the same
message "git add" (no other arguments) gives, which I think is a
mistake.  "git add --refresh" is like "git add -u" in that the
affected paths are determined by the index, and running these
commands while your index is still empty can just be a silent no-op.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] add: don't complain when adding empty project root
  2013-12-26 18:54     ` Junio C Hamano
@ 2013-12-26 19:24       ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2013-12-26 19:24 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Nguyễn Thái Ngọc Duy, git, tfnico

Junio C Hamano <gitster@pobox.com> writes:

> Regarding "git add --refresh" (no other arguments), it would say
> "Nothing specified, nothing added.", and that is unrelated to the
> breakage reported and fixed in this thread, I think.  It is the same
> message "git add" (no other arguments) gives, which I think is a
> mistake.  "git add --refresh" is like "git add -u" in that the
> affected paths are determined by the index, and running these
> commands while your index is still empty can just be a silent no-op.

Something like this...

 builtin/add.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index d7e3e44..84e8a3e 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -483,8 +483,10 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		 (implicit_dot ? ADD_CACHE_IMPLICIT_DOT : 0);
 
 	if (require_pathspec && argc == 0) {
-		fprintf(stderr, _("Nothing specified, nothing added.\n"));
-		fprintf(stderr, _("Maybe you wanted to say 'git add .'?\n"));
+		if (!refresh_only) {
+			fprintf(stderr, _("Nothing specified, nothing added.\n"));
+			fprintf(stderr, _("Maybe you wanted to say 'git add .'?\n"));
+		}
 		return 0;
 	}
 

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] add: don't complain when adding empty project root
  2013-12-24 23:49         ` Duy Nguyen
@ 2014-01-30 11:39           ` Torsten Bögershausen
  2014-01-31 18:10             ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Torsten Bögershausen @ 2014-01-30 11:39 UTC (permalink / raw)
  To: Duy Nguyen, Torsten Bögershausen
  Cc: Git Mailing List, Junio C Hamano, Thomas Ferris Nicolaisen

[]
> filepattern is related to current directory too (e.g. "*.sh" from "t"
> won't cover git-rebase.sh, ":/*.sh" does). Yes a patch to update
> git-add.txt to use the term "pathspec" instead of "filepattern" would
> be nice. A pointer to pathspec glossary could help discover
> case-insensitive matching, negative matching..


To somewhat end this thread: 
I haven't been able to find a patch that really improves the documentation,
because "Documentation/git-add.txt" has already been updated:

commit 30784198b766b19a639c199e4365f2a805fc08c6
Author: Junio C Hamano <gitster@pobox.com>
Date:   Thu Feb 14 15:51:43 2013 -0800

    Documentation/git-add: kill remaining <filepattern>
--------------------

And all changes are here:
http://git-htmldocs.googlecode.com/git/git-add.html

But, look at
https://www.kernel.org/pub/software/scm/git/docs/git-add.html

This page seems to need an update too, and I wonder why:
a) The makefile did'nt re-generate html even if it should have
b) That page is not owned or updated by the git.git maintainer
c) Any other reason?


Does anybody know how to trigger an update at kernel.org?

/Torsten

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] add: don't complain when adding empty project root
  2014-01-30 11:39           ` Torsten Bögershausen
@ 2014-01-31 18:10             ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2014-01-31 18:10 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: Duy Nguyen, Git Mailing List, Thomas Ferris Nicolaisen

Torsten Bögershausen <tboegi@web.de> writes:

> But, look at
> https://www.kernel.org/pub/software/scm/git/docs/git-add.html
>
> This page seems to need an update too, and I wonder why:
> a) The makefile did'nt re-generate html even if it should have
> b) That page is not owned or updated by the git.git maintainer
> c) Any other reason?

Sorry, but if I understand correctly, k.org these days requires each
file to be GPG signed and uploaded individually (i.e. there is no
way as far as I can tell to say "here is a tarball, and I've even
signed it with my key to convince you that it is from me. Please
accept it and then unpack the contents there").  There is no way I'd
do that every time I update git-htmldocs repository for 500+ files.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2014-01-31 18:10 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-18  8:06 git add -A fails in empty repository since 1.8.5 Thomas Ferris Nicolaisen
2013-12-18  8:44 ` Antoine Pelisse
2013-12-18 11:59   ` Duy Nguyen
2013-12-18 18:54     ` Junio C Hamano
2013-12-18 19:24       ` Matthieu Moy
2013-12-18 19:56         ` Junio C Hamano
2013-12-18 20:57     ` Junio C Hamano
2013-12-19  0:49       ` Duy Nguyen
2013-12-23  9:02 ` [PATCH] add: don't complain when adding empty project root Nguyễn Thái Ngọc Duy
2013-12-23 17:48   ` Torsten Bögershausen
2013-12-23 23:46     ` Duy Nguyen
2013-12-24 21:48       ` Torsten Bögershausen
2013-12-24 23:49         ` Duy Nguyen
2014-01-30 11:39           ` Torsten Bögershausen
2014-01-31 18:10             ` Junio C Hamano
2013-12-26 17:25   ` Jonathan Nieder
2013-12-26 18:54     ` Junio C Hamano
2013-12-26 19:24       ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).