* [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7
@ 2023-05-17 19:06 Todd Zullinger
2023-05-18 18:45 ` Jeff King
0 siblings, 1 reply; 11+ messages in thread
From: Todd Zullinger @ 2023-05-17 19:06 UTC (permalink / raw)
To: git; +Cc: Matthew John Cheetham, Jeff King
After applying the imap-send.c patch¹ on RHEL/CentOS 7, I
noticed the http tests fail because the Apache httpd config
is not valid with httpd-2.4.6² on CentOS 7.
The tests fail with:
Parse errors: No plan found in TAP output
The problem is that CGIPassAuth, added in 988aad99b4 (t5563:
add tests for basic and anoymous HTTP access, 2023-02-27) is
not supported by httpd < 2.4.13:
Starting httpd on port 10410
[Wed May 17 17:06:52.184409 2023] [core:warn] [pid 477886] AH00111: Config variable ${LIB_HTTPD_SVN} is not defined
[Wed May 17 17:06:52.184495 2023] [core:warn] [pid 477886] AH00111: Config variable ${LIB_HTTPD_SVNPATH} is not defined
AH00526: Syntax error on line 149 of /builddir/build/BUILD/git-2.41.0.rc0/t/lib-httpd/apache.conf:
Invalid command 'CGIPassAuth', perhaps misspelled or defined by a module not included in the server configuration
error: web server setup failed
Since edd060dc84 (t/lib-httpd: bump required apache version
to 2.4, 2023-02-01), we require httpd-2.4 and no longer have
any <IfVersion> conditions. I'm not sure if this a reason
to add some again (nor am I certain if httpd's IfVersion
supports minor versions).
Perhaps there's a more elegant way to fix this? (I haven't
thought of anything in patch form yet, apologies.)
I'd like to still build git for CentOS 7 and not skip all
the http tests, but if it's time to say it is not worth
supporting, I can understand. RHEL/CentOS 7 has a little
over a year left before it is EOL³.
¹ <20230517070632.71884-1-list@eworm.de>
² https://httpd.apache.org/docs/2.4/mod/core.html#cgipassauth
³ https://blog.centos.org/2023/04/end-dates-are-coming-for-centos-stream-8-and-centos-linux-7/#centos-linux-7-end-of-life-june-30-2024
Thanks,
--
Todd
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7 2023-05-17 19:06 [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7 Todd Zullinger @ 2023-05-18 18:45 ` Jeff King 2023-05-18 19:14 ` Todd Zullinger 2023-05-18 19:21 ` Jeff King 0 siblings, 2 replies; 11+ messages in thread From: Jeff King @ 2023-05-18 18:45 UTC (permalink / raw) To: Todd Zullinger; +Cc: git, Matthew John Cheetham On Wed, May 17, 2023 at 03:06:18PM -0400, Todd Zullinger wrote: > The problem is that CGIPassAuth, added in 988aad99b4 (t5563: > add tests for basic and anoymous HTTP access, 2023-02-27) is > not supported by httpd < 2.4.13: Thanks for reporting. I don't think we dug into version requirements for that (obviously CGI stuff goes back forever, so it's just that particular option). That version of Apache is "only" 8 years old. Which is old, but we often go back that far for dependencies. > Since edd060dc84 (t/lib-httpd: bump required apache version > to 2.4, 2023-02-01), we require httpd-2.4 and no longer have > any <IfVersion> conditions. I'm not sure if this a reason > to add some again (nor am I certain if httpd's IfVersion > supports minor versions). I don't think an IfVersion would be sufficient, because we need to also tell the script making use of that config that it should skip its tests. So I think we want something more like the HTTP/2 tests have: a separate script which enables the extra config, and bails if the web server setup fails. Something like this: diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index 6805229dcb..4b6d9a5817 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -191,6 +191,10 @@ enable_http2 () { test_set_prereq HTTP2 } +enable_cgipassauth () { + HTTPD_PARA="$HTTPD_PARA -DUSE_CGIPASSAUTH" +} + start_httpd() { prepare_httpd >&3 2>&4 diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index 9e6892970d..a22d138605 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -146,7 +146,9 @@ SetEnv PERL_PATH ${PERL_PATH} <LocationMatch /custom_auth/> SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} SetEnv GIT_HTTP_EXPORT_ALL + <IfDefine USE_CGIPASSAUTH> CGIPassAuth on + </IfDefine> </LocationMatch> ScriptAlias /smart/incomplete_length/git-upload-pack incomplete-length-upload-pack-v2-http.sh/ ScriptAlias /smart/incomplete_body/git-upload-pack incomplete-body-upload-pack-v2-http.sh/ diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh index f45a43b4b5..0f23f45572 100755 --- a/t/t5563-simple-http-auth.sh +++ b/t/t5563-simple-http-auth.sh @@ -5,6 +5,7 @@ test_description='test http auth header and credential helper interop' . ./test-lib.sh . "$TEST_DIRECTORY"/lib-httpd.sh +enable_cgipassauth start_httpd test_expect_success 'setup_credential_helper' ' And then the theory is that t5551 works as before (it does not try to use that config), and t5563 will either work out of the box (for recent versions), or web server setup will fail, and we'll skip all tests. BUT. That won't work if you have set GIT_TEST_HTTPD=1, rather than the default of "auto". Because then it is instructed to complain about webserver setup failing, so t5563 will fail rather than skip. So we have a few options there: 1. You use the looser value of GIT_TEST_HTTPD for CentOS tests, which would do the right thing. The downside is that if server setup failed for other reasons, we wouldn't notice and would silently skip the HTTP tests. 2. We do some kind of version check in enable_cgipassauth(), and skip tests manually if it doesn't pass. 3. You just skip the test manually on that platform with GIT_SKIP_TESTS=t5563. Obviously (1) and (3) are the least work for us upstream, but I don't think (2) would be too hard to do. -Peff ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7 2023-05-18 18:45 ` Jeff King @ 2023-05-18 19:14 ` Todd Zullinger 2023-05-18 19:22 ` Jeff King 2023-05-18 19:21 ` Jeff King 1 sibling, 1 reply; 11+ messages in thread From: Todd Zullinger @ 2023-05-18 19:14 UTC (permalink / raw) To: Jeff King; +Cc: git, Matthew John Cheetham Jeff King wrote: > On Wed, May 17, 2023 at 03:06:18PM -0400, Todd Zullinger wrote: > >> The problem is that CGIPassAuth, added in 988aad99b4 (t5563: >> add tests for basic and anoymous HTTP access, 2023-02-27) is >> not supported by httpd < 2.4.13: > > Thanks for reporting. I don't think we dug into version requirements for > that (obviously CGI stuff goes back forever, so it's just that > particular option). That version of Apache is "only" 8 years old. Which > is old, but we often go back that far for dependencies. Indeed, thanks. I don't imagine there are too many people looking to support the latest git auth mechanisms via http who are also running CentOS 7. But it's nice to not have to disable all the other http tests. >> Since edd060dc84 (t/lib-httpd: bump required apache version >> to 2.4, 2023-02-01), we require httpd-2.4 and no longer have >> any <IfVersion> conditions. I'm not sure if this a reason >> to add some again (nor am I certain if httpd's IfVersion >> supports minor versions). > > I don't think an IfVersion would be sufficient, because we need to also > tell the script making use of that config that it should skip its tests. > So I think we want something more like the HTTP/2 tests have: a separate > script which enables the extra config, and bails if the web server setup > fails. > > Something like this: [...] I'll try to apply this and see how it goes, skipping t5563, a little later in the day. > And then the theory is that t5551 works as before (it does not try to > use that config), and t5563 will either work out of the box (for recent > versions), or web server setup will fail, and we'll skip all tests. > > BUT. That won't work if you have set GIT_TEST_HTTPD=1, rather than the > default of "auto". Because then it is instructed to complain about > webserver setup failing, so t5563 will fail rather than skip. So we have > a few options there: > > 1. You use the looser value of GIT_TEST_HTTPD for CentOS tests, which > would do the right thing. The downside is that if server setup > failed for other reasons, we wouldn't notice and would silently > skip the HTTP tests. > > 2. We do some kind of version check in enable_cgipassauth(), > and skip tests manually if it doesn't pass. > > 3. You just skip the test manually on that platform with > GIT_SKIP_TESTS=t5563. > > Obviously (1) and (3) are the least work for us upstream, but I don't > think (2) would be too hard to do. Yeah, the path of least effort seems ideal here. If the tests are split I can easily skip them for CentOS 7. For something that's going away in little over a year, it's good to put at least some of the onus on folks packaging for it. And that's less cruft in the test suite after CentOS 7 is gone (or any other ancient httpd's git may wish to support). Thanks Peff! I'm really glad you know your way around the test suite so well. It'd take me far longer to figure out a good plan for something like this. -- Todd ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7 2023-05-18 19:14 ` Todd Zullinger @ 2023-05-18 19:22 ` Jeff King 0 siblings, 0 replies; 11+ messages in thread From: Jeff King @ 2023-05-18 19:22 UTC (permalink / raw) To: Todd Zullinger; +Cc: git, Matthew John Cheetham On Thu, May 18, 2023 at 03:14:50PM -0400, Todd Zullinger wrote: > > 1. You use the looser value of GIT_TEST_HTTPD for CentOS tests, which > > would do the right thing. The downside is that if server setup > > failed for other reasons, we wouldn't notice and would silently > > skip the HTTP tests. > > > > 2. We do some kind of version check in enable_cgipassauth(), > > and skip tests manually if it doesn't pass. > > > > 3. You just skip the test manually on that platform with > > GIT_SKIP_TESTS=t5563. > > > > Obviously (1) and (3) are the least work for us upstream, but I don't > > think (2) would be too hard to do. > > Yeah, the path of least effort seems ideal here. If the > tests are split I can easily skip them for CentOS 7. For > something that's going away in little over a year, it's good > to put at least some of the onus on folks packaging for it. > And that's less cruft in the test suite after CentOS 7 is > gone (or any other ancient httpd's git may wish to support). Too late. :) I just sent a patch with (2), as it was only a few extra lines. > Thanks Peff! I'm really glad you know your way around the > test suite so well. It'd take me far longer to figure out a > good plan for something like this. I was able to cheat a little, having had to figure out a similar solution for HTTP/2 not too long ago. :) -Peff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7 2023-05-18 18:45 ` Jeff King 2023-05-18 19:14 ` Todd Zullinger @ 2023-05-18 19:21 ` Jeff King 2023-05-18 19:50 ` Todd Zullinger ` (2 more replies) 1 sibling, 3 replies; 11+ messages in thread From: Jeff King @ 2023-05-18 19:21 UTC (permalink / raw) To: Todd Zullinger; +Cc: Junio C Hamano, git, Matthew John Cheetham On Thu, May 18, 2023 at 02:45:32PM -0400, Jeff King wrote: > 2. We do some kind of version check in enable_cgipassauth(), > and skip tests manually if it doesn't pass. > [...] > Obviously (1) and (3) are the least work for us upstream, but I don't > think (2) would be too hard to do. It indeed was pretty easy. So here's a patch. I'm adding Junio to the cc before any review, as this is a change in the v2.41 cycle. So I think we'd want to address this before the release. -- >8 -- Subject: [PATCH] t/lib-httpd: make CGIPassAuth support conditional Commit 988aad99b4 (t5563: add tests for basic and anoymous HTTP access, 2023-02-27) added tests that require Apache to support the CGIPassAuth directive, which was added in Apache 2.4.13. This is fairly old (~8 years), but recent enough that we still encounter it in the wild (e.g., RHEL/CentOS 7, which is not EOL until June 2024). We can live with skipping the new tests on such a platform. But unfortunately, since the directive is used unconditionally in our apache.conf, it means the web server fails to start entirely, and we cannot run other HTTP tests at all (e.g., the basic ones in t5551). We can fix that by making the config conditional, and only triggering it for t5563. That solves the problem for t5551 (which then ignores the directive entirely). For t5563, we'd see apache complain in start_httpd; with the default setting of GIT_TEST_HTTPD, we'd then skip the whole script. But that leaves one small problem: people may set GIT_TEST_HTTPD=1 explicitly, which instructs the tests to fail (rather than skip) when we can't start the webserver (to avoid accidentally missing some tests). This could be worked around by having the user manually set GIT_SKIP_TESTS on a platform with an older Apache. But we can be a bit friendlier by doing the version check ourselves and setting an appropriate prereq. We'll use the (lack of) prereq to then skip the rest of t5563. In theory we could use the prereq to skip individual tests, but in practice this whole script depends on it. Reported-by: Todd Zullinger <tmz@pobox.com> Signed-off-by: Jeff King <peff@peff.net> --- I tested this manually by mutilating the config directive to "FooBar", which would fail even on recent versions. And then tweaking the "13" in the version check up to "60" to make sure it properly skips even with recent Apache. But testing on real CentOS 7 would be very much appreciated. t/lib-httpd.sh | 14 ++++++++++++++ t/lib-httpd/apache.conf | 2 ++ t/t5563-simple-http-auth.sh | 6 ++++++ 3 files changed, 22 insertions(+) diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index 6805229dcb..2fb1b2ae56 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -191,6 +191,20 @@ enable_http2 () { test_set_prereq HTTP2 } +enable_cgipassauth () { + # We are looking for 2.4.13 or more recent. Since we only support + # 2.4 and up, no need to check for older major/minor. + if test "$HTTPD_VERSION_MAJOR" = 2 && + test "$HTTPD_VERSION_MINOR" = 4 && + test "$(echo $HTTPD_VERSION | cut -d. -f3)" -lt 13 + then + echo >&4 "apache $HTTPD_VERSION too old for CGIPassAuth" + return + fi + HTTPD_PARA="$HTTPD_PARA -DUSE_CGIPASSAUTH" + test_set_prereq CGIPASSAUTH +} + start_httpd() { prepare_httpd >&3 2>&4 diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index 9e6892970d..a22d138605 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -146,7 +146,9 @@ SetEnv PERL_PATH ${PERL_PATH} <LocationMatch /custom_auth/> SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} SetEnv GIT_HTTP_EXPORT_ALL + <IfDefine USE_CGIPASSAUTH> CGIPassAuth on + </IfDefine> </LocationMatch> ScriptAlias /smart/incomplete_length/git-upload-pack incomplete-length-upload-pack-v2-http.sh/ ScriptAlias /smart/incomplete_body/git-upload-pack incomplete-body-upload-pack-v2-http.sh/ diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh index f45a43b4b5..ab8a721ccc 100755 --- a/t/t5563-simple-http-auth.sh +++ b/t/t5563-simple-http-auth.sh @@ -5,6 +5,12 @@ test_description='test http auth header and credential helper interop' . ./test-lib.sh . "$TEST_DIRECTORY"/lib-httpd.sh +enable_cgipassauth +if ! test_have_prereq CGIPASSAUTH +then + skip_all="no CGIPassAuth support" + test_done +fi start_httpd test_expect_success 'setup_credential_helper' ' -- 2.41.0.rc0.359.g22664e20e7 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7 2023-05-18 19:21 ` Jeff King @ 2023-05-18 19:50 ` Todd Zullinger 2023-05-18 20:11 ` Todd Zullinger 2023-05-18 21:28 ` Junio C Hamano 2 siblings, 0 replies; 11+ messages in thread From: Todd Zullinger @ 2023-05-18 19:50 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, git, Matthew John Cheetham Jeff King wrote: > On Thu, May 18, 2023 at 02:45:32PM -0400, Jeff King wrote: > >> 2. We do some kind of version check in enable_cgipassauth(), >> and skip tests manually if it doesn't pass. >> [...] >> Obviously (1) and (3) are the least work for us upstream, but I don't >> think (2) would be too hard to do. > > It indeed was pretty easy. So here's a patch. I'm adding Junio to the cc > before any review, as this is a change in the v2.41 cycle. So I think > we'd want to address this before the release. Nice. > I tested this manually by mutilating the config directive to "FooBar", > which would fail even on recent versions. And then tweaking the "13" > in the version check up to "60" to make sure it properly skips even with > recent Apache. But testing on real CentOS 7 would be very much > appreciated. I have a build running now. It takes around ~20 minutes in the Fedora/EPEL build system. I wanted to make sure it was started quickly since you turned out a patch in the time it took me to write my last reply. ;) (It would have been quicker if I hadn't forgot to apply the imap-send patch as well. D'oh!) -- Todd ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7 2023-05-18 19:21 ` Jeff King 2023-05-18 19:50 ` Todd Zullinger @ 2023-05-18 20:11 ` Todd Zullinger 2023-05-18 21:31 ` Junio C Hamano 2023-05-18 21:28 ` Junio C Hamano 2 siblings, 1 reply; 11+ messages in thread From: Todd Zullinger @ 2023-05-18 20:11 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, git, Matthew John Cheetham Jeff King wrote: > I tested this manually by mutilating the config directive to "FooBar", > which would fail even on recent versions. And then tweaking the "13" > in the version check up to "60" to make sure it properly skips even with > recent Apache. But testing on real CentOS 7 would be very much > appreciated. I tested on RHEL/CentOS 7 and it does what it says on the tin: t5563-simple-http-auth.sh .......................... skipped: no CGIPassAuth support Thanks! -- Todd ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7 2023-05-18 20:11 ` Todd Zullinger @ 2023-05-18 21:31 ` Junio C Hamano 0 siblings, 0 replies; 11+ messages in thread From: Junio C Hamano @ 2023-05-18 21:31 UTC (permalink / raw) To: Todd Zullinger; +Cc: Jeff King, git, Matthew John Cheetham Todd Zullinger <tmz@pobox.com> writes: > Jeff King wrote: >> I tested this manually by mutilating the config directive to "FooBar", >> which would fail even on recent versions. And then tweaking the "13" >> in the version check up to "60" to make sure it properly skips even with >> recent Apache. But testing on real CentOS 7 would be very much >> appreciated. > > I tested on RHEL/CentOS 7 and it does what it says on the tin: > > t5563-simple-http-auth.sh .......................... skipped: no CGIPassAuth support > > Thanks! Nice. Thanks, both. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7 2023-05-18 19:21 ` Jeff King 2023-05-18 19:50 ` Todd Zullinger 2023-05-18 20:11 ` Todd Zullinger @ 2023-05-18 21:28 ` Junio C Hamano 2023-05-18 23:10 ` Jeff King 2 siblings, 1 reply; 11+ messages in thread From: Junio C Hamano @ 2023-05-18 21:28 UTC (permalink / raw) To: Jeff King; +Cc: Todd Zullinger, git, Matthew John Cheetham Jeff King <peff@peff.net> writes: > On Thu, May 18, 2023 at 02:45:32PM -0400, Jeff King wrote: > >> 2. We do some kind of version check in enable_cgipassauth(), >> and skip tests manually if it doesn't pass. >> [...] >> Obviously (1) and (3) are the least work for us upstream, but I don't >> think (2) would be too hard to do. > > It indeed was pretty easy. So here's a patch. I'm adding Junio to the cc > before any review, as this is a change in the v2.41 cycle. So I think > we'd want to address this before the release. Thanks for being considerate, as always ;-) Very much appreciated. > -- >8 -- > Subject: [PATCH] t/lib-httpd: make CGIPassAuth support conditional > > Commit 988aad99b4 (t5563: add tests for basic and anoymous HTTP access, > 2023-02-27) added tests that require Apache to support the CGIPassAuth > directive, which was added in Apache 2.4.13. This is fairly old (~8 > years), but recent enough that we still encounter it in the wild (e.g., > RHEL/CentOS 7, which is not EOL until June 2024). nitpick: we are fine to encountering 2.4.13 in the wild---encountering something a bit older than that is problematic. A quick internet search tells me that CentOS 7 ships Apache 2.4.6, so if we trust that... ... fairly old (~8 years), but recent enough that we still encounter versions older than that in the wild (e.g. CentOS 7, which is not EOL until June 2024, comes with Apache 2.4.6 from 2014 plus security fixes). or something? > I tested this manually by mutilating the config directive to "FooBar", > which would fail even on recent versions. And then tweaking the "13" > in the version check up to "60" to make sure it properly skips even with > recent Apache. But testing on real CentOS 7 would be very much > appreciated. Indeed, we'd love to see tests on a real thing, especially if we were to fast-track this. FWIW, the manual test you outlined above sounds like a very sensible way to "emulate" under the condition that there is no access to the real thing. > +enable_cgipassauth () { > + # We are looking for 2.4.13 or more recent. Since we only support > + # 2.4 and up, no need to check for older major/minor. > + if test "$HTTPD_VERSION_MAJOR" = 2 && > + test "$HTTPD_VERSION_MINOR" = 4 && > + test "$(echo $HTTPD_VERSION | cut -d. -f3)" -lt 13 As HTTPD_VERSION comes from $LIB_HTTPD_PATH -v | sed -n 's|^Server version: Apache/\([0-9.]*\).*|p' and parses a line like "Server version: Apache/2.4.6 (CentOS)", unless somebody ships 2.4 without any digit after it, the above should be safe ;-) > + then > + echo >&4 "apache $HTTPD_VERSION too old for CGIPassAuth" > + return > + fi > + HTTPD_PARA="$HTTPD_PARA -DUSE_CGIPASSAUTH" > + test_set_prereq CGIPASSAUTH > +} OK. > diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf > index 9e6892970d..a22d138605 100644 > --- a/t/lib-httpd/apache.conf > +++ b/t/lib-httpd/apache.conf > @@ -146,7 +146,9 @@ SetEnv PERL_PATH ${PERL_PATH} > <LocationMatch /custom_auth/> > SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} > SetEnv GIT_HTTP_EXPORT_ALL > + <IfDefine USE_CGIPASSAUTH> > CGIPassAuth on > + </IfDefine> > </LocationMatch> > ScriptAlias /smart/incomplete_length/git-upload-pack incomplete-length-upload-pack-v2-http.sh/ > ScriptAlias /smart/incomplete_body/git-upload-pack incomplete-body-upload-pack-v2-http.sh/ > diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh > index f45a43b4b5..ab8a721ccc 100755 > --- a/t/t5563-simple-http-auth.sh > +++ b/t/t5563-simple-http-auth.sh > @@ -5,6 +5,12 @@ test_description='test http auth header and credential helper interop' > . ./test-lib.sh > . "$TEST_DIRECTORY"/lib-httpd.sh > > +enable_cgipassauth > +if ! test_have_prereq CGIPASSAUTH > +then > + skip_all="no CGIPassAuth support" > + test_done > +fi > start_httpd > > test_expect_success 'setup_credential_helper' ' Yup, very cleanly done. Thanks. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7 2023-05-18 21:28 ` Junio C Hamano @ 2023-05-18 23:10 ` Jeff King 2023-05-18 23:23 ` Junio C Hamano 0 siblings, 1 reply; 11+ messages in thread From: Jeff King @ 2023-05-18 23:10 UTC (permalink / raw) To: Junio C Hamano; +Cc: Todd Zullinger, git, Matthew John Cheetham On Thu, May 18, 2023 at 02:28:22PM -0700, Junio C Hamano wrote: > > Commit 988aad99b4 (t5563: add tests for basic and anoymous HTTP access, > > 2023-02-27) added tests that require Apache to support the CGIPassAuth > > directive, which was added in Apache 2.4.13. This is fairly old (~8 > > years), but recent enough that we still encounter it in the wild (e.g., > > RHEL/CentOS 7, which is not EOL until June 2024). > > nitpick: we are fine to encountering 2.4.13 in the wild---encountering > something a bit older than that is problematic. A quick internet > search tells me that CentOS 7 ships Apache 2.4.6, so if we trust that... > > ... fairly old (~8 years), but recent enough that we still > encounter versions older than that in the wild (e.g. CentOS 7, > which is not EOL until June 2024, comes with Apache 2.4.6 from > 2014 plus security fixes). > > or something? Yeah, I agree what I wrote is a bit unclear. I think what I meant was "..recent enough that we'll still encounter older versions in the wild". But yours is even better, since you dug up the actual version it ships. Do you want to squash that into the commit message, or do you prefer a re-send? > > +enable_cgipassauth () { > > + # We are looking for 2.4.13 or more recent. Since we only support > > + # 2.4 and up, no need to check for older major/minor. > > + if test "$HTTPD_VERSION_MAJOR" = 2 && > > + test "$HTTPD_VERSION_MINOR" = 4 && > > + test "$(echo $HTTPD_VERSION | cut -d. -f3)" -lt 13 > > As HTTPD_VERSION comes from > > $LIB_HTTPD_PATH -v | sed -n 's|^Server version: Apache/\([0-9.]*\).*|p' > > and parses a line like "Server version: Apache/2.4.6 (CentOS)", > unless somebody ships 2.4 without any digit after it, the above > should be safe ;-) Yep. I wondered about trying to be more paranoid here, but I think there's not much point until we see a real world example. The most likely outcome of a mis-parse is that we'd claim "this looks too old" and skip the t5536 tests, which seems OK (at least nobody gets an unexpected test failure, though it may mean that they simply gloss over the problem). -Peff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7 2023-05-18 23:10 ` Jeff King @ 2023-05-18 23:23 ` Junio C Hamano 0 siblings, 0 replies; 11+ messages in thread From: Junio C Hamano @ 2023-05-18 23:23 UTC (permalink / raw) To: Jeff King; +Cc: Todd Zullinger, git, Matthew John Cheetham Jeff King <peff@peff.net> writes: > Yeah, I agree what I wrote is a bit unclear. I think what I meant was > "..recent enough that we'll still encounter older versions in the wild". > > But yours is even better, since you dug up the actual version it ships. > Do you want to squash that into the commit message, or do you prefer a > re-send? Neither. What you wrote was serviceable (and my comment was labeled "nitpick" for that reason) even though it might have been a bit unclear. >> > +enable_cgipassauth () { >> > + # We are looking for 2.4.13 or more recent. Since we only support >> > + # 2.4 and up, no need to check for older major/minor. >> > + if test "$HTTPD_VERSION_MAJOR" = 2 && >> > + test "$HTTPD_VERSION_MINOR" = 4 && >> > + test "$(echo $HTTPD_VERSION | cut -d. -f3)" -lt 13 >> >> As HTTPD_VERSION comes from >> >> $LIB_HTTPD_PATH -v | sed -n 's|^Server version: Apache/\([0-9.]*\).*|p' >> >> and parses a line like "Server version: Apache/2.4.6 (CentOS)", >> unless somebody ships 2.4 without any digit after it, the above >> should be safe ;-) > > Yep. I wondered about trying to be more paranoid here, but I think > there's not much point until we see a real world example. The most > likely outcome of a mis-parse is that we'd claim "this looks too old" > and skip the t5536 tests, which seems OK (at least nobody gets an > unexpected test failure, though it may mean that they simply gloss over > the problem). Yup, this will be in 'next' and will become part of -rc1. Thanks. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-05-18 23:23 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-17 19:06 [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7 Todd Zullinger 2023-05-18 18:45 ` Jeff King 2023-05-18 19:14 ` Todd Zullinger 2023-05-18 19:22 ` Jeff King 2023-05-18 19:21 ` Jeff King 2023-05-18 19:50 ` Todd Zullinger 2023-05-18 20:11 ` Todd Zullinger 2023-05-18 21:31 ` Junio C Hamano 2023-05-18 21:28 ` Junio C Hamano 2023-05-18 23:10 ` Jeff King 2023-05-18 23:23 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).