New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 662928 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 525042
Owner:
Closed: Nov 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Clean rietveld revert incorrectly commits one file as empty

Project Member Reported by jgruber@chromium.org, Nov 7 2016

Issue description

Version: https://chromium.googlesource.com/v8/v8/+/d5948caed59411d72fa27eca65d00a008b4e96f3

What steps will reproduce the problem?
(1) Created a revert on the web interface. The revert is clean, applied right after the original commit landed.

What is the expected output?

The state after the revert equals the state before the commit.

What do you see instead?

One file (regress-2825.js) is empty after the revert but non-empty before the revert. Presubmit checks fail because the file now does not have a copyright header (this is the only reason I noticed the missing content).

Original CL: https://codereview.chromium.org/2480223002/
Revert CL: https://codereview.chromium.org/2480283002/
 

Comment 1 by rmis...@google.com, Nov 7 2016

Cc: andyb...@chromium.org
Strange, looks like Rietveld did not calculate the patch diff for some reason.

Here is the patch of regress-2825.js (it is empty): https://codereview.chromium.org/2480283002/patch/1/100009
Here is the patch of regress-269.js (it looks right): https://codereview.chromium.org/2480283002/patch/1/90011

It is not obvious to me why it would fail to calculate the inverted patch :/
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/39b86ad453e795db162c84021793f209d3efc6e4

commit 39b86ad453e795db162c84021793f209d3efc6e4
Author: jgruber <jgruber@chromium.org>
Date: Mon Nov 07 15:46:31 2016

Manually complete failed revert

The revert somehow lost the contents of regress-2825.js.

NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
BUG= chromium:662928 

Review URL: https://codereview.chromium.org/2483863002 .

Cr-Commit-Position: refs/heads/master@{#40806}

[modify] https://crrev.com/39b86ad453e795db162c84021793f209d3efc6e4/test/mjsunit/regress/regress-2825.js

Maybe related: regress-2825.js has various line endings.

[...]
// Do not edit this file with an editor that replaces \r with \r\n.
// Variable definitions for i0 through i3 are each terminated with \r.
function f() {
  var i0 = 0;^M  var i1 = 1;^M  var i2 = 2;^M  var i3 = 3;
  var j0 = 0;
[...]

$ hexdump -c test/mjsunit/regress/regress-2825.js
[...]
0000160   (   )       {  \n           v   a   r       i   0       =    
0000170   0   ;  \r           v   a   r       i   1       =       1   ;
0000180  \r           v   a   r       i   2       =       2   ;  \r    
0000190       v   a   r       i   3       =       3   ;  \r  \n

In fact, when trying to manually fix the revert (over the web interface), the patch diff fails at exactly the affected lines: https://codereview.chromium.org/2483863002/

Mergedinto: 525042
Status: Duplicate (was: Assigned)
Rietveld is already known to not handle Windows line endings.

Sign in to add a comment