subversion-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexey Neyman <sti...@att.net>
Subject Re: FreeBSD project and subversion.
Date Tue, 05 Feb 2013 22:22:51 GMT
On Tuesday, February 05, 2013 01:55:20 PM Lathan Bidwell wrote:
> On 02/05/2013 01:14 PM, Stefan Sperling wrote:
> > On Tue, Feb 05, 2013 at 09:19:19AM -0800, Alexey Neyman wrote:
> >> There is one more weird issue with svn diff, see the script below. The
> >> issue is that "--old=A --new=B" is not opposite of "--old=B --new=A". I
> >> don't know if it is a bug or another ambuguity I am not aware of :)
> >> 
> >> Here is the script:
> >> [[[
> >> #!/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
> >> ]]]
> >> 
> >> Regards,
> >> Alexey.
> > 
> > I can reproduce this with a trunk build. Can you please file an issue
> > for this? I'm not going to get to this right away. Thanks!
> 
> Here is the issue that I see:
> 
> The --old=. get's the workspace version of ., but --new get's the
> non-changed version of /.
> 
> So, I believe it is comparing "r1" with the r2 contents of "r2" and not
> comparing both workspace versions.
>
> Rev    Contents
> 1        r1
> 2        r2
> 2*      new
> 
> 2* only get's referenced when it is in the --old position,
> 2 get's used when its in the --new position.
> 
> Please correct me if I'm wrong about this, but that is what seems to be
> a logical reasoning behind that behavior.

First, it is just the opposite: 2* gets referenced when it is in the --new, 
not --old position. Second, it is not the reasoning (why it behaves so) but 
rather a description of current behavior, which I think was rather obvious 
from the example script.

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, given that new target is optional and defaults 
to old target if not specified. If 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.
Mime
View raw message