Re: [PATCH 00/13] Update versions of libcurl and Perl

From: Eli Schwartz <eschwartz@gentoo.org>
Date: Oct 22, 2024, 03:34 UTC
Message-ID: <66bb101c-eb9f-4824-8766-750e58cd422e@gentoo.org>
In-Reply-To: <ZwmEDt7ftJabvMUH@tapette.crustytoothpaste.net>

On 10/11/24 4:01 PM, brian m. carlson wrote:

Show 13 quoted lines

> It is not effectively zero cost.  When I want to write a patch, I must
> make sure that it works on all the platforms we support, or my patch
> will get reverted or not picked up.  That means I have to expend
> additional effort when adding features to look through the supported
> versions of our dependencies and either conditionally check them or skip
> the feature.  Sometimes I have to rewrite that feature in a different
> way, or ship a compatibility stub for a system that doesn't support it.
> 
> I have actually spent a decent amount of work getting things to work
> across older versions of software, both in Git and elsewhere.  The more
> we honour the policy we have already made and agreed upon, the less work
> Git developers have to do to support adding and maintaining these
> features.

On a personal level I have a mild abhorrence of the general notion of bumping a version requirement in order to bump a requirement. I have a lot of sympathy for having a policy about what to expend effort on supporting, though!

Without getting into what the git project "should expend effort to support" (future efforts to be clear, not existing code that simply stays in place doing no significant harm)...

This patch series simplifies the codebase in order to remove workarounds for versions of curl < 7.56.0 -- it then documents the minimum supported version as 7.61.0 and there are even proposals to add a version check for that. Why? Is it currently believed that curl 7.56 through 7.60.x are going to break as a result of the modifications to ./INSTALL alone?

Instead I would suggest documenting the minimum version as 7.56 to align with reality.

Your general observation about respecting the platform support policy and not making developers expend time working around ancient dependency versions no one should be using... is something that I would say is a better fit for, well, the platform support policy.

You could instead add a section to the platform support policy detailing the minimum versions of dependencies which the git developers are willing to spend time supporting. A developer working on changes which would be onerous to backfill support for, would then have a simple, documented, easy to find policy about when it is acceptable to bump the version documented in ./INSTALL. The process would then look like:

- Code a new feature.

- Check the version table to see if maybe it was added basically
  yesterday in curl 8.7, or whether it is available in say, curl 7.75.

- Discover it was added in curl 7.59. Oh shoot! The ./INSTALL says we
  still support versions before that, but it's also super decrepit and
  nobody runs it anyway. But wait -- the platform support says we only
  care about 7.61.

- Shrug and grin. First patch in the series now bumps ./INSTALL to say
  the minimum required curl is 7.59, and if anyone disagrees then it's
  fair game to respond with. "fite me. The platform support says I don't
  have to care, we are making this change whether you like it or not".

The important distinction here is that in this model, the install requirements aren't about what you want to spend time on supporting, they are about truthfully communicating what *works* in point of fact.

Likewise, it does actually make sense to have a version check either in the build system or the code, but probably the build system, to ensure that the minimum required version which is necessary in order to successfully compile the codebase is available. It doesn't change what works and what fails -- it simply provides a clear error message. Instead of inscrutable compiler errors about CURLSSLSET_NO_BACKENDS not existing, you get:

Dependency libcurl found: NO. Found 7.51.0 but need: '>=7.56.0'

meson.build:642:7: ERROR: Dependency 'libcurl' is required but not found.

-- 
Eli Schwartz