git add -A fails in empty repository since 1.8.5
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git add -A fails in empty repository since 1.8.5
FWIW, git-bisect points to 84b8b5d (that is $gmane/230349). On Wed, Dec 18, 2013 at 9:06 AM, Thomas Ferris Nicolaisen tfn...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git add -A fails in empty repository since 1.8.5
On Wed, Dec 18, 2013 at 3:44 PM, Antoine Pelisse apeli...@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 tfn...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git add -A fails in empty repository since 1.8.5
Duy Nguyen pclo...@gmail.com writes: On Wed, Dec 18, 2013 at 3:44 PM, Antoine Pelisse apeli...@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 tfn...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git add -A fails in empty repository since 1.8.5
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git add -A fails in empty repository since 1.8.5
Duy Nguyen pclo...@gmail.com writes: On Wed, Dec 18, 2013 at 3:44 PM, Antoine Pelisse apeli...@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 tfn...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git add -A fails in empty repository since 1.8.5
On Thu, Dec 19, 2013 at 3:57 AM, Junio C Hamano gits...@pobox.com wrote: Duy Nguyen pclo...@gmail.com writes: On Wed, Dec 18, 2013 at 3:44 PM, Antoine Pelisse apeli...@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 tfn...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html