Re: Why does "svn diff" forbid "-rCOMMITTED"?

cmpilato@collab.net wrote:
> Julian Foad <julianfoad@btopenworld.com> writes:
>
>>Ben Collins-Sussman wrote:
>>
>>>Julian Foad <julianfoad@btopenworld.com> writes:
>>>
>>>>What is the reason for this bit of code that forbids "-rCOMMITTED"?
>>>>It does not forbid "-rPREV".
>>>
>>>Huh... I'm not sure why we forbid COMMITTED, but not PREV. They're
>>>very similar revision keywords. cmpilato will be back from vacation
...
>>(re. the lines leading up to the message "do_diff: COMMITTED nomenclature not supported" in subversion/libsvn_client/diff.c)
>>
>>I propose to delete those five lines, thus enabling "COMMITTED" to be used.
>
> Thus enabling COMMITTED to be used incorrectly, you mean.
>
> What is the expected behavior when COMMITTED is used in conjunction
> with a URL input path? Does it peg one particular revision number for
> whole tree, or does it get effectively re-evaluated for each path
> (like BASE and WORKING do)?

First, do we agree on the meaning of COMMITTED?

  COMMITTED: the newest revision in which the item was changed before or at the BASE revision. (Therefore a working copy is required. In fact the WC admin data includes the COMMITTED revision number.) In particular, if the WC is not up to date then COMMITTED does not mean the most recently revision of the item in the repository.

  PREV: strictly (COMMITTED - 1).

These definitions are not fully stated in the Book or in the built-in help, but are what is implemented.

Second, do you agree that if PREV is allowed then COMMITTED should also be allowed, or conversely if COMMITTED is forbidden then PREV should be forbidden?

COMMITTED and PREV, like BASE and the (implied) WORKING revision types all require access to a working copy. There is a check in subversion/libsvn_client/diff.c (do_diff) that throws an error if a local-type revision is requested on a URL. That checks for BASE and WORKING and probably needs to be extended to cover COMMITTED and PREV as well. I believe that would prevent the problem that you forsee. It would allow COMMITTED to be used with locally-specified paths, just as PREV is already allowed.

Actually, that check is pretty redundant because a sensible error message comes out anyway. PREV with a URL (not checked by do_diff):

~/src/subversion> svn diff -rPREV:HEAD http://svn.collab.net/repos/svn/trunk/HACKING
svn: A path under revision control is needed for this operation
svn: svn_client__get_revision_number: need a version-controlled path to fetch local revision info.

BASE with a URL (is checked and forbidden by do_diff):

~/src/subversion> svn diff -rBASE:HEAD http://svn.collab.net/repos/svn/trunk/HACKING
svn: Incorrect parameters given
svn: do_diff: invalid revision specifier for URL path

I am looking into the whole issue of revision-type keywords. This is just a part of it.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Sep 20 01:15:26 2003