Re: [PATCH v2 0/8] post-v2.33 "drop support for ancient curl" follow-up
- From
Jeff King <peff@peff.net>
- Date
- Sep 10, 2021, 15:20 UTC
- Message-ID
- <YTt3qwVPXEkgh6LP@coredump.intra.peff.net>
- In-Reply-To
- <87lf44cvkp.fsf@evledraar.gmail.com>
On Fri, Sep 10, 2021 at 05:08:32PM +0200, Ævar Arnfjörð Bjarmason wrote:
Show 16 quoted lines
> > So I dunno. I do not have any clever solution that would have caught > > this automatically without creating an even bigger maintenance headache. > > Yeah, ideally we'd have tests for these optional features, and we'd then > just add them to GIT-BUILD-OPTIONS and skip dedicated tests > appropriately, then it would be more visible to see those tests > skipped. Presumably someone testing this would run "make test" against a > glob that includes *curl*. > > But that's not easy to do in practice since the flags are either not > visible from the outside in terms of behavior, or if they are it's all > something that requires proxies, SSL etc, which we don't test currently. > > We can and should test that, but requires e.g. extending lib-httpd.sh to > start apache with ssl support, or maybe we could do it more easily with > an optional stunnel or something...
Yeah, even after we get the right version/feature flags into the test suite, most of these are pretty obscure and hard to test.
I definitely think that's not something we should worry about for this series.
Show 8 quoted lines
> > (Though for other reasons, it might be nice to report the curl version > > from "git version --build-options". This is a bit tricky because we > > avoid linking at libcurl at all in the main binary. Definitely > > orthogonal to your series, anyway). > > We could always ship a git-version-curl and shell out to it, or embed > the version curl-config --vernum gave us at compile time via Makefile -> > -DGIT_CURL_VERSION.
Yeah, I was thinking "git remote-curl --curl-version" or something. Let's punt on it for now, though. I think this series is getting close to ready, and this is all only semi-related.
-Peff