Revert of git cl: Rework Changelist class for Rietveld/Gerrit use. (patchset #1 id:1 of https://codereview.chromium.org/1844523002/ )
Reason for revert:
argh, CQ is way too quick.
Original issue's description:
> Reland of git cl: Rework Changelist class for Rietveld/Gerrit use. (patchset #1 id:1 of https://codereview.chromium.org/1840833002/ )
>
> Reason for revert:
> with a fix.
>
> Original issue's description:
> > Revert of git cl: Rework Changelist class for Rietveld/Gerrit use. (patchset #3 id:40001 of https://codereview.chromium.org/1830973003/ )
> >
> > Reason for revert:
> > Speculative revert, see crbug.com/598428.
> >
> > Original issue's description:
> > > git cl: Rework Changelist class for Rietveld/Gerrit use.
> > >
> > > This adds pluggable codereview-specific implementations into
> > > Changelist class. The specific implementation is chosen at
> > > Changelist automatically, with Rietveld being default for
> > > backwards compatibility.
> > >
> > > Gerrit implementation for Gerrit is incomplete, and will be
> > > added in later CLs. However, it is sufficient to ensure
> > > current functionality of this tool is not diminished.
> > >
> > > Sadly, the base class isn't completely free from Rietveld
> > > assumptions because of presubmit_support. Apparently, PRESUBMIT
> > > scripts can make use of Rietveld instance for RPCs directly.
> > > This use doesn't make sense for Gerrit, which substitutes
> > > rietveld instance with a dummy object, which raises exception
> > > on any attribute access with a diagnostic message.
> > >
> > > This also includes refactoring of some related code which
> > > (ab)used ChangeList. Overall, this CL adds a few extra call to
> > > git config in order to determine which codereview to use, but
> > > but it shouldn't have any performance impact.
> > >
> > >
> > >
> > > These is a reland of these 4 CLs + a fix.
> > > patch from issue 1827523003 at patchset 20001 (http://crrev.com/1827523003#ps20001)
> > > patch from issue 1830703004 at patchset 1 (http://crrev.com/1830703004#ps1)
> > > patch from issue 1830923002 at patchset 60001 (http://crrev.com/1830923002#ps60001)
> > > patch from issue 1805193002 at patchset 380001 (http://crrev.com/1805193002#ps380001)
> > >
> > >
> > >
> > > R=machenbach@chromium.org,sergiyb@chromium.org,andybons@chromium.org
> > > BUG=579160,597638
> > >
> > > Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299506
> >
> > TBR=andybons@chromium.org,machenbach@chromium.org,sergiyb@chromium.org,tandrii@chromium.org
> > # Skipping CQ checks because original CL landed less than 1 days ago.
> > NOPRESUBMIT=true
> > NOTREECHECKS=true
> > NOTRY=true
> > BUG=579160,597638
> >
> > Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299515
>
> TBR=andybons@chromium.org,machenbach@chromium.org,sergiyb@chromium.org,dnj@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=579160,597638
>
> Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299528
TBR=andybons@chromium.org,machenbach@chromium.org,sergiyb@chromium.org,dnj@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=579160,597638
Review URL: https://codereview.chromium.org/1839973002
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@299529 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/git_cl.py b/git_cl.py
index 58b4cfb..59e4121 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -5,7 +5,7 @@
# Copyright (C) 2008 Evan Martin <martine@danga.com>
-"""A git-command for integrating reviews on Rietveld and Gerrit."""
+"""A git-command for integrating reviews on Rietveld."""
from distutils.version import LooseVersion
from multiprocessing.pool import ThreadPool
@@ -47,7 +47,6 @@
import dart_format
import fix_encoding
import gclient_utils
-import gerrit_util
import git_cache
import git_common
import git_footers
@@ -157,7 +156,7 @@
def git_set_branch_value(key, value):
- branch = GetCurrentBranch()
+ branch = Changelist().GetBranch()
if not branch:
return
@@ -169,7 +168,7 @@
def git_get_branch_default(key, default):
- branch = GetCurrentBranch()
+ branch = Changelist().GetBranch()
if branch:
git_key = 'branch.%s.%s' % (branch, key)
(_, stdout) = RunGitWithCode(['config', '--int', '--get', git_key])
@@ -814,61 +813,23 @@
def ShortBranchName(branch):
"""Convert a name like 'refs/heads/foo' to just 'foo'."""
- return branch.replace('refs/heads/', '', 1)
-
-
-def GetCurrentBranchRef():
- """Returns branch ref (e.g., refs/heads/master) or None."""
- return RunGit(['symbolic-ref', 'HEAD'],
- stderr=subprocess2.VOID, error_ok=True).strip() or None
-
-
-def GetCurrentBranch():
- """Returns current branch or None.
-
- For refs/heads/* branches, returns just last part. For others, full ref.
- """
- branchref = GetCurrentBranchRef()
- if branchref:
- return ShortBranchName(branchref)
- return None
+ return branch.replace('refs/heads/', '')
class Changelist(object):
- """Changelist works with one changelist in local branch.
-
- Supports two codereview backends: Rietveld or Gerrit, selected at object
- creation.
-
- Not safe for concurrent multi-{thread,process} use.
- """
-
- def __init__(self, branchref=None, issue=None, codereview=None, **kwargs):
- """Create a new ChangeList instance.
-
- If issue is given, the codereview must be given too.
-
- If `codereview` is given, it must be 'rietveld' or 'gerrit'.
- Otherwise, it's decided based on current configuration of the local branch,
- with default being 'rietveld' for backwards compatibility.
- See _load_codereview_impl for more details.
-
- **kwargs will be passed directly to codereview implementation.
- """
+ def __init__(self, branchref=None, issue=None, auth_config=None):
# Poke settings so we get the "configure your server" message if necessary.
global settings
if not settings:
# Happens when git_cl.py is used as a utility library.
settings = Settings()
-
- if issue:
- assert codereview, 'codereview must be known, if issue is known'
-
+ settings.GetDefaultServerUrl()
self.branchref = branchref
if self.branchref:
self.branch = ShortBranchName(self.branchref)
else:
self.branch = None
+ self.rietveld_server = None
self.upstream_branch = None
self.lookedup_issue = False
self.issue = issue or None
@@ -878,39 +839,14 @@
self.patchset = None
self.cc = None
self.watchers = ()
+ self._auth_config = auth_config
+ self._props = None
self._remote = None
+ self._rpc_server = None
- self._codereview_impl = None
- self._load_codereview_impl(codereview, **kwargs)
-
- def _load_codereview_impl(self, codereview=None, **kwargs):
- if codereview:
- codereview = codereview.lower()
- if codereview == 'gerrit':
- self._codereview_impl = _GerritChangelistImpl(self, **kwargs)
- elif codereview == 'rietveld':
- self._codereview_impl = _RietveldChangelistImpl(self, **kwargs)
- else:
- assert codereview in ('rietveld', 'gerrit')
- return
-
- # Automatic selection based on issue number set for a current branch.
- # Rietveld takes precedence over Gerrit.
- assert not self.issue
- # Whether we find issue or not, we are doing the lookup.
- self.lookedup_issue = True
- for cls in [_RietveldChangelistImpl, _GerritChangelistImpl]:
- setting = cls.IssueSetting(self.GetBranch())
- issue = RunGit(['config', setting], error_ok=True).strip()
- if issue:
- self._codereview_impl = cls(self, **kwargs)
- self.issue = int(issue)
- return
-
- # No issue is set for this branch, so decide based on repo-wide settings.
- return self._load_codereview_impl(
- codereview='gerrit' if settings.GetIsGerrit() else 'rietveld')
-
+ @property
+ def auth_config(self):
+ return self._auth_config
def GetCCList(self):
"""Return the users cc'd on this CL.
@@ -938,7 +874,8 @@
def GetBranch(self):
"""Returns the short branch name, e.g. 'master'."""
if not self.branch:
- branchref = GetCurrentBranchRef()
+ branchref = RunGit(['symbolic-ref', 'HEAD'],
+ stderr=subprocess2.VOID, error_ok=True).strip()
if not branchref:
return None
self.branchref = branchref
@@ -982,12 +919,11 @@
remote = 'origin'
upstream_branch = 'refs/heads/trunk'
else:
- DieWithError(
- 'Unable to determine default branch to diff against.\n'
- 'Either pass complete "git diff"-style arguments, like\n'
- ' git cl upload origin/master\n'
- 'or verify this branch is set up to track another \n'
- '(via the --track argument to "git checkout -b ...").')
+ DieWithError("""Unable to determine default branch to diff against.
+Either pass complete "git diff"-style arguments, like
+ git cl upload origin/master
+or verify this branch is set up to track another (via the --track argument to
+"git checkout -b ...").""")
return remote, upstream_branch
@@ -1129,24 +1065,65 @@
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:
- issue = RunGit(['config',
- self._codereview_impl.IssueSetting(self.GetBranch())],
- error_ok=True).strip()
+ issue = RunGit(['config', self._IssueSetting()], error_ok=True).strip()
self.issue = int(issue) or None if issue else None
self.lookedup_issue = True
return self.issue
+ def GetRietveldServer(self):
+ if not self.rietveld_server:
+ # If we're on a branch then get the server potentially associated
+ # with that branch.
+ if self.GetIssue():
+ rietveld_server_config = self._RietveldServer()
+ if rietveld_server_config:
+ self.rietveld_server = gclient_utils.UpgradeToHttps(RunGit(
+ ['config', rietveld_server_config], error_ok=True).strip())
+ if not self.rietveld_server:
+ self.rietveld_server = settings.GetDefaultServerUrl()
+ return self.rietveld_server
+
+ def GetGerritServer(self):
+ # We don't support multiple Gerrit servers, and assume it to be same as
+ # origin, except with a '-review' suffix for first subdomain.
+ parts = urlparse.urlparse(self.GetRemoteUrl()).netloc.split('.')
+ parts[0] = parts[0] + '-review'
+ return 'https://%s' % '.'.join(parts)
+
def GetIssueURL(self):
"""Get the URL for a particular issue."""
- issue = self.GetIssue()
- if not issue:
+ if not self.GetIssue():
return None
- return '%s/%s' % (self._codereview_impl.GetCodereviewServer(), issue)
+ if settings.GetIsGerrit():
+ return '%s/%s' % (self.GetGerritServer(), self.GetIssue())
+ return '%s/%s' % (self.GetRietveldServer(), self.GetIssue())
def GetDescription(self, pretty=False):
if not self.has_description:
if self.GetIssue():
- self.description = self._codereview_impl.FetchDescription()
+ issue = self.GetIssue()
+ try:
+ self.description = self.RpcServer().get_description(issue).strip()
+ except urllib2.HTTPError as e:
+ if e.code == 404:
+ DieWithError(
+ ('\nWhile fetching the description for issue %d, received a '
+ '404 (not found)\n'
+ 'error. It is likely that you deleted this '
+ 'issue on the server. If this is the\n'
+ 'case, please run\n\n'
+ ' git cl issue 0\n\n'
+ 'to clear the association with the deleted issue. Then run '
+ 'this command again.') % issue)
+ else:
+ DieWithError(
+ '\nFailed to fetch issue description. HTTP error %d' % e.code)
+ except urllib2.URLError as e:
+ print >> sys.stderr, (
+ 'Warning: Failed to retrieve CL description due to network '
+ 'failure.')
+ self.description = ''
+
self.has_description = True
if pretty:
wrapper = textwrap.TextWrapper()
@@ -1157,7 +1134,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:
- patchset = RunGit(['config', self._codereview_impl.PatchsetSetting()],
+ patchset = RunGit(['config', self._PatchsetSetting()],
error_ok=True).strip()
self.patchset = int(patchset) or None if patchset else None
self.lookedup_patchset = True
@@ -1165,29 +1142,47 @@
def SetPatchset(self, patchset):
"""Set this branch's patchset. If patchset=0, clears the patchset."""
- patchset_setting = self._codereview_impl.PatchsetSetting()
if patchset:
- RunGit(['config', patchset_setting, str(patchset)])
+ RunGit(['config', self._PatchsetSetting(), str(patchset)])
self.patchset = patchset
else:
- RunGit(['config', '--unset', patchset_setting],
+ RunGit(['config', '--unset', self._PatchsetSetting()],
stderr=subprocess2.PIPE, error_ok=True)
self.patchset = None
+ def GetMostRecentPatchset(self):
+ return self.GetIssueProperties()['patchsets'][-1]
+
+ def GetPatchSetDiff(self, issue, patchset):
+ return self.RpcServer().get(
+ '/download/issue%s_%s.diff' % (issue, patchset))
+
+ def GetIssueProperties(self):
+ if self._props is None:
+ issue = self.GetIssue()
+ if not issue:
+ self._props = {}
+ else:
+ self._props = self.RpcServer().get_issue_properties(issue, True)
+ return self._props
+
+ def GetApprovingReviewers(self):
+ return get_approving_reviewers(self.GetIssueProperties())
+
+ def AddComment(self, message):
+ return self.RpcServer().add_comment(self.GetIssue(), message)
+
def SetIssue(self, issue=None):
"""Set this branch's issue. If issue isn't given, clears the issue."""
- issue_setting = self._codereview_impl.IssueSetting(self.GetBranch())
- codereview_setting = self._codereview_impl.GetCodereviewServerSetting()
if issue:
self.issue = issue
- RunGit(['config', issue_setting, str(issue)])
- codereview_server = self._codereview_impl.GetCodereviewServer()
- if codereview_server:
- RunGit(['config', codereview_setting, codereview_server])
+ RunGit(['config', self._IssueSetting(), str(issue)])
+ if not settings.GetIsGerrit() and self.rietveld_server:
+ RunGit(['config', self._RietveldServer(), self.rietveld_server])
else:
current_issue = self.GetIssue()
if current_issue:
- RunGit(['config', '--unset', issue_setting])
+ RunGit(['config', '--unset', self._IssueSetting()])
self.issue = None
self.SetPatchset(None)
@@ -1237,186 +1232,6 @@
author,
upstream=upstream_branch)
- def UpdateDescription(self, description):
- self.description = description
- return self._codereview_impl.UpdateDescriptionRemote(description)
-
- def RunHook(self, committing, may_prompt, verbose, change):
- """Calls sys.exit() if the hook fails; returns a HookResults otherwise."""
- try:
- return presubmit_support.DoPresubmitChecks(change, committing,
- verbose=verbose, output_stream=sys.stdout, input_stream=sys.stdin,
- default_presubmit=None, may_prompt=may_prompt,
- rietveld_obj=self._codereview_impl.GetRieveldObjForPresubmit())
- except presubmit_support.PresubmitFailure, e:
- DieWithError(
- ('%s\nMaybe your depot_tools is out of date?\n'
- 'If all fails, contact maruel@') % e)
-
- # Forward methods to codereview specific implementation.
-
- def CloseIssue(self):
- return self._codereview_impl.CloseIssue()
-
- def GetStatus(self):
- return self._codereview_impl.GetStatus()
-
- def GetCodereviewServer(self):
- return self._codereview_impl.GetCodereviewServer()
-
- def GetApprovingReviewers(self):
- return self._codereview_impl.GetApprovingReviewers()
-
- def GetMostRecentPatchset(self):
- return self._codereview_impl.GetMostRecentPatchset()
-
- def __getattr__(self, attr):
- # This is because lots of untested code accesses Rietveld-specific stuff
- # directly, and it's hard to fix for sure. So, just let it work, and fix
- # on a cases by case basis.
- return getattr(self._codereview_impl, attr)
-
-
-class _ChangelistCodereviewBase(object):
- """Abstract base class encapsulating codereview specifics of a changelist."""
- def __init__(self, changelist):
- self._changelist = changelist # instance of Changelist
-
- def __getattr__(self, attr):
- # Forward methods to changelist.
- # TODO(tandrii): maybe clean up _GerritChangelistImpl and
- # _RietveldChangelistImpl to avoid this hack?
- return getattr(self._changelist, attr)
-
- def GetStatus(self):
- """Apply a rough heuristic to give a simple summary of an issue's review
- or CQ status, assuming adherence to a common workflow.
-
- Returns None if no issue for this branch, or specific string keywords.
- """
- raise NotImplementedError()
-
- def GetCodereviewServer(self):
- """Returns server URL without end slash, like "https://codereview.com"."""
- raise NotImplementedError()
-
- def FetchDescription(self):
- """Fetches and returns description from the codereview server."""
- raise NotImplementedError()
-
- def GetCodereviewServerSetting(self):
- """Returns git config setting for the codereview server."""
- raise NotImplementedError()
-
- @staticmethod
- def IssueSetting(branch):
- """Returns name of git config setting which stores issue number for a given
- branch."""
- raise NotImplementedError()
-
- def PatchsetSetting(self):
- """Returns name of git config setting which stores issue number."""
- raise NotImplementedError()
-
- def GetRieveldObjForPresubmit(self):
- # This is an unfortunate Rietveld-embeddedness in presubmit.
- # For non-Rietveld codereviews, this probably should return a dummy object.
- raise NotImplementedError()
-
- def UpdateDescriptionRemote(self, description):
- """Update the description on codereview site."""
- raise NotImplementedError()
-
- def CloseIssue(self):
- """Closes the issue."""
- raise NotImplementedError()
-
- def GetApprovingReviewers(self):
- """Returns a list of reviewers approving the change.
-
- Note: not necessarily committers.
- """
- raise NotImplementedError()
-
- def GetMostRecentPatchset(self):
- """Returns the most recent patchset number from the codereview site."""
- raise NotImplementedError()
-
-
-class _RietveldChangelistImpl(_ChangelistCodereviewBase):
- def __init__(self, changelist, auth_config=None, rietveld_server=None):
- super(_RietveldChangelistImpl, self).__init__(changelist)
- assert settings, 'must be initialized in _ChangelistCodereviewBase'
- settings.GetDefaultServerUrl()
-
- self._rietveld_server = rietveld_server
- self._auth_config = auth_config
- self._props = None
- self._rpc_server = None
-
- def GetAuthConfig(self):
- return self._auth_config
-
- def GetCodereviewServer(self):
- if not self._rietveld_server:
- # If we're on a branch then get the server potentially associated
- # with that branch.
- if self.GetIssue():
- rietveld_server_setting = self.GetCodereviewServerSetting()
- if rietveld_server_setting:
- self._rietveld_server = gclient_utils.UpgradeToHttps(RunGit(
- ['config', rietveld_server_setting], error_ok=True).strip())
- if not self._rietveld_server:
- self._rietveld_server = settings.GetDefaultServerUrl()
- return self._rietveld_server
-
- def FetchDescription(self):
- issue = self.GetIssue()
- assert issue
- try:
- return self.RpcServer().get_description(issue).strip()
- except urllib2.HTTPError as e:
- if e.code == 404:
- DieWithError(
- ('\nWhile fetching the description for issue %d, received a '
- '404 (not found)\n'
- 'error. It is likely that you deleted this '
- 'issue on the server. If this is the\n'
- 'case, please run\n\n'
- ' git cl issue 0\n\n'
- 'to clear the association with the deleted issue. Then run '
- 'this command again.') % issue)
- else:
- DieWithError(
- '\nFailed to fetch issue description. HTTP error %d' % e.code)
- except urllib2.URLError as e:
- print >> sys.stderr, (
- 'Warning: Failed to retrieve CL description due to network '
- 'failure.')
- return ''
-
- def GetMostRecentPatchset(self):
- return self.GetIssueProperties()['patchsets'][-1]
-
- def GetPatchSetDiff(self, issue, patchset):
- return self.RpcServer().get(
- '/download/issue%s_%s.diff' % (issue, patchset))
-
- def GetIssueProperties(self):
- if self._props is None:
- issue = self.GetIssue()
- if not issue:
- self._props = {}
- else:
- self._props = self.RpcServer().get_issue_properties(issue, True)
- return self._props
-
- def GetApprovingReviewers(self):
- return get_approving_reviewers(self.GetIssueProperties())
-
- def AddComment(self, message):
- return self.RpcServer().add_comment(self.GetIssue(), message)
-
def GetStatus(self):
"""Apply a rough heuristic to give a simple summary of an issue's review
or CQ status, assuming adherence to a common workflow.
@@ -1464,11 +1279,26 @@
return 'reply'
return 'waiting'
- def UpdateDescriptionRemote(self, description):
+ def RunHook(self, committing, may_prompt, verbose, change):
+ """Calls sys.exit() if the hook fails; returns a HookResults otherwise."""
+
+ try:
+ return presubmit_support.DoPresubmitChecks(change, committing,
+ verbose=verbose, output_stream=sys.stdout, input_stream=sys.stdin,
+ default_presubmit=None, may_prompt=may_prompt,
+ rietveld_obj=self.RpcServer())
+ except presubmit_support.PresubmitFailure, e:
+ DieWithError(
+ ('%s\nMaybe your depot_tools is out of date?\n'
+ 'If all fails, contact maruel@') % e)
+
+ def UpdateDescription(self, description):
+ self.description = description
return self.RpcServer().update_description(
self.GetIssue(), self.description)
def CloseIssue(self):
+ """Updates the description and closes the issue."""
return self.RpcServer().close_issue(self.GetIssue())
def SetFlag(self, flag, value):
@@ -1492,116 +1322,25 @@
"""
if not self._rpc_server:
self._rpc_server = rietveld.CachingRietveld(
- self.GetCodereviewServer(),
+ self.GetRietveldServer(),
self._auth_config or auth.make_auth_config())
return self._rpc_server
- @staticmethod
- def IssueSetting(branch):
- return 'branch.%s.rietveldissue' % branch
+ def _IssueSetting(self):
+ """Return the git setting that stores this change's issue."""
+ return 'branch.%s.rietveldissue' % self.GetBranch()
- def PatchsetSetting(self):
+ def _PatchsetSetting(self):
"""Return the git setting that stores this change's most recent patchset."""
return 'branch.%s.rietveldpatchset' % self.GetBranch()
- def GetCodereviewServerSetting(self):
+ def _RietveldServer(self):
"""Returns the git setting that stores this change's rietveld server."""
branch = self.GetBranch()
if branch:
return 'branch.%s.rietveldserver' % branch
return None
- def GetRieveldObjForPresubmit(self):
- return self.RpcServer()
-
-
-class _GerritChangelistImpl(_ChangelistCodereviewBase):
- def __init__(self, changelist, auth_config=None):
- # auth_config is Rietveld thing, kept here to preserve interface only.
- super(_GerritChangelistImpl, self).__init__(changelist)
- self._change_id = None
- self._gerrit_server = None # e.g. https://chromium-review.googlesource.com
- self._gerrit_host = None # e.g. chromium-review.googlesource.com
-
- def _GetGerritHost(self):
- # Lazy load of configs.
- self.GetCodereviewServer()
- return self._gerrit_host
-
- def GetCodereviewServer(self):
- if not self._gerrit_server:
- # If we're on a branch then get the server potentially associated
- # with that branch.
- if self.GetIssue():
- gerrit_server_setting = self.GetCodereviewServerSetting()
- if gerrit_server_setting:
- self._gerrit_server = RunGit(['config', gerrit_server_setting],
- error_ok=True).strip()
- if self._gerrit_server:
- self._gerrit_host = urlparse.urlparse(self._gerrit_server).netloc
- if not self._gerrit_server:
- # We assume repo to be hosted on Gerrit, and hence Gerrit server
- # has "-review" suffix for lowest level subdomain.
- parts = urlparse.urlparse(self.GetRemoteUrl()).netloc.split('.')
- parts[0] = parts[0] + '-review'
- self._gerrit_host = '.'.join(parts)
- self._gerrit_server = 'https://%s' % self._gerrit_host
- return self._gerrit_server
-
- @staticmethod
- def IssueSetting(branch):
- return 'branch.%s.gerritissue' % branch
-
- def PatchsetSetting(self):
- """Return the git setting that stores this change's most recent patchset."""
- return 'branch.%s.gerritpatchset' % self.GetBranch()
-
- def GetCodereviewServerSetting(self):
- """Returns the git setting that stores this change's Gerrit server."""
- branch = self.GetBranch()
- if branch:
- return 'branch.%s.gerritserver' % branch
- return None
-
- def GetRieveldObjForPresubmit(self):
- class ThisIsNotRietveldIssue(object):
- def __nonzero__(self):
- # This is a hack to make presubmit_support think that rietveld is not
- # defined, yet still ensure that calls directly result in a decent
- # exception message below.
- return False
-
- def __getattr__(self, attr):
- print(
- 'You aren\'t using Rietveld at the moment, but Gerrit.\n'
- 'Using Rietveld in your PRESUBMIT scripts won\'t work.\n'
- 'Please, either change your PRESUBIT to not use rietveld_obj.%s,\n'
- 'or use Rietveld for codereview.\n'
- 'See also http://crbug.com/579160.' % attr)
- raise NotImplementedError()
- return ThisIsNotRietveldIssue()
-
- def GetStatus(self):
- # TODO(tandrii)
- raise NotImplementedError()
-
- def GetMostRecentPatchset(self):
- data = gerrit_util.GetChangeDetail(self._GetGerritHost(), self.GetIssue(),
- ['CURRENT_REVISION'])
- return data['revisions'][data['current_revision']]['_number']
-
- def FetchDescription(self):
- data = gerrit_util.GetChangeDetail(self._GetGerritHost(), self.GetIssue(),
- ['COMMIT_FOOTERS', 'CURRENT_REVISION'])
- return data['revisions'][data['current_revision']]['commit_with_footers']
-
- def UpdateDescriptionRemote(self, description):
- # TODO(tandrii)
- raise NotImplementedError()
-
- def CloseIssue(self):
- gerrit_util.AbandonChange(self._GetGerritHost(), self.GetIssue(), msg='')
-
class ChangeDescription(object):
"""Contains a parsed form of the change description."""
@@ -2147,7 +1886,6 @@
changes = (
Changelist(branchref=b, auth_config=auth_config)
for b in branches.splitlines())
- # TODO(tandrii): refactor to use CLs list instead of branches list.
branches = [c.GetBranch() for c in changes]
alignment = max(5, max(len(b) for b in branches))
print 'Branches associated with reviews:'
@@ -2263,7 +2001,7 @@
except ValueError:
DieWithError('A review issue id is expected to be a number')
- cl = Changelist(issue=issue, codereview='rietveld', auth_config=auth_config)
+ cl = Changelist(issue=issue, auth_config=auth_config)
if options.comment:
cl.AddComment(options.comment)
@@ -2642,10 +2380,8 @@
def RietveldUpload(options, args, cl, change):
"""upload the patch to rietveld."""
upload_args = ['--assume_yes'] # Don't ask about untracked files.
- upload_args.extend(['--server', cl.GetCodereviewServer()])
- # TODO(tandrii): refactor this ugliness into _RietveldChangelistImpl.
- upload_args.extend(auth.auth_config_to_command_options(
- cl._codereview_impl.GetAuthConfig()))
+ upload_args.extend(['--server', cl.GetRietveldServer()])
+ upload_args.extend(auth.auth_config_to_command_options(cl.auth_config))
if options.emulate_svn_auto_props:
upload_args.append('--emulate_svn_auto_props')
@@ -2888,9 +2624,9 @@
# during the actual upload.
if not settings.GetIsGerrit() and auth_config.use_oauth2:
authenticator = auth.get_authenticator_for_host(
- cl.GetCodereviewServer(), auth_config)
+ cl.GetRietveldServer(), auth_config)
if not authenticator.has_cached_credentials():
- raise auth.LoginRequiredError(cl.GetCodereviewServer())
+ raise auth.LoginRequiredError(cl.GetRietveldServer())
# Apply watchlists on upload.
change = cl.GetChange(base_branch, None)
@@ -3484,13 +3220,12 @@
# consequences of the caller not checking this could be dire.
assert(not git_common.is_dirty_git_tree('apply'))
- # TODO(tandrii): implement for Gerrit.
if type(issue_arg) is int or issue_arg.isdigit():
# Input is an issue id. Figure out the URL.
issue = int(issue_arg)
- cl = Changelist(issue=issue, codereview='rietveld', auth_config=auth_config)
+ cl = Changelist(issue=issue, auth_config=auth_config)
patchset = cl.GetMostRecentPatchset()
- patch_data = cl._codereview_impl.GetPatchSetDiff(issue, patchset)
+ patch_data = cl.GetPatchSetDiff(issue, patchset)
else:
# Assume it's a URL to the patch. Default to https.
issue_url = gclient_utils.UpgradeToHttps(issue_arg)
@@ -3499,8 +3234,8 @@
DieWithError('Must pass an issue ID or full URL for '
'\'Download raw patch set\'')
issue = int(match.group(2))
- cl = Changelist(issue=issue, codereview='rietveld',
- rietvled_server=match.group(1), auth_config=auth_config)
+ cl = Changelist(issue=issue, auth_config=auth_config)
+ cl.rietveld_server = match.group(1)
patchset = int(match.group(3))
patch_data = urllib2.urlopen(issue_arg).read()
@@ -3544,8 +3279,7 @@
'patch from issue %(i)s at patchset '
'%(p)s (http://crrev.com/%(i)s#ps%(p)s)'
% {'i': issue, 'p': patchset})])
- cl = Changelist(codereview='rietveld', auth_config=auth_config,
- rietveld_server=cl.GetCodereviewServer())
+ cl = Changelist(auth_config=auth_config)
cl.SetIssue(issue)
cl.SetPatchset(patchset)
print "Committed patch locally."