Re: t7400 broken on pu (Mac OS X)
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 10, 2013, 17:58 UTC
- Message-ID
- <7v38y83ooo.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20130110062838.GA11634@duynguyen-vnpc.dek-tpc.internal>
Duy Nguyen <pclouds@gmail.com> writes:
Show 15 quoted lines
> 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.
Show 60 quoted lines
> -- 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< --