3966: Uploading a revision to a CR twice before publishing creates bad partial diffs
- Fixed
- Review Board
| physics********@gmai***** (Google Code) (Is this you? Claim this profile.) | |
 |
|
| 3993 |
What version are you running?
Reviewboard 2.0.19 and RBTools 0.7.4 for Windows
I am using a git repository that is mounted as a directory
on the reviewboard server.
What's the URL of the page containing the problem?
/r/232/diff/1-2/
What steps will reproduce the problem?
Summary: Upload and publish a CR then upload two move revisions
before publishing the latter. The 1-2 diff will be incorrect.
1. Create a code change with a diff (e.g. adding
a line to a file that reads "Change #1") and run 'rbt post'
2. Publish the review.
3. Update the code change (e.g. change the line to
say "Fake Change #2") and run 'rbt post -u'
4. On reviewboard, view the diffs orig-1, orig-2 and 1-2,
and verify that they show diffs between three versions
of a file with a blank line, 'Change #1' and 'Fake
Change #2', but DON'T publish.
5. Update the code change once more (e.g. change the line
to say 'Real Change #2) and run 'rbt post -u'
6. On reviewboard, view the diffs once more. Verify that
orig-2 shows that the line 'Real Change #2' has been
added. Look at the diff 1-2 and notice that it says
'Change #1' -> 'Fake Change #2', instead of
'Change #1' -> 'Real Change #2'.
What is the expected output? What do you see instead?
Expected output: All diffs accurately represent the difference
between the respective versions of the code change.
Actual output:
Diffs between orig and another commit are correct, but relative
diffs (1-2) are out of date and show non-existent changes.
What operating system are you using? What browser?
Windows 8.1 Pro 64-bit and latest Google Chrome (Version 45.0.2454.85 m)
Please provide any additional information below.
It appears that the relative diffs between intermediate revisions
and the latest revision are being stored at the moment that the latest
revision is uploaded and not updated when the revision is updated
prior to publishing it.
Thanks for the detailed reproduction steps! We generate the changes between diffs on-the-fly by patching the source files for each revision with the accompanying patches, and then generating a side-by-side diff. It sounds like this may be a caching issue. We've heard of such issues from a recent release, but haven't had a good repro case. I'll give this a try and see what we can see.
As a test, what happens if you clear the server cache? Does the problem fix itself?
-
+ NeedInfo
I looked up how to flush memcached and ended up running echo 'flush_all' | nc localhost 11211 This did not seem to fix the incorrect data. If you give me more specific steps to check I can try those.
Some new info. This appears to be a Chrome- or at least browser-related issue. I cleared Chrome's browsing history and the data corrected itself. It would seem Chrome thinks codereview/r/232/diff/1-2/ is a page it should cache, but codereview/r/232/diff/2/ is not. So at least the reviewer will see the correct diff, even if the reviewee cannot.
It looks like once I've looked at diff/2 and diff/1-2, they get cached in a cookie. When I update the request, looking at diff/2 gives me an updated diff and looking at diff/1-2 gives a 304 NOT MODIFIED.
I observed the same behavior this morning with 2.0.19 and Firefox (I don't have the FF version in front of me right now, but it should be 40.0.3 or very close). My procedure pretty much follows the one outlined above. Took me a little while to work through this. I kept re-posting from RBTools thinking I was losing my mind! Eventually a Ctrl-F5 to override the FF cache did the trick.
-
- NeedInfo + Confirmed -
+ Release-2.0.x -
- Priority:Medium + Component:DiffViewer + Priority:High -
+ chipx86
Fixed on release-2.0.x (commit dd7dcbd)
(Also, oops, I referenced the wrong bug in the commit, but yes, they are duplciates.)
-
- Confirmed + Fixed -
+ Interdiffs