diff --git a/roll_dep.py b/roll_dep.py index 5b64a7928a..d727f84f00 100755 --- a/roll_dep.py +++ b/roll_dep.py @@ -38,6 +38,24 @@ _ROLL_SUBJECT = re.compile( r'Roll recipe dependencies \(trivial\)\.' r')$') +_PUBLIC_GERRIT_HOSTS = { + 'android', + 'aomedia', + 'boringssl', + 'chromium', + 'dart', + 'dawn', + 'fuchsia', + 'gn', + 'go', + 'llvm', + 'pdfium', + 'quiche', + 'skia', + 'swiftshader', + 'webrtc', +} + class Error(Exception): pass @@ -77,10 +95,15 @@ def is_pristine(root): and not check_output(diff_cmd + ['--cached'], cwd=root).strip()) +def get_gerrit_host(url): + """Returns the host for a given Gitiles URL.""" + m = re.match(r'https://([^/]*)\.googlesource\.com/', url) + return m and m.group(1) + + def get_log_url(upstream_url, head, tot): """Returns an URL to read logs via a Web UI if applicable.""" - if re.match(r'https://[^/]*\.googlesource\.com/', upstream_url): - # gitiles + if get_gerrit_host(upstream_url): return '%s/+log/%s..%s' % (upstream_url, head[:12], tot[:12]) if upstream_url.startswith('https://github.com/'): upstream_url = upstream_url.rstrip('/') @@ -97,7 +120,7 @@ def should_show_log(upstream_url): return False if 'webrtc' in upstream_url: return False - return True + return get_gerrit_host(upstream_url) in _PUBLIC_GERRIT_HOSTS def gclient(args): @@ -105,14 +128,11 @@ def gclient(args): return check_output([sys.executable, GCLIENT_PATH] + args).strip() -def generate_commit_message(full_dir, dependency, head, roll_to, no_log, - log_limit): +def generate_commit_message(full_dir, dependency, head, roll_to, upstream_url, + show_log, log_limit): """Creates the commit message for this specific roll.""" commit_range = '%s..%s' % (head, roll_to) commit_range_for_header = '%s..%s' % (head[:9], roll_to[:9]) - upstream_url = check_output(['git', 'config', 'remote.origin.url'], - cwd=full_dir).strip() - log_url = get_log_url(upstream_url, head, roll_to) cmd = ['git', 'log', commit_range, '--date=short', '--no-merges'] logs = check_output( # Args with '=' are automatically quoted. @@ -130,11 +150,11 @@ def generate_commit_message(full_dir, dependency, head, roll_to, no_log, 's' if nb_commits > 1 else '', ('; %s trivial rolls' % rolls) if rolls else '') log_section = '' - if log_url: + if log_url := get_log_url(upstream_url, head, roll_to): log_section = log_url + '\n\n' # It is important that --no-log continues to work, as it is used by # internal -> external rollers. Please do not remove or break it. - if not no_log and should_show_log(upstream_url): + if show_log: log_section += '$ %s ' % ' '.join(cmd) log_section += '--format=\'%ad %ae %s\'\n' log_section = log_section.replace(commit_range, commit_range_for_header) @@ -247,6 +267,10 @@ def main(): '--no-log', action='store_true', help='Do not include the short log in the commit message') + parser.add_argument( + '--always-log', + action='store_true', + help='Always include the short log in the commit message') parser.add_argument('--log-limit', type=int, default=100, @@ -270,6 +294,10 @@ def main(): if args.key: parser.error( 'Can\'t use multiple paths to roll simultaneously and --key') + + if args.no_log and args.always_log: + parser.error('Can\'t use both --no-log and --always-log') + reviewers = None if args.reviewer: reviewers = list(itertools.chain(*[r.split(',') @@ -315,8 +343,17 @@ def main(): logs = [] setdep_args = [] for dependency, (head, roll_to, full_dir) in sorted(rolls.items()): + upstream_url = check_output(['git', 'config', 'remote.origin.url'], + cwd=full_dir).strip() + show_log = args.always_log or \ + (not args.no_log and should_show_log(upstream_url)) + if not show_log: + print( + f'{dependency}: Omitting git log from the commit message. ' + 'Use the `--always-log` flag to include it.') log = generate_commit_message(full_dir, dependency, head, roll_to, - args.no_log, args.log_limit) + upstream_url, show_log, + args.log_limit) logs.append(log) setdep_args.extend(['-r', '{}@{}'.format(dependency, roll_to)]) diff --git a/tests/roll_dep_test.py b/tests/roll_dep_test.py index 86fcb38dd9..a798e40f16 100755 --- a/tests/roll_dep_test.py +++ b/tests/roll_dep_test.py @@ -8,10 +8,12 @@ import os import sys import subprocess import unittest +from unittest import mock ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) sys.path.insert(0, ROOT_DIR) +import roll_dep from testing_support import fake_repos ROLL_DEP = os.path.join(ROOT_DIR, 'roll-dep') @@ -204,6 +206,46 @@ class RollDepTest(fake_repos.FakeReposTestBase): self.assertIn(expected_message, commit_message) +class CommitMessageTest(unittest.TestCase): + + def setUp(self): + self.logs = '\n'.join([ + '2024-04-05 alice Goodbye', + '2024-04-03 bob Hello World', + ]) + + # Mock the `git log` call. + mock.patch('roll_dep.check_output', return_value=self.logs).start() + self.addCleanup(mock.patch.stopall) + + def testShowShortLog(self): + message = roll_dep.generate_commit_message( + '/path/to/dir', 'dep', 'abc', 'def', + 'https://chromium.googlesource.com', True, 10) + + self.assertIn('Roll dep/ abc..def (2 commits)', message) + self.assertIn('$ git log', message) + self.assertIn(self.logs, message) + + def testHideShortLog(self): + message = roll_dep.generate_commit_message( + '/path/to/dir', 'dep', 'abc', 'def', + 'https://chromium.googlesource.com', False, 10) + + self.assertNotIn('$ git log', message) + self.assertNotIn(self.logs, message) + + def testShouldShowLogWithPublicHost(self): + self.assertTrue( + roll_dep.should_show_log( + 'https://chromium.googlesource.com/project')) + + def testShouldNotShowLogWithPrivateHost(self): + self.assertFalse( + roll_dep.should_show_log( + 'https://private.googlesource.com/project')) + + if __name__ == '__main__': level = logging.DEBUG if '-v' in sys.argv else logging.FATAL logging.basicConfig(level=level,