Welcome to the Splat alpha!

Some things are always checked during upload, but most things aren't. This hasn't
actually changed at all.

Still, verifying that the patch is good when it's uploaded is something we really,
really ought to be doing.

• Tags:

  -	Priority-Medium
  +	Priority-High
  +	Component-Reviews

• Summary:

  +	ReviewBoard does not verify patch when uploading

• Status:

  +	Confirmed

• Duplicates:

  +	522 - svn copy causes problems

bug 522 has been merged into this issue.
522 really asks for a fix for "svn copy". Rejecting it is merely a workaround for the
crash.

I noticed that diff 5 on this review has the same problem (I didn't post it, not sure
how an invalid path was posted):

http://reviews.review-board.org/r/527/diff/5/

• Duplicates:

  +	1217 - Enhanced error detection when uploading diff

• Duplicates:

  +	1197 - No Error Message when updating a diff that's malformed

My comment is more about Bug 522 : "svn copy" followed by "svn diff":

Subversion 1.5 is able to provide a somewhat correctly looking diff:

Index: targetver_copy.h
===================================================================
--- targetver_copy.h    (revision 152)
+++ targetver_copy.h    (working copy)
@@ -11,3 +11,4 @@
 #define _WIN32_WINNT 0x0501     // Change this to the appropriate value to target 
other versions of Windows.
 #endif

+// A tiny change

In this example the diff correctly shows a tiny change and the base revision yet 
contains the new name.

RB fails to find this new name in SVN and rejects the patch. 

Do you guys have a way to encode "I have copied a file and want to diff against its 
predecessor" information in the patch? If so, perhaps the quick fix would be to 
write a script to process such changes?

This appears to be fixed these days. I don't know when, but certainly in 1.5 it's no
longer a problem.

• Status:

  -	Confirmed
  +	Fixed

Not true...

• Status:

  -	Fixed
  +	New

I think the problem being described is actually a variation on Issue 2184

We still run in to this on an almost daily basis. In our case it happens when you post a review for a Mercurial revision that is on top of some other revisions that haven't been pushed to the server yet.

When you go to view the diff you get a message like:

Diff currently unavailable.
Error: The patch to 'tests/test_repository.py' didn't apply cleanly. The temporary files have been left in '/tmp/reviewboard.D5U2wT' for debugging purposes. `patch` returned: patching file /tmp/reviewboard.D5U2wT/tmpTJqk5x Hunk #1 FAILED at 86. Hunk #2 FAILED at 235. 2 out of 2 hunks FAILED -- saving rejects to file /tmp/reviewboard.D5U2wT/tmpTJqk5x-new.rej

for every file in the diff.

I think at the very least the server should perform a check when you hit the "Publish" button to make sure your diff applies before allowing you to publish it.

Bumped into this when updating a diff that had a parent with a diff that had the same parent (which I re-uploaded).

Prior to this attempt I had been able to update it fine. Now it will appear to work on the upload dialog (if I forget the parent diff it correctly prevents the upload) and fail:

The patch to '.project' didn't apply cleanly. The temporary files have been left in '/tmp/reviewboard.Pqwdm6' for debugging purposes. `patch` returned: patching file /tmp/reviewboard.Pqwdm6/tmpVG6SvM Reversed (or previously applied) patch detected! Assume -R? [n] Apply anyway? [n] Skipping patch. 1 out of 1 hunk ignored -- saving rejects to file /tmp/reviewboard.Pqwdm6/tmpVG6SvM-new.rej

Traceback (most recent call last):
  File "/usr/lib/python2.4/site-packages/ReviewBoard-1.6.9-py2.4.egg/reviewboard/diffviewer/views.py", line 151, in view_diff
    interdiffset, highlighting, True)
  File "/usr/lib/python2.4/site-packages/ReviewBoard-1.6.9-py2.4.egg/reviewboard/diffviewer/diffutils.py", line 1071, in get_diff_files
    large_data=True)
  File "/usr/lib/python2.4/site-packages/Djblets-0.6.19-py2.4.egg/djblets/util/misc.py", line 156, in cache_memoize
    data = lookup_callable()
  File "/usr/lib/python2.4/site-packages/ReviewBoard-1.6.9-py2.4.egg/reviewboard/diffviewer/diffutils.py", line 1070, in <lambda>
    enable_syntax_highlighting)),
  File "/usr/lib/python2.4/site-packages/ReviewBoard-1.6.9-py2.4.egg/reviewboard/diffviewer/diffutils.py", line 552, in get_chunks
    new = get_patched_file(old, filediff)
  File "/usr/lib/python2.4/site-packages/ReviewBoard-1.6.9-py2.4.egg/reviewboard/diffviewer/diffutils.py", line 374, in get_patched_file
    return patch(filediff.diff, buffer, filediff.dest_file)
  File "/usr/lib/python2.4/site-packages/ReviewBoard-1.6.9-py2.4.egg/reviewboard/diffviewer/diffutils.py", line 240, in patch
    raise Exception(_("The patch to '%s' didn't apply cleanly. The temporary " +
Exception: The patch to '.project' didn't apply cleanly. The temporary files have been left in '/tmp/reviewboard.Pqwdm6' for debugging purposes.
`patch` returned: patching file /tmp/reviewboard.Pqwdm6/tmpVG6SvM
Reversed (or previously applied) patch detected!  Assume -R? [n] 
Apply anyway? [n] 
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file /tmp/reviewboard.Pqwdm6/tmpVG6SvM-new.rej

A lot of this is fixed in the master branch.

There are still some issues when the diff won't apply correctly, but this is fixed enough that I'm going to close the bug.

• Status:

  -	New
  +	Fixed