From 9d55dca262e8657b87dbd4c5b3494e1ecb7a0dbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Ascens=C3=A3o?= Date: Mon, 12 Nov 2018 13:25:43 +0000 Subject: [PATCH 1/2] refs: show --exclude failure with --branches/tags/remotes=glob MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The documentation of `--exclude=` option from rev-list and rev-parse explicitly states that exclude patterns *should not* start with 'refs/' when used with `--branches`, `--tags` or `--remotes`. However, following this advice results in refereces not being excluded if the next `--branches`, `--tags`, `--remotes` use the optional inclusive glob. Demonstrate this failure. Signed-off-by: Rafael Ascensão Signed-off-by: Junio C Hamano --- t/t6018-rev-list-glob.sh | 60 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/t/t6018-rev-list-glob.sh b/t/t6018-rev-list-glob.sh index 0bf10d0686..8e2b136356 100755 --- a/t/t6018-rev-list-glob.sh +++ b/t/t6018-rev-list-glob.sh @@ -36,7 +36,13 @@ test_expect_success 'setup' ' git tag foo/bar master && commit master3 && git update-ref refs/remotes/foo/baz master && - commit master4 + commit master4 && + git update-ref refs/remotes/upstream/one subspace/one && + git update-ref refs/remotes/upstream/two subspace/two && + git update-ref refs/remotes/upstream/x subspace-x && + git tag qux/one subspace/one && + git tag qux/two subspace/two && + git tag qux/x subspace-x ' test_expect_success 'rev-parse --glob=refs/heads/subspace/*' ' @@ -141,6 +147,54 @@ test_expect_success 'rev-parse accumulates multiple --exclude' ' compare rev-parse "--exclude=refs/remotes/* --exclude=refs/tags/* --all" --branches ' +test_expect_failure 'rev-parse --exclude=glob with --branches=glob' ' + compare rev-parse "--exclude=subspace-* --branches=sub*" "subspace/one subspace/two" +' + +test_expect_failure 'rev-parse --exclude=glob with --tags=glob' ' + compare rev-parse "--exclude=qux/? --tags=qux/*" "qux/one qux/two" +' + +test_expect_failure 'rev-parse --exclude=glob with --remotes=glob' ' + compare rev-parse "--exclude=upstream/? --remotes=upstream/*" "upstream/one upstream/two" +' + +test_expect_failure 'rev-parse --exclude=ref with --branches=glob' ' + compare rev-parse "--exclude=subspace-x --branches=sub*" "subspace/one subspace/two" +' + +test_expect_failure 'rev-parse --exclude=ref with --tags=glob' ' + compare rev-parse "--exclude=qux/x --tags=qux/*" "qux/one qux/two" +' + +test_expect_failure 'rev-parse --exclude=ref with --remotes=glob' ' + compare rev-parse "--exclude=upstream/x --remotes=upstream/*" "upstream/one upstream/two" +' + +test_expect_failure 'rev-list --exclude=glob with --branches=glob' ' + compare rev-list "--exclude=subspace-* --branches=sub*" "subspace/one subspace/two" +' + +test_expect_failure 'rev-list --exclude=glob with --tags=glob' ' + compare rev-list "--exclude=qux/? --tags=qux/*" "qux/one qux/two" +' + +test_expect_failure 'rev-list --exclude=glob with --remotes=glob' ' + compare rev-list "--exclude=upstream/? --remotes=upstream/*" "upstream/one upstream/two" +' + +test_expect_failure 'rev-list --exclude=ref with --branches=glob' ' + compare rev-list "--exclude=subspace-x --branches=sub*" "subspace/one subspace/two" +' + +test_expect_failure 'rev-list --exclude=ref with --tags=glob' ' + compare rev-list "--exclude=qux/x --tags=qux/*" "qux/one qux/two" +' + +test_expect_failure 'rev-list --exclude=ref with --remotes=glob' ' + compare rev-list "--exclude=upstream/x --remotes=upstream/*" "upstream/one upstream/two" +' + test_expect_success 'rev-list --glob=refs/heads/subspace/*' ' compare rev-list "subspace/one subspace/two" "--glob=refs/heads/subspace/*" @@ -233,7 +287,7 @@ test_expect_success 'rev-list --tags=foo' ' test_expect_success 'rev-list --tags' ' - compare rev-list "foo/bar" "--tags" + compare rev-list "foo/bar qux/x qux/two qux/one" "--tags" ' @@ -292,7 +346,7 @@ test_expect_success 'shortlog accepts --glob/--tags/--remotes' ' "master other/three someref subspace-x subspace/one subspace/two" \ "--glob=heads/*" && compare shortlog foo/bar --tags=foo && - compare shortlog foo/bar --tags && + compare shortlog "foo/bar qux/one qux/two qux/x" --tags && compare shortlog foo/baz --remotes=foo ' From 9ab9b5df0ee2a49e0c1354a667e1e00db903f516 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Ascens=C3=A3o?= Date: Mon, 12 Nov 2018 13:25:44 +0000 Subject: [PATCH 2/2] refs: fix some exclude patterns being ignored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--exclude` from rev-list and rev-parse fails to exclude references if the next `--branches`, `--tags` or `--remotes` use the optional inclusive glob because those options are implemented as particular cases of `--glob=`, which itself requires that exclude patterns begin with 'refs/'. But it makes sense for `--branches=glob` and friends to be aware that exclusions patterns for them shouldn't be 'refs//' prefixed, the same way exclude patterns for `--branches` and friends (without the optional glob) already are. Let's record in 'refs.c:struct ref_filter' which context the exclude pattern is tied to, so refs.c:filter_refs() can decide if it should ignore the prefix when trying to match. Signed-off-by: Rafael Ascensão Signed-off-by: Junio C Hamano --- refs.c | 4 ++++ t/t6018-rev-list-glob.sh | 24 ++++++++++++------------ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/refs.c b/refs.c index de81c7be7c..539f385f61 100644 --- a/refs.c +++ b/refs.c @@ -217,6 +217,7 @@ char *resolve_refdup(const char *refname, int resolve_flags, /* The argument to filter_refs */ struct ref_filter { const char *pattern; + const char *prefix; each_ref_fn *fn; void *cb_data; }; @@ -296,6 +297,8 @@ static int filter_refs(const char *refname, const struct object_id *oid, if (wildmatch(filter->pattern, refname, 0)) return 0; + if (filter->prefix) + skip_prefix(refname, filter->prefix, &refname); return filter->fn(refname, oid, flags, filter->cb_data); } @@ -458,6 +461,7 @@ int for_each_glob_ref_in(each_ref_fn fn, const char *pattern, } filter.pattern = real_pattern.buf; + filter.prefix = prefix; filter.fn = fn; filter.cb_data = cb_data; ret = for_each_ref(filter_refs, &filter); diff --git a/t/t6018-rev-list-glob.sh b/t/t6018-rev-list-glob.sh index 8e2b136356..7dc6cbdc42 100755 --- a/t/t6018-rev-list-glob.sh +++ b/t/t6018-rev-list-glob.sh @@ -147,51 +147,51 @@ test_expect_success 'rev-parse accumulates multiple --exclude' ' compare rev-parse "--exclude=refs/remotes/* --exclude=refs/tags/* --all" --branches ' -test_expect_failure 'rev-parse --exclude=glob with --branches=glob' ' +test_expect_success 'rev-parse --exclude=glob with --branches=glob' ' compare rev-parse "--exclude=subspace-* --branches=sub*" "subspace/one subspace/two" ' -test_expect_failure 'rev-parse --exclude=glob with --tags=glob' ' +test_expect_success 'rev-parse --exclude=glob with --tags=glob' ' compare rev-parse "--exclude=qux/? --tags=qux/*" "qux/one qux/two" ' -test_expect_failure 'rev-parse --exclude=glob with --remotes=glob' ' +test_expect_success 'rev-parse --exclude=glob with --remotes=glob' ' compare rev-parse "--exclude=upstream/? --remotes=upstream/*" "upstream/one upstream/two" ' -test_expect_failure 'rev-parse --exclude=ref with --branches=glob' ' +test_expect_success 'rev-parse --exclude=ref with --branches=glob' ' compare rev-parse "--exclude=subspace-x --branches=sub*" "subspace/one subspace/two" ' -test_expect_failure 'rev-parse --exclude=ref with --tags=glob' ' +test_expect_success 'rev-parse --exclude=ref with --tags=glob' ' compare rev-parse "--exclude=qux/x --tags=qux/*" "qux/one qux/two" ' -test_expect_failure 'rev-parse --exclude=ref with --remotes=glob' ' +test_expect_success 'rev-parse --exclude=ref with --remotes=glob' ' compare rev-parse "--exclude=upstream/x --remotes=upstream/*" "upstream/one upstream/two" ' -test_expect_failure 'rev-list --exclude=glob with --branches=glob' ' +test_expect_success 'rev-list --exclude=glob with --branches=glob' ' compare rev-list "--exclude=subspace-* --branches=sub*" "subspace/one subspace/two" ' -test_expect_failure 'rev-list --exclude=glob with --tags=glob' ' +test_expect_success 'rev-list --exclude=glob with --tags=glob' ' compare rev-list "--exclude=qux/? --tags=qux/*" "qux/one qux/two" ' -test_expect_failure 'rev-list --exclude=glob with --remotes=glob' ' +test_expect_success 'rev-list --exclude=glob with --remotes=glob' ' compare rev-list "--exclude=upstream/? --remotes=upstream/*" "upstream/one upstream/two" ' -test_expect_failure 'rev-list --exclude=ref with --branches=glob' ' +test_expect_success 'rev-list --exclude=ref with --branches=glob' ' compare rev-list "--exclude=subspace-x --branches=sub*" "subspace/one subspace/two" ' -test_expect_failure 'rev-list --exclude=ref with --tags=glob' ' +test_expect_success 'rev-list --exclude=ref with --tags=glob' ' compare rev-list "--exclude=qux/x --tags=qux/*" "qux/one qux/two" ' -test_expect_failure 'rev-list --exclude=ref with --remotes=glob' ' +test_expect_success 'rev-list --exclude=ref with --remotes=glob' ' compare rev-list "--exclude=upstream/x --remotes=upstream/*" "upstream/one upstream/two" '