[PATCH] 'svn' should not silently ignore a peg revision

I believe we should make 'svn' reject any peg revision specifier on an
argument where it doesn't use one. This is a rather long email to
record some of the details behind my reasoning, and a question mainly
for Stefan Sperling about issue #3651 [3].

First some context.

$ svn cat ^/trunk/readme_at_123
  # Makes sense.

$ svn mkdir ^/trunk/doc_at_123
  # Nonsense: mkdir with a URL can only operate on the head. [2]

$ svn add foo_at_123
  # Nonsense: the target must be an unversioned local path.

We decided [1] that we should allow the user to consistently escape any
target argument by appending an '@' sign, without having to know whether
a peg revision is relevant. This was implemented in 1.6.5 and trunk by
parsing off and discarding any peg revision specifier, using the
function 'svn_cl__eat_peg_revisions()'.

That current solution is great for allowing consistent escaping, but as
a side effect 'svn' silently ignores any peg revision it isn't going to
use, which I think is bad, as I have noted in the doc string of
svn_cl__eat_peg_revisions().

So, when does a peg not make sense?

  (a) When the target is supposed to be an unversioned local path, such
as in 'svn add'. A peg never makes sense on an unversioned path. A
versioned local path is allowed with '--force', but it would still be
wrong to specify any peg revision on it because only the path on disk is
relevant to the 'add' operation.

  (b) When the target necessarily refers to a path at the head of the
repository, such as in 'svn lock TARGET' or 'svn delete TARGET-URL'.
Logically, a peg of 'HEAD' is always valid in these commands.
Logically, these commands could accept a non-head peg where TARGET is
known to exist. Then, if someone else moves TARGET to another path
while we are composing and executing this command, Subversion could do
its best to follow the move or error out if it can't do so. But unless
and until 'svn' actually uses the peg revision in this way, it should
complain, not silently ignore it.

  (c) When the target refers to a new path that will be created in a new
revision on top of the old head, such as in 'svn mkdir TARGET-URL' or
the destination of 'svn copy SOURCE DEST-URL'. In most of these cases,
the target does not exist in the repository at the time the command is
issued so there is no possible peg revision specifier. The target of a
copy may be specified as an existing directory, and in that case it
could accept a peg of 'HEAD' or even a non-head peg as described in (b).

My question is about this case:

  $ svn copy SOURCE TARGET_at_HEAD

This case falls under the last sentence of point (C) above. Issue #3651
[3] claims that the 'HEAD' peg should be ignored in this case, and that
claim is explicitly tested by copy_tests 62 [4]. (It is the only test
that fails with the attached patch.)

But is 'HEAD' supposed to be a special case on the basis that it is
redundant but still correct? If so, then we should also allow '123' iff
that is currently the head, and 'BASE' for a local path iff that is
currently the head, and so on. In other words I don't think we have
reason to claim that 'HEAD' is a special case.

The logic gets very messy if we think we must support redundant pegs.
Even if we only implement a subset of the messy logic (such as just
allowing 'HEAD') the conceptual logic is messy which is a bad thing for
documentation and learning. So my conclusion is that we should reject
any peg revision specifier, even 'HEAD', in places where we're not
prepared to use it.

If that's agreeable, I'll make it happen. (The attached patch is not
quite finished; for example it has no doc string for one of the new
functions.)

- Julian

[1] Issue #3416 "Cannot add or commit 'dir/@file.txt'",
<http://subversion.tigris.org/issues/show_bug.cgi?id=3416>.
[2] The only ways I can imagine the second example could make sense are
if it were to mean, "Make this directory, assuming that HEAD is 123, and
fail if not" or, "Make this directory, assuming that HEAD is 122 so that
the committed revision will be 123, and fail if not." But we don't have
any plans to implement such semantics, and the syntax does not make
sense for the semantics we have implemented.
[3] Issue #3651 "svn copy does not eat peg revision within copy target
path", <http://subversion.tigris.org/issues/show_bug.cgi?id=3651>.
[4] <http://svn.apache.org/viewvc?revision=952992&view=revision>