[autotest] Make push-to-prod sync more aggressive.
Before this CL, the push-to-prod git repo sync through build_externals
would try to 'pull' the relevant repos. This would fail to pull new
branches from the remote repo.
This CL re-works the sync code to make it more aggressive. It now goes
through the more reliable fetch-and-reset path.
While there: fix unittest for unset git_repo_url. There was duplicated
code that was obviously not testing the 'fetch' path at least.
BUG=chromium:476124
TEST=(1) Synced a new remote branch to an existing repo that used to
fail before this CL.
(2) (updated) unittests.
Change-Id: I6d5337532aed62afd270762f21d9f496a823f2d5
Reviewed-on: https://chromium-review.googlesource.com/265351
Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org>
Commit-Queue: Prathmesh Prabhu <pprabhu@chromium.org>
Tested-by: Prathmesh Prabhu <pprabhu@chromium.org>
diff --git a/client/common_lib/revision_control.py b/client/common_lib/revision_control.py
index d271fdc..4602ce9 100644
--- a/client/common_lib/revision_control.py
+++ b/client/common_lib/revision_control.py
@@ -123,17 +123,35 @@
timeout, ignore_status)
- def gitcmd(self, cmd, ignore_status=False):
+ def gitcmd(self, cmd, ignore_status=False, error_class=None,
+ error_msg=None):
"""
Wrapper for a git command.
@param cmd: Git subcommand (ex 'clone').
- @param ignore_status: Whether we should supress error.CmdError
- exceptions if the command did return exit code !=0 (True), or
- not supress them (False).
+ @param ignore_status: If True, ignore the CmdError raised by the
+ underlying command runner. NB: Passing in an error_class
+ impiles ignore_status=True.
+ @param error_class: When ignore_status is False, optional error
+ error class to log and raise in case of errors. Must be a
+ (sub)type of GitError.
+ @param error_msg: When passed with error_class, used as a friendly
+ error message.
"""
+ # TODO(pprabhu) Get rid of the ignore_status argument.
+ # Now that we support raising custom errors, we always want to get a
+ # return code from the command execution, instead of an exception.
+ ignore_status = ignore_status or error_class is not None
cmd = '%s %s' % (self.gen_git_cmd_base(), cmd)
- return self._run(cmd, ignore_status=ignore_status)
+ rv = self._run(cmd, ignore_status=ignore_status)
+ if rv.exit_status != 0 and error_class is not None:
+ logging.error('git command failed: %s: %s',
+ cmd, error_msg if error_msg is not None else '')
+ logging.error(rv.stderr)
+ raise error_class(error_msg if error_msg is not None
+ else rv.stderr)
+
+ return rv
def clone(self):
@@ -194,10 +212,25 @@
raise revision_control.GitCommitError('Unable to commit', rv)
+ def reset(self, branch_or_sha):
+ """
+ Reset repo to the given branch or git sha.
+
+ @param branch_or_sha: Name of a local or remote branch or git sha.
+
+ @raises GitResetError if operation fails.
+ """
+ self.gitcmd('reset --hard %s' % branch_or_sha,
+ error_class=GitResetError,
+ error_msg='Failed to reset to %s' % branch_or_sha)
+
+
def reset_head(self):
"""
Reset repo to HEAD@{0} by running git reset --hard HEAD.
+ TODO(pprabhu): cleanup. Use reset.
+
@raises GitResetError: if we fails to reset HEAD.
"""
logging.info('Resetting head on repo %s', self.repodir)
@@ -222,21 +255,49 @@
raise GitFetchError(e_msg, rv)
- def pull_or_clone(self):
+ def reinit_repo_at(self, remote_branch):
"""
- Pulls if the repo is already initialized, clones if it isn't.
+ Does all it can to ensure that the repo is at remote_branch.
+
+ This will try to be nice and detect any local changes and bail early.
+ OTOH, if it finishes successfully, it'll blow away anything and
+ everything so that local repo reflects the upstream branch requested.
"""
- # TODO beeps: if the user has local changes in the repo they're
- # pulling into, this could fail on rebase. Currently the only consumer
- # of this method is external_packages and it makes sense for
- # build_externals to fail in such a scenario. Investigate ways to get
- # this to squash local changes using git rev-parse to get the upstream
- # tracking branch name, and then do a fetch + reset head.
- if self.is_repo_initialized():
- self.pull(rebase=True)
- else:
+ if not self.is_repo_initialized():
self.clone()
+ # Play nice. Detect any local changes and bail.
+ # Re-stat all files before comparing index. This is needed for
+ # diff-index to work properly in cases when the stat info on files is
+ # stale. (e.g., you just untarred the whole git folder that you got from
+ # Alice)
+ rv = self.gitcmd('update-index --refresh -q',
+ error_class=GitError,
+ error_msg='Failed to refresh index.')
+ rv = self.gitcmd(
+ 'diff-index --quiet HEAD --',
+ error_class=GitError,
+ error_msg='Failed to check for local changes.')
+ if rv.stdout:
+ loggin.error(rv.stdout)
+ e_msg = 'Local checkout dirty. (%s)'
+ raise GitError(e_msg % rv.stdout)
+
+ # Play the bad cop. Destroy everything in your path.
+ # Don't trust the existing repo setup at all (so don't trust the current
+ # config, current branches / remotes etc).
+ self.gitcmd('config remote.origin.url %s' % self.giturl,
+ error_class=GitError,
+ error_msg='Failed to set origin.')
+ self.gitcmd('checkout -f',
+ error_class=GitError,
+ error_msg='Failed to checkout.')
+ self.gitcmd('clean -qxdf',
+ error_class=GitError,
+ error_msg='Failed to clean.')
+ self.fetch_remote()
+ self.reset('origin/%s' % remote_branch)
+
def get(self, **kwargs):
"""
diff --git a/client/common_lib/revision_control_unittest.py b/client/common_lib/revision_control_unittest.py
index 3705c19..9733101 100755
--- a/client/common_lib/revision_control_unittest.py
+++ b/client/common_lib/revision_control_unittest.py
@@ -1,5 +1,8 @@
#! /usr/bin/python
-import logging, mox, os, shutil, tempfile, utils
+import logging, mox, os, shutil, tempfile, unittest, utils
+
+# This makes autotest_lib imports available.
+import common
from autotest_lib.client.common_lib import revision_control
@@ -29,6 +32,11 @@
self.repodir,
self.repodir,
abs_work_tree=self.repodir)
+ # Create an initial commit. We really care about the common case
+ # where there exists a commit in the upstream repo.
+ self._edit('initial_commit_file', 'is_non_empty')
+ self.add()
+ self.commit('initial_commit')
else:
self.repodir = tempfile.mktemp(suffix='dependent')
self.git_repo_manager = revision_control.GitRepo(
@@ -95,7 +103,7 @@
Get everything from masters TOT squashing local changes.
If the dependent repo is empty pull from master.
"""
- self.git_repo_manager.pull_or_clone()
+ self.git_repo_manager.reinit_repo_at('master')
self.commit_hash = self.git_repo_manager.get_latest_commit_hash()
@@ -156,14 +164,8 @@
Test that git clone raises a ValueError if giturl is unset.
"""
self.dependent_repo.git_repo_manager._giturl = None
- self.mox.StubOutWithMock(revision_control.GitRepo,
- 'is_repo_initialized')
- self.dependent_repo.git_repo_manager.is_repo_initialized().AndReturn(
- False)
- self.mox.ReplayAll()
-
self.assertRaises(ValueError,
- self.dependent_repo.git_repo_manager.pull_or_clone)
+ self.dependent_repo.git_repo_manager.clone)
def testGitUrlPull(self):
@@ -171,14 +173,8 @@
Test that git pull raises a ValueError if giturl is unset.
"""
self.dependent_repo.git_repo_manager._giturl = None
- self.mox.StubOutWithMock(revision_control.GitRepo,
- 'is_repo_initialized')
- self.dependent_repo.git_repo_manager.is_repo_initialized().AndReturn(
- True)
- self.mox.ReplayAll()
-
self.assertRaises(ValueError,
- self.dependent_repo.git_repo_manager.pull_or_clone)
+ self.dependent_repo.git_repo_manager.pull)
def testGitUrlFetch(self):
@@ -186,11 +182,9 @@
Test that git fetch raises a ValueError if giturl is unset.
"""
self.dependent_repo.git_repo_manager._giturl = None
- self.mox.StubOutWithMock(revision_control.GitRepo,
- 'is_repo_initialized')
- self.dependent_repo.git_repo_manager.is_repo_initialized().AndReturn(
- True)
- self.mox.ReplayAll()
-
self.assertRaises(ValueError,
- self.dependent_repo.git_repo_manager.pull_or_clone)
+ self.dependent_repo.git_repo_manager.fetch_remote)
+
+
+if __name__ == '__main__':
+ unittest.main()
diff --git a/utils/external_packages.py b/utils/external_packages.py
index 32b116e..53f851e 100644
--- a/utils/external_packages.py
+++ b/utils/external_packages.py
@@ -1071,16 +1071,6 @@
return True
- def git_pull_or_clone_to_prod(self, git_repo):
- """
- Pull or clone repo and checkout the 'prod' branch.
-
- @param git_repo: A revision_controlGitRepo object to use for git commands.
- """
- git_repo.pull_or_clone()
- git_repo.checkout(self.PROD_BRANCH)
-
-
def build_and_install(self, unused_install_dir):
"""
Fall through method to install a package.
@@ -1123,7 +1113,7 @@
self._GIT_URL,
None,
abs_work_tree=self.temp_hdctools_dir)
- self.git_pull_or_clone_to_prod(git_repo)
+ git_repo.reinit_repo_at(self.PROD_BRANCH)
if git_repo.get_latest_commit_hash():
return True
@@ -1162,7 +1152,7 @@
local_chromite_dir,
self._GIT_URL,
abs_work_tree=local_chromite_dir)
- self.git_pull_or_clone_to_prod(git_repo)
+ git_repo.reinit_repo_at(self.PROD_BRANCH)
if git_repo.get_latest_commit_hash():
@@ -1189,7 +1179,7 @@
local_devserver_dir = os.path.join(install_dir, 'devserver')
git_repo = revision_control.GitRepo(local_devserver_dir, self._GIT_URL,
abs_work_tree=local_devserver_dir)
- self.git_pull_or_clone_to_prod(git_repo)
+ git_repo.reinit_repo_at(self.PROD_BRANCH)
if git_repo.get_latest_commit_hash():
return True
@@ -1223,7 +1213,7 @@
self._GIT_URL,
None,
abs_work_tree=self.temp_btsocket_dir.name)
- self.git_pull_or_clone_to_prod(git_repo)
+ git_repo.reinit_repo_at(self.PROD_BRANCH)
if git_repo.get_latest_commit_hash():
return True