[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

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

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 7 Feb 2013 16:00:49 +0000 (GMT)

Alexey Neyman wrote:
> [Cross-posting to dev@ for patch review]
> 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.

Hi Alexey.  Thanks for the report.  As you pointed out, I changed something in this area recently, so I'll take a closer look and fix it.  I haven't done so yet but should get to it in the next day or two.

[...]
> 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.

Oops, I didn't notice I was changing the logic.  As my log message said, I thought I was just simplifying it.

> 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.

A log message should always indicate *why* the change was made.


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

- Julian
Received on 2013-02-07 17:01:25 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.