124: Broken raw-diff
- Fixed
- Review Board
| paolo******@gmai***** (Google Code) (Is this you? Claim this profile.) | |
 |
|
| Aug. 6, 2007 |
I have posted a patch generated with svn diff for review.
A colleague wants to download it to apply it to his working copy in order
to test it.
The raw-diff returned by review-board is broken.
It should be something like:
-------------
Index: src/org/tp23/antinstaller/input/InputField.java
===================================================================
--- src/org/tp23/antinstaller/input/InputField.java (revision 630)
+++ src/org/tp23/antinstaller/input/InputField.java (working copy)
@@ -52,7 +52,7 @@ public abstract class InputField extends
private boolean isRequired;
- private String suggestedValue;
+ protected String suggestedValue;
private String force;
-------------
But it is:
-------------
--- src/org/tp23/antinstaller/input/InputField.java (revision 630)
+++ src/org/tp23/antinstaller/input/InputField.java (working copy)
@@ -52,7 +52,7 @@ public abstract class InputField extends
private boolean isRequired;
- private String suggestedValue;
+ protected String suggestedValue;
private String force;
-------------
Note the missing "Index:" line.
I noticed this with CVS also. A simple test is that a downloaded raw-diff should be complete enough to turn around and submit a new review request. But since it's missing the extra header information (like RCS file), CVS breaks.
-
+ Component-DiffViewer
This is still broken for SCMTools with extra diff headers, like CVS. The RCS line is missing, and the Index line doesn't match what was in the original path. That's because the original patch may have an abbreviated path, but the SCMTool expands it to the path from the module root. Then when you're writing out the new raw patch, your synthesized "Index:" has the expanded path. For example: Original patch: Index: foo/bar.h =================================================================== RCS file: /repo/path/src/subdir/foo/bar.h,v retrieving revision 1.1 diff -u -p -r1.1 bar.h --- foo/bar.h 16 Jul 2007 00:00:00 -0000 1.1 +++ foo/bar.h 16 Jul 2007 01:00:00 -0000 [...] Broken raw-diff gives: Index: src/subdir/foo/bar.h =================================================================== --- foo/bar.h 16 Jul 2007 00:00:00 -0000 1.1 +++ foo/bar.h 16 Jul 2007 01:00:00 -0000 [...] I suspect that the extra headers aren't being stored in the DB, but they need to be.
Yeah, this is due to lsdiff giving us bad offsets and stuff. This will be fixed with my diff parsing code.
-
- david + chipx86
This should now be fixed in SVN (r852) for any newly created diffs. There's nothing we can do for older diffs.
-
- New + Fixed
Looks good, except that now I get doubled "Index" lines when downloading the raw patch. I think you need to revert r802, or modify it to only fake the Index lines for older diffs.