* t7400 broken on pu (Mac OS X)
@ 2013-01-09 18:43 Torsten Bögershausen
2013-01-09 19:16 ` Junio C Hamano
2013-01-10 6:28 ` Duy Nguyen
0 siblings, 2 replies; 6+ messages in thread
From: Torsten Bögershausen @ 2013-01-09 18:43 UTC (permalink / raw)
To: git; +Cc: Torsten Bögershausen
The current pu fails on Mac OS, case insensitive FS.
Bisecting points out
commit 3f28e4fafc046284657945798d71c57608bee479
[snip]
Date: Sun Jan 6 13:21:07 2013 +0700
Convert add_files_to_cache to take struct pathspec
And I veryfied that the preceeding commit 05647d2d8a5dc456d1f4ef73
is OK.
It fails here:
not ok 38 - gracefully add submodule with a trailing slash
A run of a modified t7400 looks like this:
Is there anything more, that I can do to debug this?
[snip]
ok 37 - do not add files from a submodule
expecting success:
git reset --hard &&
echo 1 >&2 &&
git commit -m "commit subproject" init &&
echo 2 >&2 &&
(cd init &&
echo 3 >&2 &&
echo b > a) &&
echo 4 >&2 &&
git add init/ &&
echo 5 >&2 &&
git diff --exit-code --cached init &&
echo 6 >&2 &&
commit=$(cd init &&
echo 7 >&2 &&
git commit -m update a >/dev/null &&
echo 8 >&2 &&
git rev-parse HEAD) &&
echo 9 >&2 &&
git add init/ &&
echo 10 >&2 &&
test_must_fail git diff --exit-code --cached init &&
echo 11 >&2 &&
test $commit = $(git ls-files --stage |
sed -n "s/^160000 \([^ ]*\).*/\1/p")
HEAD is now at 57f2622 super commit 1
1
[second 1b8c63f] commit subproject
Author: A U Thor <author@example.com>
1 file changed, 1 insertion(+), 1 deletion(-)
2
3
4
5
6
7
8
9
10
test_must_fail: command succeeded: git diff --exit-code --cached init
not ok 38 - gracefully add submodule with a trailing slash
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: t7400 broken on pu (Mac OS X) 2013-01-09 18:43 t7400 broken on pu (Mac OS X) Torsten Bögershausen @ 2013-01-09 19:16 ` Junio C Hamano 2013-01-10 6:28 ` Duy Nguyen 1 sibling, 0 replies; 6+ messages in thread From: Junio C Hamano @ 2013-01-09 19:16 UTC (permalink / raw) To: Torsten Bögershausen; +Cc: git, Nguyễn Thái Ngọc Duy Torsten Bögershausen <tboegi@web.de> writes: > The current pu fails on Mac OS, case insensitive FS. > > > Bisecting points out > commit 3f28e4fafc046284657945798d71c57608bee479 > [snip] > Date: Sun Jan 6 13:21:07 2013 +0700 Next time do not [snip] but please find the author address there, and Cc such a report. I think this topic is planned to be rerolled anyway, and your report would be a valuable input while doing so. Thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: t7400 broken on pu (Mac OS X) 2013-01-09 18:43 t7400 broken on pu (Mac OS X) Torsten Bögershausen 2013-01-09 19:16 ` Junio C Hamano @ 2013-01-10 6:28 ` Duy Nguyen 2013-01-10 17:58 ` Junio C Hamano 1 sibling, 1 reply; 6+ messages in thread From: Duy Nguyen @ 2013-01-10 6:28 UTC (permalink / raw) To: Torsten Bögershausen; +Cc: git On Wed, Jan 09, 2013 at 07:43:03PM +0100, Torsten Bögershausen wrote: > The current pu fails on Mac OS, case insensitive FS. > > > Bisecting points out > commit 3f28e4fafc046284657945798d71c57608bee479 > [snip] > Date: Sun Jan 6 13:21:07 2013 +0700 > > Convert add_files_to_cache to take struct pathspec > I can reproduce it by setting core.ignorecase to true. There is a bug that I overlooked. Can you verify if this throw-away patch fixes it for you? A proper fix will be in the reroll later. -- 8< -- diff --git a/builtin/add.c b/builtin/add.c index 641037f..61cb8bd 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -155,12 +155,13 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int return seen; } -static void treat_gitlinks(const char **pathspec) +static int treat_gitlinks(const char **pathspec) { int i; + int modified = 0; if (!pathspec || !*pathspec) - return; + return modified; for (i = 0; i < active_nr; i++) { struct cache_entry *ce = active_cache[i]; @@ -171,15 +172,17 @@ static void treat_gitlinks(const char **pathspec) if (len2 <= len || pathspec[j][len] != '/' || memcmp(ce->name, pathspec[j], len)) continue; - if (len2 == len + 1) + if (len2 == len + 1) { /* strip trailing slash */ pathspec[j] = xstrndup(ce->name, len); - else + modified = 1; + } else die (_("Path '%s' is in submodule '%.*s'"), pathspec[j], len, ce->name); } } } + return modified; } static void refresh(int verbose, const struct pathspec *pathspec) @@ -418,7 +421,16 @@ int cmd_add(int argc, const char **argv, const char *prefix) if (read_cache() < 0) die(_("index file corrupt")); - treat_gitlinks(pathspec.raw); + if (treat_gitlinks(pathspec.raw)) + /* + * HACK: treat_gitlinks strips the trailing slashes + * out of submodule entries but it only affects + * raw[]. Everything in pathspec.items is not touched. + * Re-init it to propagate the change. Long term, this + * function should be moved to pathspec.c and update + * everything in a consistent way. + */ + init_pathspec(&pathspec, pathspec.raw); if (add_new_files) { int baselen; -- 8< -- ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: t7400 broken on pu (Mac OS X) 2013-01-10 6:28 ` Duy Nguyen @ 2013-01-10 17:58 ` Junio C Hamano 2013-01-10 19:06 ` Torsten Bögershausen 2013-01-11 11:03 ` Duy Nguyen 0 siblings, 2 replies; 6+ messages in thread From: Junio C Hamano @ 2013-01-10 17:58 UTC (permalink / raw) To: Duy Nguyen; +Cc: Torsten Bögershausen, git Duy Nguyen <pclouds@gmail.com> writes: > On Wed, Jan 09, 2013 at 07:43:03PM +0100, Torsten Bögershausen wrote: >> The current pu fails on Mac OS, case insensitive FS. >> >> >> Bisecting points out >> commit 3f28e4fafc046284657945798d71c57608bee479 >> [snip] >> Date: Sun Jan 6 13:21:07 2013 +0700 >> >> Convert add_files_to_cache to take struct pathspec >> > > I can reproduce it by setting core.ignorecase to true. There is a bug > that I overlooked. Can you verify if this throw-away patch fixes it > for you? A proper fix will be in the reroll later. I can see why it is wrong to let pathspec.raw be rewritten without making matching change to the containing pathspec, but I find it strange why it matters only on case-insensitive codepath. I agree with the "Hack" comment that the canonicalization should be done at a higher level upfront. Then ls-files does not need its own strip_trailing_slash_from_submodules(), and check_path_for_gitlink() can (and should---the callers of "check_anything" would not expect the function to change things) stop rewriting its parameter. Thanks for a quick response. > -- 8< -- > diff --git a/builtin/add.c b/builtin/add.c > index 641037f..61cb8bd 100644 > --- a/builtin/add.c > +++ b/builtin/add.c > @@ -155,12 +155,13 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int > return seen; > } > > -static void treat_gitlinks(const char **pathspec) > +static int treat_gitlinks(const char **pathspec) > { > int i; > + int modified = 0; > > if (!pathspec || !*pathspec) > - return; > + return modified; > > for (i = 0; i < active_nr; i++) { > struct cache_entry *ce = active_cache[i]; > @@ -171,15 +172,17 @@ static void treat_gitlinks(const char **pathspec) > if (len2 <= len || pathspec[j][len] != '/' || > memcmp(ce->name, pathspec[j], len)) > continue; > - if (len2 == len + 1) > + if (len2 == len + 1) { > /* strip trailing slash */ > pathspec[j] = xstrndup(ce->name, len); > - else > + modified = 1; > + } else > die (_("Path '%s' is in submodule '%.*s'"), > pathspec[j], len, ce->name); > } > } > } > + return modified; > } > > static void refresh(int verbose, const struct pathspec *pathspec) > @@ -418,7 +421,16 @@ int cmd_add(int argc, const char **argv, const char *prefix) > > if (read_cache() < 0) > die(_("index file corrupt")); > - treat_gitlinks(pathspec.raw); > + if (treat_gitlinks(pathspec.raw)) > + /* > + * HACK: treat_gitlinks strips the trailing slashes > + * out of submodule entries but it only affects > + * raw[]. Everything in pathspec.items is not touched. > + * Re-init it to propagate the change. Long term, this > + * function should be moved to pathspec.c and update > + * everything in a consistent way. > + */ > + init_pathspec(&pathspec, pathspec.raw); > > if (add_new_files) { > int baselen; > -- 8< -- ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: t7400 broken on pu (Mac OS X) 2013-01-10 17:58 ` Junio C Hamano @ 2013-01-10 19:06 ` Torsten Bögershausen 2013-01-11 11:03 ` Duy Nguyen 1 sibling, 0 replies; 6+ messages in thread From: Torsten Bögershausen @ 2013-01-10 19:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: Duy Nguyen, Torsten Bögershausen, git On 10.01.13 18:58, Junio C Hamano wrote: > Duy Nguyen <pclouds@gmail.com> writes: > >> On Wed, Jan 09, 2013 at 07:43:03PM +0100, Torsten Bögershausen wrote: >>> The current pu fails on Mac OS, case insensitive FS. >>> >>> >>> Bisecting points out >>> commit 3f28e4fafc046284657945798d71c57608bee479 >>> [snip] >>> Date: Sun Jan 6 13:21:07 2013 +0700 >>> >>> Convert add_files_to_cache to take struct pathspec >>> >> I can reproduce it by setting core.ignorecase to true. There is a bug >> that I overlooked. Can you verify if this throw-away patch fixes it >> for you? A proper fix will be in the reroll later. > I can see why it is wrong to let pathspec.raw be rewritten without > making matching change to the containing pathspec, but I find it > strange why it matters only on case-insensitive codepath. > > I agree with the "Hack" comment that the canonicalization should be > done at a higher level upfront. Then ls-files does not need its own > strip_trailing_slash_from_submodules(), and check_path_for_gitlink() > can (and should---the callers of "check_anything" would not expect > the function to change things) stop rewriting its parameter. > > Thanks for a quick response. > The patch fixes t7400. Thanks from my side as well /Torsten ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: t7400 broken on pu (Mac OS X) 2013-01-10 17:58 ` Junio C Hamano 2013-01-10 19:06 ` Torsten Bögershausen @ 2013-01-11 11:03 ` Duy Nguyen 1 sibling, 0 replies; 6+ messages in thread From: Duy Nguyen @ 2013-01-11 11:03 UTC (permalink / raw) To: Junio C Hamano; +Cc: Torsten Bögershausen, git On Fri, Jan 11, 2013 at 12:58 AM, Junio C Hamano <gitster@pobox.com> wrote: > I can see why it is wrong to let pathspec.raw be rewritten without > making matching change to the containing pathspec, but I find it > strange why it matters only on case-insensitive codepath. Yeah, I don't get it either. I can see that core.ignorecase exercises some more code, but still fail to see the link. I should get to the bottom of this and write some tests to for core.ignorecase-only code. -- Duy ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-01-11 11:04 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-01-09 18:43 t7400 broken on pu (Mac OS X) Torsten Bögershausen 2013-01-09 19:16 ` Junio C Hamano 2013-01-10 6:28 ` Duy Nguyen 2013-01-10 17:58 ` Junio C Hamano 2013-01-10 19:06 ` Torsten Bögershausen 2013-01-11 11:03 ` Duy Nguyen
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).