Re: [PATCH 4/5] http: centralize the accounting of libcurl dependencies

From: Jeff King <peff@peff.net>
Date: Sep 9, 2021, 19:26 UTC
Message-ID: <YTpf7yC+32LYRC4Z@coredump.intra.peff.net>
In-Reply-To: <xmqq5yv91wbn.fsf@gitster.g>

On Thu, Sep 09, 2021 at 10:40:44AM -0700, Junio C Hamano wrote:

Show 15 quoted lines

> > Part of me is a little sad at the duplication this creates. We could
> > just be checking
> >
> >   #ifdef CURLOPT_TCP_KEEPALIVE
> >
> > in the second hunk, without the first one at all.
> 
> I recall having exactly the same reaction when the above pattern
> first was floated.  
> 
> I still do not see how the GITCURL_* duplication is worth it.  What
> do we want to gain from having a "central registry"?
> 
> If it is to see which ones we care about, would it be sufficient to
> use the hits from "git grep -e CURL" with postprocessing?

Just brain-storming:

  - It's probably a little easier to read, with the comments all
    together with the definitions (keeping in mind that some of these
    flags may need to be checked in several spots).

  - Likewise a few of the checks are non-trivial, with multiple ANDs, so
    it may be nicer to have a single registry (though the resulting
    "GIT_CURL_HAVE_X_and_Y_and_Z" are pretty ugly to me).

  - _Possibly_ it provides a knob for the user to say
    "-DGIT_CURL_HAVE_FOO=1" by overriding our auto-detection. But since
    most of these are "do we have FOO defined? Because are about to
    reference it in the code", I don't see what that override would buy
    you.

I dunno. I admit that I do not overly care that much. Handling curl version dependencies was neglected for many years. If Ævar wants to take a more active role in handling these and this is convenient for him, I'm happy enough however it works.

-Peff