New issue
Advanced search Search tips

Issue 685326 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 657117
issue 679951



Sign in to add a comment

[WPT Export] Cr-Commit-Position is returning incorrect commit hash

Project Member Reported by jeffcarp@chromium.org, Jan 25 2017

Issue description

I believe using Cr-Commit-Position causes us to skip over the second commit if two exportable commits exist close together enough. From what I understand this is because Cr-Commit-Position does not point exactly to the position of the commit that was upstreamed.

We should retain a pointer to exactly the right Chromium commit when exporting commits to WPT.

My idea to solve this problem is to attach another footer onto any commit we export: Cr-Original-Commit-Hash which will point unambiguously to the original commit in Chromium from whence it originated. 
 
Blocking: 679951
Maybe I'm mistaken - the Cr-Commit-Position seems to be consistent with the commit it came from in most cases except reverts. For reverted commits, both Cr-Commit-Positions point to the reverted commit (using git crrev-parse).

I still think adding Cr-Original-Commit-Hash (or Cr-Commit-Hash which is shorter and maybe nicer) is a good idea to eliminate complexity and solve this reverted commit problem.
Update: we believe this is caused by a bug in git crrev-parse. 

I believe the way crrev-parse works is that it looks for the latest commit with Cr-Commit-Position: $POSITION. Because the reverted commit contains the Cr-Commit-Position of the original commit, running crrev-parse on the original commit position will always return the reverted commit.
Labels: -Pri-0 Pri-1
Summary: [WPT Export] Cr-Commit-Position is returning incorrect commit hash (was: [WPT Export] Cr-Commit-Position is not the correct ref to use)
Created a CL in depot_tools to fix the problem. Things are functioning normally with the patch.
https://chromium-review.googlesource.com/#/c/433158/
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/056bef208a5ebbff9864a9db328fdcfaa20a934b

commit 056bef208a5ebbff9864a9db328fdcfaa20a934b
Author: Jeff Carpenter <jeffcarp@chromium.org>
Date: Wed Jan 25 20:51:26 2017

Fix git-crrev-parse returning reverted commits instead of original commits

Also delete extraneous whitespace

BUG= 685326 

Change-Id: If7f68346fd27edf9a5dca315cfcfbca0decc2da6
Reviewed-on: https://chromium-review.googlesource.com/433158
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>

[modify] https://crrev.com/056bef208a5ebbff9864a9db328fdcfaa20a934b/git-crrev-parse

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/056bef208a5ebbff9864a9db328fdcfaa20a934b

commit 056bef208a5ebbff9864a9db328fdcfaa20a934b
Author: Jeff Carpenter <jeffcarp@chromium.org>
Date: Wed Jan 25 20:51:26 2017

Fix git-crrev-parse returning reverted commits instead of original commits

Also delete extraneous whitespace

BUG= 685326 

Change-Id: If7f68346fd27edf9a5dca315cfcfbca0decc2da6
Reviewed-on: https://chromium-review.googlesource.com/433158
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>

[modify] https://crrev.com/056bef208a5ebbff9864a9db328fdcfaa20a934b/git-crrev-parse

Status: Fixed (was: Started)
The bug in git crrev-parse has landed, closing.
Components: Blink>Infra>Ecosystem
Components: -Blink>Infra>Predictability

Sign in to add a comment