You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Marcin Kasperski <Ma...@softax.com.pl> on 2004/02/19 14:34:40 UTC
Messages from hooks
Some hooks can reject the change. But it seems there is no way to
tell the client why the change has been rejected (or at least
the subversion book does not tell about it).
I think it would be good idea if the hook could somehow give an
error message which would be presented to the client.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Messages from hooks
Posted by Philip Martin <ph...@codematters.co.uk>.
Ben Collins-Sussman <su...@collab.net> writes:
> 2.0.47? We've required 2.0.48 ever since svn 0.33.
Oops again! I'm using 2.0.48.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Lost in mod_dav
Posted by Philip Martin <ph...@codematters.co.uk>.
"C. Michael Pilato" <cm...@collab.net> writes:
> Ick. So mod_dav is trying to undo the failed PROPPATCH? Ah, I see
> that code now. Nasty...
Is that meant to happen? I don't think that's the error anyway.
What I see is that after both calls to svn_repos__hook_pre_revprop
have failed I end up back in dav_method_proppatch in mod_dav.c. At
line 2352 there is a call to dav_close_propdb(propdb) which ends up
destroying the pool the was used to allocate the errors returned from
mod_dav_svn (they were allocated by calling dav_svn_convert_err in
save_value in deadprops.c).
I think those errors are still in use, in particular the ctx_list
still points at them.
After calling dav_close_propdb the next line is dav_auto_checkin which
appears to reuse the memory released when the pool was destroyed, and
that corrupts the errors in the ctx_list. The next line is
dav_process_ctx_list which uses the ctx_list and tries to log the
errors that have been corrupted.
Now I have APR pool debugging enabled which may be making the problem
more visible, but using memory after a pool has been cleared is a
serious problem. I think this problem affects the 1.0.x branch,
that's why I see the messages about "Invalid or incomplete multibyte
or wide character". I notice Ben had similar messages in his log file
which indicates that he is seeing the same corruption.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Lost in mod_dav
Posted by "C. Michael Pilato" <cm...@collab.net>.
Philip Martin <ph...@codematters.co.uk> writes:
> Philip Martin <ph...@codematters.co.uk> writes:
>
> > Ben Collins-Sussman <su...@collab.net> writes:
> >
> >> I'm sure you're capable of finding the segfault. Keep us posted...
> >
> > The trunk is segfaulting here
>
> When svn_repos__hook_pre_revprop fails, it returns an error to
> mod_dav. While processing this error mod_dav does some sort of
> rollback that ends up calling svn_repos__hook_pre_revprop again!
> Is it meant to work like that?
Ick. So mod_dav is trying to undo the failed PROPPATCH? Ah, I see
that code now. Nasty...
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Lost in mod_dav
Posted by Philip Martin <ph...@codematters.co.uk>.
Philip Martin <ph...@codematters.co.uk> writes:
> Ben Collins-Sussman <su...@collab.net> writes:
>
>> I'm sure you're capable of finding the segfault. Keep us posted...
>
> The trunk is segfaulting here
When svn_repos__hook_pre_revprop fails, it returns an error to
mod_dav. While processing this error mod_dav does some sort of
rollback that ends up calling svn_repos__hook_pre_revprop again!
Is it meant to work like that?
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Messages from hooks
Posted by Philip Martin <ph...@codematters.co.uk>.
Ben Collins-Sussman <su...@collab.net> writes:
> I'm sure you're capable of finding the segfault. Keep us posted...
The trunk is segfaulting here
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1024 (LWP 14972)]
dav_log_err (r=0x81198f0, err=0x8129c60, level=3) at mod_dav.c:585
585 if (errscan->desc == NULL)
(gdb) p errscan
$1 = (dav_error *) 0x230
(gdb) bt
#0 dav_log_err (r=0x81198f0, err=0x8129c60, level=3) at mod_dav.c:585
#1 0x403cd734 in dav_prop_log_errors (ctx=0x0) at mod_dav.c:2175
#2 0x403cd780 in dav_process_ctx_list (func=0x403cd710 <dav_prop_log_errors>,
ctx_list=0x0, stop_on_error=0, reverse=0) at mod_dav.c:2202
#3 0x403cdb0f in dav_method_proppatch (r=0x81198f0) at mod_dav.c:2358
#4 0x080812d5 in ap_run_handler (r=0x0) at config.c:194
#5 0x080818b0 in ap_invoke_handler (r=0x8129920) at config.c:401
#6 0x0806e5f6 in ap_process_request (r=0x403dc648) at http_request.c:288
#7 0x08069bdc in ap_process_http_connection (c=0x8129920) at http_core.c:293
#8 0x0808c365 in ap_run_process_connection (c=0x0) at connection.c:85
#9 0x0807f924 in child_main (child_num_arg=0) at prefork.c:694
#10 0x0807fb3b in make_child (s=0x3, slot=135436576) at prefork.c:734
#11 0x0807fb98 in startup_children (number_to_start=5) at prefork.c:806
#12 0x080803ea in ap_mpm_run (_pconf=0x80ba040, plog=0x80c2d80, s=0x80c5250)
at prefork.c:1022
#13 0x080867f8 in main (argc=2, argv=0xbffffc14) at main.c:660
I've switched to the 1.0 branch and it doesn't segfault, but I still
don't see the hook errors properly and the apache log contains
[Thu Feb 19 17:36:29 2004] [error] [client 127.0.0.1] Could not MERGE resource "/obj/repo/!svn/act/76ffe201-b8d3-0310-a82d-f775fc91260c" into "/obj/repo". [409, #0]
[Thu Feb 19 17:36:29 2004] [error] [client 127.0.0.1] An error occurred while committing the transaction. [409, #165001]
[Thu Feb 19 17:36:29 2004] [error] [client 127.0.0.1] (84)Invalid or incomplete multibyte or wide character: 'pre-commit' hook failed with error output:
[409, #165001]
and
[Thu Feb 19 17:37:53 2004] [error] [client 127.0.0.1] Could not execute PROPPATCH. [500, #206]
[Thu Feb 19 17:37:53 2004] [error] [client 127.0.0.1] 'pre-revprop-change' hook failed with error output:
[500, #165001]
[Thu Feb 19 17:37:53 2004] [error] [client 127.0.0.1] (84)Invalid or incomplete multibyte or wide character: 'pre-revprop-change' hook failed with error output:
[500, #165001]
I'm sure we used to send hook errors over ra_dav (apart from
post-commit) or is my memory playing tricks?
I don't understand "Invalid or incomplete multibyte or wide character"
either, everything I amd doing is using ASCII.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Messages from hooks
Posted by Ben Collins-Sussman <su...@collab.net>.
On Thu, 2004-02-19 at 11:12, Philip Martin wrote:
> > Using trunk@HEAD code, I don't get a segfault like you, nor do I get
> > that weird bit of corrupted text in the errorlog (!õ #?ö ï).
>
> I'm using Apache/APR 2.0.47 and the whole lot is built with pool
> debugging.
2.0.47? We've required 2.0.48 ever since svn 0.33. Perhaps your apache
and svn are compiling against a newer apr/apu?
I'm sure you're capable of finding the segfault. Keep us posted...
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Messages from hooks
Posted by Philip Martin <ph...@codematters.co.uk>.
Ben Collins-Sussman <su...@collab.net> writes:
> On Thu, 2004-02-19 at 10:47, Philip Martin wrote:
>
>> $ cat repo/hooks/pre-revprop-change
>> #!/bin/sh
>> echo bug >$2
>
> Did you mean ">&2" there?
Oops! Yes, my script has ">&2".
> Using trunk@HEAD code, I don't get a segfault like you, nor do I get
> that weird bit of corrupted text in the errorlog (!õ #?ö ï).
I'm using Apache/APR 2.0.47 and the whole lot is built with pool
debugging.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Messages from hooks
Posted by Ben Collins-Sussman <su...@collab.net>.
On Thu, 2004-02-19 at 17:45, Philip Martin wrote:
> > [Thu Feb 19 11:02:19 2004] [error] [client 127.0.0.1] (84)Invalid or
> > incomplete multibyte or wide character: 'pre-revprop-change' hook
> > failed with error output:
>
> This is caused by the (84) which is the value of the global errno when
> we call dav_svn_convert_err! It's not clear to me what we should do
> about this. errno is only valid when a system call fails, otherwise
> the value is undefined. In this case the value of errno changes from
> 2 to 84 when we run the hook, but it does that whether or not the hook
> returns an error, i.e. if I change my hook to "exit 0" I still get
> errno set to 84. I don't know whether it is better for
> dav_svn_convert_err to set errno to zero, possibly losing information,
> or to leave it alone, possibly providing erroneous information.
Greg Stein, can you comment on this?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Messages from hooks
Posted by Philip Martin <ph...@codematters.co.uk>.
Ben Collins-Sussman <su...@collab.net> writes:
> Using trunk@HEAD code, I don't get a segfault like you, nor do I get
> that weird bit of corrupted text in the errorlog (!õ #?ö ï). Instead, I
> see:
After a bit of debugging and some valgrind runs I now think there are
two bugs. The first is a trunk@HEAD only problem and it is the one
that caused the segfault and dodgy characters. It's a use of memory
after a pool has been cleared and I can fix it using this patch which
uses a pool with a longer lifetime (it also always uses the fixed
error text for two reasons: we don't want two errors with the same
text and the serr->message pool is about to be destroyed):
Index: subversion/mod_dav_svn/deadprops.c
===================================================================
--- subversion/mod_dav_svn/deadprops.c (revision 8758)
+++ subversion/mod_dav_svn/deadprops.c (working copy)
@@ -158,9 +158,8 @@
propname, value, db->resource->pool);
if (serr != NULL)
return dav_svn_convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
- serr->message ?
- serr->message : "could not change a property",
- db->p);
+ "could not change a property",
+ db->resource->pool);
/* a change to the props was made; make sure our cached copy is gone */
db->props = NULL;
There are two other dav_svn_convert_err in this file, I think they
should also use db->resource->pool but all this DAV stuff is a bit
alien to me. I think I'll commit a change for all three onto the
trunk.
The second bug is present on both the trunk and the 1.0.x branch, but
it's not so serious. It's the bug responsible for the "Invalid or
incomplete multibyte or wide character"
> [Thu Feb 19 11:02:19 2004] [error] [client 127.0.0.1] (84)Invalid or
> incomplete multibyte or wide character: 'pre-revprop-change' hook
> failed with error output:
This is caused by the (84) which is the value of the global errno when
we call dav_svn_convert_err! It's not clear to me what we should do
about this. errno is only valid when a system call fails, otherwise
the value is undefined. In this case the value of errno changes from
2 to 84 when we run the hook, but it does that whether or not the hook
returns an error, i.e. if I change my hook to "exit 0" I still get
errno set to 84. I don't know whether it is better for
dav_svn_convert_err to set errno to zero, possibly losing information,
or to leave it alone, possibly providing erroneous information.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Messages from hooks
Posted by Ben Collins-Sussman <su...@collab.net>.
On Thu, 2004-02-19 at 10:47, Philip Martin wrote:
> $ cat repo/hooks/pre-revprop-change
> #!/bin/sh
> echo bug >$2
Did you mean ">&2" there?
> $ cat /usr/local/apache2/logs/error_log
>
> [Thu Feb 19 16:45:38 2004] [notice] Apache/2.0.48 (Unix) DAV/2 SVN/0.37.0+ configured -- resuming normal operations
> [Thu Feb 19 16:45:45 2004] [error] [client 127.0.0.1] 'pre-revprop-change' hook failed with error output:
> bug
> [500, #165001]
> [Thu Feb 19 16:45:45 2004] [error] [client 127.0.0.1] (84)Invalid or incomplete multibyte or wide character: 'pre-revprop-change' hook failed with error output:
> bug
> oa [500, #165001]
> !õ #?ö ï [500, #165001]4] [error] [client 127.0.0.1]
> [Thu Feb 19 16:45:46 2004] [notice] child pid 1413 exit signal Segmentation fault (11)
> [Thu Feb 19 16:45:46 2004] [notice] child pid 1412 exit signal Segmentation fault (11)
>
Using trunk@HEAD code, I don't get a segfault like you, nor do I get
that weird bit of corrupted text in the errorlog (!õ #?ö ï). Instead, I
see:
[Thu Feb 19 11:02:19 2004] [error] [client 127.0.0.1]
'pre-revprop-change' hook
failed with error output:
/usr/local/svn/foorepo/hooks/pre-revprop-change: line 2: 1: No such file
or directory
[500, #165001]
[Thu Feb 19 11:02:19 2004] [error] [client 127.0.0.1] (84)Invalid or
incomplete
multibyte or wide character: 'pre-revprop-change' hook failed with error
output:/usr/local/svn/foorepo/hooks/pre-revprop-change: line 2: 1: No
such file or directory
[500, #165001]
[Thu Feb 19 11:02:19 2004] [error] [client 127.0.0.1] Could not execute
PROPPATCH. [500, #206]
[Thu Feb 19 11:02:19 2004] [error] [client 127.0.0.1]
'pre-revprop-change' hook
failed with error output:
/usr/local/svn/foorepo/hooks/pre-revprop-change: line 2: 1: No such file
or directory
[500, #165001]
[Thu Feb 19 11:02:19 2004] [error] [client 127.0.0.1] (84)Invalid or
incomplete
multibyte or wide character: 'pre-revprop-change' hook failed with error
output:/usr/local/svn/foorepo/hooks/pre-revprop-change: line 2: 1: No
such file or directory
[500, #165001]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Messages from hooks
Posted by Philip Martin <ph...@codematters.co.uk>.
Ben Collins-Sussman <su...@collab.net> writes:
> Calm down, guys. :-) This is issue 443, a well-known, long-time
> shortcoming in mod_dav_svn. Read that issue for background.
I'm not so sure. Issue 443 is the post-commit hook, while the OP is
complaining about the pre-revprop-change hook. The pre-revprop-change
hook *should* return an error over ra_dav, but it doesn't appear to be
working. Using trunk@HEAD I'm getting a core dump in mod_dav_svn :-(
$ cat repo/hooks/pre-revprop-change
#!/bin/sh
echo bug >$2
exit 1
$ svn ps --revprop -r1 svn:log bar http://localhost:8888/obj/repo
../svn/subversion/libsvn_ra_dav/fetch.c:1172: (apr_err=175002)
svn: DAV request failed; it's possible that the repository's pre-revprop-change hook either failed or is non-existent
../svn/subversion/libsvn_ra_dav/props.c:1072: (apr_err=175008)
svn: At least one property change failed; repository is unchanged
$ cat /usr/local/apache2/logs/error_log
[Thu Feb 19 16:45:38 2004] [notice] Apache/2.0.48 (Unix) DAV/2 SVN/0.37.0+ configured -- resuming normal operations
[Thu Feb 19 16:45:45 2004] [error] [client 127.0.0.1] 'pre-revprop-change' hook failed with error output:
bug
[500, #165001]
[Thu Feb 19 16:45:45 2004] [error] [client 127.0.0.1] (84)Invalid or incomplete multibyte or wide character: 'pre-revprop-change' hook failed with error output:
bug
oa [500, #165001]
!õ #?ö ï [500, #165001]4] [error] [client 127.0.0.1]
[Thu Feb 19 16:45:46 2004] [notice] child pid 1413 exit signal Segmentation fault (11)
[Thu Feb 19 16:45:46 2004] [notice] child pid 1412 exit signal Segmentation fault (11)
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Messages from hooks
Posted by Ben Collins-Sussman <su...@collab.net>.
On Thu, 2004-02-19 at 10:05, Marcin Kasperski wrote:
> > > I can't comment on the protocol but doesn't it possess some
> > > way of telling the client what error has happened (even in
> > > non hook-related case)?
> >
> > I don't know if WebDAV/DeltaV supports passing error
> > information back (other than "the request failed" and a status
> > code, as it's doing now).
>
> I don't know DAV internals but considering the fact it works over
> HTTP, at worst one can add
Calm down, guys. :-) This is issue 443, a well-known, long-time
shortcoming in mod_dav_svn. Read that issue for background.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Messages from hooks
Posted by Marcin Kasperski <Ma...@softax.com.pl>.
> > I can't comment on the protocol but doesn't it possess some
> > way of telling the client what error has happened (even in
> > non hook-related case)?
>
> I don't know if WebDAV/DeltaV supports passing error
> information back (other than "the request failed" and a status
> code, as it's doing now).
I don't know DAV internals but considering the fact it works over
HTTP, at worst one can add
X-Subversion-ErrorMsg: <message goes here>
to the HTTP headers. Moreover, at the first glance, DAV specifies
'responsedescription' element so one can write
HTTP/1.1 207 Multi-Status
Content-Type: text/xml; charset="utf-8"
Content-Length: xxxx
<?xml version="1.0" encoding="utf-8" ?>
<d:multistatus xmlns:d="DAV:">
<d:response>
<d:href>http://www.foo.bar/directory/element</d:href>
<d:status>HTTP/1.1 412 Precondition Failed</d:status>
<d:responsedescription>Subversion pre-commit hook failed
with message: comits to the stable branch must mention bug
numbers.</d:responsedescription>
</d:response>
</d:multistatus>
(as i read http://www.ietf.org/rfc/rfc2518.txt, seems that
responsedescription can be placed both within response and
sideways to it, I do not know which solution is preferable).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Messages from hooks
Posted by Juanma Barranquero <jm...@wke.es>.
On Thu, 19 Feb 2004 16:25:33 +0100
Marcin Kasperski <Ma...@softax.com.pl> wrote:
> If I understand correctly, for serious/large/distributed
> development one is to use http access. And those are the cases
> when custom hooks are used more frequently than in small&simple
> repositories.
I'm just a Subversion user, not a developer, so I can't really comment
on this, but I get the impression that svnserve could be use for fairly
large setups.
> I can't comment on the protocol but doesn't it possess some way
> of telling the client what error has happened (even in non
> hook-related case)?
I don't know if WebDAV/DeltaV supports passing error information back
(other than "the request failed" and a status code, as it's doing now).
> email also does not seem to be good fit. Even leaving the problem
> whom to send this email, this really does not seem to be natural
> to run command line utility and then check the inbox. And email
> does not always come immediately.
Sure. You don't have to convince me, I already think Subversion hooks
are a weak spot on an otherwise wonderful system. I'm just saying that's
the way it is now.
Juanma
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Messages from hooks
Posted by Marcin Kasperski <Ma...@softax.com.pl>.
Dnia czw 19. lutego 2004 16:06, Juanma Barranquero napisał:
> On Thu, 19 Feb 2004 15:34:40 +0100
>
> Marcin Kasperski <Ma...@softax.com.pl> wrote:
> > I think it would be good idea if the hook could somehow give
> > an error message which would be presented to the client.
>
> In svn:// and file:// cases (svnserve and local access), you
> already get the STDERR of the hook:
If I understand correctly, for serious/large/distributed
development one is to use http access. And those are the cases
when custom hooks are used more frequently than in small&simple
repositories.
> D:\> svn propset --revprop svn:log -r 3 "Test message."
> http://localhost/test svn: DAV request failed; it's possible
> that the repository's pre-revprop-change hook either failed or
> is non-existent
> svn: At least one property change failed; repository is
> unchanged
>
> Allowing that would need to develop a protocol to send that
> info back to the client, and that's non-trivial,
I can't comment on the protocol but doesn't it possess some way
of telling the client what error has happened (even in non
hook-related case)?
> and perhaps even unneeded.
Imagine complicated commit hooks which (for instance) require
some special format of commit message (there is a frequent
example of bug number while commiting in stable branch), check
some special permissions, check the file format (for instance
'no win-1250 HTML please, use iso-8859-2), ... So someone
commits. The commit fails. What happened?
> After all, the hook could send
> e-mail on failure, or write a log somewhere...
log solution is obviously wrong, in the serious installation no
developer has access to the log.
email also does not seem to be good fit. Even leaving the problem
whom to send this email, this really does not seem to be natural
to run command line utility and then check the inbox. And email
does not always come immediately.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Messages from hooks
Posted by Juanma Barranquero <jm...@wke.es>.
On Thu, 19 Feb 2004 15:34:40 +0100
Marcin Kasperski <Ma...@softax.com.pl> wrote:
> I think it would be good idea if the hook could somehow give an
> error message which would be presented to the client.
In svn:// and file:// cases (svnserve and local access), you already get
the STDERR of the hook:
D:\> svn propset --revprop svn:log -r 3 "Test message." file:///D:/my/repo/test
svn: 'pre-revprop-change' hook failed with error output:
Rejected because I say so.
D:\> svn propset --revprop svn:log -r 3 "Test message." svn://localhost/
svn: 'pre-revprop-change' hook failed with error output:
Rejected because I say so.
but not so in the http:// case:
D:\> svn propset --revprop svn:log -r 3 "Test message." http://localhost/test
svn: DAV request failed; it's possible that the repository's pre-revprop-change
hook either failed or is non-existent
svn: At least one property change failed; repository is unchanged
Allowing that would need to develop a protocol to send that info back to
the client, and that's non-trivial, and perhaps even unneeded. After all,
the hook could send e-mail on failure, or write a log somewhere...
Juanma
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org