> this is not a complete review, just clafirying some of philip's points.
>
> On Tue, 10 May 2005, Philip Martin wrote:
>
>> "Madan US" <madan@collab.net> writes:
>>
[...]
>>
>> Hmm, a quick experiment shows that the existing hook output appears to
>> get escaped over ra_dav, but not encoded. The hook needs to output
>> UTF-8 if it is to be displayed by the client, otherwise I just see
>> "svn: General svn error from server".
>>
> It definitely needs to be XML-escaped. Whether it should be recoded
> from the locale encoding is another topic - and should be a separate
> change IMO. Oh, and it needs base4 encoding as well, or filtering so
> that invalid XML chars don't come through.
Is the pre-commit hook stderr xml-escaped???? I wonder why no one pointed
that out.... :?
>
>
>> > + post_commit_hook = svntest.main.get_post_commit_hook_path
>> > (repo_dir) + svntest.main.file_append (post_commit_hook,
>> > + """#!/bin/sh
>> > + echo "Post-commit Hook says nothing
>> > doing on stderr" > /dev/stderr + exit -1
>>
>> What about Windows? At present you will have to Skip() the test on
>> some platforms.
>>
> And is 2> more portable than /dev/stderr?
>
>
>> > Index: subversion/libsvn_ra_svn/client.c
>> > ===================================================================
>> > --- subversion/libsvn_ra_svn/client.c (revision 14664)
>> > +++ subversion/libsvn_ra_svn/client.c (working copy)
>> > @@ -785,15 +785,18 @@
>> > svn_revnum_t new_rev;
>> > const char *committed_date;
>> > const char *committed_author;
>> > + const char *post_commit_err;
>> >
>> > SVN_ERR(handle_auth_request(ccb->sess_baton, ccb->pool));
>> > - SVN_ERR(svn_ra_svn_read_tuple(ccb->sess_baton->conn, ccb->pool,
>> > "r(?c)(?c)", + SVN_ERR(svn_ra_svn_read_tuple(ccb->sess_baton->conn,
>> > ccb->pool, + "r(?c)(?c)(?c)",
>>
>> Will the new server send the post-commit error to old clients? Will
>> the old clients know how to handle it? The old clients must continue
>> to work when used with the new server.
>>
> Exta elements in the tuples are ignored by both server and client. But
> it should be tested to make sure, of course.
>
> BUT, will the new client handle an old server? I'd say no, since the
> client will expect an extra element (?c). So you need to have an
> question mark before the left paren as well: "r(?c)(?c)?(?c)". Noe that
> you ned the parens to support further extensibility.
>
>
>
> Regards,
> //Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri May 20 14:19:00 2005