Malcolm Rowe wrote:
>
> I might try to get some time to look at the whole patch later this
> weekend, but initially, I'm wondering if there's any reason we need to
> announce the command's availability via a capability - does the editor
> interface break if we try unknown commands?
>
> IIRC we attempt to run at least lock, unlock, and log-related commands
> through the svn protocol and just report an error ('unsupported
> feature', I think) back to the client if that failed (and in the log
> case, I think we fall back to a slower method of doing the same thing).
>
> Can we not do that here?
>
In principle we might. The hacking document unfortunately does not not
provide any guidance on when the use which method. So, based on the "be
strict in what you sent & loose in what you receive"-paradigm that I
learned in my networking course (:-), I decided to use the capability
system to avoid sending an unknown request.
If we simply check for the "unsupported feature"-error, however, there
is the effect of errors on the pipe-lined editor-command system. If the
change-rev-prop editor command does not exist, I expect this system to
abort the whole commmit/edit run. I do not think that aborting the commit
for this reason is desirable in general. See next.
>
> Seperate to that is the question of what the command-line tools should
> do if run against an older server, of course.
>
I don't think that question is completely separate. Shouldn't we let the
client application decide whether the lack of the change-rev-prop editor
command is a fatal error? If the client application does not consider the
absence of the change-rev-prop command an error, then the commit should
proceeed, and the server cannot send back an error, right?
A client application could in principle follow up in such an error case
with a separate change-rev-prop command (ignoring for now the discussion
of the case where this command is refused by the server). This fallback
method would however not be possible if the commit has been aborted.
I guess this example provides a more functional reason to use the
capability method.
>
> Ignoring the problem (as I suspect the current patch does) probably
> isn't the right thing to do.
>
Yes, I fixed that. My code now returns the SVN_ERR_RA_NOT_IMPLEMENTED
error instead of SVN_NO_ERROR.
With regards,
Gerco.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Feb 24 14:46:02 2007