| Message ID | 20250127-pks-reftable-drop-git-compat-util-v1-0-6e280a564877@pks.im (mailing list archive) |
|---|---|
| Headers | show |
| Series | reftable: stop using "git-compat-util.h" | expand |
Patrick Steinhardt <ps@pks.im> writes: > This patch series stops using all kinds of helpers exposed by our > "git-compat-util.h" header and open-codes them instead. In order to keep > us from using these helpers by accident the final step is to pull out > POSIX-related bits and pieces into a new "compat/posix.h" header, which > the reftable library then uses instead of "git-compat-util.h". Very nice. Is there something we can also do in order to keep reftable from using stale version of these helpers that it copied with this series when we make improvements on our side to the original? I think the answer might be "then use a common library implementation that is used by both Git and reftable", but then we might be in the same place as before? I dunno. What do "libification" folks think? Anybody?
On Mon, Jan 27, 2025 at 09:44:24AM -0800, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > This patch series stops using all kinds of helpers exposed by our > > "git-compat-util.h" header and open-codes them instead. In order to keep > > us from using these helpers by accident the final step is to pull out > > POSIX-related bits and pieces into a new "compat/posix.h" header, which > > the reftable library then uses instead of "git-compat-util.h". > > Very nice. > > Is there something we can also do in order to keep reftable from using > stale version of these helpers that it copied with this series when > we make improvements on our side to the original? Overall the amount of duplication is quite limited. It looks like a lot in this patch series, but many of the changes are really only a couple of lines of code that are quite unlikely to grow stale. The subsystems that really are complex (think tempfiles, lockfiles) use the same underlying implementation as Git does via shims in "reftable/system.c". My hope is that by starting to use the reftable library in libgit2 we'll get additional test coverage thereof that helps weed out any issues we have, including issues in the compatibility layer. But I also assume that almost all bugs that we'll find will not be in the low-level code, but rather in the business logic of reftables themselves. > I think the answer might be "then use a common library > implementation that is used by both Git and reftable", but then we > might be in the same place as before? I dunno. I don't really have a good idea for how to do this. Using a common library would likely push us into the same place as before indeed, unless we started to split out standalone files that aren't allowed to link against anything but the "compat/" directory. But even if we had that it would be quite painful to take the reftable library and reuse it somewhere else, because now it's more than just "cp -r reftable/" and reimplementing system-specific bits in "reftable/system.c". I guess time will tell how much of a problem this really is. As said, my prediction is that we won't face many bugs in the low-level code, and then the question becomes moot anyway. But if time proves me wrong I'll think a bit more about potential solutions. Patrick
Patrick Steinhardt <ps@pks.im> writes: > I guess time will tell how much of a problem this really is. As said, my > prediction is that we won't face many bugs in the low-level code, and > then the question becomes moot anyway. But if time proves me wrong I'll > think a bit more about potential solutions. Sure. I am OK with the approach of playing by the ear. Thanks.
Hi, this patch series is the final step to fully decouple the reftable library from the rest of the Git codebase. The goal of this is to make the library reusable by other projects like libgit2 by simply copying over the source files, making Git the canonical upstream for reftable functionality. This patch series stops using all kinds of helpers exposed by our "git-compat-util.h" header and open-codes them instead. In order to keep us from using these helpers by accident the final step is to pull out POSIX-related bits and pieces into a new "compat/posix.h" header, which the reftable library then uses instead of "git-compat-util.h". The series is built on top of master at 5f8f7081f7 (The third batch, 2025-01-23) with ps/reftable-sign-compare at 33319b0976 (reftable: address trivial -Wsign-compare warnings, 2025-01-20) merged into it. There is a trivial merge conflict with ps/zlib-ng that can be solved like this: diff --cc reftable/system.h index e4a8944a70,d02eacea8f..0000000000 --- a/reftable/system.h +++ b/reftable/system.h @@@ -11,15 -11,9 +11,15 @@@ https://developers.google.com/open-sour /* This header glues the reftable library to the rest of Git */ -#include "git-compat-util.h" +#include "compat/posix.h" - #include <zlib.h> + #include "compat/zlib-compat.h" +/* + * Return a random 32 bit integer. This function is expected to return + * pre-seeded data. + */ +uint32_t reftable_rand(void); + /* * An implementation-specific temporary file. By making this specific to the * implementation it becomes possible to tie temporary files into any kind of Thanks! Patrick --- Patrick Steinhardt (19): reftable/stack: stop using `read_in_full()` reftable/stack: stop using `write_in_full()` reftable/blocksource: stop using `xmmap()` reftable/record: stop using `COPY_ARRAY()` reftable/record: stop using `BUG()` in `reftable_record_init()` reftable/record: don't `BUG()` in `reftable_record_cmp()` reftable: stop using `BUG()` in trivial cases reftable/basics: stop using `st_mult()` in array allocators reftable/basics: provide wrappers for big endian conversion reftable/reader: stop using `ARRAY_SIZE()` macro reftable/system: introduce `reftable_rand()` reftable/stack: stop using `sleep_millisec()` reftable/basics: stop using `SWAP()` macro reftable/basics: stop using `UNUSED` annotation compat/mingw: split out POSIX-related bits compat/msvc: split out POSIX-related bits git-compat-util.h: split out POSIX-emulating bits reftable: decouple from Git codebase by pulling in "compat/posix.h" Makefile: skip reftable library for Coccinelle Makefile | 2 +- compat/{mingw.c => mingw/compat-util.c} | 0 compat/mingw/compat-util.h | 220 +++++++++++++ compat/{mingw.h => mingw/posix.h} | 216 +------------ compat/{msvc.c => msvc/compat-util.c} | 2 +- compat/msvc/compat-util.h | 7 + compat/{msvc.h => msvc/posix.h} | 8 +- compat/posix.h | 541 ++++++++++++++++++++++++++++++++ config.mak.uname | 6 +- contrib/buildsystems/CMakeLists.txt | 2 +- git-compat-util.h | 535 +------------------------------ meson.build | 8 +- reftable/basics.c | 19 -- reftable/basics.h | 123 +++++++- reftable/block.c | 16 +- reftable/blocksource.c | 21 +- reftable/iter.c | 20 +- reftable/merged.c | 27 +- reftable/pq.c | 40 ++- reftable/pq.h | 2 +- reftable/reader.c | 33 +- reftable/record.c | 96 +++--- reftable/record.h | 6 +- reftable/stack.c | 52 ++- reftable/system.c | 7 + reftable/system.h | 9 +- reftable/writer.c | 29 +- t/unit-tests/t-reftable-basics.c | 28 +- t/unit-tests/t-reftable-pq.c | 22 +- t/unit-tests/t-reftable-record.c | 42 ++- 30 files changed, 1214 insertions(+), 925 deletions(-) --- base-commit: 5f8f7081f7761acdf83d0a4c6819fe3d724f01d7 change-id: 20241119-pks-reftable-drop-git-compat-util-470f2bfde562