Re: git --literal-pathspecs add -u says "fatal: pathspec ':/' did not match any files"

[Noam Postavsky]

On Sat, Oct 24, 2015 at 3:13 AM, Victor Leschuk  wrote:
> The problem is that in the absence of explicit argument we set the list of
> files to special path ":/" which means repo root:

> And after that we treat it as regular file

Aha.

> Maybe it'll make sense to modify file_exists() to treat ":/" specially.

I'm not sure how this would translate into code, but it would make
more sense to me if we undo the effect of --literal-pathspecs when
setting arguments to ":/". After all, it's obviously not meant to be a
literal file so it shouldn't treated as one (also this way still
allows a user to pass --literal-pathspecs with ":/" to add an actual
directory named ":" (not that anyone should ever want that)).
--
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 --literal-pathspecs add -u says "fatal: pathspec ':/' did not match any files"

[Junio C Hamano]

Victor Leschuk  writes:

> Maybe it'll make sense to modify file_exists() to treat ":/"
> specially.

The real problem is that the code assumes that it can internally use
":/" to mean "everything from the top", and with global 'literal
pathspec' magic, that is not the case.  "did not match" is a red
herring.  Even if you disable that typo-catcher, the resulting
pathspec will match only paths inside a directory whose name is ':',
so you would break 'add -u' in a more grave way.

Disabling the typo-catcher has another problem.  When an end user
says "git --literal-pathspecs add -u :/", the user means "I happen
to have this funnily named directory ':' and want to update
everything under it, but because :/ is normally taken as special, I
am passing --literal-pathspecs".  And if that ':' does not exist,
the user must get the "well you may have misspelt it" error.

But a code you are changing cannot tell if ":/" came from the end
user, or if it the one added by our code upon seeing "-u" or "-A"
without any pathspecs, to make that distinction so that you disable
it only for our internal ":/".

I think the right approach is something along the line of this patch
instead.

-- >8 --
Subject: add: simplify -u/-A without pathspec

Since Git 2.0, "add -u" and "add -A" run from a subdirectory without
any pathspec mean "everything in the working tree" (before 2.0, they
were limited to the current directory).  The limiting to the current
directory was implemented by inserting "." to the command line when
the end user did not give us any pathspec.  At 2.0, we updated the
code to insert ":/" (instead of '.') to consider everything from the
top-level, by using a pathspec magic "top".

The call to parse_pathspec() using the command line arguments is,
however, made with PATHSPEC_PREFER_FULL option since 5a76aff1 (add:
convert to use parse_pathspec, 2013-07-14), which predates Git 2.0.
In retrospect, there was no need to turn "adding . to limit to the
directory" into "adding :/ to unlimit to everywhere" in Git 2.0;
instead we could just have done "if there is no pathspec on the
command line, just let it be".  The parse_pathspec() then would give
us a pathspec that matches everything and all is well.

Incidentally such a simplifcation also fixes a corner case bug that
stems from the fact that ":/" does not necessarily mean any magic.
A user would say "git --literal-pathspecs add -u :/" from the
command line when she has a directory ':' and wants to add
everything in it (and she knows that her :/ will be taken as
'everything under the sun' magic pathspec unless she disables the
magic with --literal-pathspecs).  The internal use of ':/' would
behave the same way as such an explicitly given ":/" when run with
"--literal-pathspecs", and will not add everything under the sun as
the code originally intended.

Signed-off-by: Junio C Hamano 
---

 builtin/add.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index b2a5c57..145f06e 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -336,14 +336,8 @@ int cmd_add(int argc, const char **argv, const char 
*prefix)
if (!show_only && ignore_missing)
die(_("Option --ignore-missing can only be used together with 
--dry-run"));
 
-   if ((0 < addremove_explicit || take_worktree_changes) && !argc) {
-   static const char *whole[2] = { ":/", NULL };
-   argc = 1;
-   argv = whole;
-   }
-
add_new_files = !take_worktree_changes && !refresh_only;
-   require_pathspec = !take_worktree_changes;
+   require_pathspec = !(take_worktree_changes || (0 < addremove_explicit));
 
hold_locked_index(_file, 1);
 
--
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 --literal-pathspecs add -u says "fatal: pathspec ':/' did not match any files"

[Victor Leschuk]

Hello Noam,

The problem is that in the absence of explicit argument we set the list 
of files to special path ":/" which means repo root:


if ((0 < addremove_explicit || take_worktree_changes) && !argc) {
static const char *whole[2] = { ":/", NULL };
argc = 1;
argv = whole;
}

And after that we treat it as regular file


if (!seen[i] && path[0] &&
((pathspec.items[i].magic &
  (PATHSPEC_GLOB | PATHSPEC_ICASE)) ||
 !file_exists(path))) {  /* < 
file_exists() here just checks lstat() result */
Maybe it'll make sense to modify file_exists() to treat ":/" specially. 
Something like that:


diff --git a/dir.c b/dir.c
index 109ceea..6cae3b9 100644
--- a/dir.c
+++ b/dir.c
@@ -2103,6 +2103,10 @@ int read_directory(struct dir_struct *dir, const 
char *path, int len, const stru

 int file_exists(const char *f)
 {
struct stat sb;
+   if (!strcmp(f, ":/")) {
+   /* ":/" - root dir, always exists */
+   return 1;
+   }
return lstat(f, ) == 0;
 }

--
Victor

On 10/24/2015 02:39 AM, Noam Postavsky wrote:

~/tmp/tmprepo$ git init
Initialized empty Git repository in /home/npostavs/tmp/tmprepo/.git/
~/tmp/tmprepo$ git --literal-pathspecs add -u
fatal: pathspec ':/' did not match any files
~/tmp/tmprepo$ git --version
git version 2.6.1

It was reported[1] that 2.0.2 and several following versions also fail
with the same error; I found that version 1.9.5 succeeds.

Adding a "." argument:

git --literal-pathspecs add -u .

succeeds in all versions.

[1]: https://github.com/magit/magit/issues/2354#issuecomment-150665961
--
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

git --literal-pathspecs add -u says "fatal: pathspec ':/' did not match any files"

[Noam Postavsky]

~/tmp/tmprepo$ git init
Initialized empty Git repository in /home/npostavs/tmp/tmprepo/.git/
~/tmp/tmprepo$ git --literal-pathspecs add -u
fatal: pathspec ':/' did not match any files
~/tmp/tmprepo$ git --version
git version 2.6.1

It was reported[1] that 2.0.2 and several following versions also fail
with the same error; I found that version 1.9.5 succeeds.

Adding a "." argument:

   git --literal-pathspecs add -u .

succeeds in all versions.

[1]: https://github.com/magit/magit/issues/2354#issuecomment-150665961
--
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