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

Re: Issue #2873, diff for a single added file

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Tue, 26 Feb 2008 18:09:25 +0000

Martin Hauner wrote (on 9th Feb):
> i am looking at issue 2873
> (http://subversion.tigris.org/issues/show_bug.cgi?id=2873 ).
> It is about creating a diff for a single added file. This currently
> fails with a file doesn't exist in revision error.

Thanks for looking at this, Martin.

[...]
> My diff case is handled in diff_repos_repos which calls
> diff_prepare_repos_repos, which
> in turn calls svn_client__repos_locations.
>
> There are two places where it fails. The first failures happens in
> svn_client__repos_locations.
> It tries find the path at the two revisions. That fails for the
> revision without the file. I have
> added a must_exist parameter which (if set to FALSE) makes it set the
> missing location to
> the same location as the other revision. This lets the method run
> without failure.

OK - yes, you need to do something so that the caller can determine the
non-existence. I'm not sure that setting the missing location equal to the
other location is the right solution. That sounds like providing false information.

> The next error happens in diff_prepare_repos_repos after returning from
> svn_client__repos_locations.
> svn_ra_check_path is run on both locations. If the path check returns
> svn_node_node
> it fails. By disabling both svn_node_none checks, it returns the wanted
> single added file patch.
>
> I wonder why this check is there at all. Even if it is not what the
> user wished to see, why
> shouldn't svn create a diff against a revision where the path does not
> (yet) exist?

I've looked back at some of the history of this function with "svn blame".

Once upon a time, it said "if the two URLs are the same, then allow one of them
to be missing". (I don't know why it was only allowed if the two URLs were the
same.) This presumably worked well for your case of an added file, but
unfortunately it didn't work for something that had been MOVED rather than
added - bug report #1616. Therefore this special case was removed.

r7897:
[[[
Finish issue #1616 -- 'svn diff -rX:Y wcpath' may lie.

* subversion/libsvn_client/diff.c
   (diff_repos_repos): Remove the special-case exception of the "both
     sides of the diff must exist" policy.

* subversion/tests/clients/cmdline/diff_tests.py
   (diff_targets): Expect the new diff behavior.
]]]

At that time, I think Subversion could not trace the history across a move, but
now it can (if it searches backward from the more recent version). It now
handles the case of something that has been moved, and I think therefore we can
now allow a diff in which one side is missing.

> Attached is a small patch with my changes (against 1.4.6).

I understand it's not a patch that is ready to submit so you didn't try to
present it as such, but you could make it easier for people like me to read and
understand the next patch if you do some of the things requested in
<http://subversion.tigris.org/hacking.html#patches>.

Does the test suite pass with this patch?

If so, this might be quite close to being ready.

Thanks.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-02-26 19:09:41 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.