Reland "git-cl: Remove unused and duplicate functions."
This is a reland of e3a49aa40576da2427447f7fedb670d5d59216e5
GetRelativeRoot was fixed to use '.' instead of None
Original change's description:
> git-cl: Remove unused and duplicate functions.
>
> Remove unused functions, and use scm.GIT where possible to reduce
> duplication.
>
> Change-Id: I24f05e7b3ae04743e97b6665ee668f44d6acc294
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2116938
> Reviewed-by: Anthony Polito <apolito@google.com>
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Change-Id: I54bd1d34d3501a38bf8af2f9e70059d58fa538a5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2125293
Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
diff --git a/git_cl.py b/git_cl.py
index d1cdac0..8b59c49 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -9,7 +9,6 @@
from __future__ import print_function
-from distutils.version import LooseVersion
import base64
import collections
import datetime
@@ -116,6 +115,12 @@
# File name for yapf style config files.
YAPF_CONFIG_FILENAME = '.style.yapf'
+# The issue, patchset and codereview server are stored on git config for each
+# branch under branch.<branch-name>.<config-key>.
+ISSUE_CONFIG_KEY = 'gerritissue'
+PATCHSET_CONFIG_KEY = 'gerritpatchset'
+CODEREVIEW_SERVER_CONFIG_KEY = 'gerritserver'
+
# Shortcut since it quickly becomes repetitive.
Fore = colorama.Fore
@@ -201,20 +206,6 @@
return RunGitWithCode(args, suppress_stderr=True)[1]
-def IsGitVersionAtLeast(min_version):
- prefix = 'git version '
- version = RunGit(['--version']).strip()
- return (version.startswith(prefix) and
- LooseVersion(version[len(prefix):]) >= LooseVersion(min_version))
-
-
-def BranchExists(branch):
- """Return True if specified branch exists."""
- code, _ = RunGitWithCode(['rev-parse', '--verify', branch],
- suppress_stderr=True)
- return not code
-
-
def time_sleep(seconds):
# Use this so that it can be mocked in tests without interfering with python
# system machinery.
@@ -258,12 +249,6 @@
result = gclient_utils.AskForData('Please, type yes or no: ').lower()
-def _git_branch_config_key(branch, key):
- """Helper method to return Git config key for a branch."""
- assert branch, 'branch name is required to set git config for it'
- return 'branch.%s.%s' % (branch, key)
-
-
def _get_properties_from_options(options):
prop_list = getattr(options, 'properties', [])
properties = dict(x.split('=', 1) for x in prop_list)
@@ -275,46 +260,6 @@
return properties
-# TODO(crbug.com/976104): Remove this function once git-cl try-results has
-# migrated to use buildbucket v2
-def _buildbucket_retry(operation_name, http, *args, **kwargs):
- """Retries requests to buildbucket service and returns parsed json content."""
- try_count = 0
- while True:
- response, content = http.request(*args, **kwargs)
- try:
- content_json = json.loads(content)
- except ValueError:
- content_json = None
-
- # Buildbucket could return an error even if status==200.
- if content_json and content_json.get('error'):
- error = content_json.get('error')
- if error.get('code') == 403:
- raise BuildbucketResponseException(
- 'Access denied: %s' % error.get('message', ''))
- msg = 'Error in response. Reason: %s. Message: %s.' % (
- error.get('reason', ''), error.get('message', ''))
- raise BuildbucketResponseException(msg)
-
- if response.status == 200:
- if content_json is None:
- raise BuildbucketResponseException(
- 'Buildbucket returned invalid JSON content: %s.\n'
- 'Please file bugs at http://crbug.com, '
- 'component "Infra>Platform>Buildbucket".' %
- content)
- return content_json
- if response.status < 500 or try_count >= 2:
- raise httplib2.HttpLib2Error(content)
-
- # status >= 500 means transient failures.
- logging.debug('Transient errors when %s. Will retry.', operation_name)
- time_sleep(0.5 + (1.5 * try_count))
- try_count += 1
- assert False, 'unreachable'
-
-
def _call_buildbucket(http, buildbucket_host, method, request):
"""Calls a buildbucket v2 method and returns the parsed json response."""
headers = {
@@ -786,7 +731,7 @@
@staticmethod
def GetRelativeRoot():
- return RunGit(['rev-parse', '--show-cdup']).strip()
+ return scm.GIT.GetCheckoutRoot('.')
def GetRoot(self):
if self.root is None:
@@ -1031,12 +976,6 @@
self.cc = ','.join(filter(None, (base_cc, more_cc))) or ''
return self.cc
- def GetCCListWithoutDefault(self):
- """Return the users cc'd on this CL excluding default ones."""
- if self.cc is None:
- self.cc = ','.join(self.more_cc)
- return self.cc
-
def ExtendCC(self, more_cc):
"""Extends the list of users to cc on this CL based on the changed files."""
self.more_cc.extend(more_cc)
@@ -1056,10 +995,6 @@
self.GetBranch() # Poke the lazy loader.
return self.branchref
- def ClearBranch(self):
- """Clears cached branch data of this object."""
- self.branch = self.branchref = None
-
def _GitGetBranchConfigValue(self, key, default=None):
return scm.GIT.GetBranchConfig(
settings.GetRoot(), self.GetBranch(), key, default)
@@ -1091,7 +1026,7 @@
def GetCommonAncestorWithUpstream(self):
upstream_branch = self.GetUpstreamBranch()
- if not BranchExists(upstream_branch):
+ if not scm.GIT.IsValidRevision(settings.GetRoot(), upstream_branch):
DieWithError('The upstream for the current branch (%s) does not exist '
'anymore.\nPlease fix it and try again.' % self.GetBranch())
return git_common.get_or_create_merge_base(self.GetBranch(),
@@ -1138,55 +1073,6 @@
self._remote = (remote, 'refs/remotes/%s/%s' % (remote, branch))
return self._remote
- def GitSanityChecks(self, upstream_git_obj):
- """Checks git repo status and ensures diff is from local commits."""
-
- if upstream_git_obj is None:
- if self.GetBranch() is None:
- print('ERROR: Unable to determine current branch (detached HEAD?)',
- file=sys.stderr)
- else:
- print('ERROR: No upstream branch.', file=sys.stderr)
- return False
-
- # Verify the commit we're diffing against is in our current branch.
- upstream_sha = RunGit(['rev-parse', '--verify', upstream_git_obj]).strip()
- common_ancestor = RunGit(['merge-base', upstream_sha, 'HEAD']).strip()
- if upstream_sha != common_ancestor:
- print('ERROR: %s is not in the current branch. You may need to rebase '
- 'your tracking branch' % upstream_sha, file=sys.stderr)
- return False
-
- # List the commits inside the diff, and verify they are all local.
- commits_in_diff = RunGit(
- ['rev-list', '^%s' % upstream_sha, 'HEAD']).splitlines()
- code, remote_branch = RunGitWithCode(['config', 'gitcl.remotebranch'])
- remote_branch = remote_branch.strip()
- if code != 0:
- _, remote_branch = self.GetRemoteBranch()
-
- commits_in_remote = RunGit(
- ['rev-list', '^%s' % upstream_sha, remote_branch]).splitlines()
-
- common_commits = set(commits_in_diff) & set(commits_in_remote)
- if common_commits:
- print('ERROR: Your diff contains %d commits already in %s.\n'
- 'Run "git log --oneline %s..HEAD" to get a list of commits in '
- 'the diff. If you are using a custom git flow, you can override'
- ' the reference used for this check with "git config '
- 'gitcl.remotebranch <git-ref>".' % (
- len(common_commits), remote_branch, upstream_git_obj),
- file=sys.stderr)
- return False
- return True
-
- def GetGitBaseUrlFromConfig(self):
- """Return the configured base URL from branch.<branchname>.baseurl.
-
- Returns None if it is not set.
- """
- return self._GitGetBranchConfigValue('base-url')
-
def GetRemoteUrl(self):
"""Return the configured remote URL, e.g. 'git://example.org/foo.git/'.
@@ -1237,7 +1123,7 @@
def GetIssue(self):
"""Returns the issue number as a int or None if not set."""
if self.issue is None and not self.lookedup_issue:
- self.issue = self._GitGetBranchConfigValue(self.IssueConfigKey())
+ self.issue = self._GitGetBranchConfigValue(ISSUE_CONFIG_KEY)
if self.issue is not None:
self.issue = int(self.issue)
self.lookedup_issue = True
@@ -1273,7 +1159,7 @@
def GetPatchset(self):
"""Returns the patchset number as a int or None if not set."""
if self.patchset is None and not self.lookedup_patchset:
- self.patchset = self._GitGetBranchConfigValue(self.PatchsetConfigKey())
+ self.patchset = self._GitGetBranchConfigValue(PATCHSET_CONFIG_KEY)
if self.patchset is not None:
self.patchset = int(self.patchset)
self.lookedup_patchset = True
@@ -1289,30 +1175,28 @@
self.patchset = None
else:
self.patchset = int(patchset)
- self._GitSetBranchConfigValue(
- self.PatchsetConfigKey(), str(self.patchset))
+ self._GitSetBranchConfigValue(PATCHSET_CONFIG_KEY, str(self.patchset))
def SetIssue(self, issue=None):
"""Set this branch's issue. If issue isn't given, clears the issue."""
assert self.GetBranch()
if issue:
issue = int(issue)
- self._GitSetBranchConfigValue(
- self.IssueConfigKey(), str(issue))
+ self._GitSetBranchConfigValue(ISSUE_CONFIG_KEY, str(issue))
self.issue = issue
codereview_server = self.GetCodereviewServer()
if codereview_server:
self._GitSetBranchConfigValue(
- self.CodereviewServerConfigKey(),
- codereview_server)
+ CODEREVIEW_SERVER_CONFIG_KEY, codereview_server)
else:
# Reset all of these just to be clean.
reset_suffixes = [
'last-upload-hash',
- self.IssueConfigKey(),
- self.PatchsetConfigKey(),
- self.CodereviewServerConfigKey(),
- ] + self._PostUnsetIssueProperties()
+ ISSUE_CONFIG_KEY,
+ PATCHSET_CONFIG_KEY,
+ CODEREVIEW_SERVER_CONFIG_KEY,
+ 'gerritsquashhash',
+ ]
for prop in reset_suffixes:
try:
self._GitSetBranchConfigValue(prop, None)
@@ -1477,8 +1361,7 @@
return options.message.strip()
# Use the subject of the last commit as title by default.
- title = RunGit(
- ['show', '-s', '--format=%s', 'HEAD']).strip()
+ title = RunGit(['show', '-s', '--format=%s', 'HEAD']).strip()
if options.force:
return title
user_title = gclient_utils.AskForData('Title for patchset [%s]: ' % title)
@@ -1526,7 +1409,7 @@
options, git_diff_args, custom_cl_base, change_desc)
if not ret:
self._GitSetBranchConfigValue(
- 'last-upload-hash', RunGit(['rev-parse', 'HEAD']).strip())
+ 'last-upload-hash', scm.GIT.ResolveCommit(settings.GetRoot(), 'HEAD'))
# Run post upload hooks, if specified.
if settings.GetRunPostUploadHook():
self.RunPostUploadHook(
@@ -1605,7 +1488,7 @@
# with that branch.
if self.GetIssue() and self.GetBranch():
self._gerrit_server = self._GitGetBranchConfigValue(
- self.CodereviewServerConfigKey())
+ CODEREVIEW_SERVER_CONFIG_KEY)
if self._gerrit_server:
self._gerrit_host = urllib.parse.urlparse(self._gerrit_server).netloc
if not self._gerrit_server:
@@ -1648,18 +1531,6 @@
# Fall back on still unique, but less efficient change number.
return str(self.GetIssue())
- @classmethod
- def IssueConfigKey(cls):
- return 'gerritissue'
-
- @classmethod
- def PatchsetConfigKey(cls):
- return 'gerritpatchset'
-
- @classmethod
- def CodereviewServerConfigKey(cls):
- return 'gerritserver'
-
def EnsureAuthenticated(self, force, refresh=None):
"""Best effort check that user is authenticated with Gerrit server."""
if settings.GetGerritSkipEnsureAuthenticated():
@@ -1757,13 +1628,6 @@
(self.GetIssue(), self.GetIssueOwner(), details['email']))
confirm_or_exit(action='upload')
- def _PostUnsetIssueProperties(self):
- """Which branch-specific properties to erase when unsetting issue."""
- return ['gerritsquashhash']
-
- def GetGerritObjForPresubmit(self):
- return presubmit_support.GerritAccessor(self._GetGerritHost())
-
def GetStatus(self):
"""Applies a rough heuristic to give a simple summary of an issue's review
or CQ status, assuming adherence to a common workflow.
@@ -2099,7 +1963,7 @@
if self.GetBranch():
self.SetIssue(parsed_issue_arg.issue)
self.SetPatchset(patchset)
- fetched_hash = RunGit(['rev-parse', 'FETCH_HEAD']).strip()
+ fetched_hash = scm.GIT.ResolveCommit(settings.GetRoot(), 'FETCH_HEAD')
self._GitSetBranchConfigValue('last-upload-hash', fetched_hash)
self._GitSetBranchConfigValue('gerritsquashhash', fetched_hash)
else:
@@ -2485,9 +2349,7 @@
# the tree hash of the parent branch. The upside is less likely bogus
# requests to reupload parent change just because it's uploadhash is
# missing, yet the downside likely exists, too (albeit unknown to me yet).
- parent = RunGit(['config',
- 'branch.%s.gerritsquashhash' % upstream_branch_name],
- error_ok=True).strip()
+ parent = self._GitGetBranchConfigValue('gerritsquashhash', '')
# Verify that the upstream branch has been uploaded too, otherwise
# Gerrit will create additional CLs when uploading.
if not parent or (RunGitSilent(['rev-parse', upstream_branch + ':']) !=
@@ -2838,76 +2700,6 @@
"""
return re.sub(cls.BAD_HASH_TAG_CHUNK, '-', tag).strip('-').lower()
- def update_with_git_number_footers(self, parent_hash, parent_msg, dest_ref):
- """Updates this commit description given the parent.
-
- This is essentially what Gnumbd used to do.
- Consult https://goo.gl/WMmpDe for more details.
- """
- assert parent_msg # No, orphan branch creation isn't supported.
- assert parent_hash
- assert dest_ref
- parent_footer_map = git_footers.parse_footers(parent_msg)
- # This will also happily parse svn-position, which GnumbD is no longer
- # supporting. While we'd generate correct footers, the verifier plugin
- # installed in Gerrit will block such commit (ie git push below will fail).
- parent_position = git_footers.get_position(parent_footer_map)
-
- # Cherry-picks may have last line obscuring their prior footers,
- # from git_footers perspective. This is also what Gnumbd did.
- cp_line = None
- if (self._description_lines and
- re.match(self.CHERRY_PICK_LINE, self._description_lines[-1])):
- cp_line = self._description_lines.pop()
-
- top_lines, footer_lines, _ = git_footers.split_footers(self.description)
-
- # Original-ify all Cr- footers, to avoid re-lands, cherry-picks, or just
- # user interference with actual footers we'd insert below.
- for i, line in enumerate(footer_lines):
- k, v = git_footers.parse_footer(line) or (None, None)
- if k and k.startswith('Cr-'):
- footer_lines[i] = '%s: %s' % ('Cr-Original-' + k[len('Cr-'):], v)
-
- # Add Position and Lineage footers based on the parent.
- lineage = list(reversed(parent_footer_map.get('Cr-Branched-From', [])))
- if parent_position[0] == dest_ref:
- # Same branch as parent.
- number = int(parent_position[1]) + 1
- else:
- number = 1 # New branch, and extra lineage.
- lineage.insert(0, '%s-%s@{#%d}' % (parent_hash, parent_position[0],
- int(parent_position[1])))
-
- footer_lines.append('Cr-Commit-Position: %s@{#%d}' % (dest_ref, number))
- footer_lines.extend('Cr-Branched-From: %s' % v for v in lineage)
-
- self._description_lines = top_lines
- if cp_line:
- self._description_lines.append(cp_line)
- if self._description_lines[-1] != '':
- self._description_lines.append('') # Ensure footer separator.
- self._description_lines.extend(footer_lines)
-
-
-def get_approving_reviewers(props, disapproval=False):
- """Retrieves the reviewers that approved a CL from the issue properties with
- messages.
-
- Note that the list may contain reviewers that are not committer, thus are not
- considered by the CQ.
-
- If disapproval is True, instead returns reviewers who 'not lgtm'd the CL.
- """
- approval_type = 'disapproval' if disapproval else 'approval'
- return sorted(
- set(
- message['sender']
- for message in props['messages']
- if message[approval_type] and message['sender'] in props['reviewers']
- )
- )
-
def FindCodereviewSettingsFile(filename='codereview.settings'):
"""Finds the given file starting in the cwd and going up.
@@ -2989,11 +2781,6 @@
return f.read(2).startswith('#!')
-# TODO(bpastene) Remove once a cleaner fix to crbug.com/600473 presents itself.
-def DownloadHooks(*args, **kwargs):
- pass
-
-
def DownloadGerritHook(force):
"""Downloads and installs a Gerrit commit-msg hook.
@@ -3327,7 +3114,7 @@
@metrics.collector.collect_metrics('git cl baseurl')
def CMDbaseurl(parser, args):
"""Gets or sets base-url for this branch."""
- branchref = RunGit(['symbolic-ref', 'HEAD']).strip()
+ branchref = scm.GIT.GetBranchRef(settings.GetRoot())
branch = scm.GIT.ShortBranchName(branchref)
_, args = parser.parse_args(args)
if not args:
@@ -3808,9 +3595,8 @@
git_config[name] = val
for branch in branches:
- config_key = _git_branch_config_key(scm.GIT.ShortBranchName(branch),
- Changelist.IssueConfigKey())
- issue = git_config.get(config_key)
+ issue = git_config.get(
+ 'branch.%s.%s' % (scm.GIT.ShortBranchName(branch), ISSUE_CONFIG_KEY))
if issue:
issue_branch_map.setdefault(int(issue), []).append(branch)
if not args:
@@ -4858,9 +4644,8 @@
help='Show all owners for a particular file')
options, args = parser.parse_args(args)
- author = RunGit(['config', 'user.email']).strip() or None
-
cl = Changelist()
+ author = cl.GetAuthor()
if options.show_all:
for arg in args:
@@ -5081,8 +4866,7 @@
dart_diff_files = [x for x in diff_files if MatchingFileType(x, ['.dart'])]
gn_diff_files = [x for x in diff_files if MatchingFileType(x, GN_EXTS)]
- top_dir = os.path.normpath(
- RunGit(["rev-parse", "--show-toplevel"]).rstrip('\n'))
+ top_dir = settings.GetRoot()
return_value = _RunClangFormatDiff(opts, clang_diff_files, top_dir,
upstream_commit)
@@ -5244,15 +5028,14 @@
target_issue = str(issue_arg.issue)
- issueprefix = Changelist.IssueConfigKey()
output = RunGit(['config', '--local', '--get-regexp',
- r'branch\..*\.%s' % issueprefix],
+ r'branch\..*\.' + ISSUE_CONFIG_KEY],
error_ok=True)
branches = []
for key, issue in [x.split() for x in output.splitlines()]:
if issue == target_issue:
- branches.append(re.sub(r'branch\.(.*)\.%s' % issueprefix, r'\1', key))
+ branches.append(re.sub(r'branch\.(.*)\.' + ISSUE_CONFIG_KEY, r'\1', key))
if len(branches) == 0:
print('No branch found for issue %s.' % target_issue)