442: Diff viewer choking on svn diffs with deleted files

mmastr******@gmai***** (Google Code) (Is this you? Claim this profile.)
March 22, 2009 
When I move or delete files in my svn tree, reviewboard chokes on the svn
diff file (it looks like it's trying to actually do diffs on files that are
no longer there).

What's the URL of the page containing the problem?
http://reviewboard.glgdev.com/r/225/diff/#index_header
What steps will reproduce the problem?
1. Delete a couple source files, or move them from one package (my source
tree is in java).
2. submit a review
 
You will see output looking something like the following when you try to
view the diff in reviewboard:



The patch to
'svn://glgdevss01/GLG_JAVA_REPO/sandbox/cod/backend/src/java/com/glgroup/entityextraction/DrugFinder.java'
didn't apply cleanly. The temporary files have been left in
'/tmp/reviewboard.FixWRp' for debugging purposes. `patch` returned:
patching file /tmp/reviewboard.FixWRp/tmpdHbLxX 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.FixWRp/tmpdHbLxX-new.rej

Traceback (most recent call last):
  File "/home/tree/reviewboard/reviewboard/diffviewer/views.py", line 81,
in view_diff
    files = get_diff_files(diffset, None, interdiffset, highlighting)
  File "/home/tree/reviewboard/reviewboard/diffviewer/diffutils.py", line
561, in get_diff_files
    enable_syntax_highlighting)
  File "/home/tree/reviewboard/reviewboard/diffviewer/diffutils.py", line
500, in generate_files
    lambda: get_chunks(filediff.diffset,
  File "/home/tree/reviewboard/reviewboard/djblets/util/misc.py", line 47,
in cache_memoize
    data = lookup_callable()
  File "/home/tree/reviewboard/reviewboard/diffviewer/diffutils.py", line
503, in <lambda>
    enable_syntax_highlighting))
  File "/home/tree/reviewboard/reviewboard/diffviewer/diffutils.py", line
299, in get_chunks
    new = get_patched_file(old, filediff)
  File "/home/tree/reviewboard/reviewboard/diffviewer/diffutils.py", line
212, in get_patched_file
    return patch(filediff.diff, buffer, filediff.dest_file)
  File "/home/tree/reviewboard/reviewboard/diffviewer/diffutils.py", line
110, in patch
    (filename, tempdir, p.stdout.read()))
Exception: The patch to
'svn://glgdevss01/GLG_JAVA_REPO/sandbox/cod/backend/src/java/com/glgroup/entityextraction/DrugFinder.java'
didn't apply cleanly. The temporary files have been left in
'/tmp/reviewboard.FixWRp' for debugging purposes.
`patch` returned: patching file /tmp/reviewboard.FixWRp/tmpdHbLxX
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.FixWRp/tmpdHbLxX-new.rej


I'm running windows xp and using firefox.
david
#1 david
  • +Component-DiffViewer
#2 c.gosch%*********@gtempacc******** (Google Code) (Is this you? Claim this profile.) 
This is equal to issue 394.
chipx86
#3 chipx86 
It's a completely separate thing, unrelated to issue 394. SVN and CVS handle
revisions and deleted files differently.
#4 arthu******@gmai***** (Google Code) (Is this you? Claim this profile.) 
Any word on this issue? Moving around/deleting files is a pretty common task, so this breaks a lot of reviews.
#5 pru****@gmai***** (Google Code) (Is this you? Claim this profile.) 
I'm experiencing the same thing.
I think it's an svn problem generating the patch, but I don't know if reviewboard can
do something to avoid this error, it's really annoying not being able to review a
file move.
#6 arthu******@gmai***** (Google Code) (Is this you? Claim this profile.) 
As it's been suggested in other places, you can just do a post review.
#7 jeff.ham*********@gmai***** (Google Code) (Is this you? Claim this profile.) 
Hey Arthur: what do you mean by "you can just do post review"?
#8 arthu******@gmai***** (Google Code) (Is this you? Claim this profile.) 
Well there's a post-review script that lets you create a review after it's been committed. See here: http://code.google.com/p/reviewboard/wiki/Using_PostReview
david
#9 david 
As was mentioned, svn doesn't generate a correct diff for this situation. The
post-review script does.
  • +NotABug
#10 fbea*****@gmai***** (Google Code) (Is this you? Claim this profile.) 
We too have been experiencing this issue since summer 2009.  We use svn client 1.6+.
 The diff format changed somewhere around svn 1.5.  Unfortunately, we don't use
post-review and if we use "svn mv" we can't post the DIFF we generated with "svn
diff" in RB 1.0 or 1.5beta.

The priority of this issue should be escalated because:
- not everybody uses post-review - especially since breaking larger changesets into
multiple reviews seems easiest when done manually
- the post-review script looks to be doing some interesting things to the subversion
diff to make it consumable by RB...is that really how we want to drive the user
experience (i.e. forcing users to use post-review)?
- @trowbrds is referencing an old documentation page which, when redirected, contains
references to the latest/trunk CLI-options like --diff-filename that don't apply to
all versions of post-review.