diff --git a/scm.py b/scm.py index 30bb3d17e..d53b4f4ed 100644 --- a/scm.py +++ b/scm.py @@ -140,6 +140,10 @@ class GIT(object): results.append(('%s ' % m.group(1)[0], m.group(2))) return results + @staticmethod + def IsWorkTreeDirty(cwd): + return GIT.Capture(['status', '-s'], cwd=cwd) != '' + @staticmethod def GetEmail(cwd): """Retrieves the user email address if known.""" @@ -389,6 +393,17 @@ class GIT(object): root = GIT.Capture(['rev-parse', '--show-cdup'], cwd=cwd) return os.path.abspath(os.path.join(cwd, root)) + @staticmethod + def GetGitDir(cwd): + return os.path.abspath(GIT.Capture(['rev-parse', '--git-dir'], cwd=cwd)) + + @staticmethod + def IsInsideWorkTree(cwd): + try: + return GIT.Capture(['rev-parse', '--is-inside-work-tree'], cwd=cwd) + except (OSError, subprocess2.CalledProcessError): + return False + @staticmethod def GetGitSvnHeadRev(cwd): """Gets the most recently pulled git-svn revision.""" diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py index ff03c03e1..bf9c3534d 100755 --- a/tests/scm_unittest.py +++ b/tests/scm_unittest.py @@ -85,13 +85,16 @@ class GitWrapperTestCase(BaseSCMTestCase): 'GetCheckoutRoot', 'GetDifferentFiles', 'GetEmail', + 'GetGitDir', 'GetGitSvnHeadRev', 'GetPatchName', 'GetSha1ForSvnRev', 'GetSVNBranch', 'GetUpstreamBranch', 'IsGitSvn', + 'IsInsideWorkTree', 'IsValidRevision', + 'IsWorkTreeDirty', 'MatchSvnGlob', 'ParseGitSvnSha1', 'ShortBranchName', diff --git a/tests/trychange_unittest.py b/tests/trychange_unittest.py index 3e6e0b43f..069d0d374 100755 --- a/tests/trychange_unittest.py +++ b/tests/trychange_unittest.py @@ -46,10 +46,12 @@ class TryChangeUnittest(TryChangeTestsBase): """General trychange.py tests.""" def testMembersChanged(self): members = [ - 'DieWithError', 'EPILOG', 'Escape', 'GIT', 'GetMungedDiff', 'GuessVCS', + 'DieWithError', 'EPILOG', 'Escape', 'GIT', 'GIT_PATCH_DIR_BASENAME', + 'GetMungedDiff', 'GuessVCS', 'GIT_BRANCH_FILE', 'HELP_STRING', 'InvalidScript', 'NoTryServerAccess', 'OptionParser', 'PrintSuccess', - 'RunCommand', 'RunGit', 'SCM', 'SVN', 'TryChange', 'USAGE', 'breakpad', + 'RunCommand', 'RunGit', 'SCM', 'SVN', 'TryChange', 'USAGE', 'contextlib', + 'breakpad', 'datetime', 'errno', 'fix_encoding', 'gcl', 'gclient_utils', 'gen_parser', 'getpass', 'itertools', 'json', 'logging', 'optparse', 'os', 'posixpath', 're', 'scm', 'shutil', 'subprocess2', 'sys', 'tempfile', 'urllib', diff --git a/trychange.py b/trychange.py index f93d9a418..b744fc6dd 100755 --- a/trychange.py +++ b/trychange.py @@ -4,10 +4,11 @@ # found in the LICENSE file. """Client-side script to send a try job to the try server. It communicates to -the try server by either writting to a svn repository or by directly connecting -to the server by HTTP. +the try server by either writting to a svn/git repository or by directly +connecting to the server by HTTP. """ +import contextlib import datetime import errno import getpass @@ -70,6 +71,9 @@ Examples: -f include/b.h """ +GIT_PATCH_DIR_BASENAME = os.path.join('git-try', 'patches-git') +GIT_BRANCH_FILE = 'ref' +_GIT_PUSH_ATTEMPTS = 3 def DieWithError(message): print >> sys.stderr, message @@ -164,7 +168,10 @@ class SCM(object): 'port': self.GetCodeReviewSetting('TRYSERVER_HTTP_PORT'), 'host': self.GetCodeReviewSetting('TRYSERVER_HTTP_HOST'), 'svn_repo': self.GetCodeReviewSetting('TRYSERVER_SVN_URL'), + 'git_repo': self.GetCodeReviewSetting('TRYSERVER_GIT_URL'), 'project': self.GetCodeReviewSetting('TRYSERVER_PROJECT'), + # Primarily for revision=auto + 'revision': self.GetCodeReviewSetting('TRYSERVER_REVISION'), 'root': self.GetCodeReviewSetting('TRYSERVER_ROOT'), 'patchlevel': self.GetCodeReviewSetting('TRYSERVER_PATCHLEVEL'), } @@ -410,7 +417,9 @@ def _GenTSBotSpec(checkouts, change, changed_files, options): def _ParseSendChangeOptions(bot_spec, options): - """Parse common options passed to _SendChangeHTTP and _SendChangeSVN.""" + """Parse common options passed to _SendChangeHTTP, _SendChangeSVN and + _SendChangeGit. + """ values = [ ('user', options.user), ('name', options.name), @@ -481,6 +490,44 @@ def _SendChangeHTTP(bot_spec, options): raise NoTryServerAccess('%s is unaccessible. Got:\n%s' % (url, response)) +@contextlib.contextmanager +def _TempFilename(name, contents=None): + """Create a temporary directory, append the specified name and yield. + + In contrast to NamedTemporaryFile, does not keep the file open. + Deletes the file on __exit__. + """ + temp_dir = tempfile.mkdtemp(prefix=name) + try: + path = os.path.join(temp_dir, name) + if contents: + with open(path, 'w') as f: + f.write(contents) + yield path + finally: + shutil.rmtree(temp_dir, True) + + +@contextlib.contextmanager +def _PrepareDescriptionAndPatchFiles(description, options): + """Creates temporary files with description and patch. + + __enter__ called on the return value returns a tuple of patch_filename and + description_filename. + + Args: + description: contents of description file. + options: patchset options object. Must have attributes: user, + name (of patch) and diff (contents of patch). + """ + current_time = str(datetime.datetime.now()).replace(':', '.') + patch_basename = '%s.%s.%s.diff' % (Escape(options.user), + Escape(options.name), current_time) + with _TempFilename('description', description) as description_filename: + with _TempFilename(patch_basename, options.diff) as patch_filename: + yield patch_filename, description_filename + + def _SendChangeSVN(bot_spec, options): """Send a change to the try server by committing a diff file on a subversion server.""" @@ -497,45 +544,141 @@ def _SendChangeSVN(bot_spec, options): if options.dry_run: return - # Create a temporary directory, put a uniquely named file in it with the diff - # content and svn import that. - temp_dir = tempfile.mkdtemp() - temp_file = tempfile.NamedTemporaryFile() - try: - try: - # Description - temp_file.write(description) - temp_file.flush() - - # Diff file - current_time = str(datetime.datetime.now()).replace(':', '.') - file_name = (Escape(options.user) + '.' + Escape(options.name) + - '.%s.diff' % current_time) - full_path = os.path.join(temp_dir, file_name) - with open(full_path, 'wb') as f: - f.write(options.diff) - - # Committing it will trigger a try job. - if sys.platform == "cygwin": - # Small chromium-specific issue here: - # git-try uses /usr/bin/python on cygwin but svn.bat will be used - # instead of /usr/bin/svn by default. That causes bad things(tm) since - # Windows' svn.exe has no clue about cygwin paths. Hence force to use - # the cygwin version in this particular context. - exe = "/usr/bin/svn" - else: - exe = "svn" - command = [exe, 'import', '-q', temp_dir, options.svn_repo, '--file', - temp_file.name] - if scm.SVN.AssertVersion("1.5")[0]: - command.append('--no-ignore') + with _PrepareDescriptionAndPatchFiles(description, options) as ( + patch_filename, description_filename): + if sys.platform == "cygwin": + # Small chromium-specific issue here: + # git-try uses /usr/bin/python on cygwin but svn.bat will be used + # instead of /usr/bin/svn by default. That causes bad things(tm) since + # Windows' svn.exe has no clue about cygwin paths. Hence force to use + # the cygwin version in this particular context. + exe = "/usr/bin/svn" + else: + exe = "svn" + patch_dir = os.path.dirname(patch_filename) + command = [exe, 'import', '-q', patch_dir, options.svn_repo, '--file', + description_filename] + if scm.SVN.AssertVersion("1.5")[0]: + command.append('--no-ignore') + try: subprocess2.check_call(command) except subprocess2.CalledProcessError, e: raise NoTryServerAccess(str(e)) - finally: - temp_file.close() - shutil.rmtree(temp_dir, True) + + +def _GetPatchGitRepo(git_url): + """Gets a path to a Git repo with patches. + + Stores patches in .git/git-try/patches-git directory, a git repo. If it + doesn't exist yet or its origin URL is different, cleans up and clones it. + If it existed before, then pulls changes. + + Does not support SVN repo. + + Returns a path to the directory with patches. + """ + git_dir = scm.GIT.GetGitDir(os.getcwd()) + patch_dir = os.path.join(git_dir, GIT_PATCH_DIR_BASENAME) + + logging.info('Looking for git repo for patches') + # Is there already a repo with the expected url or should we clone? + clone = True + if os.path.exists(patch_dir) and scm.GIT.IsInsideWorkTree(patch_dir): + existing_url = scm.GIT.Capture( + ['config', '--local', 'remote.origin.url'], + cwd=patch_dir) + clone = existing_url != git_url + + if clone: + if os.path.exists(patch_dir): + logging.info('Cleaning up') + shutil.rmtree(patch_dir, True) + logging.info('Cloning patch repo') + scm.GIT.Capture(['clone', git_url, GIT_PATCH_DIR_BASENAME], cwd=git_dir) + email = scm.GIT.GetEmail(cwd=os.getcwd()) + scm.GIT.Capture(['config', '--local', 'user.email', email], cwd=patch_dir) + else: + if scm.GIT.IsWorkTreeDirty(patch_dir): + logging.info('Work dir is dirty: hard reset!') + scm.GIT.Capture(['reset', '--hard'], cwd=patch_dir) + logging.info('Updating patch repo') + scm.GIT.Capture(['pull', 'origin', 'master'], cwd=patch_dir) + + return os.path.abspath(patch_dir) + + +def _SendChangeGit(bot_spec, options): + """Send a change to the try server by committing a diff file to a GIT repo""" + if not options.git_repo: + raise NoTryServerAccess('Please use the --git_repo option to specify the ' + 'try server git repository to connect to.') + + values = _ParseSendChangeOptions(bot_spec, options) + comment_subject = '%s.%s' % (options.user, options.name) + comment_body = ''.join("%s=%s\n" % (k, v) for k, v in values) + description = '%s\n\n%s' % (comment_subject, comment_body) + logging.info('Sending by GIT') + logging.info(description) + logging.info(options.git_repo) + logging.info(options.diff) + if options.dry_run: + return + + patch_dir = _GetPatchGitRepo(options.git_repo) + def patch_git(*args): + return scm.GIT.Capture(list(args), cwd=patch_dir) + def add_and_commit(filename, comment_filename): + patch_git('add', filename) + patch_git('commit', '-F', comment_filename) + + assert scm.GIT.IsInsideWorkTree(patch_dir) + assert not scm.GIT.IsWorkTreeDirty(patch_dir) + + with _PrepareDescriptionAndPatchFiles(description, options) as ( + patch_filename, description_filename): + logging.info('Committing patch') + target_branch = ('refs/patches/' + + os.path.basename(patch_filename).replace(' ','-')) + target_filename = os.path.join(patch_dir, 'patch.diff') + branch_file = os.path.join(patch_dir, GIT_BRANCH_FILE) + try: + # Crete a new branch and put the patch there + patch_git('checkout', '--orphan', target_branch) + patch_git('reset') + patch_git('clean', '-f') + shutil.copyfile(patch_filename, target_filename) + add_and_commit(target_filename, description_filename) + assert not scm.GIT.IsWorkTreeDirty(patch_dir) + + # Update the branch file in the master + patch_git('checkout', 'master') + + def update_branch(): + with open(branch_file, 'w') as f: + f.write(target_branch) + add_and_commit(branch_file, description_filename) + + update_branch() + + # Push master and target_branch to origin. + logging.info('Pushing patch') + for attempt in xrange(_GIT_PUSH_ATTEMPTS): + try: + patch_git('push', 'origin', 'master', target_branch) + except subprocess2.CalledProcessError as e: + is_last = attempt == _GIT_PUSH_ATTEMPTS - 1 + if is_last: + raise NoTryServerAccess(str(e)) + # Fetch, reset, update branch file again. + patch_git('fetch', 'origin') + patch_git('reset', '--hard', 'origin/master') + update_branch() + except subprocess2.CalledProcessError, e: + # Restore state. + patch_git('checkout', 'master') + patch_git('reset', '--hard', 'origin/master') + raise def PrintSuccess(bot_spec, options): @@ -651,7 +794,9 @@ def gen_parser(prog): " and exit. Do not send patch. Like --dry_run" " but less verbose.") group.add_option("-r", "--revision", - help="Revision to use for the try job; default: the " + help="Revision to use for the try job. If 'auto' is " + "specified, it is resolved to the revision a patch is " + "generated against (Git only). Default: the " "revision will be determined by the try server; see " "its waterfall for more info") group.add_option("-c", "--clobber", action="store_true", @@ -737,6 +882,18 @@ def gen_parser(prog): help="SVN url to use to write the changes in; --use_svn is " "implied when using --svn_repo") parser.add_option_group(group) + + group = optparse.OptionGroup(parser, "Access the try server with Git") + group.add_option("--use_git", + action="store_const", + const=_SendChangeGit, + dest="send_patch", + help="Use GIT to talk to the try server") + group.add_option("-G", "--git_repo", + metavar="GIT_URL", + help="GIT url to use to write the changes in; --use_git is " + "implied when using --git_repo") + parser.add_option_group(group) return parser @@ -805,7 +962,6 @@ def TryChange(argv, try: changed_files = None # Always include os.getcwd() in the checkout settings. - checkouts = [] path = os.getcwd() file_list = [] @@ -819,12 +975,29 @@ def TryChange(argv, # Clear file list so that the correct list will be retrieved from the # upstream branch. file_list = [] - checkouts.append(GuessVCS(options, path, file_list)) - checkouts[0].AutomagicalSettings() + + current_vcs = GuessVCS(options, path, file_list) + current_vcs.AutomagicalSettings() + options = current_vcs.options + vcs_is_git = type(current_vcs) is GIT + + # So far, git_repo doesn't work with SVN + if options.git_repo and not vcs_is_git: + parser.error('--git_repo option is supported only for GIT repositories') + + # If revision==auto, resolve it + if options.revision and options.revision.lower() == 'auto': + if not vcs_is_git: + parser.error('--revision=auto is supported only for GIT repositories') + options.revision = scm.GIT.Capture( + ['rev-parse', current_vcs.diff_against], + cwd=path) + + checkouts = [current_vcs] for item in options.sub_rep: # Pass file_list=None because we don't know the sub repo's file list. checkout = GuessVCS(options, - os.path.join(checkouts[0].checkout_root, item), + os.path.join(current_vcs.checkout_root, item), None) if checkout.checkout_root in [c.checkout_root for c in checkouts]: parser.error('Specified the root %s two times.' % @@ -833,9 +1006,10 @@ def TryChange(argv, can_http = options.port and options.host can_svn = options.svn_repo + can_git = options.git_repo # If there was no transport selected yet, now we must have enough data to # select one. - if not options.send_patch and not (can_http or can_svn): + if not options.send_patch and not (can_http or can_svn or can_git): parser.error('Please specify an access method.') # Convert options.diff into the content of the diff. @@ -913,23 +1087,30 @@ def TryChange(argv, print ' %s' % (bot[0]) return 0 - # Send the patch. + # Determine sending protocol if options.send_patch: # If forced. - options.send_patch(bot_spec, options) - PrintSuccess(bot_spec, options) - return 0 - try: - if can_http: - _SendChangeHTTP(bot_spec, options) + senders = [options.send_patch] + else: + # Try sending patch using avaialble protocols + all_senders = [ + (_SendChangeHTTP, can_http), + (_SendChangeSVN, can_svn), + (_SendChangeGit, can_git) + ] + senders = [sender for sender, can in all_senders if can] + + # Send the patch. + for sender in senders: + try: + sender(bot_spec, options) PrintSuccess(bot_spec, options) return 0 - except NoTryServerAccess: - if not can_svn: - raise - _SendChangeSVN(bot_spec, options) - PrintSuccess(bot_spec, options) - return 0 + except NoTryServerAccess: + is_last = sender == senders[-1] + if is_last: + raise + assert False, "Unreachable code" except (InvalidScript, NoTryServerAccess), e: if swallow_exception: return 1