Patrick Steinhardt <ps@xxxxxx> writes:
Hello,
> Hi,
>
> this is the second version of my patch series that fixes preexisting
> bugs with exclude patterns and wires them up for the reftable backend.
>
> Changes compared to v1:
>
> - Some typo fixes in commit messages.
>
> - Mention that the newly introduced function in patch 1 will be used
> in patch 2, which is why it's declared in "refs.h".
>
> - Use `check()` instead of `check_int(ret, ==, 0)`.
>
> - Sort exclude patterns lexicographically. This fixes a bug where we
> might not correctly skip over some refs when patterns are passed to
> the reftable backend in non-lexicographic order. Add a test for
> this.
>
> Thanks!
>
> Patrick
>
> Patrick Steinhardt (6):
> refs: properly apply exclude patterns to namespaced refs
> builtin/receive-pack: fix exclude patterns when announcing refs
> Makefile: stop listing test library objects twice
> t/unit-tests: introduce reftable library
> reftable/reader: make table iterator reseekable
> refs/reftable: wire up support for exclude patterns
>
> Makefile | 8 +-
> builtin/receive-pack.c | 18 ++-
> refs.c | 35 +++++-
> refs.h | 9 ++
> refs/reftable-backend.c | 133 ++++++++++++++++++++++-
> reftable/reader.c | 1 +
> t/t1419-exclude-refs.sh | 49 +++++++--
> t/t5509-fetch-push-namespaces.sh | 9 ++
> t/unit-tests/lib-reftable.c | 93 ++++++++++++++++
> t/unit-tests/lib-reftable.h | 20 ++++
> t/unit-tests/t-reftable-merged.c | 163 +++++++++++++++-------------
> t/unit-tests/t-reftable-reader.c | 96 ++++++++++++++++
> t/unit-tests/t-reftable-readwrite.c | 130 +++++++---------------
> t/unit-tests/t-reftable-stack.c | 25 ++---
> trace2.h | 1 +
> trace2/tr2_ctr.c | 5 +
> 16 files changed, 594 insertions(+), 201 deletions(-)
> create mode 100644 t/unit-tests/lib-reftable.c
> create mode 100644 t/unit-tests/lib-reftable.h
> create mode 100644 t/unit-tests/t-reftable-reader.c
>
> Range-diff against v1:
> 1: 8d347bc5599 ! 1: 7497166422e refs: properly apply exclude patterns to namespaced refs
> @@ Commit message
> to the non-stripped ones that still have the namespace prefix. In fact,
> the "transfer.hideRefs" machinery does the former and applies to the
> stripped reference by default, but rules can have "^" prefixed to switch
> - this behaviour to iinstead match against the rull reference name.
> + this behaviour to instead match against the full reference name.
>
> Namespaces are exclusively handled at the generic "refs" layer, the
> respective backends have no clue that such a thing even exists. This
> @@ Commit message
> refs in the tests, and then we indeed surface the breakage.
>
> Fix this bug by prefixing exclude patterns with the namespace in the
> - generic layer.
> + generic layer. The newly introduced function will be used outside of
> + "refs.c" in the next patch, so we add a declaration to "refs.h".
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
>
> 2: 0317a5a7ede = 2: 3dc6ae936c8 builtin/receive-pack: fix exclude patterns when announcing refs
> 3: 503c44e6cab = 3: 4ba503520e6 Makefile: stop listing test library objects twice
> 4: 3df4040dd3c = 4: 6747076420f t/unit-tests: introduce reftable library
> 5: a281f936a2b ! 5: 3278cdf92fe reftable/reader: make table iterator reseekable
> @@ t/unit-tests/t-reftable-reader.c (new)
> + block_source_from_strbuf(&source, &buf);
> +
> + ret = reftable_reader_new(&reader, &source, "name");
> -+ check_int(ret, ==, 0);
> ++ check(!ret);
> +
> + reftable_reader_init_ref_iterator(reader, &it);
> + ret = reftable_iterator_seek_ref(&it, "");
> -+ check_int(ret, ==, 0);
> ++ check(!ret);
> + ret = reftable_iterator_next_ref(&it, &ref);
> -+ check_int(ret, ==, 0);
> ++ check(!ret);
> +
> -+ ret = reftable_ref_record_equal(&ref, &records[0], 20);
> ++ ret = reftable_ref_record_equal(&ref, &records[0], GIT_SHA1_RAWSZ);
> + check_int(ret, ==, 1);
> +
> + ret = reftable_iterator_next_ref(&it, &ref);
> @@ t/unit-tests/t-reftable-reader.c (new)
> + block_source_from_strbuf(&source, &buf);
> +
> + ret = reftable_reader_new(&reader, &source, "name");
> -+ check_int(ret, ==, 0);
> ++ check(!ret);
> +
> + reftable_reader_init_ref_iterator(reader, &it);
> +
> + for (size_t i = 0; i < 5; i++) {
> + ret = reftable_iterator_seek_ref(&it, "");
> -+ check_int(ret, ==, 0);
> ++ check(!ret);
> + ret = reftable_iterator_next_ref(&it, &ref);
> -+ check_int(ret, ==, 0);
> ++ check(!ret);
> +
> + ret = reftable_ref_record_equal(&ref, &records[0], GIT_SHA1_RAWSZ);
> + check_int(ret, ==, 1);
> 6: f3922b81db6 ! 6: 050f4906393 refs/reftable: wire up support for exclude patterns
> @@ refs/reftable-backend.c: static struct ref_iterator_vtable reftable_ref_iterator
> .abort = reftable_ref_iterator_abort
> };
>
> ++static int qsort_strcmp(const void *va, const void *vb)
> ++{
> ++ const char *a = *(const char **)va;
> ++ const char *b = *(const char **)vb;
> ++ return strcmp(a, b);
> ++}
> ++
> +static char **filter_exclude_patterns(const char **exclude_patterns)
> +{
> + size_t filtered_size = 0, filtered_alloc = 0;
> @@ refs/reftable-backend.c: static struct ref_iterator_vtable reftable_ref_iterator
> + }
> +
> + if (filtered_size) {
> ++ QSORT(filtered, filtered_size, qsort_strcmp);
> + ALLOC_GROW(filtered, filtered_size + 1, filtered_alloc);
> + filtered[filtered_size++] = NULL;
> + }
> @@ t/t1419-exclude-refs.sh: test_expect_success 'several overlapping excluded regio
> + assert_jumps 3 perf;;
> + *)
> + BUG "unhandled ref format $GIT_DEFAULT_REF_FORMAT";;
> ++ esac
> ++'
> ++
> ++test_expect_success 'unordered excludes' '
> ++ for_each_ref__exclude refs/heads \
> ++ refs/heads/foo refs/heads/baz >actual 2>perf &&
> ++ for_each_ref refs/heads/bar refs/heads/quux >expect &&
> ++
> ++ test_cmp expect actual &&
> ++ case "$GIT_DEFAULT_REF_FORMAT" in
> ++ files)
> ++ assert_jumps 1 perf;;
> ++ reftable)
> ++ assert_jumps 2 perf;;
> ++ *)
> ++ BUG "unhandled ref format $GIT_DEFAULT_REF_FORMAT";;
> + esac
> '
>
The range diff here looks good based on my earlier review. Thanks
Attachment:
signature.asc
Description: PGP signature
