threads / discuss / 35699
Subject: problematic git log submodule-dir/
Hi all.
i've noticed that 'git log submodule-dir' and 'git log submodule-dir/' return different results (module's head changes vs. nothing). is it a bug? looks like a trailing slash is a problem for git-log.
tested on git-1.8.5.3-2.fc20.x86_64.
BR, Paweł.
-- gpg key fingerprint = 60B4 9886 AD53 EB3E 88BB 1EB5 C52E D01B 683B 9411
Jens LehmannRe: problematic git log submodule-dir/
Am 20.01.2014 19:25, schrieb Paweł Sikora:
> i've noticed that 'git log submodule-dir' and 'git log submodule-dir/' > return different results (module's head changes vs. nothing). is it a bug? > looks like a trailing slash is a problem for git-log.
I think this is a bug, and for example "git diff" has similar problems. This is especially nasty as shell auto-completion adds the '/' at the end.
Duy, without having looked into the code myself yet: is this something that might be easily fixed by using PATHSPEC_STRIP_SUBMODULE_SLASH*?
Duy NguyenRe: problematic git log submodule-dir/
On Thu, Jan 23, 2014 at 3:35 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 20.01.2014 19:25, schrieb Paweł Sikora: >> i've noticed that 'git log submodule-dir' and 'git log submodule-dir/' >> return different results (module's head changes vs. nothing). is it a bug? >> looks like a trailing slash is a problem for git-log. > > I think this is a bug, and for example "git diff" has similar problems. > This is especially nasty as shell auto-completion adds the '/' at the > end. > > Duy, without having looked into the code myself yet: is this something > that might be easily fixed by using PATHSPEC_STRIP_SUBMODULE_SLASH*?
I think so. But I dislike those preprocessing because it looks inefficient and the same change may be needed for other diff commands as well. Maybe we can handle this at diff or log-tree.c level. Will look further into it tonight.
-- Duy
Nguyễn Thái Ngọc Duy[PATCH 1/2] tree-walk.c: ignore trailing slash on submodule in tree_entry_interesting()
We do ignore trailing slash on a directory, so pathspec "abc/" matches directory "abc". A submodule is also a directory. Apply the same logic to it. This makes "git log submodule-path" and "git log submodule-path/" produce the same output.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- t/t4010-diff-pathspec.sh | 17 +++++++++++++++++ tree-walk.c | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh index af5134b..15a4912 100755 --- a/t/t4010-diff-pathspec.sh +++ b/t/t4010-diff-pathspec.sh @@ -110,4 +110,21 @@ test_expect_success 'diff-tree -r with wildcard' ' test_cmp expected result ' +test_expect_success 'setup submodules' ' + test_tick && + git init submod && + ( cd submod && test_commit first; ) && + git add submod && + git commit -m first && + ( cd submod && test_commit second; ) && + git add submod && + git commit -m second +' + +test_expect_success 'diff-tree ignores trailing slash on submodule path' ' + git diff --name-only HEAD^ HEAD submod >expect && + git diff --name-only HEAD^ HEAD submod/ >actual && + test_cmp expect actual +' + test_done diff --git a/tree-walk.c b/tree-walk.c index 680afda..c29b6a3 100644 --- a/tree-walk.c +++ b/tree-walk.c @@ -543,7 +543,7 @@ static int match_entry(const struct pathspec_item *item, if (matchlen > pathlen) { if (match[pathlen] != '/') return 0; - if (!S_ISDIR(entry->mode)) + if (!S_ISDIR(entry->mode) && !S_ISGITLINK(entry->mode)) return 0; }
-- 1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy[PATCH 2/2] t4010: match_pathspec_depth() and trailing slash after submodule
When you do "git diff HEAD submodule/", "submodule" from the index is picked out and match_pathspec_depth() in charge of matching it with the pathspec "submodule/".
Unlike tree_entry_interesting(), match_pathspec_depth() has no knowledge about entry mode to realize "submodule" is a directory and treat the trailing slash specially. And it does not have too, mostly, because the index only contains files, not directories (not until submodules come)
I have no solutions for it (no, stripping '/' at pathspec preprocessing phase seems like a workaround than a solution). So let's mark it. Maybe I or somebody else could revisit it later.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- t/t4010-diff-pathspec.sh | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh index 15a4912..b54251a 100755 --- a/t/t4010-diff-pathspec.sh +++ b/t/t4010-diff-pathspec.sh @@ -127,4 +127,10 @@ test_expect_success 'diff-tree ignores trailing slash on submodule path' ' test_cmp expect actual ' +test_expect_failure 'diff-cache ignores trailing slash on submodule path' ' + git diff --name-only HEAD^ submod >expect && + git diff --name-only HEAD^ submod/ >actual && + test_cmp expect actual +' + test_done
-- 1.8.5.2.240.g8478abd
Junio C HamanoRe: [PATCH 2/2] t4010: match_pathspec_depth() and trailing slash after submodule
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> When you do "git diff HEAD submodule/", "submodule" from the index is > picked out and match_pathspec_depth() in charge of matching it with > the pathspec "submodule/".
Is "... is called" or something missing at the end of this sentence?
> Unlike tree_entry_interesting(), match_pathspec_depth() has no > knowledge about entry mode to realize "submodule" is a directory and > treat the trailing slash specially. And it does not have too, mostly,
s/too/to/, I think.
> because the index only contains files, not directories (not until > submodules come) > > I have no solutions for it (no, stripping '/' at pathspec > preprocessing phase seems like a workaround than a solution). So let's > mark it. Maybe I or somebody else could revisit it later. > > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> > --- > t/t4010-diff-pathspec.sh | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh > index 15a4912..b54251a 100755 > --- a/t/t4010-diff-pathspec.sh > +++ b/t/t4010-diff-pathspec.sh > @@ -127,4 +127,10 @@ test_expect_success 'diff-tree ignores trailing slash on submodule path' ' > test_cmp expect actual > ' > > +test_expect_failure 'diff-cache ignores trailing slash on submodule path' ' > + git diff --name-only HEAD^ submod >expect && > + git diff --name-only HEAD^ submod/ >actual &&
I actually doubt that the second line is expecting the right behaviour in the first place. As far as the top-level project is concerned, "submod" is the name it wants, as there is nothing underneath it. Even if asked to recurse infinite levels, the caller shouldn't be feeding paths like "submod/a/b/c" to match_pathspec_depth() in the first place, no?
> + test_cmp expect actual > +' > + > test_done
Jens LehmannRe: [PATCH 2/2] t4010: match_pathspec_depth() and trailing slash after submodule
Am 23.01.2014 22:09, schrieb Junio C Hamano:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: >> diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh >> index 15a4912..b54251a 100755 >> --- a/t/t4010-diff-pathspec.sh >> +++ b/t/t4010-diff-pathspec.sh >> @@ -127,4 +127,10 @@ test_expect_success 'diff-tree ignores trailing slash on submodule path' ' >> test_cmp expect actual >> ' >> >> +test_expect_failure 'diff-cache ignores trailing slash on submodule path' ' >> + git diff --name-only HEAD^ submod >expect && >> + git diff --name-only HEAD^ submod/ >actual && > > I actually doubt that the second line is expecting the right > behaviour in the first place. As far as the top-level project is > concerned, "submod" is the name it wants, as there is nothing > underneath it. Even if asked to recurse infinite levels, the caller > shouldn't be feeding paths like "submod/a/b/c" to > match_pathspec_depth() in the first place, no?
Agreed, "submod/a/b/c" would not make sense here. But a single trailing '/' does mark "submod" as a directory, which I think is ok for a submodule. And it makes life easier for the user if we accept that, as shell completion will add it there automatically.
Junio C HamanoRe: [PATCH 2/2] t4010: match_pathspec_depth() and trailing slash after submodule
Jens Lehmann <Jens.Lehmann@web.de> writes:
> ... But a single > trailing '/' does mark "submod" as a directory, which I think is > ok for a submodule. And it makes life easier for the user if we > accept that, as shell completion will add it there automatically.
OK, that would be annoying.
Perhaps the completion is what is broken here, then? I dunno, and haven't thought things through.
Duy NguyenRe: [PATCH 2/2] t4010: match_pathspec_depth() and trailing slash after submodule
On Fri, Jan 24, 2014 at 4:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jens Lehmann <Jens.Lehmann@web.de> writes: > >> ... But a single >> trailing '/' does mark "submod" as a directory, which I think is >> ok for a submodule. And it makes life easier for the user if we >> accept that, as shell completion will add it there automatically. > > OK, that would be annoying. > > Perhaps the completion is what is broken here, then? I dunno, and > haven't thought things through.
My reasoning is more simple minded: if "git diff HEAD HEAD^ submod/" works, the user would expect "git diff HEAD submod/" to work too. I think I've got something working with a bit refactoring. Will send out soon.
-- Duy
Nguyễn Thái Ngọc Duy[PATCH v2 0/9] About the trailing slashes
So this is the reroll that makes
git diff HEAD submodule git diff HEAD submodule/
and
git diff HEAD HEAD^ submodule git diff HEAD HEAD^ submodule/
behave the same way. The main patches are 04/09 and 09/09. The rest is just refactoring and cleaning up.
While looking at this, I found a funny behavior of fill_directory.
$ git init
$ mkdir b
$ >b/c
$ >b/d
$ git status b
Untracked files:
b/
$ git status b/
Untracked files:
b/c b/dNotice how the trailing slash produces different untracked listing. This is because of common_prefix_len(). In the "b" case, common_prefix_len() returns empty prefix, so read_directory reads top directory, traverses through, reaches "b" and eventually calls treat_directory() on it, which results in "b/" in the output.
In the "b/" case, common_prefix_len() found the prefix "b", so read_directory() starts at "b" instead of b's parent. treat_directory() is never called on "b" itself, which results in "b/c" and "b/d".
I'm tempted to make it consistent, favoring "b/" output. But that involves extra lstat, or reordering the complex read_directory() code. This is probably very unusual case to pay attention to anyway.
Nguyễn Thái Ngọc Duy (9):
Convert some match_pathspec_depth() to ce_path_match()
Convert some match_pathspec_depth() to dir_path_match()
Rename match_pathspec_depth() to match_pathspec()
dir.c: prepare match_pathspec_item for taking more flags
match_pathspec: match pathspec "foo/" against directory "foo"
Pass directory indicator to match_pathspec_item()
clean: replace match_pathspec() with dir_path_match()
clean: use cache_name_is_other()
tree-walk.c: ignore trailing slash on submodule in
tree_entry_interesting()builtin/add.c | 3 +-- builtin/checkout.c | 3 +-- builtin/clean.c | 24 +++--------------------- builtin/commit.c | 2 +- builtin/grep.c | 6 ++---- builtin/ls-files.c | 9 ++++++--- builtin/ls-tree.c | 2 +- builtin/rm.c | 2 +- builtin/update-index.c | 3 ++- cache.h | 2 -- diff-lib.c | 5 +++-- dir.c | 40 ++++++++++++++++++++++++++-------------- dir.h | 24 +++++++++++++++++++++--- pathspec.c | 2 +- preload-index.c | 3 ++- read-cache.c | 8 +------- rerere.c | 4 ++-- resolve-undo.c | 2 +- revision.c | 3 ++- t/t4010-diff-pathspec.sh | 23 +++++++++++++++++++++++ t/t6131-pathspec-icase.sh | 6 +++--- tree-walk.c | 2 +- wt-status.c | 6 +++--- 23 files changed, 107 insertions(+), 77 deletions(-)
-- 1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy[PATCH v2 1/9] Convert some match_pathspec_depth() to ce_path_match()
This helps reduce the number of match_pathspec_depth() call sites and show how match_pathspec_depth() is used.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- builtin/checkout.c | 3 +-- builtin/commit.c | 2 +- builtin/grep.c | 2 +- builtin/rm.c | 2 +- builtin/update-index.c | 3 ++- cache.h | 2 -- diff-lib.c | 5 +++-- dir.h | 7 +++++++ pathspec.c | 2 +- preload-index.c | 3 ++- read-cache.c | 8 +------- resolve-undo.c | 2 +- revision.c | 3 ++- wt-status.c | 2 +- 14 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c index 5df3837..ada51fa 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -297,8 +297,7 @@ static int checkout_paths(const struct checkout_opts *opts, * match_pathspec() for _all_ entries when * opts->source_tree != NULL. */ - if (match_pathspec_depth(&opts->pathspec, ce->name, ce_namelen(ce), - 0, ps_matched)) + if (ce_path_match(ce, &opts->pathspec, ps_matched)) ce->ce_flags |= CE_MATCHED; } diff --git a/builtin/commit.c b/builtin/commit.c index 3767478..26b2986 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -234,7 +234,7 @@ static int list_paths(struct string_list *list, const char *with_tree, if (ce->ce_flags & CE_UPDATE) continue; - if (!match_pathspec_depth(pattern, ce->name, ce_namelen(ce), 0, m)) + if (!ce_path_match(ce, pattern, m)) continue; item = string_list_insert(list, ce->name); if (ce_skip_worktree(ce)) diff --git a/builtin/grep.c b/builtin/grep.c index 63f8603..3d924c2 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -379,7 +379,7 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int const struct cache_entry *ce = active_cache[nr]; if (!S_ISREG(ce->ce_mode)) continue; - if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL)) + if (!ce_path_match(ce, pathspec, NULL)) continue; /* * If CE_VALID is on, we assume worktree file and its cache entry diff --git a/builtin/rm.c b/builtin/rm.c index 3a0e0ea..0564218 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -308,7 +308,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix) for (i = 0; i < active_nr; i++) { const struct cache_entry *ce = active_cache[i]; - if (!match_pathspec_depth(&pathspec, ce->name, ce_namelen(ce), 0, seen)) + if (!ce_path_match(ce, &pathspec, seen)) continue; ALLOC_GROW(list.entry, list.nr + 1, list.alloc); list.entry[list.nr].name = ce->name; diff --git a/builtin/update-index.c b/builtin/update-index.c index e3a10d7..aaa6f78 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -12,6 +12,7 @@ #include "resolve-undo.h" #include "parse-options.h" #include "pathspec.h" +#include "dir.h" /* * Default to not allowing changes to the list of files. The @@ -564,7 +565,7 @@ static int do_reupdate(int ac, const char **av, struct cache_entry *old = NULL; int save_nr; - if (ce_stage(ce) || !ce_path_match(ce, &pathspec)) + if (ce_stage(ce) || !ce_path_match(ce, &pathspec, NULL)) continue; if (has_head) old = read_one_ent(NULL, head_sha1, diff --git a/cache.h b/cache.h index 83a2726..3c558f8 100644 --- a/cache.h +++ b/cache.h @@ -500,8 +500,6 @@ extern void *read_blob_data_from_index(struct index_state *, const char *, unsig extern int ie_match_stat(const struct index_state *, const struct cache_entry *, struct stat *, unsigned int); extern int ie_modified(const struct index_state *, const struct cache_entry *, struct stat *, unsigned int); -extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec); - #define HASH_WRITE_OBJECT 1 #define HASH_FORMAT_CHECK 2 extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags); diff --git a/diff-lib.c b/diff-lib.c index 346cac6..2eddc66 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -11,6 +11,7 @@ #include "unpack-trees.h" #include "refs.h" #include "submodule.h" +#include "dir.h" /* * diff-files @@ -108,7 +109,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option) if (diff_can_quit_early(&revs->diffopt)) break; - if (!ce_path_match(ce, &revs->prune_data)) + if (!ce_path_match(ce, &revs->prune_data, NULL)) continue; if (ce_stage(ce)) { @@ -438,7 +439,7 @@ static int oneway_diff(const struct cache_entry * const *src, if (tree == o->df_conflict_entry) tree = NULL; - if (ce_path_match(idx ? idx : tree, &revs->prune_data)) { + if (ce_path_match(idx ? idx : tree, &revs->prune_data, NULL)) { do_oneway_diff(o, idx, tree); if (diff_can_quit_early(&revs->diffopt)) { o->exiting_early = 1; diff --git a/dir.h b/dir.h index 9b7e4e7..42793e5 100644 --- a/dir.h +++ b/dir.h @@ -205,4 +205,11 @@ extern int git_fnmatch(const struct pathspec_item *item, const char *pattern, const char *string, int prefix); +static inline int ce_path_match(const struct cache_entry *ce, + const struct pathspec *pathspec, + char *seen) +{ + return match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, seen); +} + #endif diff --git a/pathspec.c b/pathspec.c index 6cb9fd3..8043099 100644 --- a/pathspec.c +++ b/pathspec.c @@ -33,7 +33,7 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec, return; for (i = 0; i < active_nr; i++) { const struct cache_entry *ce = active_cache[i]; - match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, seen); + ce_path_match(ce, pathspec, seen); } } diff --git a/preload-index.c b/preload-index.c index 8c44ceb..968ee25 100644 --- a/preload-index.c +++ b/preload-index.c @@ -3,6 +3,7 @@ */ #include "cache.h" #include "pathspec.h" +#include "dir.h" #ifdef NO_PTHREADS static void preload_index(struct index_state *index, @@ -53,7 +54,7 @@ static void *preload_thread(void *_data) continue; if (ce_uptodate(ce)) continue; - if (!ce_path_match(ce, &p->pathspec)) + if (!ce_path_match(ce, &p->pathspec, NULL)) continue; if (threaded_has_symlink_leading_path(&cache, ce->name, ce_namelen(ce))) continue; diff --git a/read-cache.c b/read-cache.c index 33dd676..23eb251 100644 --- a/read-cache.c +++ b/read-cache.c @@ -728,11 +728,6 @@ int ce_same_name(const struct cache_entry *a, const struct cache_entry *b) return ce_namelen(b) == len && !memcmp(a->name, b->name, len); } -int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec) -{ - return match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL); -} - /* * We fundamentally don't like some paths: we don't want * dot or dot-dot anywhere, and for obvious reasons don't @@ -1149,8 +1144,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, if (ignore_submodules && S_ISGITLINK(ce->ce_mode)) continue; - if (pathspec && - !match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, seen)) + if (pathspec && !ce_path_match(ce, pathspec, seen)) filtered = 1; if (ce_stage(ce)) { diff --git a/resolve-undo.c b/resolve-undo.c index c09b006..67d1543 100644 --- a/resolve-undo.c +++ b/resolve-undo.c @@ -182,7 +182,7 @@ void unmerge_index(struct index_state *istate, const struct pathspec *pathspec) for (i = 0; i < istate->cache_nr; i++) { const struct cache_entry *ce = istate->cache[i]; - if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL)) + if (!ce_path_match(ce, pathspec, NULL)) continue; i = unmerge_index_entry_at(istate, i); } diff --git a/revision.c b/revision.c index a68fde6..e3dff61 100644 --- a/revision.c +++ b/revision.c @@ -16,6 +16,7 @@ #include "line-log.h" #include "mailmap.h" #include "commit-slab.h" +#include "dir.h" volatile show_early_output_fn_t show_early_output; @@ -1396,7 +1397,7 @@ static void prepare_show_merge(struct rev_info *revs) const struct cache_entry *ce = active_cache[i]; if (!ce_stage(ce)) continue; - if (ce_path_match(ce, &revs->prune_data)) { + if (ce_path_match(ce, &revs->prune_data, NULL)) { prune_num++; prune = xrealloc(prune, sizeof(*prune) * prune_num); prune[prune_num-2] = ce->name; diff --git a/wt-status.c b/wt-status.c index 4e55810..295c09e 100644 --- a/wt-status.c +++ b/wt-status.c @@ -510,7 +510,7 @@ static void wt_status_collect_changes_initial(struct wt_status *s) struct wt_status_change_data *d; const struct cache_entry *ce = active_cache[i]; - if (!ce_path_match(ce, &s->pathspec)) + if (!ce_path_match(ce, &s->pathspec, NULL)) continue; it = string_list_insert(&s->change, ce->name); d = it->util;
-- 1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy[PATCH v2 2/9] Convert some match_pathspec_depth() to dir_path_match()
This helps reduce the number of match_pathspec_depth() call sites and show how m_p_d() is used. And it usage is:
- match against an index entry (ce_path_match or match_pathspec_depth in ls-files)
- match against a dir_entry from read_directory (dir_path_match and match_pathspec_depth in clean.c, which will be converted later)
- resolve-undo (rerere.c and ls-files.c)
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- builtin/add.c | 3 +-- builtin/grep.c | 4 +--- builtin/ls-files.c | 2 +- dir.h | 7 +++++++ wt-status.c | 4 ++-- 5 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c index 2a2722f..672adc0 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -208,8 +208,7 @@ static char *prune_directory(struct dir_struct *dir, struct pathspec *pathspec, i = dir->nr; while (--i >= 0) { struct dir_entry *entry = *src++; - if (match_pathspec_depth(pathspec, entry->name, entry->len, - prefix, seen)) + if (dir_path_match(entry, pathspec, prefix, seen)) *dst++ = entry; else if (flag & WARN_IMPLICIT_DOT) /* diff --git a/builtin/grep.c b/builtin/grep.c index 3d924c2..69ac2d8 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -524,9 +524,7 @@ static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec, fill_directory(&dir, pathspec); for (i = 0; i < dir.nr; i++) { - const char *name = dir.entries[i]->name; - int namelen = strlen(name); - if (!match_pathspec_depth(pathspec, name, namelen, 0, NULL)) + if (!dir_path_match(dir.entries[i], pathspec, 0, NULL)) continue; hit |= grep_file(opt, dir.entries[i]->name); if (hit && opt->status_only) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index e1cf6d8..e238608 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -64,7 +64,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent) if (len >= ent->len) die("git ls-files: internal error - directory entry not superset of prefix"); - if (!match_pathspec_depth(&pathspec, ent->name, ent->len, len, ps_matched)) + if (!dir_path_match(ent, &pathspec, len, ps_matched)) return; fputs(tag, stdout); diff --git a/dir.h b/dir.h index 42793e5..65f54b6 100644 --- a/dir.h +++ b/dir.h @@ -212,4 +212,11 @@ static inline int ce_path_match(const struct cache_entry *ce, return match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, seen); } +static inline int dir_path_match(const struct dir_entry *ent, + const struct pathspec *pathspec, + int prefix, char *seen) +{ + return match_pathspec_depth(pathspec, ent->name, ent->len, prefix, seen); +} + #endif diff --git a/wt-status.c b/wt-status.c index 295c09e..a452407 100644 --- a/wt-status.c +++ b/wt-status.c @@ -552,7 +552,7 @@ static void wt_status_collect_untracked(struct wt_status *s) for (i = 0; i < dir.nr; i++) { struct dir_entry *ent = dir.entries[i]; if (cache_name_is_other(ent->name, ent->len) && - match_pathspec_depth(&s->pathspec, ent->name, ent->len, 0, NULL)) + dir_path_match(ent, &s->pathspec, 0, NULL)) string_list_insert(&s->untracked, ent->name); free(ent); } @@ -560,7 +560,7 @@ static void wt_status_collect_untracked(struct wt_status *s) for (i = 0; i < dir.ignored_nr; i++) { struct dir_entry *ent = dir.ignored[i]; if (cache_name_is_other(ent->name, ent->len) && - match_pathspec_depth(&s->pathspec, ent->name, ent->len, 0, NULL)) + dir_path_match(ent, &s->pathspec, 0, NULL)) string_list_insert(&s->ignored, ent->name); free(ent); }
-- 1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy[PATCH v2 3/9] Rename match_pathspec_depth() to match_pathspec()
A long time ago, for some reason I was not happy with match_pathspec(). I created a better version, match_pathspec_depth() that was suppose to replace match_pathspec() eventually. match_pathspec() has finally been gone since 6 months ago. Use the shorter name for match_pathspec_depth().
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- builtin/clean.c | 4 ++-- builtin/ls-files.c | 6 ++++-- builtin/ls-tree.c | 2 +- dir.c | 20 ++++++++++---------- dir.h | 10 +++++----- rerere.c | 4 ++-- t/t6131-pathspec-icase.sh | 6 +++--- 7 files changed, 27 insertions(+), 25 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c index 2f26297..f59c753 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -961,8 +961,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix) die_errno("Cannot lstat '%s'", ent->name); if (pathspec.nr) - matches = match_pathspec_depth(&pathspec, ent->name, - len, 0, NULL); + matches = match_pathspec(&pathspec, ent->name, + len, 0, NULL); if (S_ISDIR(st.st_mode)) { if (remove_directories || (matches == MATCHED_EXACTLY)) { diff --git a/builtin/ls-files.c b/builtin/ls-files.c index e238608..02db0e1 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -139,7 +139,8 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce) if (len >= ce_namelen(ce)) die("git ls-files: internal error - cache entry not superset of prefix"); - if (!match_pathspec_depth(&pathspec, ce->name, ce_namelen(ce), len, ps_matched)) + if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce), + len, ps_matched)) return; if (tag && *tag && show_valid_bit && @@ -195,7 +196,8 @@ static void show_ru_info(void) len = strlen(path); if (len < max_prefix_len) continue; /* outside of the prefix */ - if (!match_pathspec_depth(&pathspec, path, len, max_prefix_len, ps_matched)) + if (!match_pathspec(&pathspec, path, len, + max_prefix_len, ps_matched)) continue; /* uninterested */ for (i = 0; i < 3; i++) { if (!ui->mode[i]) diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c index 65ec931..51184df 100644 --- a/builtin/ls-tree.c +++ b/builtin/ls-tree.c @@ -171,7 +171,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix) * show_recursive() rolls its own matching code and is * generally ignorant of 'struct pathspec'. The magic mask * cannot be lifted until it is converted to use - * match_pathspec_depth() or tree_entry_interesting() + * match_pathspec() or tree_entry_interesting() */ parse_pathspec(&pathspec, PATHSPEC_GLOB | PATHSPEC_ICASE, PATHSPEC_PREFER_CWD, diff --git a/dir.c b/dir.c index d10a63f..8960ea1 100644 --- a/dir.c +++ b/dir.c @@ -218,7 +218,7 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix, * The normal call pattern is: * 1. prefix = common_prefix_len(ps); * 2. prune something, or fill_directory - * 3. match_pathspec_depth() + * 3. match_pathspec() * * 'prefix' at #1 may be shorter than the command's prefix and * it's ok for #2 to match extra files. Those extras will be @@ -282,10 +282,10 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix, * pathspec did not match any names, which could indicate that the * user mistyped the nth pathspec. */ -static int match_pathspec_depth_1(const struct pathspec *ps, - const char *name, int namelen, - int prefix, char *seen, - int exclude) +static int do_match_pathspec(const struct pathspec *ps, + const char *name, int namelen, + int prefix, char *seen, + int exclude) { int i, retval = 0; @@ -350,15 +350,15 @@ static int match_pathspec_depth_1(const struct pathspec *ps, return retval; } -int match_pathspec_depth(const struct pathspec *ps, - const char *name, int namelen, - int prefix, char *seen) +int match_pathspec(const struct pathspec *ps, + const char *name, int namelen, + int prefix, char *seen) { int positive, negative; - positive = match_pathspec_depth_1(ps, name, namelen, prefix, seen, 0); + positive = do_match_pathspec(ps, name, namelen, prefix, seen, 0); if (!(ps->magic & PATHSPEC_EXCLUDE) || !positive) return positive; - negative = match_pathspec_depth_1(ps, name, namelen, prefix, seen, 1); + negative = do_match_pathspec(ps, name, namelen, prefix, seen, 1); return negative ? 0 : positive; } diff --git a/dir.h b/dir.h index 65f54b6..c31ed9a 100644 --- a/dir.h +++ b/dir.h @@ -132,9 +132,9 @@ struct dir_struct { extern int simple_length(const char *match); extern int no_wildcard(const char *string); extern char *common_prefix(const struct pathspec *pathspec); -extern int match_pathspec_depth(const struct pathspec *pathspec, - const char *name, int namelen, - int prefix, char *seen); +extern int match_pathspec(const struct pathspec *pathspec, + const char *name, int namelen, + int prefix, char *seen); extern int within_depth(const char *name, int namelen, int depth, int max_depth); extern int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec); @@ -209,14 +209,14 @@ static inline int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec, char *seen) { - return match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, seen); + return match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen); } static inline int dir_path_match(const struct dir_entry *ent, const struct pathspec *pathspec, int prefix, char *seen) { - return match_pathspec_depth(pathspec, ent->name, ent->len, prefix, seen); + return match_pathspec(pathspec, ent->name, ent->len, prefix, seen); } #endif diff --git a/rerere.c b/rerere.c index 1f2d21a..34a21c4 100644 --- a/rerere.c +++ b/rerere.c @@ -672,8 +672,8 @@ int rerere_forget(struct pathspec *pathspec) find_conflict(&conflict); for (i = 0; i < conflict.nr; i++) { struct string_list_item *it = &conflict.items[i]; - if (!match_pathspec_depth(pathspec, it->string, strlen(it->string), - 0, NULL)) + if (!match_pathspec(pathspec, it->string, + strlen(it->string), 0, NULL)) continue; rerere_forget_one_path(it->string, &merge_rr); } diff --git a/t/t6131-pathspec-icase.sh b/t/t6131-pathspec-icase.sh index a7c7ff5..39fc3f6 100755 --- a/t/t6131-pathspec-icase.sh +++ b/t/t6131-pathspec-icase.sh @@ -69,7 +69,7 @@ test_expect_success 'tree_entry_interesting matches :(icase)bar with empty prefi test_cmp expect actual ' -test_expect_success 'match_pathspec_depth matches :(icase)bar' ' +test_expect_success 'match_pathspec matches :(icase)bar' ' cat <<-EOF >expect && BAR bAr @@ -79,7 +79,7 @@ test_expect_success 'match_pathspec_depth matches :(icase)bar' ' test_cmp expect actual ' -test_expect_success 'match_pathspec_depth matches :(icase)bar with prefix' ' +test_expect_success 'match_pathspec matches :(icase)bar with prefix' ' cat <<-EOF >expect && fOo/BAR fOo/bAr @@ -89,7 +89,7 @@ test_expect_success 'match_pathspec_depth matches :(icase)bar with prefix' ' test_cmp expect actual ' -test_expect_success 'match_pathspec_depth matches :(icase)bar with empty prefix' ' +test_expect_success 'match_pathspec matches :(icase)bar with empty prefix' ' cat <<-EOF >expect && bar fOo/BAR
-- 1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy[PATCH v2 4/9] dir.c: prepare match_pathspec_item for taking more flags
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- dir.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/dir.c b/dir.c index 8960ea1..35e09bd 100644 --- a/dir.c +++ b/dir.c @@ -195,6 +195,8 @@ int within_depth(const char *name, int namelen, return 1; } +#define DO_MATCH_EXCLUDE 1 + /* * Does 'match' match the given name? * A match is found if @@ -208,7 +210,7 @@ int within_depth(const char *name, int namelen, * It returns 0 when there is no match. */ static int match_pathspec_item(const struct pathspec_item *item, int prefix, - const char *name, int namelen) + const char *name, int namelen, unsigned flags) { /* name/namelen has prefix cut off by caller */ const char *match = item->match + prefix; @@ -285,9 +287,9 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix, static int do_match_pathspec(const struct pathspec *ps, const char *name, int namelen, int prefix, char *seen, - int exclude) + unsigned flags) { - int i, retval = 0; + int i, retval = 0, exclude = flags & DO_MATCH_EXCLUDE; GUARD_PATHSPEC(ps, PATHSPEC_FROMTOP | @@ -327,7 +329,8 @@ static int do_match_pathspec(const struct pathspec *ps, */ if (seen && ps->items[i].magic & PATHSPEC_EXCLUDE) seen[i] = MATCHED_FNMATCH; - how = match_pathspec_item(ps->items+i, prefix, name, namelen); + how = match_pathspec_item(ps->items+i, prefix, name, + namelen, flags); if (ps->recursive && (ps->magic & PATHSPEC_MAXDEPTH) && ps->max_depth != -1 && @@ -355,10 +358,14 @@ int match_pathspec(const struct pathspec *ps, int prefix, char *seen) { int positive, negative; - positive = do_match_pathspec(ps, name, namelen, prefix, seen, 0); + unsigned flags = 0; + positive = do_match_pathspec(ps, name, namelen, + prefix, seen, flags); if (!(ps->magic & PATHSPEC_EXCLUDE) || !positive) return positive; - negative = do_match_pathspec(ps, name, namelen, prefix, seen, 1); + negative = do_match_pathspec(ps, name, namelen, + prefix, seen, + flags | DO_MATCH_EXCLUDE); return negative ? 0 : positive; }
-- 1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy[PATCH v2 5/9] match_pathspec: match pathspec "foo/" against directory "foo"
Currently we do support matching pathspec "foo/" against directory "foo". That is because match_pathspec() has no way to tell "foo" is a directory and matching "foo/" against _file_ "foo" is wrong.
The callers can now tell match_pathspec if "foo" is a directory, we could make an exception for this case. Code is not executed though because no callers pass the flag yet.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- dir.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/dir.c b/dir.c index 35e09bd..7b50072 100644 --- a/dir.c +++ b/dir.c @@ -196,6 +196,7 @@ int within_depth(const char *name, int namelen, } #define DO_MATCH_EXCLUDE 1 +#define DO_MATCH_DIRECTORY 2 /* * Does 'match' match the given name? @@ -259,7 +260,11 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix, if (match[matchlen-1] == '/' || name[matchlen] == '/') return MATCHED_RECURSIVELY; - } + } else if ((flags & DO_MATCH_DIRECTORY) && + match[matchlen - 1] == '/' && + namelen == matchlen - 1 && + !ps_strncmp(item, match, name, namelen)) + return MATCHED_EXACTLY; if (item->nowildcard_len < item->len && !git_fnmatch(item, match, name,
-- 1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy[PATCH v2 6/9] Pass directory indicator to match_pathspec_item()
This patch activates the DO_MATCH_DIRECTORY code in m_p_i(), which makes "git diff HEAD submodule/" and "git diff HEAD submodule" produce the same output. Previously only the version without trailing slash returns the difference (if any).
That's the effect of new ce_path_match(). dir_path_match() is not executed by the new tests. And it should not introduce regressions.
Previously if path "dir/" is passed in with pathspec "dir/", they obviously match. With new dir_path_match(), the path becomes _directory_ "dir" vs pathspec "dir/", which is not executed by the old code path in m_p_i(). The new code path is executed and produces the same result.
The other case is pathspec "dir" and path "dir/" is now turned to "dir" (with DO_MATCH_DIRECTORY). Still the same result before or after the patch.
So why change? Because of the next patch about clean.c.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- builtin/clean.c | 2 +- builtin/ls-files.c | 5 +++-- dir.c | 4 ++-- dir.h | 10 +++++++--- rerere.c | 2 +- t/t4010-diff-pathspec.sh | 17 +++++++++++++++++ 6 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c index f59c753..4c9680a 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -962,7 +962,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) if (pathspec.nr) matches = match_pathspec(&pathspec, ent->name, - len, 0, NULL); + len, 0, NULL, 0); if (S_ISDIR(st.st_mode)) { if (remove_directories || (matches == MATCHED_EXACTLY)) { diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 02db0e1..47c3880 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -140,7 +140,8 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce) die("git ls-files: internal error - cache entry not superset of prefix"); if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce), - len, ps_matched)) + len, ps_matched, + S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode))) return; if (tag && *tag && show_valid_bit && @@ -197,7 +198,7 @@ static void show_ru_info(void) if (len < max_prefix_len) continue; /* outside of the prefix */ if (!match_pathspec(&pathspec, path, len, - max_prefix_len, ps_matched)) + max_prefix_len, ps_matched, 0)) continue; /* uninterested */ for (i = 0; i < 3; i++) { if (!ui->mode[i]) diff --git a/dir.c b/dir.c index 7b50072..2fd0ebd 100644 --- a/dir.c +++ b/dir.c @@ -360,10 +360,10 @@ static int do_match_pathspec(const struct pathspec *ps, int match_pathspec(const struct pathspec *ps, const char *name, int namelen, - int prefix, char *seen) + int prefix, char *seen, int is_dir) { int positive, negative; - unsigned flags = 0; + unsigned flags = is_dir ? DO_MATCH_DIRECTORY : 0; positive = do_match_pathspec(ps, name, namelen, prefix, seen, flags); if (!(ps->magic & PATHSPEC_EXCLUDE) || !positive) diff --git a/dir.h b/dir.h index c31ed9a..55e5345 100644 --- a/dir.h +++ b/dir.h @@ -134,7 +134,7 @@ extern int no_wildcard(const char *string); extern char *common_prefix(const struct pathspec *pathspec); extern int match_pathspec(const struct pathspec *pathspec, const char *name, int namelen, - int prefix, char *seen); + int prefix, char *seen, int is_dir); extern int within_depth(const char *name, int namelen, int depth, int max_depth); extern int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec); @@ -209,14 +209,18 @@ static inline int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec, char *seen) { - return match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen); + return match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen, + S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode)); } static inline int dir_path_match(const struct dir_entry *ent, const struct pathspec *pathspec, int prefix, char *seen) { - return match_pathspec(pathspec, ent->name, ent->len, prefix, seen); + int has_trailing_dir = ent->len && ent->name[ent->len - 1] == '/'; + int len = has_trailing_dir ? ent->len - 1 : ent->len; + return match_pathspec(pathspec, ent->name, len, prefix, seen, + has_trailing_dir); } #endif diff --git a/rerere.c b/rerere.c index 34a21c4..d55aa8a 100644 --- a/rerere.c +++ b/rerere.c @@ -673,7 +673,7 @@ int rerere_forget(struct pathspec *pathspec) for (i = 0; i < conflict.nr; i++) { struct string_list_item *it = &conflict.items[i]; if (!match_pathspec(pathspec, it->string, - strlen(it->string), 0, NULL)) + strlen(it->string), 0, NULL, 0)) continue; rerere_forget_one_path(it->string, &merge_rr); } diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh index af5134b..167af53 100755 --- a/t/t4010-diff-pathspec.sh +++ b/t/t4010-diff-pathspec.sh @@ -110,4 +110,21 @@ test_expect_success 'diff-tree -r with wildcard' ' test_cmp expected result ' +test_expect_success 'setup submodules' ' + test_tick && + git init submod && + ( cd submod && test_commit first; ) && + git add submod && + git commit -m first && + ( cd submod && test_commit second; ) && + git add submod && + git commit -m second +' + +test_expect_success 'diff-cache ignores trailing slash on submodule path' ' + git diff --name-only HEAD^ submod >expect && + git diff --name-only HEAD^ submod/ >actual && + test_cmp expect actual +' + test_done
-- 1.8.5.2.240.g8478abd
Eric SunshineRe: [PATCH v2 6/9] Pass directory indicator to match_pathspec_item()
On Fri, Jan 24, 2014 at 8:40 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh > index af5134b..167af53 100755 > --- a/t/t4010-diff-pathspec.sh > +++ b/t/t4010-diff-pathspec.sh > @@ -110,4 +110,21 @@ test_expect_success 'diff-tree -r with wildcard' ' > test_cmp expected result > ' > > +test_expect_success 'setup submodules' ' > + test_tick && > + git init submod && > + ( cd submod && test_commit first; ) &&
Unnecessary semicolon might confuse the reader into thinking something unusual is going on here.
> + git add submod && > + git commit -m first && > + ( cd submod && test_commit second; ) &&
Ditto.
> + git add submod && > + git commit -m second > +' > + > +test_expect_success 'diff-cache ignores trailing slash on submodule path' ' > + git diff --name-only HEAD^ submod >expect && > + git diff --name-only HEAD^ submod/ >actual && > + test_cmp expect actual > +' > + > test_done
Duy NguyenRe: [PATCH v2 6/9] Pass directory indicator to match_pathspec_item()
On Sat, Jan 25, 2014 at 4:22 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Fri, Jan 24, 2014 at 8:40 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote: >> diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh >> index af5134b..167af53 100755 >> --- a/t/t4010-diff-pathspec.sh >> +++ b/t/t4010-diff-pathspec.sh >> @@ -110,4 +110,21 @@ test_expect_success 'diff-tree -r with wildcard' ' >> test_cmp expected result >> ' >> >> +test_expect_success 'setup submodules' ' >> + test_tick &&
Also the test_tick here. I thought I would need to match SHA-1 but I did not and still forgot to take this line out.
>> + git init submod && >> + ( cd submod && test_commit first; ) && > > Unnecessary semicolon might confuse the reader into thinking something > unusual is going on here. > >> + git add submod && >> + git commit -m first && >> + ( cd submod && test_commit second; ) && > > Ditto. > >> + git add submod && >> + git commit -m second >> +' >> + >> +test_expect_success 'diff-cache ignores trailing slash on submodule path' ' >> + git diff --name-only HEAD^ submod >expect && >> + git diff --name-only HEAD^ submod/ >actual && >> + test_cmp expect actual >> +' >> + >> test_done
-- Duy
Nguyễn Thái Ngọc Duy[PATCH v2 7/9] clean: replace match_pathspec() with dir_path_match()
This instance was left out when many match_pathspec() call sites that take input from dir_entry were converted to dir_path_match() because it passed a path with the trailing slash stripped out to match_pathspec() while the others did not. Stripping for all call sites back then would be a regression because match_pathspec() did not know how to match pathspec foo/ against _directory_ foo (the stripped version of path "foo/").
match_pathspec() knows how to do it now. And dir_path_match() strips the trailing slash also. Use the new function, because the stripping code is removed in the next patch.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- builtin/clean.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c index 4c9680a..5adb52d 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -961,8 +961,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) die_errno("Cannot lstat '%s'", ent->name); if (pathspec.nr) - matches = match_pathspec(&pathspec, ent->name, - len, 0, NULL, 0); + matches = dir_path_match(ent, &pathspec, 0, NULL); if (S_ISDIR(st.st_mode)) { if (remove_directories || (matches == MATCHED_EXACTLY)) {
-- 1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy[PATCH v2 8/9] clean: use cache_name_is_other()
cmd_clean() has the exact same code of index_name_is_other(). Reduce code duplication.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- builtin/clean.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c index 5adb52d..cb02a53 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -933,29 +933,12 @@ int cmd_clean(int argc, const char **argv, const char *prefix) for (i = 0; i < dir.nr; i++) { struct dir_entry *ent = dir.entries[i]; - int len, pos; int matches = 0; - const struct cache_entry *ce; struct stat st; const char *rel; - /* - * Remove the '/' at the end that directory - * walking adds for directory entries. - */ - len = ent->len; - if (len && ent->name[len-1] == '/') - len--; - pos = cache_name_pos(ent->name, len); - if (0 <= pos) - continue; /* exact match */ - pos = -pos - 1; - if (pos < active_nr) { - ce = active_cache[pos]; - if (ce_namelen(ce) == len && - !memcmp(ce->name, ent->name, len)) - continue; /* Yup, this one exists unmerged */ - } + if (!cache_name_is_other(ent->name, ent->len)) + continue; if (lstat(ent->name, &st)) die_errno("Cannot lstat '%s'", ent->name);
-- 1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy[PATCH v2 9/9] tree-walk.c: ignore trailing slash on submodule in tree_entry_interesting()
We do ignore trailing slash on a directory, so pathspec "abc/" matches directory "abc". A submodule is also a directory. Apply the same logic to it. This makes "git log submodule-path" and "git log submodule-path/" produce the same output.
Reported-by: Paweł Sikora <pawel.sikora@agmk.net> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- t/t4010-diff-pathspec.sh | 6 ++++++ tree-walk.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh index 167af53..2424af0 100755 --- a/t/t4010-diff-pathspec.sh +++ b/t/t4010-diff-pathspec.sh @@ -127,4 +127,10 @@ test_expect_success 'diff-cache ignores trailing slash on submodule path' ' test_cmp expect actual ' +test_expect_success 'diff-tree ignores trailing slash on submodule path' ' + git diff --name-only HEAD^ HEAD submod >expect && + git diff --name-only HEAD^ HEAD submod/ >actual && + test_cmp expect actual +' + test_done diff --git a/tree-walk.c b/tree-walk.c index 680afda..c29b6a3 100644 --- a/tree-walk.c +++ b/tree-walk.c @@ -543,7 +543,7 @@ static int match_entry(const struct pathspec_item *item, if (matchlen > pathlen) { if (match[pathlen] != '/') return 0; - if (!S_ISDIR(entry->mode)) + if (!S_ISDIR(entry->mode) && !S_ISGITLINK(entry->mode)) return 0; }
-- 1.8.5.2.240.g8478abd
Junio C HamanoRe: [PATCH v2 0/9] About the trailing slashes
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> While looking at this, I found a funny behavior of fill_directory. > > $ git init > $ mkdir b > $ >b/c > $ >b/d > $ git status b > Untracked files: > b/ > $ git status b/ > Untracked files: > b/c b/d > > Notice how the trailing slash produces different untracked listing. > This is because of common_prefix_len(). In the "b" case, > common_prefix_len() returns empty prefix, so read_directory reads top > directory, traverses through, reaches "b" and eventually calls > treat_directory() on it, which results in "b/" in the output. > > In the "b/" case, common_prefix_len() found the prefix "b", so > read_directory() starts at "b" instead of b's parent. > treat_directory() is never called on "b" itself, which results in > "b/c" and "b/d".
Hmm, this feels like a borderline case.
If the user asked "git status ?", or even "git status '?'", it seems to me that the user wanted to get "git status" output but with a scope limited to top-level entries with a single letter. And the unlimited output asks to consolidate output for directories that have nothing interesting. "git status b" and "git status '[b]'" are asking similar thing, but not just limiting the scope to any one letter but to 'b'. So the output you showed above looks correct to me. On the other hand, the other request "git status b/" seems to me that the user is very aware that 'b' is a directory and wants to look inside, so again the output you showed looks correct to me.
How does "git status '[b]/'" behave?
Duy NguyenRe: [PATCH v2 0/9] About the trailing slashes
On Sat, Jan 25, 2014 at 2:22 AM, Junio C Hamano <gitster@pobox.com> wrote:
> How does "git status '[b]/'" behave?
It outputs "b/" (like "git status b"). I think that's because common_prefix_len() stops looking at the first wildcard char, '['. So common prefix is empty, just like "b".
-- Duy
Junio C HamanoRe: [PATCH v2 0/9] About the trailing slashes
Thanks; will try to rebase on top of more recent codebase and then review.