| Message ID | cover.1726476401.git.ps@pks.im (mailing list archive) |
|---|---|
| Headers | show |
| Series | refs/reftable: wire up exclude patterns | expand |
Patrick Steinhardt <ps@pks.im> 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@pks.im> > > 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
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@pks.im> 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 '