* [PATCH] http: fix v1 protocol tests with apache httpd < 2.4
@ 2017-12-31 2:32 Todd Zullinger
2018-01-03 0:21 ` Brandon Williams
2018-01-03 5:56 ` Jeff King
0 siblings, 2 replies; 6+ messages in thread
From: Todd Zullinger @ 2017-12-31 2:32 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, Jeff King, Junio C Hamano
The apache config used by tests was updated to use the SetEnvIf
directive to set the Git-Protocol header in 19113a26b6 ("http: tell
server that the client understands v1", 2017-10-16).
Setting the Git-Protocol header is restricted to httpd >= 2.4, but
mod_setenvif and the SetEnvIf directive work with lower versions, at
least as far back as 2.0, according to the httpd documentation:
https://httpd.apache.org/docs/2.0/mod/mod_setenvif.html
Drop the restriction. Tested with httpd 2.2 and 2.4.
Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
These tests fail with 2.16.0-rc0 on CentOS-6, which uses
httpd-2.2.
I removed the version restriction entirely rather than adjust
the version. I believe SetEnvIf works on httpd >= 2.0. I'm
not sure if we aim to support anything less than httpd 2.0,
but I'm betting not. If that's incorrect, I can add some
IfVersion conditions.
As noted in the commit message, I only tested with httpd 2.2
and 2.4 (on CentOS 6/7 and Fedora 26-28). If anyone has older
httpd systems which need support, it would be great if they
could test this before 2.16.0 is finalized, so we can avoid
shipping with any failing tests.
t/lib-httpd/apache.conf | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index df19436314..724d9ae462 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -25,6 +25,9 @@ ErrorLog error.log
<IfModule !mod_headers.c>
LoadModule headers_module modules/mod_headers.so
</IfModule>
+<IfModule !mod_setenvif.c>
+ LoadModule setenvif_module modules/mod_setenvif.so
+</IfModule>
<IfVersion < 2.4>
LockFile accept.lock
@@ -67,9 +70,6 @@ LockFile accept.lock
<IfModule !mod_unixd.c>
LoadModule unixd_module modules/mod_unixd.so
</IfModule>
-<IfModule !mod_setenvif.c>
- LoadModule setenvif_module modules/mod_setenvif.so
-</IfModule>
</IfVersion>
PassEnv GIT_VALGRIND
@@ -79,9 +79,7 @@ PassEnv ASAN_OPTIONS
PassEnv GIT_TRACE
PassEnv GIT_CONFIG_NOSYSTEM
-<IfVersion >= 2.4>
- SetEnvIf Git-Protocol ".*" GIT_PROTOCOL=$0
-</IfVersion>
+SetEnvIf Git-Protocol ".*" GIT_PROTOCOL=$0
Alias /dumb/ www/
Alias /auth/dumb/ www/auth/dumb/
--
2.16.0.rc0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] http: fix v1 protocol tests with apache httpd < 2.4
2017-12-31 2:32 [PATCH] http: fix v1 protocol tests with apache httpd < 2.4 Todd Zullinger
@ 2018-01-03 0:21 ` Brandon Williams
2018-01-03 0:39 ` Todd Zullinger
2018-01-03 5:56 ` Jeff King
1 sibling, 1 reply; 6+ messages in thread
From: Brandon Williams @ 2018-01-03 0:21 UTC (permalink / raw)
To: Todd Zullinger; +Cc: git, Jeff King, Junio C Hamano
On 12/30, Todd Zullinger wrote:
> The apache config used by tests was updated to use the SetEnvIf
> directive to set the Git-Protocol header in 19113a26b6 ("http: tell
> server that the client understands v1", 2017-10-16).
>
> Setting the Git-Protocol header is restricted to httpd >= 2.4, but
> mod_setenvif and the SetEnvIf directive work with lower versions, at
> least as far back as 2.0, according to the httpd documentation:
>
> https://httpd.apache.org/docs/2.0/mod/mod_setenvif.html
>
> Drop the restriction. Tested with httpd 2.2 and 2.4.
>
> Signed-off-by: Todd Zullinger <tmz@pobox.com>
> ---
>
> These tests fail with 2.16.0-rc0 on CentOS-6, which uses
> httpd-2.2.
>
> I removed the version restriction entirely rather than adjust
> the version. I believe SetEnvIf works on httpd >= 2.0. I'm
> not sure if we aim to support anything less than httpd 2.0,
> but I'm betting not. If that's incorrect, I can add some
> IfVersion conditions.
>
> As noted in the commit message, I only tested with httpd 2.2
> and 2.4 (on CentOS 6/7 and Fedora 26-28). If anyone has older
> httpd systems which need support, it would be great if they
> could test this before 2.16.0 is finalized, so we can avoid
> shipping with any failing tests.
Seems good to me. I think I was just being overly cautious when i was
implementing those patches. Thanks!
>
> t/lib-httpd/apache.conf | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
> index df19436314..724d9ae462 100644
> --- a/t/lib-httpd/apache.conf
> +++ b/t/lib-httpd/apache.conf
> @@ -25,6 +25,9 @@ ErrorLog error.log
> <IfModule !mod_headers.c>
> LoadModule headers_module modules/mod_headers.so
> </IfModule>
> +<IfModule !mod_setenvif.c>
> + LoadModule setenvif_module modules/mod_setenvif.so
> +</IfModule>
>
> <IfVersion < 2.4>
> LockFile accept.lock
> @@ -67,9 +70,6 @@ LockFile accept.lock
> <IfModule !mod_unixd.c>
> LoadModule unixd_module modules/mod_unixd.so
> </IfModule>
> -<IfModule !mod_setenvif.c>
> - LoadModule setenvif_module modules/mod_setenvif.so
> -</IfModule>
> </IfVersion>
>
> PassEnv GIT_VALGRIND
> @@ -79,9 +79,7 @@ PassEnv ASAN_OPTIONS
> PassEnv GIT_TRACE
> PassEnv GIT_CONFIG_NOSYSTEM
>
> -<IfVersion >= 2.4>
> - SetEnvIf Git-Protocol ".*" GIT_PROTOCOL=$0
> -</IfVersion>
> +SetEnvIf Git-Protocol ".*" GIT_PROTOCOL=$0
>
> Alias /dumb/ www/
> Alias /auth/dumb/ www/auth/dumb/
> --
> 2.16.0.rc0
>
--
Brandon Williams
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] http: fix v1 protocol tests with apache httpd < 2.4
2018-01-03 0:21 ` Brandon Williams
@ 2018-01-03 0:39 ` Todd Zullinger
2018-01-03 6:17 ` Jeff King
0 siblings, 1 reply; 6+ messages in thread
From: Todd Zullinger @ 2018-01-03 0:39 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, Jeff King, Junio C Hamano
Brandon Williams wrote:
> Seems good to me. I think I was just being overly cautious when i was
> implementing those patches. Thanks!
Cool, thanks for taking a look.
In this case, the test isn't dependent on apache > 2.4, but
before I checked that I wondered if we had any way to run a
test only if the apache version was greater or lesser than
some release. Luckily, I didn't have to work out such a
method. :)
I don't know if there's a clean way to do that
automatically, short of parsing the output of 'httpd -v'
should we ever need to add such a prereq.
--
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Sometimes you get the blues because your baby leaves you. Sometimes
you get'em 'cause she comes back.
-- B.B. King
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] http: fix v1 protocol tests with apache httpd < 2.4
2017-12-31 2:32 [PATCH] http: fix v1 protocol tests with apache httpd < 2.4 Todd Zullinger
2018-01-03 0:21 ` Brandon Williams
@ 2018-01-03 5:56 ` Jeff King
1 sibling, 0 replies; 6+ messages in thread
From: Jeff King @ 2018-01-03 5:56 UTC (permalink / raw)
To: Todd Zullinger; +Cc: git, Brandon Williams, Junio C Hamano
On Sat, Dec 30, 2017 at 09:32:34PM -0500, Todd Zullinger wrote:
> The apache config used by tests was updated to use the SetEnvIf
> directive to set the Git-Protocol header in 19113a26b6 ("http: tell
> server that the client understands v1", 2017-10-16).
>
> Setting the Git-Protocol header is restricted to httpd >= 2.4, but
> mod_setenvif and the SetEnvIf directive work with lower versions, at
> least as far back as 2.0, according to the httpd documentation:
>
> https://httpd.apache.org/docs/2.0/mod/mod_setenvif.html
>
> Drop the restriction. Tested with httpd 2.2 and 2.4.
Makes sense. I think the only way this could backfire is if somebody has
a funny build that doesn't include mod_setenvif at all. But I don't
think we can know that for sure without applying this and seeing if
anybody screams.
> I removed the version restriction entirely rather than adjust
> the version. I believe SetEnvIf works on httpd >= 2.0. I'm
> not sure if we aim to support anything less than httpd 2.0,
> but I'm betting not. If that's incorrect, I can add some
> IfVersion conditions.
IIRC, anything less than 2.0 is broken already. And it's not worth
changing that, given the age (and I think there were some pretty severe
hardships in making 1.3.x, but it's been a while).
-Peff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] http: fix v1 protocol tests with apache httpd < 2.4
2018-01-03 0:39 ` Todd Zullinger
@ 2018-01-03 6:17 ` Jeff King
2018-01-03 16:21 ` Todd Zullinger
0 siblings, 1 reply; 6+ messages in thread
From: Jeff King @ 2018-01-03 6:17 UTC (permalink / raw)
To: Todd Zullinger; +Cc: Brandon Williams, git, Junio C Hamano
On Tue, Jan 02, 2018 at 07:39:46PM -0500, Todd Zullinger wrote:
> Brandon Williams wrote:
> > Seems good to me. I think I was just being overly cautious when i was
> > implementing those patches. Thanks!
>
> Cool, thanks for taking a look.
>
> In this case, the test isn't dependent on apache > 2.4, but
> before I checked that I wondered if we had any way to run a
> test only if the apache version was greater or lesser than
> some release. Luckily, I didn't have to work out such a
> method. :)
>
> I don't know if there's a clean way to do that
> automatically, short of parsing the output of 'httpd -v'
> should we ever need to add such a prereq.
In the general case, we could probably define an endpoint within an <If>
block, and then try to access the endpoint from the test script.
E.g., something like:
<IfVersion >= 2.4>
Alias /have-2.4.txt www/yes.txt
</IfVersion>
in the apache config, and then:
test_lazy_prereq APACHE24 '
echo yes >"$HTTPD_DOCUMENT_ROOT_PATH/yes.txt" &&
curl -f "$HTTPD_URL/have-2.4.txt"
'
in the test script (of course we may not want to depend on having
command-line curl, but we could replace that with "git ls-remote" or
similar).
One nice thing about that approach is that it can be extended to other
"If" blocks, like if we have a particular module available, or if ssl is
configured.
-Peff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] http: fix v1 protocol tests with apache httpd < 2.4
2018-01-03 6:17 ` Jeff King
@ 2018-01-03 16:21 ` Todd Zullinger
0 siblings, 0 replies; 6+ messages in thread
From: Todd Zullinger @ 2018-01-03 16:21 UTC (permalink / raw)
To: Jeff King; +Cc: Brandon Williams, git, Junio C Hamano
Jeff King wrote:
> On Tue, Jan 02, 2018 at 07:39:46PM -0500, Todd Zullinger wrote:
>> I don't know if there's a clean way to do that
>> automatically, short of parsing the output of 'httpd -v'
>> should we ever need to add such a prereq.
>
> In the general case, we could probably define an endpoint within an <If>
> block, and then try to access the endpoint from the test script.
>
> E.g., something like:
>
> <IfVersion >= 2.4>
> Alias /have-2.4.txt www/yes.txt
> </IfVersion>
>
> in the apache config, and then:
>
> test_lazy_prereq APACHE24 '
> echo yes >"$HTTPD_DOCUMENT_ROOT_PATH/yes.txt" &&
> curl -f "$HTTPD_URL/have-2.4.txt"
> '
>
> in the test script (of course we may not want to depend on having
> command-line curl, but we could replace that with "git ls-remote" or
> similar).
>
> One nice thing about that approach is that it can be extended to other
> "If" blocks, like if we have a particular module available, or if ssl is
> configured.
That's quite elegant. I even modified an IfVersion block
and didn't think about using it that way to create a prereq.
Neat!
--
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
You're not drunk if you can lie on the floor without holding on.
-- Dean Martin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-01-03 16:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-31 2:32 [PATCH] http: fix v1 protocol tests with apache httpd < 2.4 Todd Zullinger
2018-01-03 0:21 ` Brandon Williams
2018-01-03 0:39 ` Todd Zullinger
2018-01-03 6:17 ` Jeff King
2018-01-03 16:21 ` Todd Zullinger
2018-01-03 5:56 ` Jeff King
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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).