Re: t7400 broken on pu (Mac OS X)
On Fri, Jan 11, 2013 at 12:58 AM, Junio C Hamano gits...@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 -- 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: t7400 broken on pu (Mac OS X)
Duy Nguyen pclo...@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 --
--
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: t7400 broken on pu (Mac OS X)
On 10.01.13 18:58, Junio C Hamano wrote: Duy Nguyen pclo...@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 -- 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
t7400 broken on pu (Mac OS X)
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/^16 \([^ ]*\).*/\1/p) HEAD is now at 57f2622 super commit 1 1 [second 1b8c63f] commit subproject Author: A U Thor aut...@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 -- 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: t7400 broken on pu (Mac OS X)
Torsten Bögershausen tbo...@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. -- 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: t7400 broken on pu (Mac OS X)
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 --
--
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
