{"thread":{"id":"59761","subject":"[BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7","startedAt":"2023-05-17T19:06:29Z","lastAt":"2023-05-18T23:23:38Z","messageCount":11,"participants":["Todd Zullinger","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"477474","messageId":"ZGUlqu7sP7yxbaTI@pobox.com","threadId":"59761","inReplyTo":null,"subject":"[BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-05-17T19:06:18Z","receivedAt":"2023-05-17T19:06:29Z","isPatch":false,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"After applying the imap-send.c patch¹ on RHEL/CentOS 7, I\nnoticed the http tests fail because the Apache httpd config\nis not valid with httpd-2.4.6² on CentOS 7.\n\nThe tests fail with:\n\n    Parse errors: No plan found in TAP output\n\nThe problem is that CGIPassAuth, added in 988aad99b4 (t5563:\nadd tests for basic and anoymous HTTP access, 2023-02-27) is\nnot supported by httpd < 2.4.13:\n\n    Starting httpd on port 10410\n    [Wed May 17 17:06:52.184409 2023] [core:warn] [pid 477886] AH00111: Config variable ${LIB_HTTPD_SVN} is not defined\n    [Wed May 17 17:06:52.184495 2023] [core:warn] [pid 477886] AH00111: Config variable ${LIB_HTTPD_SVNPATH} is not defined\n    AH00526: Syntax error on line 149 of /builddir/build/BUILD/git-2.41.0.rc0/t/lib-httpd/apache.conf:\n    Invalid command 'CGIPassAuth', perhaps misspelled or defined by a module not included in the server configuration\n    error: web server setup failed\n\nSince edd060dc84 (t/lib-httpd: bump required apache version\nto 2.4, 2023-02-01), we require httpd-2.4 and no longer have\nany <IfVersion> conditions.  I'm not sure if this a reason\nto add some again (nor am I certain if httpd's IfVersion\nsupports minor versions).\n\nPerhaps there's a more elegant way to fix this?  (I haven't\nthought of anything in patch form yet, apologies.)\n\nI'd like to still build git for CentOS 7 and not skip all\nthe http tests, but if it's time to say it is not worth\nsupporting, I can understand.  RHEL/CentOS 7 has a little\nover a year left before it is EOL³.\n\n¹ <20230517070632.71884-1-list@eworm.de>\n² https://httpd.apache.org/docs/2.4/mod/core.html#cgipassauth\n³ 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\n\nThanks,\n\n-- \nTodd\n"},{"id":"477536","messageId":"20230518184532.GC557383@coredump.intra.peff.net","threadId":"59761","inReplyTo":"ZGUlqu7sP7yxbaTI@pobox.com","subject":"Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-05-18T18:45:32Z","receivedAt":"2023-05-18T18:45:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 17, 2023 at 03:06:18PM -0400, Todd Zullinger wrote:\n\n> The problem is that CGIPassAuth, added in 988aad99b4 (t5563:\n> add tests for basic and anoymous HTTP access, 2023-02-27) is\n> not supported by httpd < 2.4.13:\n\nThanks for reporting. I don't think we dug into version requirements for\nthat (obviously CGI stuff goes back forever, so it's just that\nparticular option). That version of Apache is \"only\" 8 years old. Which\nis old, but we often go back that far for dependencies.\n\n> Since edd060dc84 (t/lib-httpd: bump required apache version\n> to 2.4, 2023-02-01), we require httpd-2.4 and no longer have\n> any <IfVersion> conditions.  I'm not sure if this a reason\n> to add some again (nor am I certain if httpd's IfVersion\n> supports minor versions).\n\nI don't think an IfVersion would be sufficient, because we need to also\ntell the script making use of that config that it should skip its tests.\nSo I think we want something more like the HTTP/2 tests have: a separate\nscript which enables the extra config, and bails if the web server setup\nfails.\n\nSomething like this:\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 6805229dcb..4b6d9a5817 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -191,6 +191,10 @@ enable_http2 () {\n \ttest_set_prereq HTTP2\n }\n \n+enable_cgipassauth () {\n+\tHTTPD_PARA=\"$HTTPD_PARA -DUSE_CGIPASSAUTH\"\n+}\n+\n start_httpd() {\n \tprepare_httpd >&3 2>&4\n \ndiff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf\nindex 9e6892970d..a22d138605 100644\n--- a/t/lib-httpd/apache.conf\n+++ b/t/lib-httpd/apache.conf\n@@ -146,7 +146,9 @@ SetEnv PERL_PATH ${PERL_PATH}\n <LocationMatch /custom_auth/>\n \tSetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}\n \tSetEnv GIT_HTTP_EXPORT_ALL\n+\t<IfDefine USE_CGIPASSAUTH>\n \tCGIPassAuth on\n+\t</IfDefine>\n </LocationMatch>\n ScriptAlias /smart/incomplete_length/git-upload-pack incomplete-length-upload-pack-v2-http.sh/\n ScriptAlias /smart/incomplete_body/git-upload-pack incomplete-body-upload-pack-v2-http.sh/\ndiff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh\nindex f45a43b4b5..0f23f45572 100755\n--- a/t/t5563-simple-http-auth.sh\n+++ b/t/t5563-simple-http-auth.sh\n@@ -5,6 +5,7 @@ test_description='test http auth header and credential helper interop'\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n \n+enable_cgipassauth\n start_httpd\n \n test_expect_success 'setup_credential_helper' '\n\nAnd then the theory is that t5551 works as before (it does not try to\nuse that config), and t5563 will either work out of the box (for recent\nversions), or web server setup will fail, and we'll skip all tests.\n\nBUT. That won't work if you have set GIT_TEST_HTTPD=1, rather than the\ndefault of \"auto\". Because then it is instructed to complain about\nwebserver setup failing, so t5563 will fail rather than skip. So we have\na few options there:\n\n  1. You use the looser value of GIT_TEST_HTTPD for CentOS tests, which\n     would do the right thing. The downside is that if server setup\n     failed for other reasons, we wouldn't notice and would silently\n     skip the HTTP tests.\n\n  2. We do some kind of version check in enable_cgipassauth(),\n     and skip tests manually if it doesn't pass.\n\n  3. You just skip the test manually on that platform with\n     GIT_SKIP_TESTS=t5563.\n\nObviously (1) and (3) are the least work for us upstream, but I don't\nthink (2) would be too hard to do.\n\n-Peff\n"},{"id":"477537","messageId":"ZGZ5KsbkwoT68J1S@pobox.com","threadId":"59761","inReplyTo":"20230518184532.GC557383@coredump.intra.peff.net","subject":"Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-05-18T19:14:50Z","receivedAt":"2023-05-18T19:14:56Z","isPatch":false,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Jeff King wrote:\n> On Wed, May 17, 2023 at 03:06:18PM -0400, Todd Zullinger wrote:\n> \n>> The problem is that CGIPassAuth, added in 988aad99b4 (t5563:\n>> add tests for basic and anoymous HTTP access, 2023-02-27) is\n>> not supported by httpd < 2.4.13:\n> \n> Thanks for reporting. I don't think we dug into version requirements for\n> that (obviously CGI stuff goes back forever, so it's just that\n> particular option). That version of Apache is \"only\" 8 years old. Which\n> is old, but we often go back that far for dependencies.\n\nIndeed, thanks.  I don't imagine there are too many people\nlooking to support the latest git auth mechanisms via http\nwho are also running CentOS 7.  But it's nice to not have to\ndisable all the other http tests.\n\n>> Since edd060dc84 (t/lib-httpd: bump required apache version\n>> to 2.4, 2023-02-01), we require httpd-2.4 and no longer have\n>> any <IfVersion> conditions.  I'm not sure if this a reason\n>> to add some again (nor am I certain if httpd's IfVersion\n>> supports minor versions).\n> \n> I don't think an IfVersion would be sufficient, because we need to also\n> tell the script making use of that config that it should skip its tests.\n> So I think we want something more like the HTTP/2 tests have: a separate\n> script which enables the extra config, and bails if the web server setup\n> fails.\n> \n> Something like this:\n\n[...]\n\nI'll try to apply this and see how it goes, skipping t5563,\na little later in the day.\n\n> And then the theory is that t5551 works as before (it does not try to\n> use that config), and t5563 will either work out of the box (for recent\n> versions), or web server setup will fail, and we'll skip all tests.\n> \n> BUT. That won't work if you have set GIT_TEST_HTTPD=1, rather than the\n> default of \"auto\". Because then it is instructed to complain about\n> webserver setup failing, so t5563 will fail rather than skip. So we have\n> a few options there:\n> \n>   1. You use the looser value of GIT_TEST_HTTPD for CentOS tests, which\n>      would do the right thing. The downside is that if server setup\n>      failed for other reasons, we wouldn't notice and would silently\n>      skip the HTTP tests.\n> \n>   2. We do some kind of version check in enable_cgipassauth(),\n>      and skip tests manually if it doesn't pass.\n> \n>   3. You just skip the test manually on that platform with\n>      GIT_SKIP_TESTS=t5563.\n> \n> Obviously (1) and (3) are the least work for us upstream, but I don't\n> think (2) would be too hard to do.\n\nYeah, the path of least effort seems ideal here.  If the\ntests are split I can easily skip them for CentOS 7.  For\nsomething that's going away in little over a year, it's good\nto put at least some of the onus on folks packaging for it.\nAnd that's less cruft in the test suite after CentOS 7 is\ngone (or any other ancient httpd's git may wish to support).\n\nThanks Peff!  I'm really glad you know your way around the\ntest suite so well.  It'd take me far longer to figure out a\ngood plan for something like this.\n\n-- \nTodd\n"},{"id":"477538","messageId":"20230518192102.GA1514485@coredump.intra.peff.net","threadId":"59761","inReplyTo":"20230518184532.GC557383@coredump.intra.peff.net","subject":"Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-05-18T19:21:02Z","receivedAt":"2023-05-18T19:21:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 18, 2023 at 02:45:32PM -0400, Jeff King wrote:\n\n>   2. We do some kind of version check in enable_cgipassauth(),\n>      and skip tests manually if it doesn't pass.\n> [...]\n> Obviously (1) and (3) are the least work for us upstream, but I don't\n> think (2) would be too hard to do.\n\nIt indeed was pretty easy. So here's a patch. I'm adding Junio to the cc\nbefore any review, as this is a change in the v2.41 cycle. So I think\nwe'd want to address this before the release.\n\n-- >8 --\nSubject: [PATCH] t/lib-httpd: make CGIPassAuth support conditional\n\nCommit 988aad99b4 (t5563: add tests for basic and anoymous HTTP access,\n2023-02-27) added tests that require Apache to support the CGIPassAuth\ndirective, which was added in Apache 2.4.13. This is fairly old (~8\nyears), but recent enough that we still encounter it in the wild (e.g.,\nRHEL/CentOS 7, which is not EOL until June 2024).\n\nWe can live with skipping the new tests on such a platform. But\nunfortunately, since the directive is used unconditionally in our\napache.conf, it means the web server fails to start entirely, and we\ncannot run other HTTP tests at all (e.g., the basic ones in t5551).\n\nWe can fix that by making the config conditional, and only triggering it\nfor t5563. That solves the problem for t5551 (which then ignores the\ndirective entirely). For t5563, we'd see apache complain in start_httpd;\nwith the default setting of GIT_TEST_HTTPD, we'd then skip the whole\nscript.\n\nBut that leaves one small problem: people may set GIT_TEST_HTTPD=1\nexplicitly, which instructs the tests to fail (rather than skip) when we\ncan't start the webserver (to avoid accidentally missing some tests).\n\nThis could be worked around by having the user manually set\nGIT_SKIP_TESTS on a platform with an older Apache. But we can be a bit\nfriendlier by doing the version check ourselves and setting an\nappropriate prereq. We'll use the (lack of) prereq to then skip the rest\nof t5563. In theory we could use the prereq to skip individual tests, but\nin practice this whole script depends on it.\n\nReported-by: Todd Zullinger <tmz@pobox.com>\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI tested this manually by mutilating the config directive to \"FooBar\",\nwhich would fail even on recent versions. And then tweaking the \"13\"\nin the version check up to \"60\" to make sure it properly skips even with\nrecent Apache. But testing on real CentOS 7 would be very much\nappreciated.\n\n t/lib-httpd.sh              | 14 ++++++++++++++\n t/lib-httpd/apache.conf     |  2 ++\n t/t5563-simple-http-auth.sh |  6 ++++++\n 3 files changed, 22 insertions(+)\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 6805229dcb..2fb1b2ae56 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -191,6 +191,20 @@ enable_http2 () {\n \ttest_set_prereq HTTP2\n }\n \n+enable_cgipassauth () {\n+\t# We are looking for 2.4.13 or more recent. Since we only support\n+\t# 2.4 and up, no need to check for older major/minor.\n+\tif test \"$HTTPD_VERSION_MAJOR\" = 2 &&\n+\t   test \"$HTTPD_VERSION_MINOR\" = 4 &&\n+\t   test \"$(echo $HTTPD_VERSION | cut -d. -f3)\" -lt 13\n+\tthen\n+\t\techo >&4 \"apache $HTTPD_VERSION too old for CGIPassAuth\"\n+\t\treturn\n+\tfi\n+\tHTTPD_PARA=\"$HTTPD_PARA -DUSE_CGIPASSAUTH\"\n+\ttest_set_prereq CGIPASSAUTH\n+}\n+\n start_httpd() {\n \tprepare_httpd >&3 2>&4\n \ndiff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf\nindex 9e6892970d..a22d138605 100644\n--- a/t/lib-httpd/apache.conf\n+++ b/t/lib-httpd/apache.conf\n@@ -146,7 +146,9 @@ SetEnv PERL_PATH ${PERL_PATH}\n <LocationMatch /custom_auth/>\n \tSetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}\n \tSetEnv GIT_HTTP_EXPORT_ALL\n+\t<IfDefine USE_CGIPASSAUTH>\n \tCGIPassAuth on\n+\t</IfDefine>\n </LocationMatch>\n ScriptAlias /smart/incomplete_length/git-upload-pack incomplete-length-upload-pack-v2-http.sh/\n ScriptAlias /smart/incomplete_body/git-upload-pack incomplete-body-upload-pack-v2-http.sh/\ndiff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh\nindex f45a43b4b5..ab8a721ccc 100755\n--- a/t/t5563-simple-http-auth.sh\n+++ b/t/t5563-simple-http-auth.sh\n@@ -5,6 +5,12 @@ test_description='test http auth header and credential helper interop'\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n \n+enable_cgipassauth\n+if ! test_have_prereq CGIPASSAUTH\n+then\n+\tskip_all=\"no CGIPassAuth support\"\n+\ttest_done\n+fi\n start_httpd\n \n test_expect_success 'setup_credential_helper' '\n-- \n2.41.0.rc0.359.g22664e20e7\n\n"},{"id":"477539","messageId":"20230518192248.GB1514485@coredump.intra.peff.net","threadId":"59761","inReplyTo":"ZGZ5KsbkwoT68J1S@pobox.com","subject":"Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-05-18T19:22:48Z","receivedAt":"2023-05-18T19:22:51Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 18, 2023 at 03:14:50PM -0400, Todd Zullinger wrote:\n\n> >   1. You use the looser value of GIT_TEST_HTTPD for CentOS tests, which\n> >      would do the right thing. The downside is that if server setup\n> >      failed for other reasons, we wouldn't notice and would silently\n> >      skip the HTTP tests.\n> > \n> >   2. We do some kind of version check in enable_cgipassauth(),\n> >      and skip tests manually if it doesn't pass.\n> > \n> >   3. You just skip the test manually on that platform with\n> >      GIT_SKIP_TESTS=t5563.\n> > \n> > Obviously (1) and (3) are the least work for us upstream, but I don't\n> > think (2) would be too hard to do.\n> \n> Yeah, the path of least effort seems ideal here.  If the\n> tests are split I can easily skip them for CentOS 7.  For\n> something that's going away in little over a year, it's good\n> to put at least some of the onus on folks packaging for it.\n> And that's less cruft in the test suite after CentOS 7 is\n> gone (or any other ancient httpd's git may wish to support).\n\nToo late. :) I just sent a patch with (2), as it was only a few extra\nlines.\n\n> Thanks Peff!  I'm really glad you know your way around the\n> test suite so well.  It'd take me far longer to figure out a\n> good plan for something like this.\n\nI was able to cheat a little, having had to figure out a similar\nsolution for HTTP/2 not too long ago. :)\n\n-Peff\n"},{"id":"477540","messageId":"ZGaBjLsJUiNEaLu2@pobox.com","threadId":"59761","inReplyTo":"20230518192102.GA1514485@coredump.intra.peff.net","subject":"Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-05-18T19:50:36Z","receivedAt":"2023-05-18T19:50:53Z","isPatch":false,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Jeff King wrote:\n> On Thu, May 18, 2023 at 02:45:32PM -0400, Jeff King wrote:\n> \n>>   2. We do some kind of version check in enable_cgipassauth(),\n>>      and skip tests manually if it doesn't pass.\n>> [...]\n>> Obviously (1) and (3) are the least work for us upstream, but I don't\n>> think (2) would be too hard to do.\n> \n> It indeed was pretty easy. So here's a patch. I'm adding Junio to the cc\n> before any review, as this is a change in the v2.41 cycle. So I think\n> we'd want to address this before the release.\n\nNice.\n\n> I tested this manually by mutilating the config directive to \"FooBar\",\n> which would fail even on recent versions. And then tweaking the \"13\"\n> in the version check up to \"60\" to make sure it properly skips even with\n> recent Apache. But testing on real CentOS 7 would be very much\n> appreciated.\n\nI have a build running now.  It takes around ~20 minutes in\nthe Fedora/EPEL build system.  I wanted to make sure it was\nstarted quickly since you turned out a patch in the time it\ntook me to write my last reply. ;)\n\n(It would have been quicker if I hadn't forgot to apply the\nimap-send patch as well.  D'oh!)\n\n-- \nTodd\n"},{"id":"477562","messageId":"ZGaGZACGNLeAsZRp@pobox.com","threadId":"59761","inReplyTo":"20230518192102.GA1514485@coredump.intra.peff.net","subject":"Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-05-18T20:11:16Z","receivedAt":"2023-05-18T20:11:28Z","isPatch":false,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Jeff King wrote:\n> I tested this manually by mutilating the config directive to \"FooBar\",\n> which would fail even on recent versions. And then tweaking the \"13\"\n> in the version check up to \"60\" to make sure it properly skips even with\n> recent Apache. But testing on real CentOS 7 would be very much\n> appreciated.\n\nI tested on RHEL/CentOS 7 and it does what it says on the tin:\n\nt5563-simple-http-auth.sh .......................... skipped: no CGIPassAuth support\n\nThanks!\n\n-- \nTodd\n"},{"id":"477567","messageId":"xmqqpm6xs51l.fsf@gitster.g","threadId":"59761","inReplyTo":"20230518192102.GA1514485@coredump.intra.peff.net","subject":"Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-18T21:28:22Z","receivedAt":"2023-05-18T21:29:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, May 18, 2023 at 02:45:32PM -0400, Jeff King wrote:\n>\n>>   2. We do some kind of version check in enable_cgipassauth(),\n>>      and skip tests manually if it doesn't pass.\n>> [...]\n>> Obviously (1) and (3) are the least work for us upstream, but I don't\n>> think (2) would be too hard to do.\n>\n> It indeed was pretty easy. So here's a patch. I'm adding Junio to the cc\n> before any review, as this is a change in the v2.41 cycle. So I think\n> we'd want to address this before the release.\n\nThanks for being considerate, as always ;-)  Very much appreciated.\n\n> -- >8 --\n> Subject: [PATCH] t/lib-httpd: make CGIPassAuth support conditional\n>\n> Commit 988aad99b4 (t5563: add tests for basic and anoymous HTTP access,\n> 2023-02-27) added tests that require Apache to support the CGIPassAuth\n> directive, which was added in Apache 2.4.13. This is fairly old (~8\n> years), but recent enough that we still encounter it in the wild (e.g.,\n> RHEL/CentOS 7, which is not EOL until June 2024).\n\nnitpick: we are fine to encountering 2.4.13 in the wild---encountering\nsomething a bit older than that is problematic.  A quick internet\nsearch tells me that CentOS 7 ships Apache 2.4.6, so if we trust that...\n\n    ... fairly old (~8 years), but recent enough that we still\n    encounter versions older than that in the wild (e.g.  CentOS 7,\n    which is not EOL until June 2024, comes with Apache 2.4.6 from\n    2014 plus security fixes).\n\nor something?\n\n> I tested this manually by mutilating the config directive to \"FooBar\",\n> which would fail even on recent versions. And then tweaking the \"13\"\n> in the version check up to \"60\" to make sure it properly skips even with\n> recent Apache. But testing on real CentOS 7 would be very much\n> appreciated.\n\nIndeed, we'd love to see tests on a real thing, especially if we\nwere to fast-track this.  FWIW, the manual test you outlined above\nsounds like a very sensible way to \"emulate\" under the condition\nthat there is no access to the real thing.\n\n> +enable_cgipassauth () {\n> +\t# We are looking for 2.4.13 or more recent. Since we only support\n> +\t# 2.4 and up, no need to check for older major/minor.\n> +\tif test \"$HTTPD_VERSION_MAJOR\" = 2 &&\n> +\t   test \"$HTTPD_VERSION_MINOR\" = 4 &&\n> +\t   test \"$(echo $HTTPD_VERSION | cut -d. -f3)\" -lt 13\n\nAs HTTPD_VERSION comes from \n\n\t$LIB_HTTPD_PATH -v | sed -n 's|^Server version: Apache/\\([0-9.]*\\).*|p'\n\nand parses a line like \"Server version: Apache/2.4.6 (CentOS)\",\nunless somebody ships 2.4 without any digit after it, the above\nshould be safe ;-)\n\n> +\tthen\n> +\t\techo >&4 \"apache $HTTPD_VERSION too old for CGIPassAuth\"\n> +\t\treturn\n> +\tfi\n> +\tHTTPD_PARA=\"$HTTPD_PARA -DUSE_CGIPASSAUTH\"\n> +\ttest_set_prereq CGIPASSAUTH\n> +}\n\nOK.\n\n> diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf\n> index 9e6892970d..a22d138605 100644\n> --- a/t/lib-httpd/apache.conf\n> +++ b/t/lib-httpd/apache.conf\n> @@ -146,7 +146,9 @@ SetEnv PERL_PATH ${PERL_PATH}\n>  <LocationMatch /custom_auth/>\n>  \tSetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}\n>  \tSetEnv GIT_HTTP_EXPORT_ALL\n> +\t<IfDefine USE_CGIPASSAUTH>\n>  \tCGIPassAuth on\n> +\t</IfDefine>\n>  </LocationMatch>\n>  ScriptAlias /smart/incomplete_length/git-upload-pack incomplete-length-upload-pack-v2-http.sh/\n>  ScriptAlias /smart/incomplete_body/git-upload-pack incomplete-body-upload-pack-v2-http.sh/\n> diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh\n> index f45a43b4b5..ab8a721ccc 100755\n> --- a/t/t5563-simple-http-auth.sh\n> +++ b/t/t5563-simple-http-auth.sh\n> @@ -5,6 +5,12 @@ test_description='test http auth header and credential helper interop'\n>  . ./test-lib.sh\n>  . \"$TEST_DIRECTORY\"/lib-httpd.sh\n>  \n> +enable_cgipassauth\n> +if ! test_have_prereq CGIPASSAUTH\n> +then\n> +\tskip_all=\"no CGIPassAuth support\"\n> +\ttest_done\n> +fi\n>  start_httpd\n>  \n>  test_expect_success 'setup_credential_helper' '\n\nYup, very cleanly done.\n\nThanks.\n"},{"id":"477568","messageId":"xmqqlehls4x2.fsf@gitster.g","threadId":"59761","inReplyTo":"ZGaGZACGNLeAsZRp@pobox.com","subject":"Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-18T21:31:05Z","receivedAt":"2023-05-18T21:32:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger <tmz@pobox.com> writes:\n\n> Jeff King wrote:\n>> I tested this manually by mutilating the config directive to \"FooBar\",\n>> which would fail even on recent versions. And then tweaking the \"13\"\n>> in the version check up to \"60\" to make sure it properly skips even with\n>> recent Apache. But testing on real CentOS 7 would be very much\n>> appreciated.\n>\n> I tested on RHEL/CentOS 7 and it does what it says on the tin:\n>\n> t5563-simple-http-auth.sh .......................... skipped: no CGIPassAuth support\n>\n> Thanks!\n\nNice.  Thanks, both.\n"},{"id":"477570","messageId":"20230518231056.GA1752284@coredump.intra.peff.net","threadId":"59761","inReplyTo":"xmqqpm6xs51l.fsf@gitster.g","subject":"Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-05-18T23:10:56Z","receivedAt":"2023-05-18T23:11:01Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 18, 2023 at 02:28:22PM -0700, Junio C Hamano wrote:\n\n> > Commit 988aad99b4 (t5563: add tests for basic and anoymous HTTP access,\n> > 2023-02-27) added tests that require Apache to support the CGIPassAuth\n> > directive, which was added in Apache 2.4.13. This is fairly old (~8\n> > years), but recent enough that we still encounter it in the wild (e.g.,\n> > RHEL/CentOS 7, which is not EOL until June 2024).\n> \n> nitpick: we are fine to encountering 2.4.13 in the wild---encountering\n> something a bit older than that is problematic.  A quick internet\n> search tells me that CentOS 7 ships Apache 2.4.6, so if we trust that...\n> \n>     ... fairly old (~8 years), but recent enough that we still\n>     encounter versions older than that in the wild (e.g.  CentOS 7,\n>     which is not EOL until June 2024, comes with Apache 2.4.6 from\n>     2014 plus security fixes).\n> \n> or something?\n\nYeah, I agree what I wrote is a bit unclear. I think what I meant was\n\"..recent enough that we'll still encounter older versions in the wild\".\n\nBut yours is even better, since you dug up the actual version it ships.\nDo you want to squash that into the commit message, or do you prefer a\nre-send?\n\n> > +enable_cgipassauth () {\n> > +\t# We are looking for 2.4.13 or more recent. Since we only support\n> > +\t# 2.4 and up, no need to check for older major/minor.\n> > +\tif test \"$HTTPD_VERSION_MAJOR\" = 2 &&\n> > +\t   test \"$HTTPD_VERSION_MINOR\" = 4 &&\n> > +\t   test \"$(echo $HTTPD_VERSION | cut -d. -f3)\" -lt 13\n> \n> As HTTPD_VERSION comes from \n> \n> \t$LIB_HTTPD_PATH -v | sed -n 's|^Server version: Apache/\\([0-9.]*\\).*|p'\n> \n> and parses a line like \"Server version: Apache/2.4.6 (CentOS)\",\n> unless somebody ships 2.4 without any digit after it, the above\n> should be safe ;-)\n\nYep. I wondered about trying to be more paranoid here, but I think\nthere's not much point until we see a real world example. The most\nlikely outcome of a mis-parse is that we'd claim \"this looks too old\"\nand skip the t5536 tests, which seems OK (at least nobody gets an\nunexpected test failure, though it may mean that they simply gloss over\nthe problem).\n\n-Peff\n"},{"id":"477571","messageId":"xmqqy1llql56.fsf@gitster.g","threadId":"59761","inReplyTo":"20230518231056.GA1752284@coredump.intra.peff.net","subject":"Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-18T23:23:33Z","receivedAt":"2023-05-18T23:23:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Yeah, I agree what I wrote is a bit unclear. I think what I meant was\n> \"..recent enough that we'll still encounter older versions in the wild\".\n>\n> But yours is even better, since you dug up the actual version it ships.\n> Do you want to squash that into the commit message, or do you prefer a\n> re-send?\n\nNeither.  What you wrote was serviceable (and my comment was labeled\n\"nitpick\" for that reason) even though it might have been a bit\nunclear.\n\n>> > +enable_cgipassauth () {\n>> > +\t# We are looking for 2.4.13 or more recent. Since we only support\n>> > +\t# 2.4 and up, no need to check for older major/minor.\n>> > +\tif test \"$HTTPD_VERSION_MAJOR\" = 2 &&\n>> > +\t   test \"$HTTPD_VERSION_MINOR\" = 4 &&\n>> > +\t   test \"$(echo $HTTPD_VERSION | cut -d. -f3)\" -lt 13\n>> \n>> As HTTPD_VERSION comes from \n>> \n>> \t$LIB_HTTPD_PATH -v | sed -n 's|^Server version: Apache/\\([0-9.]*\\).*|p'\n>> \n>> and parses a line like \"Server version: Apache/2.4.6 (CentOS)\",\n>> unless somebody ships 2.4 without any digit after it, the above\n>> should be safe ;-)\n>\n> Yep. I wondered about trying to be more paranoid here, but I think\n> there's not much point until we see a real world example. The most\n> likely outcome of a mis-parse is that we'd claim \"this looks too old\"\n> and skip the t5536 tests, which seems OK (at least nobody gets an\n> unexpected test failure, though it may mean that they simply gloss over\n> the problem).\n\nYup, this will be in 'next' and will become part of -rc1.\n\nThanks.\n"}]}