From 96b57b9b59091007162adfa4035a6383e9757815 Mon Sep 17 00:00:00 2001 From: Joanna Wang Date: Mon, 1 Aug 2022 22:48:38 +0000 Subject: [PATCH] [no-sync] bot_update: Remove previous no-sync exp changes and implement new one. Bug: 1339472 Change-Id: I250dd72dd806d26ba6cbe213280ecd65fbc66123 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3789864 Commit-Queue: Joanna Wang Reviewed-by: Gavin Mak --- .../bot_update/resources/bot_update.py | 89 ++++++------------- tests/bot_update_coverage_test.py | 10 +++ 2 files changed, 39 insertions(+), 60 deletions(-) diff --git a/recipes/recipe_modules/bot_update/resources/bot_update.py b/recipes/recipe_modules/bot_update/resources/bot_update.py index c67d13b34..681305f4f 100755 --- a/recipes/recipe_modules/bot_update/resources/bot_update.py +++ b/recipes/recipe_modules/bot_update/resources/bot_update.py @@ -83,10 +83,9 @@ GOT_REVISION_MAPPINGS = { } # List of bot update experiments -EXP_NO_SYNC = 'no_sync' # Don't fetch/sync if current revision is recent enough - -# Don't sync if the checkout is less than 6 hours old. -NO_SYNC_MAX_DELAY_S = 6 * 60 * 60 +# Gclient will skip deps syncing if there have been no DEPS changes +# since the last sync on the bot. +EXP_NO_SYNC = 'no-sync' GCLIENT_TEMPLATE = """solutions = %(solutions)s @@ -411,10 +410,14 @@ def git_config_if_not_set(key, value): git('config', '--global', '--unset', key) -def gclient_sync( - with_branch_heads, with_tags, revisions, - patch_refs, gerrit_reset, - gerrit_rebase_patch_ref, download_topics=False): +def gclient_sync(with_branch_heads, + with_tags, + revisions, + patch_refs, + gerrit_reset, + gerrit_rebase_patch_ref, + download_topics=False, + experiments=None): args = ['sync', '--verbose', '--reset', '--force', '--nohooks', '--noprehooks', '--delete_unversioned_trees'] if with_branch_heads: @@ -436,6 +439,9 @@ def gclient_sync( if download_topics: args.append('--download-topics') + if EXP_NO_SYNC in experiments: + args.extend(['--experiment', EXP_NO_SYNC]) + try: call_gclient(*args) except SubprocessFailed as e: @@ -652,47 +658,22 @@ def _set_git_config(fn): def git_checkouts(solutions, revisions, refs, no_fetch_tags, git_cache_dir, - cleanup_dir, enforce_fetch, experiments): + cleanup_dir, enforce_fetch): build_dir = os.getcwd() - synced = [] for sln in solutions: sln_dir = path.join(build_dir, sln['name']) - did_sync = _git_checkout( - sln, sln_dir, revisions, refs, no_fetch_tags, git_cache_dir, - cleanup_dir, enforce_fetch, experiments) - if did_sync: - synced.append(sln['name']) - return synced - - -def _git_checkout_needs_sync(sln_url, sln_dir, refs): - if not path.exists(sln_dir): - return True - for ref in refs: - try: - remote_ref = ref_to_remote_ref(ref) - commit_time = git('show', '-s', '--format=%ct', remote_ref, cwd=sln_dir) - commit_time = int(commit_time) - except SubprocessError: - return True - if time.time() - commit_time >= NO_SYNC_MAX_DELAY_S: - return True - return False + _git_checkout(sln, sln_dir, revisions, refs, no_fetch_tags, git_cache_dir, + cleanup_dir, enforce_fetch) def _git_checkout(sln, sln_dir, revisions, refs, no_fetch_tags, git_cache_dir, - cleanup_dir, enforce_fetch, experiments): + cleanup_dir, enforce_fetch): name = sln['name'] url = sln['url'] branch, revision = get_target_branch_and_revision(name, url, revisions) pin = revision if COMMIT_HASH_RE.match(revision) else None - if (EXP_NO_SYNC in experiments - and not _git_checkout_needs_sync(url, sln_dir, refs)): - git('checkout', '--force', pin or branch, '--', cwd=sln_dir) - return False - populate_cmd = (['cache', 'populate', '-v', '--cache-dir', git_cache_dir, url, '--reset-fetch-config']) if no_fetch_tags: @@ -756,7 +737,7 @@ def _git_checkout(sln, sln_dir, revisions, refs, no_fetch_tags, git_cache_dir, # happens to have the exact same name. git('checkout', '--force', pin or branch, '--', cwd=sln_dir) git('clean', '-dff', cwd=sln_dir) - return True + return except SubprocessFailed as e: # Exited abnormally, there's probably something wrong. print('Something failed: %s.' % str(e)) @@ -767,8 +748,6 @@ def _git_checkout(sln, sln_dir, revisions, refs, no_fetch_tags, git_cache_dir, else: raise - return True - def _git_disable_gc(cwd): git('config', 'gc.auto', '0', cwd=cwd) @@ -838,9 +817,8 @@ def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only, # invoking DEPS. print('Fetching Git checkout') - synced_solutions = git_checkouts( - solutions, revisions, refs, no_fetch_tags, git_cache_dir, cleanup_dir, - enforce_fetch, experiments) + git_checkouts(solutions, revisions, refs, no_fetch_tags, git_cache_dir, + cleanup_dir, enforce_fetch) # Ensure our build/ directory is set up with the correct .gclient file. gclient_configure(solutions, target_os, target_os_only, target_cpu, @@ -860,14 +838,10 @@ def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only, # Let gclient do the DEPS syncing. # The branch-head refspec is a special case because it's possible Chrome # src, which contains the branch-head refspecs, is DEPSed in. - gclient_sync( - BRANCH_HEADS_REFSPEC in refs, - TAGS_REFSPEC in refs, - gc_revisions, - patch_refs, - gerrit_reset, - gerrit_rebase_patch_ref, - download_topics) + gclient_sync(BRANCH_HEADS_REFSPEC in refs, TAGS_REFSPEC in refs, gc_revisions, + patch_refs, gerrit_reset, gerrit_rebase_patch_ref, + download_topics, experiments) + # Now that gclient_sync has finished, we should revert any .DEPS.git so that # presubmit doesn't complain about it being modified. @@ -880,8 +854,6 @@ def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only, gclient_configure(solutions, target_os, target_os_only, target_cpu, git_cache_dir) - return synced_solutions - def parse_revisions(revisions, root): """Turn a list of revision specs into a nice dictionary. @@ -1072,7 +1044,6 @@ def checkout(options, git_slns, specs, revisions, step_text): pass should_delete_dirty_file = False - synced_solutions = [] experiments = [] if options.experiments: experiments = options.experiments.split(',') @@ -1111,12 +1082,12 @@ def checkout(options, git_slns, specs, revisions, step_text): gerrit_reset=not options.gerrit_no_reset, experiments=experiments) - synced_solutions = ensure_checkout(**checkout_parameters) + ensure_checkout(**checkout_parameters) should_delete_dirty_file = True except GclientSyncFailed: print('We failed gclient sync, lets delete the checkout and retry.') ensure_no_checkout(dir_names, options.cleanup_dir) - synced_solutions = ensure_checkout(**checkout_parameters) + ensure_checkout(**checkout_parameters) should_delete_dirty_file = True except PatchFailed as e: # Tell recipes information such as root, got_revision, etc. @@ -1128,8 +1099,7 @@ def checkout(options, git_slns, specs, revisions, step_text): patch_failure=True, failed_patch_body=e.output, step_text='%s PATCH FAILED' % step_text, - fixed_revisions=revisions, - synced_solutions=synced_solutions) + fixed_revisions=revisions) should_delete_dirty_file = True raise finally: @@ -1169,8 +1139,7 @@ def checkout(options, git_slns, specs, revisions, step_text): step_text=step_text, fixed_revisions=revisions, properties=got_revisions, - manifest=manifest, - synced_solutions=synced_solutions) + manifest=manifest) def print_debug_info(): diff --git a/tests/bot_update_coverage_test.py b/tests/bot_update_coverage_test.py index 3bf8ab373..aeef59f02 100755 --- a/tests/bot_update_coverage_test.py +++ b/tests/bot_update_coverage_test.py @@ -228,6 +228,16 @@ class BotUpdateUnittests(unittest.TestCase): self.assertTrue(found) return self.call.records + def testGclientNoSyncExperiment(self): + ref = 'refs/changes/12/345/6' + repo = 'https://chromium.googlesource.com/v8/v8' + self.params['patch_refs'] = ['%s@%s' % (repo, ref)] + self.params['experiments'] = bot_update.EXP_NO_SYNC + bot_update.ensure_checkout(**self.params) + args = self.gclient.records[0] + idx = args.index('--experiment') + self.assertEqual(args[idx+1], bot_update.EXP_NO_SYNC) + def testApplyPatchOnGclient(self): ref = 'refs/changes/12/345/6' repo = 'https://chromium.googlesource.com/v8/v8'