Issue 2333: svn diff URL1 URL2 / diff URL2 URL1

[Malcolm Rowe <ma...@farside.org.uk>]

Hello all,

I've been looking at issue 2333. The initial report was that the output
from svn diff differs depending on the order of the arguments.

I think that the particular instance reported in that report can be
demonstrated by just the following (using svn 1.2.3):

$ svn log -r1084 -v svn://svn.debian.org/pkg-exim4/exim
------------------------------------------------------------------------
r1084 | zugschlus | 2005-04-03 07:36:17 +0100 (Sun, 03 Apr 2005) | 3 lines
Changed paths:
   D /exim/trunk/debian/exim4-config-simple
   A /exim4-config-simple/trunk/exim4-config-simple (from /exim/trunk/debian/exim4-config-simple:1083)

move exim4-config-simple to its own part of the repository, out of the
main package

------------------------------------------------------------------------
$ svn diff -r1083:1084 svn://svn.debian.org/pkg-exim4/
(which shows only the additions to /exim4-config-simple, and no deletions
to /exim)
or
$ svn diff -r1083:1084 svn://svn.debian.org/pkg-exim4/exim
(which returns no output)

I believe that the problem here is equivalent to that demonstrated with
the following script:

#/bin/sh

REPO=file://`pwd`/repo

rm -rf repo wc

svnadmin create repo
svn co $REPO wc

cd wc
mkdir A B A/P
echo content > A/P/x
svn add A B
svn ci -m 'initial checkin'

svn mv $REPO/A/P \
        $REPO/B \
        -m 'Move P from A to B'

Now compare the two following diffs, neither of which are displaying any
deletions (and also note the revision numbers that are displayed next
to each file compared with the revisions that were requested):

$ svn diff -r1:2 $REPO
Index: B/P/x
===================================================================
--- B/P/x       (revision 0)
+++ B/P/x       (revision 2)
@@ -0,0 +1 @@
+content

$ svn diff -r2:1 $REPO
Index: A/P/x
===================================================================
--- A/P/x       (revision 0)
+++ A/P/x       (revision 1)
@@ -0,0 +1 @@
+content

Note that the results shown above would be correct for 'svn diff -r0:2'
and '-r0:1' respectively.

Regards,
Malcolm

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Regression test for issue #2333

[Julian Foad <ju...@btopenworld.com>]

Malcolm Rowe wrote:
> Ping.
> 
> Is anyone able to review this patch or the original analysis, or does the
> lack of any response mean that I've committed some dreadful faux pas?

Sorry for the lack of response.  Thanks for paying attention to this issue, and 
for the reminder.  You're doing the right things, it's just that nobody in the 
last few days has taken it upon themselves to review this.  Like me, they may 
have had your message marked for attention but not got around to it yet.

> Analysis: http://svn.haxx.se/dev/archive-2005-08/1343.shtml
> Patch (new regression test): http://svn.haxx.se/dev/archive-2005-08/1363.shtml
> [Yes, there's a missing parenthesis in the log message. If this is all
> that's wrong, I'll be very happy.]
> 
> Summary for the impatient:
> svn diff produces the wrong output if a directory is renamed.
> 
> To reproduce:
> $ svn log -r14414 -v http://svn.collab.net/repos/svn
>   - shows /trunk/packages/rpm/wbel-3 being renamed to .../rhel-3.
> $ svn diff -r14413:14414 http://svn.collab.net/repos/svn | less
>   - shows only the additions, not the deletions.
> 
> Questions:
> 1. Is this a bug?

Yes.

> 2. Are the regression tests correct?

Yes, I think so.  And well written, thank you.

I've committed it as r16079.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Regression test for issue #2333

[Malcolm Rowe <ma...@farside.org.uk>]

Ping.

Is anyone able to review this patch or the original analysis, or does the
lack of any response mean that I've committed some dreadful faux pas?

Analysis: http://svn.haxx.se/dev/archive-2005-08/1343.shtml
Patch (new regression test): http://svn.haxx.se/dev/archive-2005-08/1363.shtml
[Yes, there's a missing parenthesis in the log message. If this is all
that's wrong, I'll be very happy.]

Summary for the impatient:
svn diff produces the wrong output if a directory is renamed.

To reproduce:
$ svn log -r14414 -v http://svn.collab.net/repos/svn
  - shows /trunk/packages/rpm/wbel-3 being renamed to .../rhel-3.
$ svn diff -r14413:14414 http://svn.collab.net/repos/svn | less
  - shows only the additions, not the deletions.

Questions:
1. Is this a bug?
2. Are the regression tests correct?

Thanks,
Malcolm

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

[PATCH] Regression test for issue #2333

[Malcolm Rowe <ma...@farside.org.uk>]

Following on from my original message, I've attempted to put together a
patch (attached, for review) to add a regression test for issue #2333.

The basic problem seems to be that there are problems with reporting diffs
following a directory rename. For example, a pre-commit repos-wc diff shows
only the deletion of the files under the old name, while a post-commit
repos-wc diff shows both the deletion _and_ the addition.

I'm fairly sure this is wrong, though I might not be entirely correct about
what the expected result should be.

Please be somewhat gentle with the review: not only is this my first
contributed patch to Subversion, it's also my first-ever Python program,
assembled cargo-cult-style from the other examples in that file.

I would appreciate a thorough review of the tests themselves, as it's
obviously pretty hard to double-check that the expected results are
correct without also fixing the problem, something that's beyond my
level of skill at the moment.

[[[
New XFail test for diff on a renamed directory, for issue #2333.

* subversion/tests/clients/cmdline/diff_tests.py
  (diff_renamed_dir: New test.
  (test_list): Add the new test, as XFail.
]]]

Regards,
Malcolm

Re: Issue 2333: svn diff URL1 URL2 / diff URL2 URL1

[Malcolm Rowe <ma...@farside.org.uk>]

Hi all,

After further analysis, I think that this may be the 'well-known' problem of
'svn diff' not being able to properly report on directory deletes, rather
than anything specifically to do with directory moves or renames.

(for example: 'svn diff -r15333:15334 http://svn.collab.net/repos/svn'
returns nothing, rather than the expected diff produced by the deletion of
the non-empty /trunk/doc/translations/norwegian_nb directory).

I say 'well-known' because, although there appears to be no issue logged or
XFail test, if you look in subversion/tests/clients/cmdline/diff_tests.py,
there's a commented out test, and comment that reads "### TODO: directory
delete doesn't work yet", so presumably somebody knows about it.

While I'm still not entirely sure what works and what doesn't (I seemed to
get some contradictory results while I was testing), I'm fairly sure that
repos->repos diffs never report directory deletions, whether from a delete
or a rename. Fortunately, the other common use cases seem to work fine.

I'm guessing that the primary cause of this is that the directory contents
aren't available to the client. (And indeed, for an update or merge against
a working copy, you wouldn't _want_ to download the deleted contents). So
any fix has probably got to add an option to extend the remote report to
include the complete contents of deleted directories (or have the client
fetch the directory contents out-of-band, but I suspect this would be even
harder, as well as being messier).

That sounds beyond what I might be able to do, unfortunately, so I probably
won't be spending any more time on this issue.

Regards,
Malcolm

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org