httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Re: svn commit: r230592 - in /httpd/httpd/branches/2.0.x: CHANGES STATUS modules/proxy/proxy_http.c
Date Sun, 07 Aug 2005 03:08:44 GMT
At 08:39 PM 8/6/2005, Jeff Trawick wrote:
>On 8/6/05, wrowe@apache.org <wrowe@apache.org> wrote:
>> Author: wrowe
>> Date: Sat Aug  6 14:29:05 2005
>> New Revision: 230592
>> 
>> URL: http://svn.apache.org/viewcvs?rev=230592&view=rev
>> Log:
>> 
>>  As much as it pains me, seriously, it seems that reviewing the re-backport
>>  of this code was too illegible for review, so it seems we will need to
>>  re-review a fresh backport from httpd trunk.
>
>It looks to me that we have lost our second of two chances to go
>through a stepwise, single-problem/single-solution approach to
>resolving the issues with this code, even after multiple comments
>stating that mixing that set of changes was undesired.  It isn't
>impossible to move forward from this point, but I don't understand why
>we're still in big-patch mode after those previous comments.

Hold on a second ;-)  Are you suggesting this original backport
patch was 'small' enough for review?  I'd challenge that the reasons
why the errors were not caught was that the original backport was the
patch that was too large, and there simply weren't enough eyeballs
on it (no insult intended; you know how many iterations my later
changes went through.)

The other side of the coin, however, is that all agree the old code
in proxy_http is horribly broken.  If we agree trunk/ both behaves
correctly and is now more legible, shouldn't we put far more trust
in the code on trunk/ than the cruft in 2.0.x?

I'm suggesting that the review should have occurred commit-by-commit 
on trunk (C-t-R).  The entire history of these changes is sitting
on trunk, with all the wrinkles from the beginning of the effort.

Since we have little faith that sufficient 'R' occurred as we moved
through each bit of your and my efforts, perhaps it's too much to 
ask people to have faith in the resulting backport.

If I have folks look at each 'little piece', I'm happy to 'precommit'
each of the side-effects (reformatting, apr_natstrcasecmp, etc) in
separate commits that make the end-result patch easier to read.

But the bottom line; do you really want me to hand a "STACK" of 
patches representing each of these changes, which will inevitable
not apply cleanly unless layered in a specific order (and even then...)

What if I take Joe's breakdown of issues and ask for each of them to
have a separate vote (without individual patch files, the main patch
file can be used to see 'what will change') and the moment we have 
3 +1's I'll commit that small layer, making the final, harry refactor 
patch of the body stream/spool code just a wee bit easier to read.  
Does this appeal to everyone?

Forgive me if I've been humorless about this; already pointed out
I had burned 60, maybe 80 hours of my early summer on this mess,
all because, when I added proxy trace and tried to actually send
bodies, the proxy request body processing would simply hang on
the back end because the proxied request was simply mishandled.
Apache 1.3?  Was close.  Apache 2.1?  Even closer.  2.0.x - blah.
Fixing one small quirk would point out yet another until various
bits of the code looked nothing as they had started as.

I'll do my best to put on a smile and slog through the 'correct' 
solution in a manner 'agreeable' to both you and Joe :)

Bill



Mime
View raw message