You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Vivek <vi...@collab.net> on 2005/04/12 12:34:41 UTC

[PATCH] fix for 'make davautocheck'

[[[
     A patch to make the make davautocheck really work
]]]


Re: revised [PATCH] fix for 'make davautocheck'

Posted by VK Sameer <sa...@collab.net>.
On Mon, 2005-04-18 at 14:20 +0200, Peter N. Lundblad wrote:

> I don't know why you commented out the line that ensures that a module is
> bultin if it isn't present in the modules directory. Since it works with
> that line on my Debian system, I added it back in. Tell me if something
> breaks because of that - and in that case, the line should really have
> been removed (or an explanation added).

My apologies, that should have been uncommented before generating the
patch. Thanks for catching it and the commit.

Regards
Sameer


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: revised [PATCH] fix for 'make davautocheck'

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Mon, 18 Apr 2005, VK Sameer wrote:

> Hopefully, this version of the patch addresses all of Peter's and
> Marcus's comments. Additional checks are present for htpasswd2 and
> looking for files in SBINDIR as well as BINDIR.
>
Tweaked somewhat and committed in r14286; proposed for backport to 1.2.x
as well.

I don't know why you commented out the line that ensures that a module is
bultin if it isn't present in the modules directory. Since it works with
that line on my Debian system, I added it back in. Tell me if something
breaks because of that - and in that case, the line should really have
been removed (or an explanation added).

Thanks for working on this Vivek and VK.
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: revised [PATCH] fix for 'make davautocheck'

Posted by VK Sameer <sa...@collab.net>.
Hopefully, this version of the patch addresses all of Peter's and
Marcus's comments. Additional checks are present for htpasswd2 and
looking for files in SBINDIR as well as BINDIR.

Regards
Sameer


Re: revised [PATCH] fix for 'make davautocheck'

Posted by VK Sameer <sa...@collab.net>.
Another update, based on darix's comment about using ${variable:+set}
instead of ${variable+set} (catches a null value as well as
non-existence).

Regards
Sameer

Re: revised [PATCH] fix for 'make davautocheck'

Posted by VK Sameer <sa...@collab.net>.
Updated log and patch based on Peter Lundblad's comments in IRC.

Regards
Sameer

Re: revised [PATCH] fix for 'make davautocheck'

Posted by VK Sameer <sa...@collab.net>.
If the patch looks ok, could this be committed? We have a few boxes
where this test needs to be run.

Thanks
Sameer


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: revised [PATCH] fix for 'make davautocheck'

Posted by VK Sameer <sa...@collab.net>.
Responding to this since Vivek is out today.

On Wed, 2005-04-13 at 22:12, Max Bowsher wrote:

> Defines APXS variable, but does not use it.

It _is_ used in a few places notably when finding the location of httpd.
But a bare apxs was also being used. Fixed.

> Uses both ` ` and $( ). Be consistent.

Good point, fixed.

> Pointless to define a function which is called just once, and that, 
> immediately after its definition.

Removed.

> Please follow the guidelines in HACKING regarding log messages.

Hope the updated log message is ok.

Regards
Sameer

Re: revised [PATCH] fix for 'make davautocheck'

Posted by Max Bowsher <ma...@ukf.net>.
Vivek wrote:
>> On Wed, 2005-04-13 at 18:37, Peter N. Lundblad wrote:
>>> On Wed, 13 Apr 2005, Vivek wrote:
>>>
>>>>
>>> Nice to see that you're working on this, since I was also trying to get
>>> this working, to not have to mess up my main Apache installation with 
>>> test
>>> stuff.
>>>
>>> On Debian, apxs for Apache 2 is called apxs2. It might be a good idea to
>>> make this script adhere to the --with-apxs config option. Another would 
>>> be
>>> to have the script get the Apache binary name from apxs as well.
>>>
>> ok. The script have been modified according to your suggestions. Thanks.
>> And thanks to sameer@collab.net for working with me for all this.

Thanks.

Further issues:

Defines APXS variable, but does not use it.

Uses both ` ` and $( ). Be consistent.

Pointless to define a function which is called just once, and that, 
immediately after its definition.

Do not leave old code present but commented out, that's what the history is 
for.

Please follow the guidelines in HACKING regarding log messages.


Max.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: revised [PATCH] fix for 'make davautocheck'

Posted by Vivek <vi...@collab.net>.
On Wed, 2005-04-13 at 18:37, Peter N. Lundblad wrote:
> On Wed, 13 Apr 2005, Vivek wrote:
> 
> > 
> Nice to see that you're working on this, since I was also trying to get
> this working, to not have to mess up my main Apache installation with test
> stuff.
> 
> On Debian, apxs for Apache 2 is called apxs2. It might be a good idea to
> make this script adhere to the --with-apxs config option. Another would be
> to have the script get the Apache binary name from apxs as well.
> 
ok. The script have been modified according to your suggestions. Thanks.
And thanks to sameer@collab.net for working with me for all this.
  
> Regards,
> //Peter
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org

Re: revised [PATCH] fix for 'make davautocheck'

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 13 Apr 2005, Vivek wrote:

> On Wed, 2005-04-13 at 16:02, Max Bowsher wrote:
> > Vivek wrote:
> > >> [[[
> > >>     A patch to make the make davautocheck really work
> > >> ]]]
> >
> > That's not a fix, that's just changing one hardcoded path for another.
> >
> > The proper way is to ask apxs where the modules live.
> >
> > Max.
> Yes, thank you Max. It has been taken care of in this patch.

Nice to see that you're working on this, since I was also trying to get
this working, to not have to mess up my main Apache installation with test
stuff.

On Debian, apxs for Apache 2 is called apxs2. It might be a good idea to
make this script adhere to the --with-apxs config option. Another would be
to have the script get the Apache binary name from apxs as well.

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

revised [PATCH] fix for 'make davautocheck'

Posted by Vivek <vi...@collab.net>.
On Wed, 2005-04-13 at 16:02, Max Bowsher wrote:
> Vivek wrote:
> >> [[[
> >>     A patch to make the make davautocheck really work
> >> ]]]
> 
> That's not a fix, that's just changing one hardcoded path for another.
> 
> The proper way is to ask apxs where the modules live.
> 
> Max.
Yes, thank you Max. It has been taken care of in this patch. 
Vivek


Re: [PATCH] fix for 'make davautocheck'

Posted by Max Bowsher <ma...@ukf.net>.
Vivek wrote:
>> [[[
>>     A patch to make the make davautocheck really work
>> ]]]

That's not a fix, that's just changing one hardcoded path for another.

The proper way is to ask apxs where the modules live.

Max.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org