Re: svn commit: r10185 - in trunk/subversion: include libsvn_ra_svn svnserve

[Greg Hudson <gh...@MIT.EDU>]

On Thu, 2004-07-08 at 17:18, Peter N. Lundblad wrote:
> We need to be careful about backwards compability. This is a bug in 1.0
> servers, so we can't fall back gracefully.

Could I get clarification on this point?  The bug I'm aware of is in the
client (libsvn_client invokes ra_lib->change_revprop in violation of its
API, but since that's the only way to remove a revprop we want to amend
the API and make it work over ra_svn).

As I understand Josh's change:

  * Setting a revprop will continue to work, since the syntax for that
hasn't changed.  (That's why I didn't suggest using (?s) for the value,
even though that's what we would have done originally had we thought
about this.)

  * Deleting a revprop will work against a 1.1 server.  Against a 1.0
server, it will fail with "Connection closed unexpectedly" (server sees
malformed network data and closes the connection).  This isn't graceful,
but it's certainly no worse than the assertion failure you get with a
1.0 client, so I'm not sure how much machinery is warranted making it
fail more gracefully against an old server.

(We could backport this change to 1.0.6, in which case it's a >=1.0.6
vs. <=1.0.5 issue instead of a 1.1 vs. 1.0 issue.)

> > -    params:   ( rev:number name:string value:string )
> > +    params:   ( rev:number name:string [ value:string ] )

Hm, I hadn't realized the protocol document has no way of expressing a
tuple with an optional part, unless the entire contents of the tuple are
optional.  I'll take care of that.


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

Re: svn commit: r10185 - in trunk/subversion: include libsvn_ra_svn svnserve

[kf...@collab.net]

"Peter N. Lundblad" <pe...@famlundblad.se> writes:
> Adding a remove-rev-prop command would be nearly as simple and allows an
> informative message. That said, the current fix is clearly an improvement,
> but we will still have to answer user question about closed connections
> when deleting revprops.

1.0.6 will probably be the shortest-lived 1.0.x release we ever make,
so let's just leave this change as-is, deal with any questions that
come up, and pretty soon everyone will be in a >=1.1 universe and the
problem will go away by itself.


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

Re: svn commit: r10185 - in trunk/subversion: include libsvn_ra_svn svnserve

["Peter N. Lundblad" <pe...@famlundblad.se>]

On Fri, 9 Jul 2004, Greg Hudson wrote:

> On Fri, 2004-07-09 at 04:51, Peter N. Lundblad wrote:
> > >   * Deleting a revprop will work against a 1.1 server.  Against a 1.0
> > > server, it will fail with "Connection closed unexpectedly" (server sees
> > > malformed network data and closes the connection).  This isn't graceful,
> > > but it's certainly no worse than the assertion failure you get with a
> > > 1.0 client, so I'm not sure how much machinery is warranted making it
> > > fail more gracefully against an old server.
> > >
> > If we added a way for the client to know that it shouldn't issue a
> > change-rev-prop without the value, it can give a better error message.
> > "Connection closed..." is confusing, to say the least.
>
> Uh, sure, there's no question that we *could*, with enough effort,
> produce a better error message in this case.  But my point is, the
> simple fix doesn't make it any worse than it was already, and simplicity
> is good.
>
Adding a remove-rev-prop command would be nearly as simple and allows an
informative message. That said, the current fix is clearly an improvement,
but we will still have to answer user question about closed connections
when deleting revprops.

Regards,
//Peter

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

Re: svn commit: r10185 - in trunk/subversion: include libsvn_ra_svn svnserve

[Greg Hudson <gh...@MIT.EDU>]

On Fri, 2004-07-09 at 04:51, Peter N. Lundblad wrote:
> >   * Deleting a revprop will work against a 1.1 server.  Against a 1.0
> > server, it will fail with "Connection closed unexpectedly" (server sees
> > malformed network data and closes the connection).  This isn't graceful,
> > but it's certainly no worse than the assertion failure you get with a
> > 1.0 client, so I'm not sure how much machinery is warranted making it
> > fail more gracefully against an old server.
> >
> If we added a way for the client to know that it shouldn't issue a
> change-rev-prop without the value, it can give a better error message.
> "Connection closed..." is confusing, to say the least.

Uh, sure, there's no question that we *could*, with enough effort,
produce a better error message in this case.  But my point is, the
simple fix doesn't make it any worse than it was already, and simplicity
is good.

> BTW, do we want to document the 1.1 additions to the protocol (I am
> thinking of get-locations and get-file-revs), so other clients know that
> these might fail with "unknown command"?

Well, we do have version control on the file itself, so I'm not sure
that's really necessary.  Perhaps we could add a note that the protocol
document may be extended in compatible ways, and people should check out
prior versions if they're interested in interacting with prior-version
clients and servers.


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

Re: svn commit: r10185 - in trunk/subversion: include libsvn_ra_svn svnserve

["Peter N. Lundblad" <pe...@famlundblad.se>]

On Thu, 8 Jul 2004, Greg Hudson wrote:

> On Thu, 2004-07-08 at 17:18, Peter N. Lundblad wrote:
> > We need to be careful about backwards compability. This is a bug in 1.0
> > servers, so we can't fall back gracefully.
>
> Could I get clarification on this point?  The bug I'm aware of is in the
> client (libsvn_client invokes ra_lib->change_revprop in violation of its
> API, but since that's the only way to remove a revprop we want to amend
> the API and make it work over ra_svn).
>
> As I understand Josh's change:
>
>   * Setting a revprop will continue to work, since the syntax for that
> hasn't changed.  (That's why I didn't suggest using (?s) for the value,
> even though that's what we would have done originally had we thought
> about this.)
>
>   * Deleting a revprop will work against a 1.1 server.  Against a 1.0
> server, it will fail with "Connection closed unexpectedly" (server sees
> malformed network data and closes the connection).  This isn't graceful,
> but it's certainly no worse than the assertion failure you get with a
> 1.0 client, so I'm not sure how much machinery is warranted making it
> fail more gracefully against an old server.
>
If we added a way for the client to know that it shouldn't issue a
change-rev-prop without the value, it can give a better error message.
"Connection closed..." is confusing, to say the least.

> (We could backport this change to 1.0.6, in which case it's a >=1.0.6
> vs. <=1.0.5 issue instead of a 1.1 vs. 1.0 issue.)
>
Then it is less of a problem, since we recommend people to upgrade anyway.

> > > -    params:   ( rev:number name:string value:string )
> > > +    params:   ( rev:number name:string [ value:string ] )
>
> Hm, I hadn't realized the protocol document has no way of expressing a
> tuple with an optional part, unless the entire contents of the tuple are
> optional.  I'll take care of that.
>
Good.

BTW, do we want to document the 1.1 additions to the protocol (I am
thinking of get-locations and get-file-revs), so other clients know that
these might fail with "unknown command"?

Regards,
//Peter

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