Re: [PATCH 1/4] t/lib-httpd: bump required apache version to 2.2

From: Todd Zullinger <tmz@pobox.com>
Date: Feb 2, 2023, 04:39 UTC
Message-ID: <Y9s+ifALGYniOVHK@pobox.com>
In-Reply-To: <Y9pO8bBZjXjEZhR/@coredump.intra.peff.net>

Jeff King wrote:

Show 37 quoted lines

> Apache 2.2 was released in 2005, almost 18 years ago. We can probably
> assume that people are running a version at least that old (and the
> stakes for removing it are fairly low, as the worst case is that they
> would not run the http tests against their ancient version).
> 
> Dropping support for the older versions cleans up the config file a
> little, and will also enable us to bump the required version further
> (with more cleanups) in a future patch.
> 
> Note that the file actually checks for version 2.1. In apache's
> versioning scheme, odd numbered versions are for development and even
> numbers are for stable releases. So 2.1 and 2.2 are effectively the same
> from our perspective.
> 
> Older versions would just fail to start, which would generally cause us
> to skip the tests. However, we do have version detection code in
> lib-httpd.sh which produces a nicer error message, so let's update that,
> too. I didn't bother handling the case of "3.0", etc. Apache has been on
> 2.x for 21 years, with no signs of bumping the major version.  And if
> they eventually do, I suspect there will be enough breaking changes that
> we'd need to update more than just the numeric version check. We can
> worry about that hypothetical when it happens.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  t/lib-httpd.sh          | 11 +++++++----
>  t/lib-httpd/apache.conf |  8 --------
>  2 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
> index 608949ea80..8fc411ff41 100644
> --- a/t/lib-httpd.sh
> +++ b/t/lib-httpd.sh
> @@ -99,16 +99,19 @@ then
>  fi
>  
>  HTTPD_VERSION=$($LIB_HTTPD_PATH -v | \

Trivial, but is it worth getting rid of the unnecessary backslash, while you're improving things here? Maybe that's a mild tangent for code that's otherwise not adjusted very often?

The backslash was present when lib-httpd.sh was added in faa4bc35a0 (http-push: add regression tests, 2008-02-27). The line was last touched in e429dfd5e4 (t/lib-httpd.sh: use the $( ... ) construct for command substitution, 2015-12-22).

> -	sed -n 's/^Server version: Apache\/\([0-9]*\)\..*$/\1/p; q')
> +	sed -n 's/^Server version: Apache\/\([0-9.]*\).*$/\1/p; q')
> +HTTPD_VERSION_MAJOR=$(echo $HTTPD_VERSION | cut -d. -f1)
> +HTTPD_VERSION_MINOR=$(echo $HTTPD_VERSION | cut -d. -f2)

-- 
Todd