[no-sync] Save previous values to files.

Tested locally with chromium/src.
going from 17s -> <1s

Bug: 1339472
Change-Id: I71b90dcd6a7934ea7c9722dd040d63645792078e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3791746
Commit-Queue: Joanna Wang <jojwang@chromium.org>
Reviewed-by: Gavin Mak <gavinmak@google.com>

gclient.py

@@ -130,8 +130,9 @@ DEPOT_TOOLS_DIR = os.path.dirname(os.path.abspath(os.path.realpath(__file__)))
 # one, e.g. if a spec explicitly says `cache_dir = None`.)
 UNSET_CACHE_DIR = object()
 
+PREVIOUS_CUSTOM_VARS_FILE = '.gclient_previous_custom_vars'
+PREVIOUS_SYNC_COMMITS_FILE = '.gclient_previous_sync_commits'
 
-PREVIOUS_CUSTOM_VARS = 'GCLIENT_PREVIOUS_CUSTOM_VARS'
 PREVIOUS_SYNC_COMMITS = 'GCLIENT_PREVIOUS_SYNC_COMMITS'
 
 NO_SYNC_EXPERIMENT = 'no-sync'
@@ -1717,11 +1718,34 @@ it or fix the checkout.
       gclient_utils.SyntaxErrorToError(filename, e)
     return scope.get('entries', {})
 
+  def _ExtractFileJsonContents(self, default_filename):
+    # type: (str) -> Mapping[str,Any]
+    f = os.path.join(self.root_dir, default_filename)
+
+    if not os.path.exists(f):
+      logging.info('File %s does not exist.' % f)
+      return {}
+
+    with open(f, 'r') as open_f:
+      logging.info('Reading content from file %s' % f)
+      content = open_f.read().rstrip()
+      if content:
+        return json.loads(content)
+    return {}
+
+  def _WriteFileContents(self, default_filename, content):
+    # type: (str, str) -> None
+    f = os.path.join(self.root_dir, default_filename)
+
+    with open(f, 'w') as open_f:
+      logging.info('Writing to file %s' % f)
+      open_f.write(content)
+
   def _EnforceSkipSyncRevisions(self, patch_refs):
     # type: (Mapping[str, str]) -> Mapping[str, str]
     """Checks for and enforces revisions for skipping deps syncing."""
-    previous_sync_commits = json.loads(
-        os.environ.get(PREVIOUS_SYNC_COMMITS, '{}'))
+    previous_sync_commits = self._ExtractFileJsonContents(
+        PREVIOUS_SYNC_COMMITS_FILE)
 
     if not previous_sync_commits:
       return {}
@@ -1747,14 +1771,16 @@ it or fix the checkout.
 
     # We cannot skip syncing if there are custom_vars that differ from the
     # previous run's custom_vars.
-    previous_custom_vars = json.loads(os.environ.get(PREVIOUS_CUSTOM_VARS,
-                                                     '{}'))
+    previous_custom_vars = self._ExtractFileJsonContents(
+        PREVIOUS_CUSTOM_VARS_FILE)
+
     cvs_by_name = {s.name: s.custom_vars for s in self.dependencies}
 
     skip_sync_revisions = {}
     for name, commit in previous_sync_commits.items():
-      previous_vars = previous_custom_vars.get(name, {})
-      if previous_vars == cvs_by_name.get(name):
+      previous_vars = previous_custom_vars.get(name)
+      if previous_vars == cvs_by_name.get(name) or (not previous_vars and
+                                                    not cvs_by_name.get(name)):
         skip_sync_revisions[name] = commit
       else:
         print('We cannot skip syncs when custom_vars for solutions have '
@@ -1808,14 +1834,25 @@ it or fix the checkout.
     delete_unversioned_trees is set to true.
     """
 
-    entries = [i.name for i in self.root.subtree(False) if i.url]
+    entry_names_and_sync = [(i.name, i._should_sync)
+                            for i in self.root.subtree(False) if i.url]
+    entries = []
+    if entry_names_and_sync:
+      entries, _ = zip(*entry_names_and_sync)
     full_entries = [os.path.join(self.root_dir, e.replace('/', os.path.sep))
                     for e in entries]
+    no_sync_entries = [
+        name for name, should_sync in entry_names_and_sync if not should_sync
+    ]
 
     for entry, prev_url in self._ReadEntries().items():
       if not prev_url:
         # entry must have been overridden via .gclient custom_deps
         continue
+      if any(entry.startswith(sln) for sln in no_sync_entries):
+        # Dependencies of solutions that skipped syncing would not
+        # show up in `entries`.
+        continue
       # Fix path separator on Windows.
       entry_fixed = entry.replace('/', os.path.sep)
       e_dir = os.path.join(self.root_dir, entry_fixed)
@@ -1945,13 +1982,16 @@ it or fix the checkout.
       if NO_SYNC_EXPERIMENT in self._options.experiments:
         skip_sync_revisions = self._EnforceSkipSyncRevisions(patch_refs)
 
-      # Store solutions' custom_vars on memory to compare in the next run.
-      # All dependencies added later are inherited from the current
-      # self.dependencies.
-      custom_vars = {}
-      for dep in self.dependencies:
-        custom_vars[dep.name] = dep.custom_vars
-      os.environ[PREVIOUS_CUSTOM_VARS] = json.dumps(sorted(custom_vars))
+    # Store solutions' custom_vars on memory to compare in the next run.
+    # All dependencies added later are inherited from the current
+    # self.dependencies.
+    custom_vars = {
+        dep.name: dep.custom_vars
+        for dep in self.dependencies if dep.custom_vars
+    }
+    if custom_vars:
+      self._WriteFileContents(PREVIOUS_CUSTOM_VARS_FILE,
+                              json.dumps(custom_vars))
 
     # Disable progress for non-tty stdout.
     should_show_progress = (
@@ -2011,6 +2051,9 @@ it or fix the checkout.
         pm = Progress('Running hooks', 1)
       self.RunHooksRecursively(self._options, pm)
 
+    self._WriteFileContents(PREVIOUS_SYNC_COMMITS_FILE,
+                            os.environ.get(PREVIOUS_SYNC_COMMITS, '{}'))
+
     return 0
 
   def PrintRevInfo(self):

tests/gclient_no_sync_smoketest.py

@@ -23,6 +23,16 @@ import subprocess2
 from testing_support import fake_repos
 
 
+def write(filename, content):
+  """Writes the content of a file and create the directories as needed."""
+  filename = os.path.abspath(filename)
+  dirname = os.path.dirname(filename)
+  if not os.path.isdir(dirname):
+    os.makedirs(dirname)
+  with open(filename, 'w') as f:
+    f.write(content)
+
+
 class GClientSmokeGIT(gclient_smoketest_base.GClientSmokeBase):
   """Smoke tests for the no-sync experiment."""
 
@@ -63,8 +73,14 @@ class GClientSmokeGIT(gclient_smoketest_base.GClientSmokeBase):
     patch_ref = self.FAKE_REPOS.git_hashes['repo_1'][3][0]  # DEPS 2
 
     # Previous run did a sync at revision_1
-    self.env[gclient.PREVIOUS_SYNC_COMMITS] = json.dumps({'src': revision_1})
-    self.env[gclient.PREVIOUS_CUSTOM_VARS] = json.dumps({'src': {'mac': True}})
+    write(
+        os.path.join(self.root_dir, gclient.PREVIOUS_SYNC_COMMITS_FILE),
+        json.dumps({'src': revision_1}))
+    write(
+        os.path.join(self.root_dir, gclient.PREVIOUS_CUSTOM_VARS_FILE),
+        json.dumps({'src': {
+            'mac': True
+        }}))
 
     # We checkout src at revision_2 which has a different DEPS
     # but that should not matter because patch_ref and revision_1
@@ -117,8 +133,14 @@ class GClientSmokeGIT(gclient_smoketest_base.GClientSmokeBase):
     patch_ref = self.FAKE_REPOS.git_hashes['repo_1'][3][0]  # DEPS 2
 
     # Previous run did a sync at revision_1
-    self.env[gclient.PREVIOUS_SYNC_COMMITS] = json.dumps({'src': revision_1})
-    self.env[gclient.PREVIOUS_CUSTOM_VARS] = json.dumps({'src': {'mac': True}})
+    write(
+        os.path.join(self.root_dir, gclient.PREVIOUS_SYNC_COMMITS_FILE),
+        json.dumps({'src': revision_1}))
+    write(
+        os.path.join(self.root_dir, gclient.PREVIOUS_CUSTOM_VARS_FILE),
+        json.dumps({'src': {
+            'mac': True
+        }}))
 
     self.gclient([
         'sync',
@@ -179,7 +201,9 @@ class GClientSmokeGIT(gclient_smoketest_base.GClientSmokeBase):
     patch_ref = self.FAKE_REPOS.git_hashes['repo_1'][3][0]  # DEPS 1
 
     # Previous run did a sync at revision_1
-    self.env[gclient.PREVIOUS_SYNC_COMMITS] = json.dumps({'src': revision_1})
+    write(
+        os.path.join(self.root_dir, gclient.PREVIOUS_SYNC_COMMITS_FILE),
+        json.dumps({'src': revision_1}))
     # No PREVIOUS_CUSTOM_VARS
 
     # We checkout src at revision_2 which has a different DEPS
@@ -225,7 +249,9 @@ class GClientSmokeGIT(gclient_smoketest_base.GClientSmokeBase):
     patch_ref = self.FAKE_REPOS.git_hashes['repo_1'][3][0]  # DEPS 1
 
     # Previous run did a sync at revision_1
-    self.env[gclient.PREVIOUS_SYNC_COMMITS] = json.dumps({'src': revision_2})
+    write(
+        os.path.join(self.root_dir, gclient.PREVIOUS_SYNC_COMMITS_FILE),
+        json.dumps({'src': revision_2}))
 
     # We checkout src at revision_1 which has the same DEPS
     # but that should not matter because patch_ref and revision_2

tests/gclient_test.py

@@ -1566,9 +1566,10 @@ class GclientTest(trial_dir.TestCase):
         os.path.join('foo/src', 'DEPS'), 'deps = {\n'
         '  "bar": "https://example.com/bar.git@bar_version",\n'
         '}')
+    write(gclient.PREVIOUS_SYNC_COMMITS_FILE,
+          json.dumps({'foo/src': '1234'}))
     options, _ = gclient.OptionParser().parse_args([])
-    os.environ[gclient.PREVIOUS_SYNC_COMMITS] = json.dumps(
-        {'foo/src': '1234'})
+
     client = gclient.GClient.LoadCurrentConfig(options)
     patch_refs = {'foo/src': '1222', 'somedeps': '1111'}
     self.assertEqual({}, client._EnforceSkipSyncRevisions(patch_refs))
@@ -1601,7 +1602,6 @@ class GclientTest(trial_dir.TestCase):
         os.path.join('novars', 'DEPS'), 'deps = {\n'
         '  "poo": "https://example.com/poo.git@poo_version",\n'
         '}')
-
     previous_custom_vars = {
         'samevars': {
             'checkout_foo': 'true'
@@ -1610,16 +1610,17 @@ class GclientTest(trial_dir.TestCase):
             'checkout_chicken': 'false'
         },
     }
-    os.environ[gclient.PREVIOUS_CUSTOM_VARS] = json.dumps(previous_custom_vars)
-    options, _ = gclient.OptionParser().parse_args([])
-
-    patch_refs = {'samevars': '1222'}
+    write(gclient.PREVIOUS_CUSTOM_VARS_FILE,
+          json.dumps(previous_custom_vars))
     previous_sync_commits = {'samevars': '10001',
                              'diffvars': '10002',
                              'novars': '10003'}
-    os.environ[
-        gclient.PREVIOUS_SYNC_COMMITS] = json.dumps(previous_sync_commits)
+    write(gclient.PREVIOUS_SYNC_COMMITS_FILE,
+          json.dumps(previous_sync_commits))
+
+    options, _ = gclient.OptionParser().parse_args([])
 
+    patch_refs = {'samevars': '1222'}
     expected_skip_sync_revisions = {'samevars': '10001', 'novars': '10003'}
 
     client = gclient.GClient.LoadCurrentConfig(options)