From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: Jens.Lehmann@web.de, dennis@kaarsemaker.net,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH] pathspec: adjust prefixlen after striping trailing slash
Date: Sat, 18 Apr 2015 08:19:06 +0700 [thread overview]
Message-ID: <1429319946-19890-1-git-send-email-pclouds@gmail.com> (raw)
In-Reply-To: <55300D2C.9030903@web.de>
A path(spec) from git perspective consists of two parts, the prefix,
and the rest. The prefix covers the part of `pwd` after expanding ".."
components. The split is to support case-insensitive match in a sane
way (see 93d9353, especially the big comment block added in dir.c).
Normally the prefix is found after prefix_path_gently() and that will
be it. Unfortunately the *STRIP_SUBMODULE* flags can strip the
trailing slash (see 2ce53f9 for the reason) and cut into this prefix
part. In the test, the prefix is "submodule/", but the final path is
just "submodule". We need to readjust prefix info when this happens.
The assert() that catches this is turned to die() to make sure it's
always active. prefix_pathspec() is not in any critical path to be a
performance concern because of this change.
93d9353 (parse_pathspec: accept :(icase)path syntax - 2013-07-14)
2ce53f9 (git add: do not add files from a submodule - 2009-01-02)
Noticed-by: djanos_ (via irc)
Helped-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
On Fri, Apr 17, 2015 at 2:27 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> The problem seems to be that "subrepo" is still registered as a
> submodule in the index. But we should see a proper error message
> instead of an assert in that case ... CCed Duy who knows much
> more about pathspec.c than me.
This deals with the bug in pathspec code. I leave it to you to decide
how git-add should do when a submodule is registered in index, but
it's no longer a submodule in worktree. Yeah maybe it should error
out.
pathspec.c | 18 +++++++++++++++---
t/t3703-add-magic-pathspec.sh | 8 ++++++++
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index 9304ee3..aa5e2c7 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -262,7 +262,6 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
} else
item->original = elt;
item->len = strlen(item->match);
- item->prefix = prefixlen;
if ((flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) &&
(item->len >= 1 && item->match[item->len - 1] == '/') &&
@@ -292,6 +291,15 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
elt, ce_len, ce->name);
}
+ /*
+ * Adjust prefixlen if the above trailing slash stripping cuts
+ * into the prefix part
+ */
+ if ((flags & (PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE)) &&
+ prefixlen > item->len)
+ prefixlen = item->len;
+
if (magic & PATHSPEC_LITERAL)
item->nowildcard_len = item->len;
else {
@@ -299,6 +307,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
if (item->nowildcard_len < prefixlen)
item->nowildcard_len = prefixlen;
}
+ item->prefix = prefixlen;
item->flags = 0;
if (magic & PATHSPEC_GLOB) {
/*
@@ -313,8 +322,11 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
}
/* sanity checks, pathspec matchers assume these are sane */
- assert(item->nowildcard_len <= item->len &&
- item->prefix <= item->len);
+ if (!(item->nowildcard_len <= item->len &&
+ item->prefix <= item->len))
+ die("BUG: item->nowildcard_len (%d) or item->prefix (%d)"
+ " is longer than item->len (%d)",
+ item->nowildcard_len, item->prefix, item->len);
return magic;
}
diff --git a/t/t3703-add-magic-pathspec.sh b/t/t3703-add-magic-pathspec.sh
index 5115de7..cced8c4 100755
--- a/t/t3703-add-magic-pathspec.sh
+++ b/t/t3703-add-magic-pathspec.sh
@@ -55,4 +55,12 @@ test_expect_success COLON_DIR 'a file with the same (short) magic name exists' '
git add -n "./:/bar"
'
+test_expect_success 'prefix is updated after trailing slash is stripped' '
+ git init submodule &&
+ ( cd submodule && test_commit test ) &&
+ git add submodule &&
+ mv submodule/.git submodule/dotgit &&
+ ( cd submodule && git add . )
+'
+
test_done
--
2.3.0.rc1.137.g477eb31
next prev parent reply other threads:[~2015-04-18 1:19 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-13 16:55 assert failed in submodule edge case Dennis Kaarsemaker
2015-04-13 16:57 ` Dennis Kaarsemaker
2015-04-16 19:27 ` Jens Lehmann
2015-04-18 1:19 ` Nguyễn Thái Ngọc Duy [this message]
2015-04-19 12:53 ` [PATCH] pathspec: adjust prefixlen after striping trailing slash Jens Lehmann
2015-04-20 1:34 ` Duy Nguyen
2015-04-20 5:37 ` Junio C Hamano
2015-04-20 5:52 ` Duy Nguyen
2015-04-21 21:08 ` Junio C Hamano
2015-04-22 19:14 ` Jens Lehmann
2015-04-22 19:58 ` Junio C Hamano
2015-04-22 22:32 ` Jens Lehmann
2015-04-23 3:47 ` Junio C Hamano
2015-06-14 13:16 ` Duy Nguyen
2015-06-14 21:34 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1429319946-19890-1-git-send-email-pclouds@gmail.com \
--to=pclouds@gmail.com \
--cc=Jens.Lehmann@web.de \
--cc=dennis@kaarsemaker.net \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).