Re: [PATCH 1/1] Fix the order of consuming unpackLimit config settings.

From: Jeff King <peff@peff.net>
Date: Aug 21, 2023, 20:30 UTC
Message-ID: <20230821203051.GA1800481@coredump.intra.peff.net>
In-Reply-To: <20230817215325.2550998-2-taylorsantiago@google.com>

On Thu, Aug 17, 2023 at 02:53:25PM -0700, Taylor Santiago wrote:

> The documentation for fetch.unpackLimit states that fetch.unpackLimit
> should be treated as higher priority than transfer.unpackLimit, but the
> intended order is currently inverted.

Looks like this has been broken since it was introduced in e28714c527 (Consolidate {receive,fetch}.unpackLimit, 2007-01-24).

Sometimes when documentation and code have diverged for so long, we prefer to change the documentation instead, to avoid disrupting users. But that would make these weirdly unlike most other "specific overrides general" config options. And the fact that the bug has existed for so long without anyone noticing implies to me that nobody really tries to mix and match them much.

So I'm in favor of fixing them[1]. Doesn't receive-pack have the same bug, though? And we'd probably want to have tests for both.

-Peff

[1] Commit e28714c527 does mention deprecating the operation-specific
    ones. In my experience (and I did use transfer.unpackLimit quite a
    bit for server-side code at GitHub) there is no real need for have
    operation-specific ones. OTOH it is work to deprecate them, and they
    are not causing a big maintenance burden. So it is probably simplest
    to keep them.