{"thread":{"id":"47516","subject":"[PATCH] http: fix v1 protocol tests with apache httpd < 2.4","startedAt":"2017-12-31T02:32:43Z","lastAt":"2018-01-03T16:21:55Z","messageCount":6,"participants":["Todd Zullinger","Brandon Williams","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"335568","messageId":"20171231023234.21215-1-tmz@pobox.com","threadId":"47516","inReplyTo":null,"subject":"[PATCH] http: fix v1 protocol tests with apache httpd < 2.4","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2017-12-31T02:32:34Z","receivedAt":"2017-12-31T02:32:43Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"The apache config used by tests was updated to use the SetEnvIf\ndirective to set the Git-Protocol header in 19113a26b6 (\"http: tell\nserver that the client understands v1\", 2017-10-16).\n\nSetting the Git-Protocol header is restricted to httpd >= 2.4, but\nmod_setenvif and the SetEnvIf directive work with lower versions, at\nleast as far back as 2.0, according to the httpd documentation:\n\n https://httpd.apache.org/docs/2.0/mod/mod_setenvif.html\n\nDrop the restriction. Tested with httpd 2.2 and 2.4.\n\nSigned-off-by: Todd Zullinger \n---\n\nThese tests fail with 2.16.0-rc0 on CentOS-6, which uses\nhttpd-2.2.\n\nI removed the version restriction entirely rather than adjust\nthe version. I believe SetEnvIf works on httpd >= 2.0. I'm\nnot sure if we aim to support anything less than httpd 2.0,\nbut I'm betting not. If that's incorrect, I can add some\nIfVersion conditions.\n\nAs noted in the commit message, I only tested with httpd 2.2\nand 2.4 (on CentOS 6/7 and Fedora 26-28). If anyone has older\nhttpd systems which need support, it would be great if they\ncould test this before 2.16.0 is finalized, so we can avoid\nshipping with any failing tests.\n\n t/lib-httpd/apache.conf | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf\nindex df19436314..724d9ae462 100644\n--- a/t/lib-httpd/apache.conf\n+++ b/t/lib-httpd/apache.conf\n@@ -25,6 +25,9 @@ ErrorLog error.log\n \n \tLoadModule headers_module modules/mod_headers.so\n \n+\n+\tLoadModule setenvif_module modules/mod_setenvif.so\n+\n \n \n LockFile accept.lock\n@@ -67,9 +70,6 @@ LockFile accept.lock\n \n \tLoadModule unixd_module modules/mod_unixd.so\n \n-\n-\tLoadModule setenvif_module modules/mod_setenvif.so\n-\n \n \n PassEnv GIT_VALGRIND\n@@ -79,9 +79,7 @@ PassEnv ASAN_OPTIONS\n PassEnv GIT_TRACE\n PassEnv GIT_CONFIG_NOSYSTEM\n \n-= 2.4>\n-\tSetEnvIf Git-Protocol \".*\" GIT_PROTOCOL=$0\n-\n+SetEnvIf Git-Protocol \".*\" GIT_PROTOCOL=$0\n \n Alias /dumb/ www/\n Alias /auth/dumb/ www/auth/dumb/\n-- \n2.16.0.rc0\n\n"},{"id":"335673","messageId":"20180103002145.GA242551@google.com","threadId":"47516","inReplyTo":"20171231023234.21215-1-tmz@pobox.com","subject":"Re: [PATCH] http: fix v1 protocol tests with apache httpd < 2.4","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2018-01-03T00:21:45Z","receivedAt":"2018-01-03T00:21:53Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 12/30, Todd Zullinger wrote:\n> The apache config used by tests was updated to use the SetEnvIf\n> directive to set the Git-Protocol header in 19113a26b6 (\"http: tell\n> server that the client understands v1\", 2017-10-16).\n> \n> Setting the Git-Protocol header is restricted to httpd >= 2.4, but\n> mod_setenvif and the SetEnvIf directive work with lower versions, at\n> least as far back as 2.0, according to the httpd documentation:\n> \n> https://httpd.apache.org/docs/2.0/mod/mod_setenvif.html\n> \n> Drop the restriction. Tested with httpd 2.2 and 2.4.\n> \n> Signed-off-by: Todd Zullinger \n> ---\n> \n> These tests fail with 2.16.0-rc0 on CentOS-6, which uses\n> httpd-2.2.\n> \n> I removed the version restriction entirely rather than adjust\n> the version. I believe SetEnvIf works on httpd >= 2.0. I'm\n> not sure if we aim to support anything less than httpd 2.0,\n> but I'm betting not. If that's incorrect, I can add some\n> IfVersion conditions.\n> \n> As noted in the commit message, I only tested with httpd 2.2\n> and 2.4 (on CentOS 6/7 and Fedora 26-28). If anyone has older\n> httpd systems which need support, it would be great if they\n> could test this before 2.16.0 is finalized, so we can avoid\n> shipping with any failing tests.\n\nSeems good to me. I think I was just being overly cautious when i was\nimplementing those patches. Thanks!\n\n> \n> t/lib-httpd/apache.conf | 10 ++++------\n> 1 file changed, 4 insertions(+), 6 deletions(-)\n> \n> diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf\n> index df19436314..724d9ae462 100644\n> --- a/t/lib-httpd/apache.conf\n> +++ b/t/lib-httpd/apache.conf\n> @@ -25,6 +25,9 @@ ErrorLog error.log\n> \n> \tLoadModule headers_module modules/mod_headers.so\n> \n> +\n> +\tLoadModule setenvif_module modules/mod_setenvif.so\n> +\n> \n> \n> LockFile accept.lock\n> @@ -67,9 +70,6 @@ LockFile accept.lock\n> \n> \tLoadModule unixd_module modules/mod_unixd.so\n> \n> -\n> -\tLoadModule setenvif_module modules/mod_setenvif.so\n> -\n> \n> \n> PassEnv GIT_VALGRIND\n> @@ -79,9 +79,7 @@ PassEnv ASAN_OPTIONS\n> PassEnv GIT_TRACE\n> PassEnv GIT_CONFIG_NOSYSTEM\n> \n> -= 2.4>\n> -\tSetEnvIf Git-Protocol \".*\" GIT_PROTOCOL=$0\n> -\n> +SetEnvIf Git-Protocol \".*\" GIT_PROTOCOL=$0\n> \n> Alias /dumb/ www/\n> Alias /auth/dumb/ www/auth/dumb/\n> -- \n> 2.16.0.rc0\n> \n\n-- \nBrandon Williams\n"},{"id":"335674","messageId":"20180103003946.GC12108@zaya.teonanacatl.net","threadId":"47516","inReplyTo":"20180103002145.GA242551@google.com","subject":"Re: [PATCH] http: fix v1 protocol tests with apache httpd < 2.4","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-01-03T00:39:46Z","receivedAt":"2018-01-03T00:39:54Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Brandon Williams wrote:\n> Seems good to me. I think I was just being overly cautious when i was\n> implementing those patches. Thanks!\n\nCool, thanks for taking a look.\n\nIn this case, the test isn't dependent on apache > 2.4, but\nbefore I checked that I wondered if we had any way to run a\ntest only if the apache version was greater or lesser than\nsome release. Luckily, I didn't have to work out such a\nmethod. :)\n\nI don't know if there's a clean way to do that\nautomatically, short of parsing the output of 'httpd -v'\nshould we ever need to add such a prereq.\n\n-- \nTodd\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nSometimes you get the blues because your baby leaves you. Sometimes\nyou get'em 'cause she comes back.\n -- B.B. King\n\n"},{"id":"335704","messageId":"20180103055646.GA25752@sigill.intra.peff.net","threadId":"47516","inReplyTo":"20171231023234.21215-1-tmz@pobox.com","subject":"Re: [PATCH] http: fix v1 protocol tests with apache httpd < 2.4","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-03T05:56:47Z","receivedAt":"2018-01-03T05:56:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Dec 30, 2017 at 09:32:34PM -0500, Todd Zullinger wrote:\n\n> The apache config used by tests was updated to use the SetEnvIf\n> directive to set the Git-Protocol header in 19113a26b6 (\"http: tell\n> server that the client understands v1\", 2017-10-16).\n> \n> Setting the Git-Protocol header is restricted to httpd >= 2.4, but\n> mod_setenvif and the SetEnvIf directive work with lower versions, at\n> least as far back as 2.0, according to the httpd documentation:\n> \n> https://httpd.apache.org/docs/2.0/mod/mod_setenvif.html\n> \n> Drop the restriction. Tested with httpd 2.2 and 2.4.\n\nMakes sense. I think the only way this could backfire is if somebody has\na funny build that doesn't include mod_setenvif at all. But I don't\nthink we can know that for sure without applying this and seeing if\nanybody screams.\n\n> I removed the version restriction entirely rather than adjust\n> the version. I believe SetEnvIf works on httpd >= 2.0. I'm\n> not sure if we aim to support anything less than httpd 2.0,\n> but I'm betting not. If that's incorrect, I can add some\n> IfVersion conditions.\n\nIIRC, anything less than 2.0 is broken already. And it's not worth\nchanging that, given the age (and I think there were some pretty severe\nhardships in making 1.3.x, but it's been a while).\n\n-Peff\n"},{"id":"335705","messageId":"20180103061703.GB25752@sigill.intra.peff.net","threadId":"47516","inReplyTo":"20180103003946.GC12108@zaya.teonanacatl.net","subject":"Re: [PATCH] http: fix v1 protocol tests with apache httpd < 2.4","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-03T06:17:04Z","receivedAt":"2018-01-03T06:17:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 02, 2018 at 07:39:46PM -0500, Todd Zullinger wrote:\n\n> Brandon Williams wrote:\n> > Seems good to me. I think I was just being overly cautious when i was\n> > implementing those patches. Thanks!\n> \n> Cool, thanks for taking a look.\n> \n> In this case, the test isn't dependent on apache > 2.4, but\n> before I checked that I wondered if we had any way to run a\n> test only if the apache version was greater or lesser than\n> some release. Luckily, I didn't have to work out such a\n> method. :)\n> \n> I don't know if there's a clean way to do that\n> automatically, short of parsing the output of 'httpd -v'\n> should we ever need to add such a prereq.\n\nIn the general case, we could probably define an endpoint within an \nblock, and then try to access the endpoint from the test script.\n\nE.g., something like:\n\n = 2.4>\n Alias /have-2.4.txt www/yes.txt\n \n\nin the apache config, and then:\n\n test_lazy_prereq APACHE24 '\n echo yes >\"$HTTPD_DOCUMENT_ROOT_PATH/yes.txt\" &&\n curl -f \"$HTTPD_URL/have-2.4.txt\"\n '\n\nin the test script (of course we may not want to depend on having\ncommand-line curl, but we could replace that with \"git ls-remote\" or\nsimilar).\n\nOne nice thing about that approach is that it can be extended to other\n\"If\" blocks, like if we have a particular module available, or if ssl is\nconfigured.\n\n-Peff\n"},{"id":"335720","messageId":"20180103162146.GJ12108@zaya.teonanacatl.net","threadId":"47516","inReplyTo":"20180103061703.GB25752@sigill.intra.peff.net","subject":"Re: [PATCH] http: fix v1 protocol tests with apache httpd < 2.4","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-01-03T16:21:46Z","receivedAt":"2018-01-03T16:21:55Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Jeff King wrote:\n> On Tue, Jan 02, 2018 at 07:39:46PM -0500, Todd Zullinger wrote:\n>> I don't know if there's a clean way to do that\n>> automatically, short of parsing the output of 'httpd -v'\n>> should we ever need to add such a prereq.\n> \n> In the general case, we could probably define an endpoint within an \n> block, and then try to access the endpoint from the test script.\n> \n> E.g., something like:\n> \n> = 2.4>\n> Alias /have-2.4.txt www/yes.txt\n> \n> \n> in the apache config, and then:\n> \n> test_lazy_prereq APACHE24 '\n> echo yes >\"$HTTPD_DOCUMENT_ROOT_PATH/yes.txt\" &&\n> curl -f \"$HTTPD_URL/have-2.4.txt\"\n> '\n> \n> in the test script (of course we may not want to depend on having\n> command-line curl, but we could replace that with \"git ls-remote\" or\n> similar).\n> \n> One nice thing about that approach is that it can be extended to other\n> \"If\" blocks, like if we have a particular module available, or if ssl is\n> configured.\n\nThat's quite elegant. I even modified an IfVersion block\nand didn't think about using it that way to create a prereq.\nNeat!\n\n-- \nTodd\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nYou're not drunk if you can lie on the floor without holding on.\n -- Dean Martin\n\n"}]}