522: svn copy causes problems

mkoe****@gmai***** (Google Code) (Is this you? Claim this profile.)
March 24, 2009
583 
What steps will reproduce the problem?
1. create a new file using "svn copy"
2. create the diff and upload it
3. view the diff

What is the expected output? 
the view off the diff

What do you see instead?
he patch to 'newFile.pl' didn't apply cleanly. The temporary files
have been left in '/tmp/reviewboard.HioriF' for debugging purposes.
`patch` returned: patching file /tmp/reviewboard.HioriF/tmpZSh4CR Hunk
#1 FAILED at 1. Hunk #2 FAILED at 14. 2 out of 2 hunks FAILED --
saving rejects to file /tmp/reviewboard.HioriF/tmpZSh4CR-new.rej

Traceback (most recent call last):
    File "/home/mkobele/reviewboard/diffviewer/views.py", line 82, in
view_diff
files = get_diff_files(diffset, None, interdiffset, highlighting)
    File "/home/mkobele/reviewboard/diffviewer/diffutils.py", line
594, in get_diff_files
    enable_syntax_highlighting)
    File "/home/mkobele/reviewboard/diffviewer/diffutils.py", line
513, in generate_files
    lambda: get_chunks(filediff.diffset,
            File "/home/mkobele/reviewboard/djblets/util/misc.py",
line 49, in cache_memoize
            data = lookup_callable()
            File "/home/mkobele/reviewboard/diffviewer/diffutils.py",
line 516, in <lambda>
            enable_syntax_highlighting))
    File "/home/mkobele/reviewboard/diffviewer/diffutils.py", line
317, in get_chunks
new = get_patched_file(old, filediff)
    File "/home/mkobele/reviewboard/diffviewer/diffutils.py", line
233, in get_patched_file
return patch(filediff.diff, buffer, filediff.dest_file)
    File "/home/mkobele/reviewboard/diffviewer/diffutils.py", line
119, in patch
(filename, tempdir, patch_output))
    Exception: The patch to 'newFile.pl' didn't apply cleanly. The
temporary files have been left in '/tmp/reviewboard.HioriF' for
debugging purposes.
    `patch` returned: patching file /tmp/reviewboard.HioriF/tmpZSh4CR
    Hunk #1 FAILED at 1.
    Hunk #2 FAILED at 14.
    2 out of 2 hunks FAILED -- saving rejects to file /tmp/
reviewboard.HioriF/tmpZSh4CR-new.rej


Please provide any additional information below.
The problem might be that the diff header looks like this:

Index: newFile.pl
===================================================================
--- newFile.pl  (revision 0)
+++ newFile.pl  (working copy)
@@ -1,8 +1,8 @@
 #!/usr/bin/perl
 #
[...]

If it was a brand new file (svn add as opposed to svn copy)
the header would like this:

Index: newFile.pl
===================================================================
--- newFile.pl  (revision 0)
+++ newFile.pl  (revision 0)
@@ -0,0 +1,202 @@
 #!/usr/bin/perl
 #
[...]
chipx86
#1 chipx86 
Try to use the post-review tool to generate/upload the diff and see if that fixes
your problem.
  • +NeedInfo
#2 mkoe****@gmai***** (Google Code) (Is this you? Claim this profile.) 
I will try that.
But that would be merely a workaround and not a fix for this issue.

I hope the core developers of review-board can have a look at it and at least avoid
that exception as seen above.

Thanks.
chipx86
#3 chipx86 
We can't do much, though. SVN diffs that do copies or renames provide broken
information and we can't know it's broken. The best we can do is display an inline
error for that file, or just drop it. Neither are great solutions. This is why
post-review must be used for these cases, as we can generate our own fixed diff.
  • -NeedInfo
    +New
  • +Component-SCMTools
david
#4 david 
With SVN, it's very hard (if not impossible) to tell the difference between a file
that has a broken copy and a file which is uploaded from the web UI with the wrong
base diff path. We may want to come up with a generic error to show in this case.

Either way, we ought to check the diff for problems when it gets uploaded instead of
when it's viewed.
  • -New
    +Confirmed
  • -Priority-Medium
    +Priority-Low
david
#5 david 
Speaking of which, I'm going to merge this into bug 583.
#6 mkoe****@gmai***** (Google Code) (Is this you? Claim this profile.) 
does this mean that "svn copy" will not be fixed?
bug 583 talks about validation, will reviewboard then simply reject diffs containing
files created by a svn copy?
david
#7 david 
There's no way for us to "fix" this. "svn diff" is what's broken, we can merely work
around it.