[PATCH] Weird 'svn diff' operation with --old/--new (was: Re: FreeBSD project and subversion.)

[Cross-posting to dev@ for patch review]

Hi,

I noticed a weird behavior of 'svn diff --old=... --new=...' where reversing
the old/new targets does not produce the reverse diff, as one would expect.

Here is the script for reproduction:

[[[
#!/bin/bash
 
REPO=/tmp/foo
WC=/tmp/foo.wc
 
rm -rf $REPO $WC
svnadmin create $REPO
svn co -q file://$REPO $WC
cd $WC
 
echo r1 > a
svn add -q a
svn ci -q -m R1
echo r2 > a
svn ci -q -m R2
svn up -q -r 1
echo new > a
echo "Issue: --old=A --new=B is not opposite of --old=B --new=A"
echo " Running: svn di --old=^/ --new=."
svn di --old=^/ --new=.
echo " Running: svn di --old=. --new=^/"
svn di --old=. --new=^/
]]]
 
And here is the output (svn 1.7.6):
[[[
Issue: --old=A --new=B is not opposite of --old=B --new=A
  Running: svn di --old=^/ --new=.
Index: a
===================================================================
--- a (.../file:///tmp/foo) (revision 2)
+++ a (working copy)
@@ -1 +1 @@
-r2
+new
  Running: svn di --old=. --new=^/
Index: a
===================================================================
--- a (working copy)
+++ a (.../file:///tmp/foo) (revision 2)
@@ -1 +1 @@
-r1
+r2
]]]

The reason for this behavior is that the code in diff-cmd.c makes the
following defaults for the old/new revisions:

      if (opt_state->start_revision.kind == svn_opt_revision_unspecified)
        opt_state->start_revision.kind = svn_path_is_url(old_target)
          ? svn_opt_revision_head : svn_opt_revision_base;

      if (opt_state->end_revision.kind == svn_opt_revision_unspecified)
        opt_state->end_revision.kind = svn_path_is_url(new_target)
          ? svn_opt_revision_head : svn_opt_revision_working;

These defaults make some sense, if the new target is not specified (it
defaults to old target in that case). If the new target is specified, and is
different from from old target, I am not so sure if it makes sense. Note that
there is a special case if both old/new targets are WC paths - in that case,
both old and new targets default to svn_opt_revision_working. So,

$ svn diff --old=foo --new=bar

will compare MODIFIED version of 'foo' against modified version of 'bar' [*],
but

$ svn diff --old=foo --new=^/bar

would compare BASE version of 'foo' against head version of 'bar. Does it make
sense? I would say, if both old and new are specified, old target's revision
should default to 'working' as well.

Problem is further aggravated since there is no commandline alias to request
svn_opt_revision_working setting from the command line: only 'head', 'prev',
'base' and 'committed' are currently available. Hence, it is not possible to
request the exact opposite of the patch using -rN:M syntax if one of the --
old/--new targets is a working copy path. Is there a reason why 'working' is
not parsed by revision_from_word()?

Also note that 'svn help diff' does not describe revision defaults for
synopsis 2 (and the default is different from synopsis 1, which requires old
revision to be specified for URLs).

PS. Stephan apparently made 'working' revision as default in his check-in
(r1442640) at least for the short-hand form, but Julian Foad, "optimizing the
logic" in r1442659 and r1442676, switched it back to be the same as --old/--
new logic.

PPS. If agreed to suggested change of default, the patch is attached.

[[[
Make svn diff --old=.. --new=.. default to WORKING revision for old target
if new target is explicitly specified.

* subversion/svn/diff-cmd.c
  (svn_cl__diff) Change defaults as described and simplify the logic.
]]]

Regards,
Alexey.