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

Re: [PATCH v2 00/10] reftable: stop using `struct strbuf`

From
shejialuo <shejialuo@gmail.com>
Date
Oct 15, 2024, 11:23 UTC
Message-ID
<Zw5QzmtOrDOXAavV@ArchLinux>
In-Reply-To
<Zw5HntIyk_DLWESC@pks.im>
On Tue, Oct 15, 2024 at 12:44:53PM +0200, Patrick Steinhardt wrote:
[snip]
Show 33 quoted lines
> > I have read some patches yesterday, I feel quite strange that we need to
> > make repetition. Could we provide a header file which requires the users
> > who need to use the reftable library to implement the interfaces?
> > 
> >     reftable_strbuf_addf(void *buf, char *fmt, va_list ap);
> > 
> > Thus, we could reuse "strbuf_addf" to implement this interface in Git.
> > As for libgit2, could we let it implement these interfaces? Although I
> > have never read the source code of libgit2, I think there should be some
> > code which could be reuse to implement these interfaces?
> > 
> > However, I do not know the context. Maybe the above is totally wrong. If
> > so, please ignore.
> 
> The thing is that we'll have repetition regardless of what we end up
> doing:
> 
>   - We could either have repetition once in the reftable library,
>     reimplementing `struct strbuf`. This can then be reused by every
>     single user of the reftable library.
> 
>   - Or we can have repetition for every single user of the reftable
>     library. For now that'd only be Git and libgit2, but we'd still have
>     repetition.
> 
> The second kind of repetition is way worse though, because now every
> user of the reftable library has a different implementation of a type
> that is as basic as a buffer. These _must_ behave the exact same across
> implementations or we will hit issues. So I'd rather have the repetition
> a single time in the reftable library such that all users of the library
> will behave the same rather than having downstream users copy the
> implementation of `struct strbuf` and making it work for their library.
> 

Yes. I agree with you it is worse to let every downstream to implement the interfaces. I know the motivation here, we want to make the whole reftable library be independent of the Git which allows the downstream to easily use the reftable library.

Show 11 quoted lines
> Also, due to the nature of `struct strbuf` not allowing for allocation
> failures we'd already have diverging behaviour. In Git you would never
> hit error code paths for allocation failures, whereas every library user
> potentially can.
> 
> So we really have to treat the reftable code base special. If we want to
> be a good citizen and be a proper upstream for projects like libgit2 we
> don't really have much of a choice than to detangle it from libgit.a. If
> we don't we may be saving 20 lines of code, but we make everybody elses
> life harder.
> 

Yes. And I do not think this is a problem right now. Thanks for this wonderful explanation.

> Patrick
Jialuo
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 49 of 63 in “reftable: stop using `struct strbuf`”
  1. 00/10 reftable: stop using `struct strbuf`Patrick Steinhardt, Oct 11, 2024
  2. 01/10 reftable: stop using `strbuf_addbuf()`Patrick Steinhardt, Oct 11, 2024
  3. 02/10 reftable: stop using `strbuf_addf()`Patrick Steinhardt, Oct 11, 2024
  4. karthik nayakOct 11, 2024
  5. Patrick SteinhardtOct 14, 2024
  6. 03/10 reftable/basics: provide new `reftable_buf` interfacePatrick Steinhardt, Oct 11, 2024
  7. karthik nayakOct 11, 2024
  8. Patrick SteinhardtOct 14, 2024
  9. 04/10 reftable: convert from `strbuf` to `reftable_buf`Patrick Steinhardt, Oct 11, 2024
  10. karthik nayakOct 11, 2024
  11. Patrick SteinhardtOct 14, 2024
  12. 05/10 reftable/blocksource: adapt interface namePatrick Steinhardt, Oct 11, 2024
  13. 06/10 t/unit-tests: check for `reftable_buf` allocation errorsPatrick Steinhardt, Oct 11, 2024
  14. 07/10 reftable/stack: adapt `format_name()` to handle allocation failuresPatrick Steinhardt, Oct 11, 2024
  15. 08/10 reftable/record: adapt `reftable_record_key()` to handle allocation failuresPatrick Steinhardt, Oct 11, 2024
  16. 09/10 reftable/stack: adapt `stack_filename()` to handle allocation failuresPatrick Steinhardt, Oct 11, 2024
  17. 10/10 reftable: handle trivial `reftable_buf` errorsPatrick Steinhardt, Oct 11, 2024
  18. karthik nayakOct 11, 2024
  19. Patrick SteinhardtOct 14, 2024
  20. 00/10 reftable: stop using `struct strbuf`Patrick Steinhardt, Oct 14, 2024
  21. 01/10 reftable: stop using `strbuf_addbuf()`Patrick Steinhardt, Oct 14, 2024
  22. Taylor BlauOct 14, 2024
  23. 02/10 reftable: stop using `strbuf_addf()`Patrick Steinhardt, Oct 14, 2024
  24. Taylor BlauOct 14, 2024
  25. Patrick SteinhardtOct 15, 2024
  26. Taylor BlauOct 15, 2024
  27. 03/10 reftable/basics: provide new `reftable_buf` interfacePatrick Steinhardt, Oct 14, 2024
  28. Taylor BlauOct 14, 2024
  29. Patrick SteinhardtOct 15, 2024
  30. Eric SunshineOct 15, 2024
  31. Taylor BlauOct 15, 2024
  32. Patrick SteinhardtOct 16, 2024
  33. Taylor BlauOct 16, 2024
  34. Patrick SteinhardtOct 17, 2024
  35. Taylor BlauOct 17, 2024
  36. 04/10 reftable: convert from `strbuf` to `reftable_buf`Patrick Steinhardt, Oct 14, 2024
  37. Taylor BlauOct 14, 2024
  38. 05/10 reftable/blocksource: adapt interface namePatrick Steinhardt, Oct 14, 2024
  39. 06/10 t/unit-tests: check for `reftable_buf` allocation errorsPatrick Steinhardt, Oct 14, 2024
  40. 07/10 reftable/stack: adapt `format_name()` to handle allocation failuresPatrick Steinhardt, Oct 14, 2024
  41. Taylor BlauOct 14, 2024
  42. 08/10 reftable/record: adapt `reftable_record_key()` to handle allocation failuresPatrick Steinhardt, Oct 14, 2024
  43. 09/10 reftable/stack: adapt `stack_filename()` to handle allocation failuresPatrick Steinhardt, Oct 14, 2024
  44. 10/10 reftable: handle trivial `reftable_buf` errorsPatrick Steinhardt, Oct 14, 2024
  45. Taylor BlauOct 14, 2024
  46. Patrick SteinhardtOct 15, 2024
  47. shejialuoOct 15, 2024
  48. Patrick SteinhardtOct 15, 2024
  49. shejialuoOct 15, 2024
  50. 00/10 reftable: stop using `struct strbuf`Patrick Steinhardt, Oct 17, 2024
  51. 01/10 reftable: stop using `strbuf_addbuf()`Patrick Steinhardt, Oct 17, 2024
  52. 02/10 reftable: stop using `strbuf_addf()`Patrick Steinhardt, Oct 17, 2024
  53. 03/10 reftable/basics: provide new `reftable_buf` interfacePatrick Steinhardt, Oct 17, 2024
  54. 04/10 reftable: convert from `strbuf` to `reftable_buf`Patrick Steinhardt, Oct 17, 2024
  55. 05/10 reftable/blocksource: adapt interface namePatrick Steinhardt, Oct 17, 2024
  56. 06/10 t/unit-tests: check for `reftable_buf` allocation errorsPatrick Steinhardt, Oct 17, 2024
  57. 07/10 reftable/stack: adapt `format_name()` to handle allocation failuresPatrick Steinhardt, Oct 17, 2024
  58. 08/10 reftable/record: adapt `reftable_record_key()` to handle allocation failuresPatrick Steinhardt, Oct 17, 2024
  59. 09/10 reftable/stack: adapt `stack_filename()` to handle allocation failuresPatrick Steinhardt, Oct 17, 2024
  60. 10/10 reftable: handle trivial `reftable_buf` errorsPatrick Steinhardt, Oct 17, 2024
  61. Taylor BlauOct 17, 2024
  62. karthik nayakOct 18, 2024
  63. Taylor BlauOct 18, 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.