New issue
Advanced search Search tips

Issue 888446 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[WPT Export] CL's corresponding PR lost Change-Id, resulting in failing exporter

Project Member Reported by foolip@chromium.org, Sep 24

Issue description

Earliest and most recent failures:
https://ci.chromium.org/buildbot/chromium.infra.cron/wpt-exporter/102191 (started 2018-09-22 12:28 AM (CEST))
https://ci.chromium.org/buildbot/chromium.infra.cron/wpt-exporter/102358

The log ends like this:
2018-09-22 18:03:24,770 - Cloning GitHub web-platform-tests/wpt into /tmp/wpt
2018-09-22 18:03:57,215 - Searching for exportable in-flight CLs.
2018-09-22 18:04:02,226 - Rejecting CL with over 1000 files: Remove pre-refresh code in BrowserNonClientFrameView (ID: I3893b7aacaba0faebc2905bdfc6b0880334c50d3) 
2018-09-22 18:04:02,234 - Found Gerrit in-flight CL: "[css-text] A leading white-space should break before handling overflow" https://chromium-review.googlesource.com/1209745
2018-09-22 18:04:15,496 - Fetched 1000 PRs from GitHub.
2018-09-22 18:04:15,505 - No in-flight PR found for CL. Creating...
2018-09-22 18:07:34,235 - Deleting old branch chromium-export-cl-1209745
2018-09-22 18:07:34,248 - Creating local branch chromium-export-cl-1209745
2018-09-22 18:07:34,743 - Author: "Javier Fernandez <jfernandez@igalia.com>"
Traceback (most recent call last):
  File "/b/rr/tmp4zGJ3h/w/src/third_party/blink/tools/wpt_export.py", line 28, in <module>
    main()
  File "/b/rr/tmp4zGJ3h/w/src/third_party/blink/tools/wpt_export.py", line 20, in main
    success = exporter.main()
  File "/b/rr/tmp4zGJ3h/w/src/third_party/blink/tools/blinkpy/w3c/test_exporter.py", line 75, in main
    self.process_gerrit_cls(open_gerrit_cls)
  File "/b/rr/tmp4zGJ3h/w/src/third_party/blink/tools/blinkpy/w3c/test_exporter.py", line 106, in process_gerrit_cls
    self.process_gerrit_cl(cl)
  File "/b/rr/tmp4zGJ3h/w/src/third_party/blink/tools/blinkpy/w3c/test_exporter.py", line 131, in process_gerrit_cl
    self.create_or_update_pr_from_inflight_cl(cl)
  File "/b/rr/tmp4zGJ3h/w/src/third_party/blink/tools/blinkpy/w3c/test_exporter.py", line 274, in create_or_update_pr_from_inflight_cl
    commit, provisional=True, pr_footer=footer, pr_branch_name=branch_name)
  File "/b/rr/tmp4zGJ3h/w/src/third_party/blink/tools/blinkpy/w3c/test_exporter.py", line 334, in create_or_update_pr_from_commit
    self.local_wpt.create_branch_with_patch(pr_branch_name, message, patch, author, force_push=updating)
  File "/b/rr/tmp4zGJ3h/w/src/third_party/blink/tools/blinkpy/w3c/local_wpt.py", line 103, in create_branch_with_patch
    self.run(['git', 'push', 'origin', branch_name])
  File "/b/rr/tmp4zGJ3h/w/src/third_party/blink/tools/blinkpy/w3c/local_wpt.py", line 50, in run
    return self.host.executive.run_command(command, cwd=self.path, **kwargs)
  File "/b/rr/tmp4zGJ3h/w/src/third_party/blink/tools/blinkpy/common/system/executive.py", line 350, in run_command
    (error_handler or self.default_error_handler)(script_error)
  File "/b/rr/tmp4zGJ3h/w/src/third_party/blink/tools/blinkpy/common/system/executive.py", line 257, in default_error_handler
    raise error
blinkpy.common.system.executive.ScriptError: Failed to run "['git', 'push', 'origin', 'chromium-export-cl-1209745']" exit_code: 1 cwd: /tmp/wpt
output: Last 500 characters of output:
sts/wpt.git
 ! [rejected]              chromium-export-cl-1209745 -> chromium-export-cl-1209745 (non-fast-forward)
error: failed to push some refs to 'https://fd33bd6a41f84861545b1a1cb173d1a380b11c9e@github.com/web-platform-tests/wpt.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
step returned non-zero exit code: 1

The CL and PR in question are:
https://chromium-review.googlesource.com/c/chromium/src/+/1209745
https://github.com/web-platform-tests/wpt/pull/12894

In the above log, the exporter fails to find a PR for CL 1209745 and tries to create a new one. This then fails because the branch chromium-export-cl-1209745 already exists.

Most likely the reason that the PR isn't found is that the description no longer has the Change-Id.

It looks like the commit message on the CL was at one point edited to not include the Change-Id:
https://chromium-review.googlesource.com/c/chromium/src/+/1209745/6

That was on Sep 11, which was also the last time that the PR was updated. After that, the association was lost.

The workaround for this will be to edit the commit message back to include the Change-Id.

To prevent this from happening again, when a CL is updated to not include a Change-Id, we simply not update the PR. If the Change-Id is added back, things will continue to work.

Open question: what if the Change-Id is changed to something else?
 
Status: Available (was: Untriaged)
Looks like the Change-Id was manually removed at PS6 for reasons unknown (perhaps just accidentally). I'm surprised that Gerrit actually allowed that to happen. In the next PS, not surprisingly, Gerrit automatically added the same Change-Id back to the description.

Change-Id is a critical metadata on Gerrit, allowing Gerrit to find the corresponding review from a git commit. https://gerrit-review.googlesource.com/Documentation/user-changeid.html It should be safe for us to rely on it.

To prevent similar incidents to happen again, we can assert the presence of Change-Id in the description when creating/updating a PR.

Sign in to add a comment