git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 0/6] refs/reftable: wire up exclude patterns

From
karthik nayak <karthik.188@gmail.com>
Date
Sep 17, 2024, 17:33 UTC
Message-ID
<CAOLa=ZTeZPeAOhSx3JnDGeeRwD9=af071FJZj+gWGqE29d4Adg@mail.gmail.com>
In-Reply-To
<cover.1726476401.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Hello,
Show 162 quoted lines
> 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
Previous: Taylor Blau
Message 36 of 36 in “refs/reftable: wire up exclude patterns”
  1. 0/6 refs/reftable: wire up exclude patternsPatrick Steinhardt, Sep 9, 2024
  2. 1/6 refs: properly apply exclude patterns to namespaced refsPatrick Steinhardt, Sep 9, 2024
  3. karthik nayakSep 13, 2024
  4. Patrick SteinhardtSep 16, 2024
  5. 2/6 builtin/receive-pack: fix exclude patterns when announcing refsPatrick Steinhardt, Sep 9, 2024
  6. karthik nayakSep 13, 2024
  7. 3/6 Makefile: stop listing test library objects twicePatrick Steinhardt, Sep 9, 2024
  8. 4/6 t/unit-tests: introduce reftable libraryPatrick Steinhardt, Sep 9, 2024
  9. 5/6 reftable/reader: make table iterator reseekablePatrick Steinhardt, Sep 9, 2024
  10. karthik nayakSep 13, 2024
  11. Patrick SteinhardtSep 16, 2024
  12. karthik nayakSep 17, 2024
  13. 6/6 refs/reftable: wire up support for exclude patternsPatrick Steinhardt, Sep 9, 2024
  14. karthik nayakSep 13, 2024
  15. Patrick SteinhardtSep 16, 2024
  16. karthik nayakSep 17, 2024
  17. karthik nayakSep 13, 2024
  18. Patrick SteinhardtSep 16, 2024
  19. 0/6 refs/reftable: wire up exclude patternsPatrick Steinhardt, Sep 16, 2024
  20. 1/6 refs: properly apply exclude patterns to namespaced refsPatrick Steinhardt, Sep 16, 2024
  21. Taylor BlauSep 17, 2024
  22. Patrick SteinhardtSep 17, 2024
  23. Taylor BlauSep 17, 2024
  24. Patrick SteinhardtSep 17, 2024
  25. Taylor BlauSep 17, 2024
  26. Patrick SteinhardtSep 17, 2024
  27. 2/6 builtin/receive-pack: fix exclude patterns when announcing refsPatrick Steinhardt, Sep 16, 2024
  28. Taylor BlauSep 17, 2024
  29. 3/6 Makefile: stop listing test library objects twicePatrick Steinhardt, Sep 16, 2024
  30. 4/6 t/unit-tests: introduce reftable libraryPatrick Steinhardt, Sep 16, 2024
  31. 5/6 reftable/reader: make table iterator reseekablePatrick Steinhardt, Sep 16, 2024
  32. 6/6 refs/reftable: wire up support for exclude patternsPatrick Steinhardt, Sep 16, 2024
  33. Taylor BlauSep 17, 2024
  34. Patrick SteinhardtSep 17, 2024
  35. Taylor BlauSep 17, 2024
  36. karthik nayakSep 17, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.