# [PATCH v4 01/10] compat: drop `uncompress2()` compatibility shim 12 messages from 2025-01-28 to 2025-01-28. Participants: Patrick Steinhardt, Junio C Hamano. Thread: https://gitlist.dev/t/62863 ## Patrick Steinhardt, 2025-01-28 08:41 Subject: [PATCH v4 01/10] compat: drop `uncompress2()` compatibility shim Message-ID: <20250128-b4-pks-compat-drop-uncompress2-v4-1-129bc36ae8f5@pks.im> URL: https://gitlist.dev/e/20250128-b4-pks-compat-drop-uncompress2-v4-1-129bc36ae8f5%40pks.im In-Reply-To: <20250128-b4-pks-compat-drop-uncompress2-v4-0-129bc36ae8f5@pks.im> ``` Our compat library has an implementation of zlib's `uncompress2()` function that gets used when linking against an old version of zlib that doesn't yet have it. The last user of `uncompress2()` got removed in 15a60b747e (reftable/block: open-code call to `uncompress2()`, 2024-04-08), so the compatibility code is not required anymore. Drop it. Signed-off-by: Patrick Steinhardt --- Makefile | 1 - compat/zlib-uncompress2.c | 96 ----------------------------------------------- git-compat-util.h | 9 ----- meson.build | 1 - 4 files changed, 107 deletions(-) diff --git a/Makefile b/Makefile index 97e8385b66..86c6c3d7ad 100644 --- a/Makefile +++ b/Makefile @@ -984,7 +984,6 @@ LIB_OBJS += commit.o LIB_OBJS += compat/nonblock.o LIB_OBJS += compat/obstack.o LIB_OBJS += compat/terminal.o -LIB_OBJS += compat/zlib-uncompress2.o LIB_OBJS += config.o LIB_OBJS += connect.o LIB_OBJS += connected.o diff --git a/compat/zlib-uncompress2.c b/compat/zlib-uncompress2.c deleted file mode 100644 index 77a1b08048..0000000000 --- a/compat/zlib-uncompress2.c +++ /dev/null @@ -1,96 +0,0 @@ -#include "git-compat-util.h" - -#if ZLIB_VERNUM < 0x1290 -/* taken from zlib's uncompr.c - - commit cacf7f1d4e3d44d871b605da3b647f07d718623f - Author: Mark Adler - Date: Sun Jan 15 09:18:46 2017 -0800 - - zlib 1.2.11 - -*/ - -/* - * Copyright (C) 1995-2003, 2010, 2014, 2016 Jean-loup Gailly, Mark Adler - * For conditions of distribution and use, see copyright notice in zlib.h - */ - -/* clang-format off */ - -/* =========================================================================== - Decompresses the source buffer into the destination buffer. *sourceLen is - the byte length of the source buffer. Upon entry, *destLen is the total size - of the destination buffer, which must be large enough to hold the entire - uncompressed data. (The size of the uncompressed data must have been saved - previously by the compressor and transmitted to the decompressor by some - mechanism outside the scope of this compression library.) Upon exit, - *destLen is the size of the decompressed data and *sourceLen is the number - of source bytes consumed. Upon return, source + *sourceLen points to the - first unused input byte. - - uncompress returns Z_OK if success, Z_MEM_ERROR if there was not enough - memory, Z_BUF_ERROR if there was not enough room in the output buffer, or - Z_DATA_ERROR if the input data was corrupted, including if the input data is - an incomplete zlib stream. -*/ -int ZEXPORT uncompress2 ( - Bytef *dest, - uLongf *destLen, - const Bytef *source, - uLong *sourceLen) { - z_stream stream; - int err; - const uInt max = (uInt)-1; - uLong len, left; - Byte buf[1]; /* for detection of incomplete stream when *destLen == 0 */ - - len = *sourceLen; - if (*destLen) { - left = *destLen; - *destLen = 0; - } - else { - left = 1; - dest = buf; - } - - stream.next_in = (z_const Bytef *)source; - stream.avail_in = 0; - stream.zalloc = (alloc_func)0; - stream.zfree = (free_func)0; - stream.opaque = (voidpf)0; - - err = inflateInit(&stream); - if (err != Z_OK) return err; - - stream.next_out = dest; - stream.avail_out = 0; - - do { - if (stream.avail_out == 0) { - stream.avail_out = left > (uLong)max ? max : (uInt)left; - left -= stream.avail_out; - } - if (stream.avail_in == 0) { - stream.avail_in = len > (uLong)max ? max : (uInt)len; - len -= stream.avail_in; - } - err = inflate(&stream, Z_NO_FLUSH); - } while (err == Z_OK); - - *sourceLen -= len + stream.avail_in; - if (dest != buf) - *destLen = stream.total_out; - else if (stream.total_out && err == Z_BUF_ERROR) - left = 1; - - inflateEnd(&stream); - return err == Z_STREAM_END ? Z_OK : - err == Z_NEED_DICT ? Z_DATA_ERROR : - err == Z_BUF_ERROR && left + stream.avail_out ? Z_DATA_ERROR : - err; -} -#else -static void *dummy_variable = &dummy_variable; -#endif diff --git a/git-compat-util.h b/git-compat-util.h index e283c46c6f..d50f487c00 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1540,15 +1540,6 @@ int common_exit(const char *file, int line, int code); #define z_const #include -#if ZLIB_VERNUM < 0x1290 -/* - * This is uncompress2, which is only available in zlib >= 1.2.9 - * (released as of early 2017). See compat/zlib-uncompress2.c. - */ -int uncompress2(Bytef *dest, uLongf *destLen, const Bytef *source, - uLong *sourceLen); -#endif - /* * This include must come after system headers, since it introduces macros that * replace system names. diff --git a/meson.build b/meson.build index 30d7a89490..12129a8b95 100644 --- a/meson.build +++ b/meson.build @@ -263,7 +263,6 @@ libgit_sources = [ 'compat/nonblock.c', 'compat/obstack.c', 'compat/terminal.c', - 'compat/zlib-uncompress2.c', 'config.c', 'connect.c', 'connected.c', -- 2.48.1.362.g079036d154.dirty ``` ## Patrick Steinhardt, 2025-01-28 08:41 Subject: [PATCH v4 00/10] compat/zlib: allow use of zlib-ng as backend Message-ID: <20250128-b4-pks-compat-drop-uncompress2-v4-0-129bc36ae8f5@pks.im> URL: https://gitlist.dev/e/20250128-b4-pks-compat-drop-uncompress2-v4-0-129bc36ae8f5%40pks.im In-Reply-To: <20250110-b4-pks-compat-drop-uncompress2-v1-0-965d0022a74d@pks.im> ``` Hi, I have recently started to play around with zlib-ng a bit, which is a hard fork of the zlib library. It describes itself as zlib replacement with optimizations for "next generation" systems. As such, it contains several implementations of central algorithms using for example SSE2, AVX2 and other vectorized CPU intrinsics that supposedly speed up in- and deflating data. And indeed, compiling Git against zlib-ng leads to a significant speedup when reading objects. The following benchmark uses git-cat-file(1) with `--batch --batch-all-objects` in the Git repository: Benchmark 1: zlib Time (mean ± σ): 52.085 s ± 0.141 s [User: 51.500 s, System: 0.456 s] Range (min … max): 52.004 s … 52.335 s 5 runs Benchmark 2: zlib-ng Time (mean ± σ): 40.324 s ± 0.134 s [User: 39.731 s, System: 0.490 s] Range (min … max): 40.135 s … 40.484 s 5 runs Summary zlib-ng ran 1.29 ± 0.01 times faster than zlib So we're looking at a ~25% speedup compared to zlib. This is of course an extreme example, as it makes us read through all objects in the repository. But regardless, it should be possible to see some sort of speedup in most commands that end up accessing the object database. This patch series refactors how we wire up zlib in our project by introducing a new "compat/zlib.h" header function. This header is then later extended to patch over the differences between zlib and zlib-ng, which is mostly just that zlib-ng has a `zng_` prefix for each of its symbols. Like this, we can support both libraries directly, and a new Meson build options allows users to pick whichever backend they like. In theory, these changes shouldn't be necessary because zlib-ng provides a compatibility layer that make it directly compatible with zlib. But most distros don't allow you to install zlib-ng with that layer is it would mean that zlib would need to be replaced globally. Instead, they typically only provide a version of zlib-ng that only has the `zng_` prefixed symbols. Given the observed speedup I do think that this is a worthwhile change so that users (or especially hosting providers) can easily switch to zlib-ng without impacting the rest of their system. Changes in v2: - Wire up zlib-ng in our Makefile. - Exercise zlib-ng via CI by adapting our "linux-musl" job to use Meson and installing zlib-ng. - Link to v1: https://lore.kernel.org/r/20250110-b4-pks-compat-drop-uncompress2-v1-0-965d0022a74d@pks.im Changes in v3: - Fix a couple of commit message typos. - Mention why we can safely drop "CC=gcc" when converting the musl job to use Meson. - Link to v2: https://lore.kernel.org/r/20250114-b4-pks-compat-drop-uncompress2-v2-0-614a2158e34e@pks.im Changes in v4: - Add a comment explaining why we can stub out `deflateSetHeader()`. - Add a comment explaining why we have to cast away constness with zlib-ng's `next_in` field. - Link to v3: https://lore.kernel.org/r/20250116-b4-pks-compat-drop-uncompress2-v3-0-f2af1f5c4a06@pks.im I've adjusted the series to be based on top of fbe8d3079d (Git 2.48, 2025-01-10) with ps/meson-weak-sha1-build at 6a0ee54f9a (meson: provide a summary of configured backends, 2024-12-30) and ps/build-meson-fixes at 4e517e68b5 (ci: wire up Visual Studio build with Meson, 2025-01-14) merged into it. This matches what Junio has in his tree -- sorry for screwing up the previous base! Thanks! Patrick --- Patrick Steinhardt (10): compat: drop `uncompress2()` compatibility shim git-compat-util: drop `z_const` define compat: introduce new "zlib.h" header git-compat-util: move include of "compat/zlib.h" into "git-zlib.h" compat/zlib: provide `deflateBound()` shim centrally compat/zlib: provide stubs for `deflateSetHeader()` git-zlib: cast away potential constness of `next_in` pointer compat/zlib: allow use of zlib-ng as backend ci: switch linux-musl to use Meson ci: make "linux-musl" job use zlib-ng .github/workflows/main.yml | 2 +- .gitlab-ci.yml | 2 +- Makefile | 21 +++++++--- archive-tar.c | 4 -- archive.c | 1 + ci/install-dependencies.sh | 4 +- ci/lib.sh | 5 +-- ci/run-build-and-tests.sh | 3 +- compat/zlib-compat.h | 53 +++++++++++++++++++++++++ compat/zlib-uncompress2.c | 96 ---------------------------------------------- config.c | 1 + csum-file.c | 3 +- environment.c | 1 + git-compat-util.h | 12 ------ git-zlib.c | 7 +--- git-zlib.h | 2 + meson.build | 24 +++++++++--- meson_options.txt | 4 ++ reftable/block.c | 1 - reftable/system.h | 1 + 20 files changed, 107 insertions(+), 140 deletions(-) Range-diff versus v3: 1: 00984b07b9 = 1: fcfcf1ed81 compat: drop `uncompress2()` compatibility shim 2: de7bf8bf15 = 2: b483553549 git-compat-util: drop `z_const` define 3: 1aea050dae = 3: f4f23ad8bc compat: introduce new "zlib.h" header 4: 40229f1c0a = 4: 14f6055809 git-compat-util: move include of "compat/zlib.h" into "git-zlib.h" 5: 230d23877f = 5: acb5212ed3 compat/zlib: provide `deflateBound()` shim centrally 6: 05e0757235 ! 6: 918cf3eb0d compat/zlib: provide stubs for `deflateSetHeader()` @@ compat/zlib-compat.h # define deflateBound(c,s) ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11) #endif ++/* ++ * zlib only gained support for setting up the gzip header in v1.2.2.1. In ++ * Git we only set the header to make archives reproducible across different ++ * operating systems, so it's fine to simply make this a no-op when using a ++ * zlib version that doesn't support this yet. ++ */ +#if ZLIB_VERNUM < 0x1221 +struct gz_header_s { + int os; 7: b10e6f35d7 ! 7: 4047b9226a git-zlib: cast away potential constness of `next_in` pointer @@ git-zlib.c: static void zlib_post_call(git_zstream *s) s->total_out = s->z.total_out; s->total_in = s->z.total_in; - s->next_in = s->z.next_in; ++ /* zlib-ng marks `next_in` as `const`, so we have to cast it away. */ + s->next_in = (unsigned char *) s->z.next_in; s->next_out = s->z.next_out; s->avail_in -= bytes_consumed; 8: 6149885889 ! 8: d8f5c87d71 compat/zlib: allow use of zlib-ng as backend @@ compat/zlib-compat.h -#endif +# define z_stream zng_stream +#define gz_header_s zng_gz_header_s - --#if ZLIB_VERNUM < 0x1221 ++ +# define crc32(crc, buf, len) zng_crc32(crc, buf, len) + +# define inflate(strm, bits) zng_inflate(strm, bits) @@ compat/zlib-compat.h +# if defined(NO_DEFLATE_BOUND) || ZLIB_VERNUM < 0x1200 +# define deflateBound(c,s) ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11) +# endif -+ + + /* + * zlib only gained support for setting up the gzip header in v1.2.2.1. In +@@ + * operating systems, so it's fine to simply make this a no-op when using a + * zlib version that doesn't support this yet. + */ +-#if ZLIB_VERNUM < 0x1221 +# if ZLIB_VERNUM < 0x1221 struct gz_header_s { int os; 9: f663af4332 = 9: 87fbc86f47 ci: switch linux-musl to use Meson 10: 376c05fe77 = 10: f3ea4c5a81 ci: make "linux-musl" job use zlib-ng --- base-commit: cbdbb490357c16eaaa6528c1d550c513a632d196 change-id: 20250110-b4-pks-compat-drop-uncompress2-eb5914459c32 ``` ## Patrick Steinhardt, 2025-01-28 08:41 Subject: [PATCH v4 02/10] git-compat-util: drop `z_const` define Message-ID: <20250128-b4-pks-compat-drop-uncompress2-v4-2-129bc36ae8f5@pks.im> URL: https://gitlist.dev/e/20250128-b4-pks-compat-drop-uncompress2-v4-2-129bc36ae8f5%40pks.im In-Reply-To: <20250128-b4-pks-compat-drop-uncompress2-v4-0-129bc36ae8f5@pks.im> ``` Before including we explicitly define `z_const` to an empty value. This has the effect that the `z_const` macro in "zconf.h" itself will remain empty instead of being defined as `const`, which effectively adapts a couple of APIs so that their parameters are not marked as being constants. It is dubious though whether this is something we actually want: not marking a parameter as a constant doesn't make it any less constant than it was. The define was added via 07564773c2 (compat: auto-detect if zlib has uncompress2(), 2022-01-24), where it was seemingly carried over from our internal compatibility shim for `uncompress2()` that was removed in the preceding commit. The commit message doesn't mention why we carry over the define and make it public, either, and I cannot think of any reason for why we would want to have it. Drop the define. Signed-off-by: Patrick Steinhardt --- git-compat-util.h | 1 - 1 file changed, 1 deletion(-) diff --git a/git-compat-util.h b/git-compat-util.h index d50f487c00..c4b4b372b4 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1537,7 +1537,6 @@ int cmd_main(int, const char **); int common_exit(const char *file, int line, int code); #define exit(code) exit(common_exit(__FILE__, __LINE__, (code))) -#define z_const #include /* -- 2.48.1.362.g079036d154.dirty ``` ## Patrick Steinhardt, 2025-01-28 08:41 Subject: [PATCH v4 03/10] compat: introduce new "zlib.h" header Message-ID: <20250128-b4-pks-compat-drop-uncompress2-v4-3-129bc36ae8f5@pks.im> URL: https://gitlist.dev/e/20250128-b4-pks-compat-drop-uncompress2-v4-3-129bc36ae8f5%40pks.im In-Reply-To: <20250128-b4-pks-compat-drop-uncompress2-v4-0-129bc36ae8f5@pks.im> ``` Introduce a new "compat/zlib-compat.h" header that we include instead of including directly. This will allow us to wire up zlib-ng as an alternative backend for zlib compression in a subsequent commit. Note that we cannot just call the file "compat/zlib.h", as that may otherwise cause us to include that file instead of . Signed-off-by: Patrick Steinhardt --- compat/zlib-compat.h | 6 ++++++ git-compat-util.h | 2 +- reftable/block.c | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/compat/zlib-compat.h b/compat/zlib-compat.h new file mode 100644 index 0000000000..bc20b884ef --- /dev/null +++ b/compat/zlib-compat.h @@ -0,0 +1,6 @@ +#ifndef COMPAT_ZLIB_H +#define COMPAT_ZLIB_H + +#include + +#endif /* COMPAT_ZLIB_H */ diff --git a/git-compat-util.h b/git-compat-util.h index c4b4b372b4..1ca2671322 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1537,7 +1537,7 @@ int cmd_main(int, const char **); int common_exit(const char *file, int line, int code); #define exit(code) exit(common_exit(__FILE__, __LINE__, (code))) -#include +#include "compat/zlib-compat.h" /* * This include must come after system headers, since it introduces macros that diff --git a/reftable/block.c b/reftable/block.c index 9858bbc7c5..bc9b079634 100644 --- a/reftable/block.c +++ b/reftable/block.c @@ -13,7 +13,7 @@ license that can be found in the LICENSE file or at #include "record.h" #include "reftable-error.h" #include "system.h" -#include +#include "compat/zlib-compat.h" int header_size(int version) { -- 2.48.1.362.g079036d154.dirty ``` ## Patrick Steinhardt, 2025-01-28 08:41 Subject: [PATCH v4 04/10] git-compat-util: move include of "compat/zlib.h" into "git-zlib.h" Message-ID: <20250128-b4-pks-compat-drop-uncompress2-v4-4-129bc36ae8f5@pks.im> URL: https://gitlist.dev/e/20250128-b4-pks-compat-drop-uncompress2-v4-4-129bc36ae8f5%40pks.im In-Reply-To: <20250128-b4-pks-compat-drop-uncompress2-v4-0-129bc36ae8f5@pks.im> ``` We include "compat/zlib.h" in "git-compat-util.h", which is unnecessarily broad given that we only have a small handful of files that use the zlib library. Move the header into "git-zlib.h" instead and adapt users of zlib to include that header. One exception is the reftable library, as we don't want to use the Git-specific wrapper of zlib there, so we include "compat/zlib.h" instead. Furthermore, we move the include into "reftable/system.h" so that users of the library other than Git can wire up zlib themselves. Signed-off-by: Patrick Steinhardt --- archive.c | 1 + config.c | 1 + csum-file.c | 3 ++- environment.c | 1 + git-compat-util.h | 2 -- git-zlib.h | 2 ++ reftable/block.c | 1 - reftable/system.h | 1 + 8 files changed, 8 insertions(+), 4 deletions(-) diff --git a/archive.c b/archive.c index b9c200cba6..8be4e7ac8d 100644 --- a/archive.c +++ b/archive.c @@ -7,6 +7,7 @@ #include "convert.h" #include "environment.h" #include "gettext.h" +#include "git-zlib.h" #include "hex.h" #include "object-name.h" #include "path.h" diff --git a/config.c b/config.c index 50f2d17b39..36f76fafe5 100644 --- a/config.c +++ b/config.c @@ -19,6 +19,7 @@ #include "convert.h" #include "environment.h" #include "gettext.h" +#include "git-zlib.h" #include "ident.h" #include "repository.h" #include "lockfile.h" diff --git a/csum-file.c b/csum-file.c index 5716016e12..78e04356d3 100644 --- a/csum-file.c +++ b/csum-file.c @@ -11,9 +11,10 @@ #define USE_THE_REPOSITORY_VARIABLE #include "git-compat-util.h" -#include "progress.h" #include "csum-file.h" +#include "git-zlib.h" #include "hash.h" +#include "progress.h" static void verify_buffer_or_die(struct hashfile *f, const void *buf, diff --git a/environment.c b/environment.c index 8389a27270..e5b361bb5d 100644 --- a/environment.c +++ b/environment.c @@ -16,6 +16,7 @@ #include "convert.h" #include "environment.h" #include "gettext.h" +#include "git-zlib.h" #include "repository.h" #include "config.h" #include "refs.h" diff --git a/git-compat-util.h b/git-compat-util.h index 1ca2671322..fb25fbf503 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1537,8 +1537,6 @@ int cmd_main(int, const char **); int common_exit(const char *file, int line, int code); #define exit(code) exit(common_exit(__FILE__, __LINE__, (code))) -#include "compat/zlib-compat.h" - /* * This include must come after system headers, since it introduces macros that * replace system names. diff --git a/git-zlib.h b/git-zlib.h index d8a670aff9..1e8d9aabcb 100644 --- a/git-zlib.h +++ b/git-zlib.h @@ -1,6 +1,8 @@ #ifndef GIT_ZLIB_H #define GIT_ZLIB_H +#include "compat/zlib-compat.h" + typedef struct git_zstream { z_stream z; unsigned long avail_in; diff --git a/reftable/block.c b/reftable/block.c index bc9b079634..38e3081c61 100644 --- a/reftable/block.c +++ b/reftable/block.c @@ -13,7 +13,6 @@ license that can be found in the LICENSE file or at #include "record.h" #include "reftable-error.h" #include "system.h" -#include "compat/zlib-compat.h" int header_size(int version) { diff --git a/reftable/system.h b/reftable/system.h index 5274eca1d0..f194a38686 100644 --- a/reftable/system.h +++ b/reftable/system.h @@ -14,6 +14,7 @@ license that can be found in the LICENSE file or at #define DISABLE_SIGN_COMPARE_WARNINGS #include "git-compat-util.h" +#include "compat/zlib-compat.h" /* * An implementation-specific temporary file. By making this specific to the -- 2.48.1.362.g079036d154.dirty ``` ## Patrick Steinhardt, 2025-01-28 08:41 Subject: [PATCH v4 05/10] compat/zlib: provide `deflateBound()` shim centrally Message-ID: <20250128-b4-pks-compat-drop-uncompress2-v4-5-129bc36ae8f5@pks.im> URL: https://gitlist.dev/e/20250128-b4-pks-compat-drop-uncompress2-v4-5-129bc36ae8f5%40pks.im In-Reply-To: <20250128-b4-pks-compat-drop-uncompress2-v4-0-129bc36ae8f5@pks.im> ``` The `deflateBound()` function has only been introduced with zlib 1.2.0. When linking against a zlib version older than that we thus provide our own compatibility shim. Move this shim into "compat/zlib.h" so that we can adapt it based on whether or not we use zlib-ng in a subsequent commit. Signed-off-by: Patrick Steinhardt --- compat/zlib-compat.h | 4 ++++ git-zlib.c | 4 ---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compat/zlib-compat.h b/compat/zlib-compat.h index bc20b884ef..96a08811a9 100644 --- a/compat/zlib-compat.h +++ b/compat/zlib-compat.h @@ -3,4 +3,8 @@ #include +#if defined(NO_DEFLATE_BOUND) || ZLIB_VERNUM < 0x1200 +# define deflateBound(c,s) ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11) +#endif + #endif /* COMPAT_ZLIB_H */ diff --git a/git-zlib.c b/git-zlib.c index d43bbeb6da..2e973320c2 100644 --- a/git-zlib.c +++ b/git-zlib.c @@ -147,10 +147,6 @@ int git_inflate(git_zstream *strm, int flush) return status; } -#if defined(NO_DEFLATE_BOUND) || ZLIB_VERNUM < 0x1200 -#define deflateBound(c,s) ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11) -#endif - unsigned long git_deflate_bound(git_zstream *strm, unsigned long size) { return deflateBound(&strm->z, size); -- 2.48.1.362.g079036d154.dirty ``` ## Patrick Steinhardt, 2025-01-28 08:41 Subject: [PATCH v4 06/10] compat/zlib: provide stubs for `deflateSetHeader()` Message-ID: <20250128-b4-pks-compat-drop-uncompress2-v4-6-129bc36ae8f5@pks.im> URL: https://gitlist.dev/e/20250128-b4-pks-compat-drop-uncompress2-v4-6-129bc36ae8f5%40pks.im In-Reply-To: <20250128-b4-pks-compat-drop-uncompress2-v4-0-129bc36ae8f5@pks.im> ``` The function `deflateSetHeader()` has been introduced with zlib v1.2.2.1, so we don't use it when linking against an older version of it. Refactor the code to instead provide a central stub via "compat/zlib.h" so that we can adapt it based on whether or not we use zlib-ng in a subsequent commit. Signed-off-by: Patrick Steinhardt --- archive-tar.c | 4 ---- compat/zlib-compat.h | 19 +++++++++++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/archive-tar.c b/archive-tar.c index e7b3489e1e..0edf13fba7 100644 --- a/archive-tar.c +++ b/archive-tar.c @@ -473,9 +473,7 @@ static const char internal_gzip_command[] = "git archive gzip"; static int write_tar_filter_archive(const struct archiver *ar, struct archiver_args *args) { -#if ZLIB_VERNUM >= 0x1221 struct gz_header_s gzhead = { .os = 3 }; /* Unix, for reproducibility */ -#endif struct strbuf cmd = STRBUF_INIT; struct child_process filter = CHILD_PROCESS_INIT; int r; @@ -486,10 +484,8 @@ static int write_tar_filter_archive(const struct archiver *ar, if (!strcmp(ar->filter_command, internal_gzip_command)) { write_block = tgz_write_block; git_deflate_init_gzip(&gzstream, args->compression_level); -#if ZLIB_VERNUM >= 0x1221 if (deflateSetHeader(&gzstream.z, &gzhead) != Z_OK) BUG("deflateSetHeader() called too late"); -#endif gzstream.next_out = outbuf; gzstream.avail_out = sizeof(outbuf); diff --git a/compat/zlib-compat.h b/compat/zlib-compat.h index 96a08811a9..6226b30c0c 100644 --- a/compat/zlib-compat.h +++ b/compat/zlib-compat.h @@ -7,4 +7,23 @@ # define deflateBound(c,s) ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11) #endif +/* + * zlib only gained support for setting up the gzip header in v1.2.2.1. In + * Git we only set the header to make archives reproducible across different + * operating systems, so it's fine to simply make this a no-op when using a + * zlib version that doesn't support this yet. + */ +#if ZLIB_VERNUM < 0x1221 +struct gz_header_s { + int os; +}; + +static int deflateSetHeader(z_streamp strm, struct gz_header_s *head) +{ + (void)(strm); + (void)(head); + return Z_OK; +} +#endif + #endif /* COMPAT_ZLIB_H */ -- 2.48.1.362.g079036d154.dirty ``` ## Patrick Steinhardt, 2025-01-28 08:41 Subject: [PATCH v4 07/10] git-zlib: cast away potential constness of `next_in` pointer Message-ID: <20250128-b4-pks-compat-drop-uncompress2-v4-7-129bc36ae8f5@pks.im> URL: https://gitlist.dev/e/20250128-b4-pks-compat-drop-uncompress2-v4-7-129bc36ae8f5%40pks.im In-Reply-To: <20250128-b4-pks-compat-drop-uncompress2-v4-0-129bc36ae8f5@pks.im> ``` The `struct git_zstream::next_in` variable points to the input data and is used in combination with `struct z_stream::next_in`. While that latter field is not marked as a constant in zlib, it is marked as such in zlib-ng. This causes a couple of compiler errors when we try to assign these fields to one another due to mismatching constness. Fix the issue by casting away the potential constness of `next_in`. Signed-off-by: Patrick Steinhardt --- git-zlib.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/git-zlib.c b/git-zlib.c index 2e973320c2..651dd9e07c 100644 --- a/git-zlib.c +++ b/git-zlib.c @@ -59,7 +59,8 @@ static void zlib_post_call(git_zstream *s) s->total_out = s->z.total_out; s->total_in = s->z.total_in; - s->next_in = s->z.next_in; + /* zlib-ng marks `next_in` as `const`, so we have to cast it away. */ + s->next_in = (unsigned char *) s->z.next_in; s->next_out = s->z.next_out; s->avail_in -= bytes_consumed; s->avail_out -= bytes_produced; -- 2.48.1.362.g079036d154.dirty ``` ## Patrick Steinhardt, 2025-01-28 08:41 Subject: [PATCH v4 08/10] compat/zlib: allow use of zlib-ng as backend Message-ID: <20250128-b4-pks-compat-drop-uncompress2-v4-8-129bc36ae8f5@pks.im> URL: https://gitlist.dev/e/20250128-b4-pks-compat-drop-uncompress2-v4-8-129bc36ae8f5%40pks.im In-Reply-To: <20250128-b4-pks-compat-drop-uncompress2-v4-0-129bc36ae8f5@pks.im> ``` The zlib-ng library is a hard fork of the old and venerable zlib library. It describes itself as zlib replacement with optimizations for "next generation" systems. As such, it contains several implementations of central algorithms using for example SSE2, AVX2 and other vectorized CPU intrinsics that supposedly speed up in- and deflating data. And indeed, compiling Git against zlib-ng leads to a significant speedup when reading objects. The following benchmark uses git-cat-file(1) with `--batch --batch-all-objects` in the Git repository: Benchmark 1: zlib Time (mean ± σ): 52.085 s ± 0.141 s [User: 51.500 s, System: 0.456 s] Range (min … max): 52.004 s … 52.335 s 5 runs Benchmark 2: zlib-ng Time (mean ± σ): 40.324 s ± 0.134 s [User: 39.731 s, System: 0.490 s] Range (min … max): 40.135 s … 40.484 s 5 runs Summary zlib-ng ran 1.29 ± 0.01 times faster than zlib So we're looking at a ~25% speedup compared to zlib. This is of course an extreme example, as it makes us read through all objects in the repository. But regardless, it should be possible to see some sort of speedup in most commands that end up accessing the object database. The zlib-ng library provides a compatibility layer that makes it a proper drop-in replacement for zlib: nothing needs to change in the build system to support it. Unfortunately though, this mode isn't easy to use on most systems because distributions do not allow you to install zlib-ng in that way, as that would mean that the zlib library would be globally replaced. Instead, many distributions provide a package that installs zlib-ng without the compatibility layer. This version does provide effectively the same APIs like zlib does, but all of the symbols are prefixed with `zng_` to avoid symbol collisions. Implement a new build option that allows us to link against zlib-ng directly. If set, we redefine zlib symbols so that we use the `zng_` prefixed versions thereof provided by that library. Like this, it becomes possible to install both zlib and zlib-ng (without the compat layer) and then pick whichever library one wants to link against for Git. Signed-off-by: Patrick Steinhardt --- Makefile | 20 +++++++++++++++----- compat/zlib-compat.h | 36 ++++++++++++++++++++++++++++++------ meson.build | 21 +++++++++++++++++---- meson_options.txt | 2 ++ 4 files changed, 64 insertions(+), 15 deletions(-) diff --git a/Makefile b/Makefile index 86c6c3d7ad..1853e6ddfa 100644 --- a/Makefile +++ b/Makefile @@ -183,7 +183,8 @@ include shared.mak # byte-order mark (BOM) when writing UTF-16 or UTF-32 and always writes in # big-endian format. # -# Define NO_DEFLATE_BOUND if your zlib does not have deflateBound. +# Define NO_DEFLATE_BOUND if your zlib does not have deflateBound. Define +# ZLIB_NG if you want to use zlib-ng instead of zlib. # # Define NO_NORETURN if using buggy versions of gcc 4.6+ and profile feedback, # as the compiler can crash (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49299) @@ -1687,11 +1688,20 @@ else endif IMAP_SEND_LDFLAGS += $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO) -ifdef ZLIB_PATH - BASIC_CFLAGS += -I$(ZLIB_PATH)/include - EXTLIBS += $(call libpath_template,$(ZLIB_PATH)/$(lib)) +ifdef ZLIB_NG + BASIC_CFLAGS += -DHAVE_ZLIB_NG + ifdef ZLIB_NG_PATH + BASIC_CFLAGS += -I$(ZLIB_NG_PATH)/include + EXTLIBS += $(call libpath_template,$(ZLIB_NG_PATH)/$(lib)) + endif + EXTLIBS += -lz-ng +else + ifdef ZLIB_PATH + BASIC_CFLAGS += -I$(ZLIB_PATH)/include + EXTLIBS += $(call libpath_template,$(ZLIB_PATH)/$(lib)) + endif + EXTLIBS += -lz endif -EXTLIBS += -lz ifndef NO_OPENSSL OPENSSL_LIBSSL = -lssl diff --git a/compat/zlib-compat.h b/compat/zlib-compat.h index 6226b30c0c..0c60e3af33 100644 --- a/compat/zlib-compat.h +++ b/compat/zlib-compat.h @@ -1,11 +1,34 @@ #ifndef COMPAT_ZLIB_H #define COMPAT_ZLIB_H -#include +#ifdef HAVE_ZLIB_NG +# include -#if defined(NO_DEFLATE_BOUND) || ZLIB_VERNUM < 0x1200 -# define deflateBound(c,s) ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11) -#endif +# define z_stream zng_stream +#define gz_header_s zng_gz_header_s + +# define crc32(crc, buf, len) zng_crc32(crc, buf, len) + +# define inflate(strm, bits) zng_inflate(strm, bits) +# define inflateEnd(strm) zng_inflateEnd(strm) +# define inflateInit(strm) zng_inflateInit(strm) +# define inflateInit2(strm, bits) zng_inflateInit2(strm, bits) +# define inflateReset(strm) zng_inflateReset(strm) + +# define deflate(strm, flush) zng_deflate(strm, flush) +# define deflateBound(strm, source_len) zng_deflateBound(strm, source_len) +# define deflateEnd(strm) zng_deflateEnd(strm) +# define deflateInit(strm, level) zng_deflateInit(strm, level) +# define deflateInit2(stream, level, method, window_bits, mem_level, strategy) zng_deflateInit2(stream, level, method, window_bits, mem_level, strategy) +# define deflateReset(strm) zng_deflateReset(strm) +# define deflateSetHeader(strm, head) zng_deflateSetHeader(strm, head) + +#else +# include + +# if defined(NO_DEFLATE_BOUND) || ZLIB_VERNUM < 0x1200 +# define deflateBound(c,s) ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11) +# endif /* * zlib only gained support for setting up the gzip header in v1.2.2.1. In @@ -13,7 +36,7 @@ * operating systems, so it's fine to simply make this a no-op when using a * zlib version that doesn't support this yet. */ -#if ZLIB_VERNUM < 0x1221 +# if ZLIB_VERNUM < 0x1221 struct gz_header_s { int os; }; @@ -24,6 +47,7 @@ static int deflateSetHeader(z_streamp strm, struct gz_header_s *head) (void)(head); return Z_OK; } -#endif +# endif +#endif /* HAVE_ZLIB_NG */ #endif /* COMPAT_ZLIB_H */ diff --git a/meson.build b/meson.build index 12129a8b95..f9e6a051e0 100644 --- a/meson.build +++ b/meson.build @@ -792,11 +792,23 @@ else build_options_config.set('NO_PERL_CPAN_FALLBACKS', '') endif -zlib = dependency('zlib', default_options: ['default_library=static', 'tests=disabled']) -if zlib.version().version_compare('<1.2.0') - libgit_c_args += '-DNO_DEFLATE_BOUND' +zlib_backend = get_option('zlib_backend') +if zlib_backend in ['auto', 'zlib-ng'] + zlib_ng = dependency('zlib-ng', required: zlib_backend == 'zlib-ng') + if zlib_ng.found() + zlib_backend = 'zlib-ng' + libgit_c_args += '-DHAVE_ZLIB_NG' + libgit_dependencies += zlib_ng + endif +endif +if zlib_backend in ['auto', 'zlib'] + zlib = dependency('zlib', default_options: ['default_library=static', 'tests=disabled']) + if zlib.version().version_compare('<1.2.0') + libgit_c_args += '-DNO_DEFLATE_BOUND' + endif + zlib_backend = 'zlib' + libgit_dependencies += zlib endif -libgit_dependencies += zlib threads = dependency('threads', required: false) if threads.found() @@ -2001,4 +2013,5 @@ summary({ 'sha1': sha1_backend, 'sha1_unsafe': sha1_unsafe_backend, 'sha256': sha256_backend, + 'zlib': zlib_backend, }, section: 'Backends') diff --git a/meson_options.txt b/meson_options.txt index 5429022f30..c962c0a676 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -57,6 +57,8 @@ option('sha1_unsafe_backend', type: 'combo', choices: ['openssl', 'block', 'Comm description: 'The backend used for hashing data with the SHA1 object format in case no cryptographic security is needed.') option('sha256_backend', type: 'combo', choices: ['openssl', 'nettle', 'gcrypt', 'block'], value: 'block', description: 'The backend used for hashing objects with the SHA256 object format.') +option('zlib_backend', type: 'combo', choices: ['auto', 'zlib', 'zlib-ng'], value: 'auto', + description: 'The backend used for compressing objects and other data.') # Build tweaks. option('macos_use_homebrew_gettext', type: 'boolean', value: true, -- 2.48.1.362.g079036d154.dirty ``` ## Patrick Steinhardt, 2025-01-28 08:41 Subject: [PATCH v4 09/10] ci: switch linux-musl to use Meson Message-ID: <20250128-b4-pks-compat-drop-uncompress2-v4-9-129bc36ae8f5@pks.im> URL: https://gitlist.dev/e/20250128-b4-pks-compat-drop-uncompress2-v4-9-129bc36ae8f5%40pks.im In-Reply-To: <20250128-b4-pks-compat-drop-uncompress2-v4-0-129bc36ae8f5@pks.im> ``` Switch over the "linux-musl" job to use Meson instead of Makefiles. This is done due to multiple reasons: - It simplifies our CI infrastructure a bit as we don't have to manually specify a couple of build options anymore. - It verifies that Meson detects and sets those build options automatically. - It makes it easier for us to wire up a new CI job using zlib-ng as backend. One platform compatibility that Meson cannot easily detect automatically is the `GIT_TEST_UTF8_LOCALE` variable used in tests. Wire up a build option for it, which we set via a new "MESONFLAGS" environment variable. Note that we also drop the CC variable, which is set to "gcc". We already default to GCC when CC is unset in "ci/lib.sh", so this is not needed. Signed-off-by: Patrick Steinhardt --- .github/workflows/main.yml | 2 +- .gitlab-ci.yml | 2 +- ci/install-dependencies.sh | 2 +- ci/lib.sh | 5 +---- ci/run-build-and-tests.sh | 3 ++- meson.build | 2 +- meson_options.txt | 2 ++ 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 7f55f8b3a9..83bf9b918b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -394,7 +394,7 @@ jobs: fail-fast: false matrix: vector: - - jobname: linux-musl + - jobname: linux-musl-meson image: alpine distro: alpine-latest # Supported until 2025-04-02. diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 4976e18a05..f737e4177c 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -67,7 +67,7 @@ test:linux: CC: clang - jobname: pedantic image: fedora:latest - - jobname: linux-musl + - jobname: linux-musl-meson image: alpine:latest - jobname: linux-meson image: ubuntu:latest diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index d1cb9fa878..5ae80b0486 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -24,7 +24,7 @@ fi case "$distro" in alpine-*) - apk add --update shadow sudo build-base curl-dev openssl-dev expat-dev gettext \ + apk add --update shadow sudo meson ninja-build gcc libc-dev curl-dev openssl-dev expat-dev gettext \ pcre2-dev python3 musl-libintl perl-utils ncurses \ apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \ bash cvs gnupg perl-cgi perl-dbd-sqlite perl-io-tty >/dev/null diff --git a/ci/lib.sh b/ci/lib.sh index 8885ee3c3f..71d85ae85a 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -378,10 +378,7 @@ linux32) CC=gcc ;; linux-musl) - CC=gcc - MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3 USE_LIBPCRE2=Yes" - MAKEFLAGS="$MAKEFLAGS NO_REGEX=Yes ICONV_OMITS_BOM=Yes" - MAKEFLAGS="$MAKEFLAGS GIT_TEST_UTF8_LOCALE=C.UTF-8" + MESONFLAGS="$MESONFLAGS -DGIT_TEST_UTF8_LOCALE=C.UTF-8" ;; linux-leaks|linux-reftable-leaks) export SANITIZE=leak diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh index 964322055f..643c6a9dcc 100755 --- a/ci/run-build-and-tests.sh +++ b/ci/run-build-and-tests.sh @@ -55,7 +55,8 @@ case "$jobname" in --fatal-meson-warnings \ --warnlevel 2 --werror \ --wrap-mode nofallback \ - -Dfuzzers=true + -Dfuzzers=true \ + $MESONFLAGS group "Build" meson compile -C build -- if test -n "$run_tests" then diff --git a/meson.build b/meson.build index f9e6a051e0..35b3c51fb1 100644 --- a/meson.build +++ b/meson.build @@ -664,7 +664,7 @@ build_options_config.set('GIT_TEST_CMP_USE_COPIED_CONTEXT', '') build_options_config.set('GIT_TEST_INDEX_VERSION', '') build_options_config.set('GIT_TEST_OPTS', '') build_options_config.set('GIT_TEST_PERL_FATAL_WARNINGS', '') -build_options_config.set('GIT_TEST_UTF8_LOCALE', '') +build_options_config.set_quoted('GIT_TEST_UTF8_LOCALE', get_option('test_utf8_locale')) build_options_config.set_quoted('LOCALEDIR', fs.as_posix(get_option('prefix') / get_option('localedir'))) build_options_config.set('GITWEBDIR', fs.as_posix(get_option('prefix') / get_option('datadir') / 'gitweb')) diff --git a/meson_options.txt b/meson_options.txt index c962c0a676..e9f8e990e3 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -99,5 +99,7 @@ option('tests', type: 'boolean', value: true, description: 'Enable building tests. This requires Perl, but is separate from the "perl" option such that you can build tests without Perl features enabled.') option('test_output_directory', type: 'string', description: 'Path to the directory used to store test outputs') +option('test_utf8_locale', type: 'string', + description: 'Name of a UTF-8 locale used for testing.') option('fuzzers', type: 'boolean', value: false, description: 'Enable building fuzzers.') -- 2.48.1.362.g079036d154.dirty ``` ## Patrick Steinhardt, 2025-01-28 08:41 Subject: [PATCH v4 10/10] ci: make "linux-musl" job use zlib-ng Message-ID: <20250128-b4-pks-compat-drop-uncompress2-v4-10-129bc36ae8f5@pks.im> URL: https://gitlist.dev/e/20250128-b4-pks-compat-drop-uncompress2-v4-10-129bc36ae8f5%40pks.im In-Reply-To: <20250128-b4-pks-compat-drop-uncompress2-v4-0-129bc36ae8f5@pks.im> ``` We don't yet have any test coverage for the new zlib-ng backend as part of our CI. Add it by installing zlib-ng in Alpine Linux, which causes Meson to pick it up automatically. Note that we are somewhat limited with regards to where we run that job: Debian-based distributions don't have zlib-ng in their repositories, Fedora has it but doesn't run tests, and Alma Linux doesn't have the package either. Alpine Linux does have it available and is running our test suite, which is why it was picked. Signed-off-by: Patrick Steinhardt --- ci/install-dependencies.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh index 5ae80b0486..be7ab4b1bf 100755 --- a/ci/install-dependencies.sh +++ b/ci/install-dependencies.sh @@ -25,7 +25,7 @@ fi case "$distro" in alpine-*) apk add --update shadow sudo meson ninja-build gcc libc-dev curl-dev openssl-dev expat-dev gettext \ - pcre2-dev python3 musl-libintl perl-utils ncurses \ + zlib-ng-dev pcre2-dev python3 musl-libintl perl-utils ncurses \ apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \ bash cvs gnupg perl-cgi perl-dbd-sqlite perl-io-tty >/dev/null ;; -- 2.48.1.362.g079036d154.dirty ``` ## Junio C Hamano, 2025-01-28 20:50 Subject: Re: [PATCH v4 00/10] compat/zlib: allow use of zlib-ng as backend Message-ID: URL: https://gitlist.dev/e/xmqqv7tymzmm.fsf%40gitster.g In-Reply-To: <20250128-b4-pks-compat-drop-uncompress2-v4-0-129bc36ae8f5@pks.im> ``` Patrick Steinhardt writes: > Changes in v4: > - Add a comment explaining why we can stub out `deflateSetHeader()`. > - Add a comment explaining why we have to cast away constness with > zlib-ng's `next_in` field. > - Link to v3: https://lore.kernel.org/r/20250116-b4-pks-compat-drop-uncompress2-v3-0-f2af1f5c4a06@pks.im > > I've adjusted the series to be based on top of fbe8d3079d (Git 2.48, > 2025-01-10) with ps/meson-weak-sha1-build at 6a0ee54f9a (meson: provide > a summary of configured backends, 2024-12-30) and ps/build-meson-fixes > at 4e517e68b5 (ci: wire up Visual Studio build with Meson, 2025-01-14) > merged into it. This matches what Junio has in his tree -- sorry for > screwing up the previous base! Looks like this is getting close to the final. Will replace. Thanks. ```