Commit 34af3640 authored by Robert Ma's avatar Robert Ma Committed by Commit Bot

[WPT export] Set git user in tmp WPT checkout

Builders don't (and shouldn't) have global git user name/email
configured, and the recipe only configures git user in the Chromium
checkout (the recipe doesn't know about the tmp WPT checkout). When git
user cannot be found (neither in the repo or globally), `git commit`
will fail. (Previously, we didn't notice the issue on BuildBot because
the bot had some leftover global configurations from an earlier version
of the recipe.)

To make sure we always have a git user configured, this change
explicitly sets git user name & email in the tmp WPT checkout to the bot
account. It is still safe to run the script manually because GitHub
allows committers to be different from the uploader.

Besides, remove an unnecessary pair of quotes in a format string for
`git show`; we don't use shell to start subprocesses so spaces don't
need to be quoted.

Bug: 803111
Change-Id: Ife020c8a18120ff607b8802c32cf9539bf6a36c9
Reviewed-on: https://chromium-review.googlesource.com/1258977
Commit-Queue: Robert Ma <robertma@chromium.org>
Reviewed-by: default avatarQuinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596290}
parent 83ec345b
...@@ -86,7 +86,7 @@ class ChromiumCommit(object): ...@@ -86,7 +86,7 @@ class ChromiumCommit(object):
def author(self): def author(self):
return self.host.executive.run_command([ return self.host.executive.run_command([
'git', 'show', '--format="%aN <%aE>"', '--no-patch', self.sha 'git', 'show', '--format=%aN <%aE>', '--no-patch', self.sha
], cwd=self.absolute_chromium_dir).strip() ], cwd=self.absolute_chromium_dir).strip()
def message(self): def message(self):
......
...@@ -17,6 +17,11 @@ WPT_REVISION_FOOTER = 'WPT-Export-Revision:' ...@@ -17,6 +17,11 @@ WPT_REVISION_FOOTER = 'WPT-Export-Revision:'
EXPORT_PR_LABEL = 'chromium-export' EXPORT_PR_LABEL = 'chromium-export'
PROVISIONAL_PR_LABEL = 'do not merge yet' PROVISIONAL_PR_LABEL = 'do not merge yet'
# These are only set in a new WPT checkout, and they should be consistent with
# the bot's GitHub account (chromium-wpt-export-bot).
DEFAULT_WPT_COMMITTER_NAME = 'Chromium WPT Sync'
DEFAULT_WPT_COMMITTER_EMAIL = 'blink-w3c-test-autoroller@chromium.org'
# TODO(qyearsley): Avoid hard-coding third_party/WebKit/LayoutTests. # TODO(qyearsley): Avoid hard-coding third_party/WebKit/LayoutTests.
CHROMIUM_WPT_DIR = 'third_party/WebKit/LayoutTests/external/wpt/' CHROMIUM_WPT_DIR = 'third_party/WebKit/LayoutTests/external/wpt/'
......
...@@ -7,7 +7,13 @@ ...@@ -7,7 +7,13 @@
import logging import logging
from blinkpy.common.system.executive import ScriptError from blinkpy.common.system.executive import ScriptError
from blinkpy.w3c.common import WPT_GH_SSH_URL_TEMPLATE, WPT_MIRROR_URL, CHROMIUM_WPT_DIR from blinkpy.w3c.common import (
CHROMIUM_WPT_DIR,
DEFAULT_WPT_COMMITTER_EMAIL,
DEFAULT_WPT_COMMITTER_NAME,
WPT_GH_SSH_URL_TEMPLATE,
WPT_MIRROR_URL,
)
_log = logging.getLogger(__name__) _log = logging.getLogger(__name__)
...@@ -42,8 +48,13 @@ class LocalWPT(object): ...@@ -42,8 +48,13 @@ class LocalWPT(object):
remote_url = WPT_MIRROR_URL remote_url = WPT_MIRROR_URL
_log.info('No credentials given, using wpt mirror URL.') _log.info('No credentials given, using wpt mirror URL.')
_log.info('It is possible for the mirror to be delayed; see https://crbug.com/698272.') _log.info('It is possible for the mirror to be delayed; see https://crbug.com/698272.')
# Do not use self.run here because self.path doesn't exist yet.
self.host.executive.run_command(['git', 'clone', remote_url, self.path]) self.host.executive.run_command(['git', 'clone', remote_url, self.path])
_log.info('Setting git user name & email in %s', self.path)
self.run(['git', 'config', 'user.name', DEFAULT_WPT_COMMITTER_NAME])
self.run(['git', 'config', 'user.email', DEFAULT_WPT_COMMITTER_EMAIL])
def run(self, command, **kwargs): def run(self, command, **kwargs):
"""Runs a command in the local WPT directory.""" """Runs a command in the local WPT directory."""
# TODO(robertma): Migrate to blinkpy.common.checkout.Git. (crbug.com/676399) # TODO(robertma): Migrate to blinkpy.common.checkout.Git. (crbug.com/676399)
......
...@@ -8,6 +8,7 @@ from blinkpy.common.host_mock import MockHost ...@@ -8,6 +8,7 @@ from blinkpy.common.host_mock import MockHost
from blinkpy.common.system.executive import ScriptError from blinkpy.common.system.executive import ScriptError
from blinkpy.common.system.executive_mock import MockExecutive, mock_git_commands from blinkpy.common.system.executive_mock import MockExecutive, mock_git_commands
from blinkpy.common.system.filesystem_mock import MockFileSystem from blinkpy.common.system.filesystem_mock import MockFileSystem
from blinkpy.w3c.common import DEFAULT_WPT_COMMITTER_EMAIL, DEFAULT_WPT_COMMITTER_NAME
from blinkpy.w3c.local_wpt import LocalWPT from blinkpy.w3c.local_wpt import LocalWPT
...@@ -34,6 +35,8 @@ class LocalWPTTest(unittest.TestCase): ...@@ -34,6 +35,8 @@ class LocalWPTTest(unittest.TestCase):
self.assertEqual(host.executive.calls, [ self.assertEqual(host.executive.calls, [
['git', 'clone', 'https://token@github.com/web-platform-tests/wpt.git', '/tmp/wpt'], ['git', 'clone', 'https://token@github.com/web-platform-tests/wpt.git', '/tmp/wpt'],
['git', 'config', 'user.name', DEFAULT_WPT_COMMITTER_NAME],
['git', 'config', 'user.email', DEFAULT_WPT_COMMITTER_EMAIL],
]) ])
def test_constructor(self): def test_constructor(self):
...@@ -55,6 +58,8 @@ class LocalWPTTest(unittest.TestCase): ...@@ -55,6 +58,8 @@ class LocalWPTTest(unittest.TestCase):
local_wpt.create_branch_with_patch('chromium-export-decafbad', 'message', 'patch', 'author <author@author.com>') local_wpt.create_branch_with_patch('chromium-export-decafbad', 'message', 'patch', 'author <author@author.com>')
self.assertEqual(host.executive.calls, [ self.assertEqual(host.executive.calls, [
['git', 'clone', 'https://token@github.com/web-platform-tests/wpt.git', '/tmp/wpt'], ['git', 'clone', 'https://token@github.com/web-platform-tests/wpt.git', '/tmp/wpt'],
['git', 'config', 'user.name', DEFAULT_WPT_COMMITTER_NAME],
['git', 'config', 'user.email', DEFAULT_WPT_COMMITTER_EMAIL],
['git', 'reset', '--hard', 'HEAD'], ['git', 'reset', '--hard', 'HEAD'],
['git', 'clean', '-fdx'], ['git', 'clean', '-fdx'],
['git', 'checkout', 'origin/master'], ['git', 'checkout', 'origin/master'],
......
...@@ -379,6 +379,7 @@ class TestExporterTest(LoggingTestCase): ...@@ -379,6 +379,7 @@ class TestExporterTest(LoggingTestCase):
self.assertFalse(success) self.assertFalse(success)
self.assertLog(['INFO: Cloning GitHub web-platform-tests/wpt into /tmp/wpt\n', self.assertLog(['INFO: Cloning GitHub web-platform-tests/wpt into /tmp/wpt\n',
'INFO: Setting git user name & email in /tmp/wpt\n',
'INFO: Searching for exportable in-flight CLs.\n', 'INFO: Searching for exportable in-flight CLs.\n',
'INFO: In-flight CLs cannot be exported due to the following error:\n', 'INFO: In-flight CLs cannot be exported due to the following error:\n',
'ERROR: Gerrit API fails.\n', 'ERROR: Gerrit API fails.\n',
...@@ -393,6 +394,7 @@ class TestExporterTest(LoggingTestCase): ...@@ -393,6 +394,7 @@ class TestExporterTest(LoggingTestCase):
self.assertFalse(success) self.assertFalse(success)
self.assertLog(['INFO: Cloning GitHub web-platform-tests/wpt into /tmp/wpt\n', self.assertLog(['INFO: Cloning GitHub web-platform-tests/wpt into /tmp/wpt\n',
'INFO: Setting git user name & email in /tmp/wpt\n',
'INFO: Searching for exportable in-flight CLs.\n', 'INFO: Searching for exportable in-flight CLs.\n',
'INFO: Searching for exportable Chromium commits.\n', 'INFO: Searching for exportable Chromium commits.\n',
'INFO: Attention: The following errors have prevented some commits from being exported:\n', 'INFO: Attention: The following errors have prevented some commits from being exported:\n',
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment