Peter N. Lundblad wrote:
> On Thu, 8 Jul 2004 jpieper@tigris.org wrote:
>
> > Author: jpieper
> > Date: Thu Jul 8 14:50:51 2004
> > New Revision: 10185
> >
> > Modified:
> > trunk/subversion/include/svn_ra.h
> > trunk/subversion/libsvn_ra_svn/client.c
> > trunk/subversion/libsvn_ra_svn/protocol
> > trunk/subversion/svnserve/serve.c
> > Log:
> > Fix revprop deletion when using ra_svn. Make the "value" portion of
> > the change-rev-prop command be optional. If it is missing, treat the
> > revprop change as a deletion.
> >
> We need to be careful about backwards compability. This is a bug in 1.0
> servers, so we can't fall back gracefully. But we want a better error
> message for an old server. Maybe it is time for a new capability keyword.
> ghudson? Otherwise, I'd say we need a new remove-rev-prop command. Also,
> see below.
I discussed this briefly with ghudson on IRC and he said this would be
a backwards compatible way to make the change.
> > Modified: trunk/subversion/libsvn_ra_svn/client.c
> > Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/libsvn_ra_svn/client.c?view=diff&rev=10185&p1=trunk/subversion/libsvn_ra_svn/client.c&r1=10184&p2=trunk/subversion/libsvn_ra_svn/client.c&r2=10185
> > ==============================================================================
> > --- trunk/subversion/libsvn_ra_svn/client.c (original)
> > +++ trunk/subversion/libsvn_ra_svn/client.c Thu Jul 8 14:50:51 2004
> > @@ -641,7 +641,7 @@
> > ra_svn_session_baton_t *sess = baton;
> > svn_ra_svn_conn_t *conn = sess->conn;
> >
> > - SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "change-rev-prop", "rcs",
> > + SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "change-rev-prop", "rc?s",
> > rev, name, value));
> > SVN_ERR(handle_auth_request(sess, pool));
> > SVN_ERR(svn_ra_svn_read_cmd_response(conn, pool, ""));
> >
> > Modified: trunk/subversion/libsvn_ra_svn/protocol
> > Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/libsvn_ra_svn/protocol?view=diff&rev=10185&p1=trunk/subversion/libsvn_ra_svn/protocol&r1=10184&p2=trunk/subversion/libsvn_ra_svn/protocol&r2=10185
> > ==============================================================================
> > --- trunk/subversion/libsvn_ra_svn/protocol (original)
> > +++ trunk/subversion/libsvn_ra_svn/protocol Thu Jul 8 14:50:51 2004
> > @@ -203,7 +203,7 @@
> > response: ( rev:number )
> >
> > change-rev-prop
> > - params: ( rev:number name:string value:string )
> > + params: ( rev:number name:string [ value:string ] )
> > response: ( )
> >
> > rev-proplist
> >
> The code doesn't match the protocol spec. [ ] means an optional *tuple*,
> not an abritrary item. So you need to add parentheses around the value
> (i.e. "(?s)" to be consistent in the protocol. Then new servers would need
> to accept an old-style request, which shouldn't be hard.
>
> So, either we are consistent and change the syntax of change-rev-prop, or
> we add a new command. I don't know which is best.
I did not discuss the change to the protocol document with him, so
that mistake is solely mine. I don't particularly have any strong
feelings on whether this should be implemented with a new command or
an inconsistent extension. I'll leave that up to the ra_svn experts.
-Josh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jul 9 02:19:48 2004