* git-add says 'pathspec did not match any files' for git repository in / @ 2011-03-25 10:02 Matthijs Kooijman 2011-03-25 12:56 ` Nguyen Thai Ngoc Duy 2011-03-25 13:49 ` [PATCH] setup: return correct prefix if worktree is '/' Nguyễn Thái Ngọc Duy 0 siblings, 2 replies; 7+ messages in thread From: Matthijs Kooijman @ 2011-03-25 10:02 UTC (permalink / raw) To: git [-- Attachment #1: Type: text/plain, Size: 1094 bytes --] Hey folks, I've been using git for keeping config files using a repository in /. In 1.7.4.1, this seems to be broken again. Committing, diffing and status work this time, but git add gives a pathspec did not match any files error when adding a file while the current working directory is not /. This is best illustrated by an example: root@ldap:/# git init Initialized empty Git repository in /.git/ root@ldap:/# cd etc/ root@ldap:/etc# gt add fstab bash: gt: command not found root@ldap:/etc# git add fstab fatal: pathspec 'fstab' did not match any files root@ldap:/etc# ls -l fstab -rw-r--r-- 1 root root 37 Mar 24 16:58 fstab root@ldap:/etc# cd / root@ldap:/# git add /etc/fstab root@ldap:/etc# git --version git version 1.7.4.1 All of this worked properly in 1.7.1. There was a different problem in 1.7.2, but that was fixed then (and with that fix, the above problem didn't occur). I haven't tested 1.7.3. I don't have time to fix this right now, so I'll be sticking to 1.7.1 for now. Perhaps someone else want to look into this. Gr. Matthijs [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 197 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: git-add says 'pathspec did not match any files' for git repository in / 2011-03-25 10:02 git-add says 'pathspec did not match any files' for git repository in / Matthijs Kooijman @ 2011-03-25 12:56 ` Nguyen Thai Ngoc Duy 2011-03-25 13:49 ` [PATCH] setup: return correct prefix if worktree is '/' Nguyễn Thái Ngọc Duy 1 sibling, 0 replies; 7+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-03-25 12:56 UTC (permalink / raw) To: Matthijs Kooijman; +Cc: git On Fri, Mar 25, 2011 at 11:02:54AM +0100, Matthijs Kooijman wrote: > Hey folks, > > I've been using git for keeping config files using a repository in /. > In 1.7.4.1, this seems to be broken again. Committing, diffing and > status work this time, but git add gives a pathspec did not match any > files error when adding a file while the current working directory is > not /. We have t1509 to guard these cases (and it does indeed show breakages). The problem is that the test requires chroot and cannot be run automatically. I need to think of writing better tests. The following makes t1509 pass again for me. You may want to try and see if it fixes it for you. diff --git a/setup.c b/setup.c index 03cd84f..c18ea9c 100644 --- a/setup.c +++ b/setup.c @@ -390,15 +390,25 @@ static const char *setup_explicit_git_dir(const char *gitdirenv, return NULL; } - if (!prefixcmp(cwd, worktree) && - cwd[strlen(worktree)] == '/') { /* cwd inside worktree */ - set_git_dir(real_path(gitdirenv)); - if (chdir(worktree)) - die_errno("Could not chdir to '%s'", worktree); - cwd[len++] = '/'; - cwd[len] = '\0'; - free(gitfile); - return cwd + strlen(worktree) + 1; + if (!prefixcmp(cwd, worktree)) { + if (strlen(worktree) == offset_1st_component(worktree)) { + set_git_dir(real_path(gitdirenv)); + if (chdir(worktree)) + die_errno("Could not chdir to '%s'", worktree); + cwd[len++] = '/'; + cwd[len] = '\0'; + free(gitfile); + return cwd + offset_1st_component(worktree); + } + if (cwd[strlen(worktree)] == '/') { /* cwd inside worktree */ + set_git_dir(real_path(gitdirenv)); + if (chdir(worktree)) + die_errno("Could not chdir to '%s'", worktree); + cwd[len++] = '/'; + cwd[len] = '\0'; + free(gitfile); + return cwd + strlen(worktree) + 1; + } } /* cwd outside worktree */ -- Duy ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] setup: return correct prefix if worktree is '/' 2011-03-25 10:02 git-add says 'pathspec did not match any files' for git repository in / Matthijs Kooijman 2011-03-25 12:56 ` Nguyen Thai Ngoc Duy @ 2011-03-25 13:49 ` Nguyễn Thái Ngọc Duy 2011-03-25 14:17 ` Michael J Gruber 2011-03-25 15:52 ` [PATCH] dir.c: do not trip over difference in "/" Michael J Gruber 1 sibling, 2 replies; 7+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2011-03-25 13:49 UTC (permalink / raw) To: git, Junio C Hamano, Matthijs Kooijman Cc: Nguyễn Thái Ngọc Duy The same old problem reappears after setup code is reworked. We tend to assume there is at least one path component in a path and forget that path can be simply '/'. Reported-by: Matthijs Kooijman <matthijs@stdin•nl> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail•com> --- cache.h | 2 ++ path.c | 35 +++++++++++++++++++++++++++++++++++ setup.c | 5 ++--- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/cache.h b/cache.h index f765cf5..f579807 100644 --- a/cache.h +++ b/cache.h @@ -741,6 +741,8 @@ int longest_ancestor_length(const char *path, const char *prefix_list); char *strip_path_suffix(const char *path, const char *suffix); int daemon_avoid_alias(const char *path); int offset_1st_component(const char *path); +int is_subdir_or_same(const char *subdir, const char *dir); +const char *skip_dir(const char *dir, int skip); /* Read and unpack a sha1 file into memory, write memory to a sha1 file */ extern int sha1_object_info(const unsigned char *, unsigned long *); diff --git a/path.c b/path.c index 4d73cc9..9f77aea 100644 --- a/path.c +++ b/path.c @@ -662,3 +662,38 @@ int offset_1st_component(const char *path) return 2 + is_dir_sep(path[2]); return is_dir_sep(path[0]); } + +/* return 1 if subdir is either dir or inside dir */ +int is_subdir_or_same(const char *subdir, const char *dir) +{ + if (!*subdir || !*dir) + die("BUG: how can I compare a dir to empty?"); + + while (*dir && *subdir && *dir == *subdir) { + dir++; + subdir++; + } + + /* help/me vs hell/yeah */ + if (*dir && *subdir) + return 0; + + if (!*subdir) + return !*dir; /* same dir */ + + /* foo/bar vs foo/ */ + if (is_dir_sep(dir[-1])) + return is_dir_sep(subdir[-1]); + + /* foo/bar vs foo */ + return is_dir_sep(*subdir); +} + +/* skip 'skip' chars and the trailing separator if any */ +const char *skip_dir(const char *dir, int skip) +{ + dir += skip; + if (is_dir_sep(*dir)) + dir++; + return dir; +} diff --git a/setup.c b/setup.c index 03cd84f..79f8ea7 100644 --- a/setup.c +++ b/setup.c @@ -390,15 +390,14 @@ static const char *setup_explicit_git_dir(const char *gitdirenv, return NULL; } - if (!prefixcmp(cwd, worktree) && - cwd[strlen(worktree)] == '/') { /* cwd inside worktree */ + if (is_subdir_or_same(cwd, worktree)) { /* cwd inside worktree? */ set_git_dir(real_path(gitdirenv)); if (chdir(worktree)) die_errno("Could not chdir to '%s'", worktree); cwd[len++] = '/'; cwd[len] = '\0'; free(gitfile); - return cwd + strlen(worktree) + 1; + return skip_dir(cwd, strlen(worktree)); } /* cwd outside worktree */ -- 1.7.4.74.g639db ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] setup: return correct prefix if worktree is '/' 2011-03-25 13:49 ` [PATCH] setup: return correct prefix if worktree is '/' Nguyễn Thái Ngọc Duy @ 2011-03-25 14:17 ` Michael J Gruber 2011-03-25 15:11 ` Nguyen Thai Ngoc Duy 2011-03-25 15:52 ` [PATCH] dir.c: do not trip over difference in "/" Michael J Gruber 1 sibling, 1 reply; 7+ messages in thread From: Michael J Gruber @ 2011-03-25 14:17 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy Cc: git, Junio C Hamano, Matthijs Kooijman Nguyễn Thái Ngọc Duy venit, vidit, dixit 25.03.2011 14:49: > The same old problem reappears after setup code is reworked. We tend > to assume there is at least one path component in a path and forget > that path can be simply '/'. > > Reported-by: Matthijs Kooijman <matthijs@stdin•nl> > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail•com> > --- > cache.h | 2 ++ > path.c | 35 +++++++++++++++++++++++++++++++++++ > setup.c | 5 ++--- > 3 files changed, 39 insertions(+), 3 deletions(-) > Wait wait, the bug bisects to 490544b (get_cwd_relative(): do not misinterpret suffix as subdirectory, 2010-05-22) for me, using git bisect run sh -c "make || exit 125; cd /etc; GIT_DIR=/tmp/a/.git ~/src/git/git-add fstab || exit 1" and a repo in /tmp/a/.git with core.worktree set to "/". The issue is that in get_relative_cwd() of dir.c, "dir" (and maybe ("cwd") may or may not end in "/", so even with dir="/etc/" and cwd="/etc" we would not recognize we are within the repo. Patch for that is coming. Note that by doing something like the above, we can test / without being root as long as we have files there which we can rely on being readable, or can rely on /tmp being there. Michael ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] setup: return correct prefix if worktree is '/' 2011-03-25 14:17 ` Michael J Gruber @ 2011-03-25 15:11 ` Nguyen Thai Ngoc Duy 0 siblings, 0 replies; 7+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-03-25 15:11 UTC (permalink / raw) To: Michael J Gruber; +Cc: git, Junio C Hamano, Matthijs Kooijman 2011/3/25 Michael J Gruber <git@drmicha•warpmail.net>: > Wait wait, the bug bisects to > > 490544b (get_cwd_relative(): do not misinterpret suffix as subdirectory, > 2010-05-22) Well, we've had a few bugs in this area lately :) That one was fixed by fbbb4e1 (get_cwd_relative(): do not misinterpret root path - 2010-11-20), which was also noticed by Mathijs. > for me, using > > git bisect run sh -c "make || exit 125; cd /etc; GIT_DIR=/tmp/a/.git > ~/src/git/git-add fstab || exit 1" > > and a repo in /tmp/a/.git with core.worktree set to "/". > > The issue is that in get_relative_cwd() of dir.c, "dir" (and maybe > ("cwd") may or may not end in "/", so even with dir="/etc/" and > cwd="/etc" we would not recognize we are within the repo. Patch for that > is coming. > > Note that by doing something like the above, we can test / without being > root as long as we have files there which we can rely on being readable, > or can rely on /tmp being there. Sounds good, as long as we stick to standard paths, like /etc/fstab. But that won't work on Windows. I don't even know if any file is always at a known location in C:\. -- Duy ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] dir.c: do not trip over difference in "/" 2011-03-25 13:49 ` [PATCH] setup: return correct prefix if worktree is '/' Nguyễn Thái Ngọc Duy 2011-03-25 14:17 ` Michael J Gruber @ 2011-03-25 15:52 ` Michael J Gruber 2011-03-26 7:59 ` Nguyen Thai Ngoc Duy 1 sibling, 1 reply; 7+ messages in thread From: Michael J Gruber @ 2011-03-25 15:52 UTC (permalink / raw) To: git Cc: Junio C Hamano, Matthijs Kooijman, Nguyễn Thái Ngọc Duy get_relative_cwd() tries to determine a common prefix for dir and cwd. The fix in 490544b (get_cwd_relative(): do not misinterpret suffix as subdirectory, 2010-05-22) made the logic less naive (so that foo-bar is not misdetected as being within foo) but broke some other cases, in particular foo not being detected as being within foo/ any more. Fix it by taking into a account that a directory name may or may not end in /. Document with a test. Reported-by: Matthijs Kooijman <matthijs@stdin•nl> Signed-off-by: Michael J Gruber <git@drmicha•warpmail.net> --- So I think this would be a proper fix. I tried to be nice and base it on the commit which I bisected the problem to. I saw much too late that their were later (failed) attempts at fixing this and a major rewrite of the test, which is quite a nuisance. Anyway, this problem needs a fix, and by taking get_cwd_relative() from this patch on top of next the problem is fixed. I have not looked into resolving the merge conflict in the test. dir.c | 23 +++++++++++++++-------- t/t1501-worktree.sh | 13 +++++++++++++ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/dir.c b/dir.c index 5615f33..b090cd0 100644 --- a/dir.c +++ b/dir.c @@ -956,16 +956,23 @@ char *get_relative_cwd(char *buffer, int size, const char *dir) dir++; cwd++; } - if (*dir) + + /* dir has more than just '/' left */ + if (*dir && ((*dir != '/') || *(dir+1))) return NULL; - switch (*cwd) { - case '\0': + /* *dir is now '\0' or '/' followed by '\0' */ + if (cwd == buffer) /* we did not move */ + return (*cwd) ? NULL : cwd; + /* we have moved at least by 1 */ + if (*dir) /* == '/' */ + return (*cwd) ? NULL : cwd; + if (!(*cwd)) /* both ended */ return cwd; - case '/': - return cwd + 1; - default: - return NULL; - } + if (*(dir-1) == '/') /* must have been common */ + return cwd; + if (*cwd == '/') + return cwd+1; + return NULL; } int is_inside_dir(const char *dir) diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh index bd8b607..f0dbdd8 100755 --- a/t/t1501-worktree.sh +++ b/t/t1501-worktree.sh @@ -63,6 +63,19 @@ cd sub/dir || exit 1 test_rev_parse 'subdirectory' false false true sub/dir/ cd ../../.. || exit 1 +say "core.worktree = absolute path/" +GIT_DIR=$(pwd)/repo.git +GIT_CONFIG=$GIT_DIR/config +git config core.worktree "$(pwd)/work/" +test_rev_parse 'outside' false false false +cd work2 +test_rev_parse 'outside2' false false false +cd ../work || exit 1 +test_rev_parse 'inside' false false true '' +cd sub/dir || exit 1 +test_rev_parse 'subdirectory' false false true sub/dir/ +cd ../../.. || exit 1 + say "GIT_WORK_TREE=relative path (override core.worktree)" GIT_DIR=$(pwd)/repo.git GIT_CONFIG=$GIT_DIR/config -- 1.7.4.1.607.g888da ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] dir.c: do not trip over difference in "/" 2011-03-25 15:52 ` [PATCH] dir.c: do not trip over difference in "/" Michael J Gruber @ 2011-03-26 7:59 ` Nguyen Thai Ngoc Duy 0 siblings, 0 replies; 7+ messages in thread From: Nguyen Thai Ngoc Duy @ 2011-03-26 7:59 UTC (permalink / raw) To: Michael J Gruber; +Cc: git, Junio C Hamano, Matthijs Kooijman 2011/3/25 Michael J Gruber <git@drmicha•warpmail.net>: > get_relative_cwd() tries to determine a common prefix for dir and cwd. > The fix in > 490544b (get_cwd_relative(): do not misinterpret suffix as subdirectory, 2010-05-22) > made the logic less naive (so that foo-bar is not misdetected as being > within foo) but broke some other cases, in particular foo not being > detected as being within foo/ any more. I'd rather kill this function off. It's only used in is_inside_dir(), we can be replaced with is_subdir_or_same() in my previous patch (more or less the same function with get_relative_cwd, but less cryptic). > diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh > index bd8b607..f0dbdd8 100755 > --- a/t/t1501-worktree.sh > +++ b/t/t1501-worktree.sh > @@ -63,6 +63,19 @@ cd sub/dir || exit 1 > test_rev_parse 'subdirectory' false false true sub/dir/ > cd ../../.. || exit 1 > > +say "core.worktree = absolute path/" > +GIT_DIR=$(pwd)/repo.git > +GIT_CONFIG=$GIT_DIR/config > +git config core.worktree "$(pwd)/work/" > +test_rev_parse 'outside' false false false > +cd work2 > +test_rev_parse 'outside2' false false false > +cd ../work || exit 1 > +test_rev_parse 'inside' false false true '' > +cd sub/dir || exit 1 > +test_rev_parse 'subdirectory' false false true sub/dir/ > +cd ../../.. || exit 1 > + > say "GIT_WORK_TREE=relative path (override core.worktree)" > GIT_DIR=$(pwd)/repo.git > GIT_CONFIG=$GIT_DIR/config I tried something similar (basically core.worktree = $(pwd)/work/) but could not reproduce. Note that worktree will be normalized by real_path() (or get_absolute_path() earlier) and the trailing '/' may have been removed. -- Duy ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-03-26 8:00 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-03-25 10:02 git-add says 'pathspec did not match any files' for git repository in / Matthijs Kooijman 2011-03-25 12:56 ` Nguyen Thai Ngoc Duy 2011-03-25 13:49 ` [PATCH] setup: return correct prefix if worktree is '/' Nguyễn Thái Ngọc Duy 2011-03-25 14:17 ` Michael J Gruber 2011-03-25 15:11 ` Nguyen Thai Ngoc Duy 2011-03-25 15:52 ` [PATCH] dir.c: do not trip over difference in "/" Michael J Gruber 2011-03-26 7:59 ` Nguyen Thai Ngoc Duy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox