From f0aeb220def22edfac9838288ad251f86da782c1 Mon Sep 17 00:00:00 2001
From: Gavin Mak <gavinmak@google.com>
Date: Tue, 8 Aug 2023 04:43:36 +0000
Subject: [PATCH] sync: Warn if partial sync state is detected

Partial syncs are not supported and can lead to strange behavior like
deleting files. Explicitly warn users on partial sync.

Bug: b/286126621, b/271507654
Change-Id: I471f78ac5942eb855bc34c80af47aa561dfa61e8
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/382154
Reviewed-by: Jason Chang <jasonnc@google.com>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Tested-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
---
 subcmds/sync.py            | 41 ++++++++++++++++++++++++-
 tests/test_subcmds_sync.py | 61 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+), 1 deletion(-)

diff --git a/subcmds/sync.py b/subcmds/sync.py
index eaca50c9..3fa6efa5 100644
--- a/subcmds/sync.py
+++ b/subcmds/sync.py
@@ -1866,6 +1866,14 @@ later is required to fix a server side protocol bug.
             mp.config.GetSyncAnalysisStateData(), "current_sync_state"
         )
 
+        self._local_sync_state.PruneRemovedProjects()
+        if self._local_sync_state.IsPartiallySynced():
+            print(
+                "warning: Partial syncs are not supported. For the best "
+                "experience, sync the entire tree.",
+                file=sys.stderr,
+            )
+
         if not opt.quiet:
             print("repo sync has finished successfully.")
 
@@ -1975,7 +1983,10 @@ class _LocalSyncState(object):
     _LAST_CHECKOUT = "last_checkout"
 
     def __init__(self, manifest):
-        self._path = os.path.join(manifest.repodir, ".repo_localsyncstate.json")
+        self._manifest = manifest
+        self._path = os.path.join(
+            self._manifest.repodir, ".repo_localsyncstate.json"
+        )
         self._time = time.time()
         self._state = None
         self._Load()
@@ -2023,6 +2034,34 @@ class _LocalSyncState(object):
         except (IOError, TypeError):
             platform_utils.remove(self._path, missing_ok=True)
 
+    def PruneRemovedProjects(self):
+        """Remove entries don't exist on disk and save."""
+        if not self._state:
+            return
+        delete = set()
+        for path in self._state:
+            gitdir = os.path.join(self._manifest.topdir, path, ".git")
+            if not os.path.exists(gitdir):
+                delete.add(path)
+        if not delete:
+            return
+        for path in delete:
+            del self._state[path]
+        self.Save()
+
+    def IsPartiallySynced(self):
+        """Return whether a partial sync state is detected."""
+        self._Load()
+        prev_checkout_t = None
+        for data in self._state.values():
+            checkout_t = data.get(self._LAST_CHECKOUT)
+            if not checkout_t:
+                return True
+            prev_checkout_t = prev_checkout_t or checkout_t
+            if prev_checkout_t != checkout_t:
+                return True
+        return False
+
 
 # This is a replacement for xmlrpc.client.Transport using urllib2
 # and supporting persistent-http[s]. It cannot change hosts from
diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py
index 00c34852..7cc93e39 100644
--- a/tests/test_subcmds_sync.py
+++ b/tests/test_subcmds_sync.py
@@ -175,12 +175,73 @@ class LocalSyncState(unittest.TestCase):
             os.listdir(self.repodir), [".repo_localsyncstate.json"]
         )
 
+    def test_partial_sync(self):
+        """Partial sync state is detected."""
+        with open(self.state._path, "w") as f:
+            f.write(
+                """
+            {
+              "projA": {
+                "last_fetch": 5,
+                "last_checkout": 5
+              },
+              "projB": {
+                "last_fetch": 5,
+                "last_checkout": 5
+              }
+            }
+            """
+            )
+
+        # Initialize state to read from the new file.
+        self.state = self._new_state()
+        projB = mock.MagicMock(relpath="projB")
+        self.assertEqual(self.state.IsPartiallySynced(), False)
+
+        self.state.SetFetchTime(projB)
+        self.state.SetCheckoutTime(projB)
+        self.assertEqual(self.state.IsPartiallySynced(), True)
+
     def test_nonexistent_project(self):
         """Unsaved projects don't have data."""
         p = mock.MagicMock(relpath="projC")
         self.assertEqual(self.state.GetFetchTime(p), None)
         self.assertEqual(self.state.GetCheckoutTime(p), None)
 
+    def test_prune_removed_projects(self):
+        """Removed projects are pruned."""
+        with open(self.state._path, "w") as f:
+            f.write(
+                """
+            {
+              "projA": {
+                "last_fetch": 5
+              },
+              "projB": {
+                "last_fetch": 7
+              }
+            }
+            """
+            )
+
+        def mock_exists(path):
+            if "projA" in path:
+                return False
+            return True
+
+        projA = mock.MagicMock(relpath="projA")
+        projB = mock.MagicMock(relpath="projB")
+        self.state = self._new_state()
+        self.assertEqual(self.state.GetFetchTime(projA), 5)
+        self.assertEqual(self.state.GetFetchTime(projB), 7)
+        with mock.patch("os.path.exists", side_effect=mock_exists):
+            self.state.PruneRemovedProjects()
+        self.assertIsNone(self.state.GetFetchTime(projA))
+
+        self.state = self._new_state()
+        self.assertIsNone(self.state.GetFetchTime(projA))
+        self.assertEqual(self.state.GetFetchTime(projB), 7)
+
 
 class GetPreciousObjectsState(unittest.TestCase):
     """Tests for _GetPreciousObjectsState."""