Re: [PATCH v6 0/5] Reftable support git-core
- From
Jeff King <peff@peff.net>
- Date
- Feb 20, 2020, 11:19 UTC
- Message-ID
- <20200220111901.GA1252160@coredump.intra.peff.net>
- In-Reply-To
- <xmqq8skyfnxm.fsf@gitster-ct.c.googlers.com>
On Wed, Feb 19, 2020 at 10:10:29AM -0800, Junio C Hamano wrote:
Show 22 quoted lines
> Han-Wen Nienhuys <hanwenn@gmail.com> writes: > > > On Wed, Feb 19, 2020 at 6:02 PM Junio C Hamano <gitster@pobox.com> wrote: > > > >> > your checker is tripping over code imported from zlib. I added a /* > >> > clang-format off */ comment to avoid reformatting this code. What do > >> > you suggest? > >> > >> Use zlib from the system instead? > > > > uncompress2 is a 2016 addition to zlib. It doesn't pass on > > gitgitgadget's CI, because it is using an older version of zlib. > > Ahh. > > It is OK (and indeed you're right that you cannot avoid it) to ship > a reasonably new snapshot as a fallback in such a case, but it still > is far more preferrable to avoid linking with the fallback snapshot > copy when a working one is available on the system, especially for a > widely used and established library like zlib, because we have one > less thing to keep up-to-date with the security patches made to the > upstream.
If this were substantial code, I'd agree. But this is really just a thin wrapper around the usual loop zlib inflate() loop that we already do in a dozen places in our code (e.g., unpack_loose_rest(), which even does the "did we consume all the bytes" check that uncompress2 allows).
I'm not sure it is worth the mental energy of adding a Makefile knob to use the zlib version if we can just open-code it ourselves (or provide our own custom helper that we always use).
And if it _is_ worth taking the zlib version because we're concerned that it may contain or gain bugfixes[1], then possibly we should be using it in lots more places (though probably not everywhere, as we do sometimes need the streaming behavior of the loop).
[1] I'll admit we've hit some subtleties with that loop in the past,
especially around corrupted or bogus inputs, but I think we've
shaken them all out these days.-Peff