Issue 721: SVN backend doesn't cope well with deleted files

Reported by Patrick Georgi, Aug 9, 2011

Our SVN backend uses --revision, which fails if the file it is 
applied on doesn't exist in HEAD.

The following example is in a test repository with "foo" 
added in r1, and deleted in r2, with r2 being HEAD. 
The first variant is similar to what is used in IDF, the second is 
what the patch changes them to.

    $ svn info --revision 1 file:///home/patrick/t/f/foo
    svn: File not found: revision 2, path '/foo'
    
    $ svn info file:///home/patrick/t/f/foo@1
    Path: foo
    Name: foo
    URL: file:///home/patrick/t/f/foo
    Repository Root: file:///home/patrick/t/f
    Repository UUID: a3aa51bb-8de1-4ac8-9cb5-b0808b5602b1
    Revision: 1
    Node Kind: file
    Last Changed Author: patrick
    Last Changed Rev: 1
    Last Changed Date: 2011-08-09 19:32:03 +0200 (Tue, 09 Aug 2011)

20110809-1-fix-svn-with-deleted-files.diff - 10.05 kB - view

Comment 1 by Thomas Keller, Aug 12, 2011

Hrm... both syntaxes apparently have their pro's and con's, because 
we had the @REV syntax previously and it made problems in 
restructured repositories (see comment 19 in the linked issue 364).

So how does SVN behave if it gets both, --revision=1 and @1?

Relations: is related to 364

Comment 2 by Patrick Georgi, Aug 12, 2011

Thanks for the reminder.. I seemed to remember such a change, but 
wasn't sure about it anymore and couldn't find that issue.

--revision=NUM ..@NUM seems to work.

In general, the mechanism seems to work like this:
1. look up FILE@rev
2. look up information about that file as it existed in rev given by 
--revision.

I'll adapt the patch this way. Shall I push it to the repo directly?

Comment 3 by Thomas Keller, Aug 12, 2011

Please also test whether the --revision=NUM ..@NUM syntax also works 
for the case in the linked issue. If it does, then yes, please adapt 
the patch accordingly and push it to develop.

If you're courageous enough, you could also refactor the 
cmd-building code a bit so we don't DRY out...

Comment 4 by Patrick Georgi, Aug 12, 2011

My test scenario for --rev NUM ...@NUM was:

r1: create foo
r2: move foo to t/bar
r3: delete t/bar

This should properly replicate both cases (file is deleted, and file 
is moved), right?

Comment 5 by Thomas Keller, Aug 12, 2011

Well, you can test this positively by leaving out either parameter 
for r2 and r3 and see if it breaks "as expected", right?

I don't have a setup ready nor do I have time to test this myself at 
the moment, I just want to ensure that we don't dig new holes while 
patching others, that's why I'm nagging you... :)

Comment 6 by Patrick Georgi, Aug 12, 2011

Committed and pushed in r57c2389aae7926b9d69d7929a71c07a9e25654f2. 
Please review the test case. It's minimalist and doesn't cover all 
the methods, but it's good enough to test this particular issue.

Comment 7 by Thomas Keller, Aug 13, 2011

I did a few minor modifications in revision 2ff2f88, but the rest 
was already fine as is. Many thanks!

Labels: Milestone:Release1.2
Status: Fixed

Indefero