New issue
Advanced search Search tips

Issue 685715 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Rietveld and git reporting different committed code [diff].

Project Member Reported by erikc...@chromium.org, Jan 26 2017

Issue description

I recently landed two CLs:

1) https://codereview.chromium.org/2649973003/
[Add files A/B]
2) https://codereview.chromium.org/2650363002/
[Update A/B]

(2) is dependent on (1). (2) caused a compile failure on an ASAN bot, but the sheriff incorrectly reverted (!). To everyone's surprise, the revert landed:

3) https://codereview.chromium.org/2658083002/

However, if you look at what rietveld claims is contained in this CL [both by diff, or just file by file], it's different from what git claims was landed:

See hash e9942e23fec9e2bca8ebd70f79feb2120943b54b.

I think what's happened here is that rietveld uploads a diff, which may or may not represent the actual patch that we try to land on trunk, which presumably just deletes A and B, without looking to see if the diff matches. This seems dangerous, since (2) has effectively been reverted, but it's very hard to tell that this is the case from looking at rietveld.
 
How can I get a link to the buildbot apply_patch that did the revert?

From (3) I managed to get to the CQ log here:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2658083002/1

But that is just a text log. 
I suspect this might be a side-effect of applying the patch via 3-way merge, but I'd like to see how the patch was applied and all the "apply / git am" args.
Components: -Infra Infra>Codereview>Rietveld
Status: Available (was: Untriaged)
As this is a Pri-3, it is unlikely to be fixed as we are moving everyone to PolyGerrit (https://polygerrit.appspot.com). Stay tuned for your dogfooding email coming this quarter.
I think this is not about polygerrit or emails, this is about applying the diff on the CQ. 
Status: WontFix (was: Available)
Closing in bulk due to Rietveld’s deprecation in favor of Gerrit. If you feel this bug should not have been closed, please feel free to re-open.

Sign in to add a comment