'svn mv' seen as just a delete by upload.py

[GoogleCodeExporter]

When moving files (with 'svn mv oldfile newfile', without editing the new
file) and uploading them with upload.py, Rietveld displayed just the delete
of oldfile, not the addition of newfile. upload.py also only said
'Uploading base file for oldfile'.

This may be related to issue 82:
http://code.google.com/p/rietveld/issues/detail?id=82

Original issue reported on code.google.com by thomaswout@gmail.com on 13 May 2009 at 11:18

[GoogleCodeExporter]

Can you give a short example how to reproduce this issue? It would be helpful 
to see
the output of "svn diff" before uploading too. 

Original comment by albrecht.andi on 15 May 2009 at 4:37

[GoogleCodeExporter]

This seems to be a systemic problem, relating to the svn diff format.
The Mercurial (and git, with the patch for issue 140) backends both use the 'new
file' name in the Index: line, which drops all information about the 'old file'
(although that's still present in the raw diff).  I'd like to see what svn diff
generates for this case, since I can't fixup the git/Mercurial backends until I 
know
what the appropriate SVN case would look like.

Original comment by canan...@gmail.com on 19 Aug 2009 at 11:05

[GoogleCodeExporter]

Short example how to reproduce:
> svn co http://scons.tigris.org/svn/scons/trunk scons
> cd scons
> svn up -r 5163
> svn move review.py bin\review.py
> upload.py
> svn st > svn_st.log
> svn diff > svn_diff.log

See upload.py result at http://codereview.appspot.com/2017045/

Original comment by techtonik@gmail.com on 24 Aug 2010 at 11:16

Attachments:

• svn_st.log
• svn_diff.log

[GoogleCodeExporter]

Thanks for the attachments and the example. The problem we have is that 
upload.py constructs everything around the diff (see upload.py in RealMain). 
And the diff in that case is just wrong. When applying the diff (and that is 
actually what the review server does) review.py is removed, but bin/review.py 
isn't created. How would a contributor generate a patch for someone with commit 
privileges in this case?

Original comment by albrecht.andi on 25 Aug 2010 at 3:45

[GoogleCodeExporter]

For people with commit privileges the patch should be in Git format to handle 
renames and copies (Git support will likely to appear in Subversion 1.7). But 
as a poor man's hack we can just add missing files manually.

1. use `svn st` to get missing files ("A  +" signature)
2. use `svn propget svn:mime-type ...` != application/octet-stream to detect if 
file is text

Original comment by techtonik@gmail.com on 25 Aug 2010 at 4:16

[GoogleCodeExporter]

Ok. I've reproduced the diff for moved README -> README.txt at 
http://codereview.appspot.com/2050041 patch set 1. The result is ugly. 
Generated diff applied to uploaded base file results in duplicated content in 
side-by-side diff view for README.txt, but! downloaded raw patch set is ok and 
can be used to create README.txt

IMHO this perfectly illustrates why SVN diff format could not be used for 
storing change sets on server side. I propose to replace it with some internal 
format, from which it will be possible to generate diffs if Git or SVN format 
with obligatory warnings about information loss due to the presence of binary 
files or copies/moves/prop changes.

Original comment by techtonik@gmail.com on 27 Aug 2010 at 5:39

[GoogleCodeExporter]

Original comment by techtonik@gmail.com on 27 Sep 2010 at 5:31

• Added labels: Priority-High
• Removed labels: Priority-Medium

[GoogleCodeExporter]

Original comment by albrecht.andi on 6 Apr 2012 at 7:19

• Added labels: Component-Scripts