From: Jeff King <peff@peff.net>
To: Todd Zullinger <tmz@pobox.com>
Cc: git@vger.kernel.org, Matthew John Cheetham <mjcheetham@outlook.com>
Subject: Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7
Date: Thu, 18 May 2023 14:45:32 -0400 [thread overview]
Message-ID: <20230518184532.GC557383@coredump.intra.peff.net> (raw)
In-Reply-To: <ZGUlqu7sP7yxbaTI@pobox.com>
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
next prev parent reply other threads:[~2023-05-18 18:45 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230518184532.GC557383@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=mjcheetham@outlook.com \
--cc=tmz@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).