sync: record superproject sync analysis state again

Since the multi-manifest change (bdcba7d), _UpdateProjectsRevisionId
rebinds its superproject_logging_data parameter to a new local dict.
The caller keeps its own empty dict and passes it to
UpdateSyncAnalysisState, so repo.syncstate.superproject.* is never
written to the manifest config or logged to trace2.

Drop the rebinding so the caller's dict is populated.

Also record superproject, haslocalmanifests and hassuperprojecttag
when no manifest declares a superproject. Before bdcba7d these keys
were logged on every sync. SyncAnalysisState does not remove old
keys, so without this a checkout that moves to a manifest without a
superproject keeps reporting the values from its last superproject
sync.

Bug: 565957530
Test: ./run_tests
Change-Id: I8796e3a34b9cf4786383d25ecc3ebf0c5409caa3
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/634843
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Ram Peri <ramperi@google.com>
Tested-by: Ram Peri <ramperi@google.com>

subcmds/sync.py

@@ -858,6 +858,11 @@ later is required to fix a server side protocol bug.
             m.superproject for m in manifest.all_children
         )
         if not have_superproject:
+            superproject_logging_data.update(
+                superproject=False,
+                haslocalmanifests=bool(manifest.HasLocalManifests),
+                hassuperprojecttag=False,
+            )
             return
 
         if opt.local_only and manifest.superproject:
@@ -882,7 +887,6 @@ later is required to fix a server side protocol bug.
             for p in all_projects:
                 per_manifest[p.manifest.path_prefix].append(p)
 
-        superproject_logging_data = {}
         need_unload = False
         for m in self.ManifestList(opt):
             if m.path_prefix not in per_manifest:

tests/test_subcmds_sync.py

@@ -215,6 +215,72 @@ def test_sync_update_projects_revision_id_respects_groups(tmp_path: Path):
             assert kwargs.get("groups") == "group1"
 
 
+def test_sync_update_projects_revision_id_populates_logging_data(
+    tmp_path: Path,
+) -> None:
+    """Test that _UpdateProjectsRevisionId fills in the caller's dict."""
+    manifest = _create_manifest_with_groups(tmp_path)
+    cmd = sync.Sync()
+    cmd.manifest = manifest
+
+    superproject = mock.MagicMock()
+    superproject.UpdateProjectsRevisionId.return_value = mock.MagicMock(
+        manifest_path=None
+    )
+    manifest._superproject = superproject
+
+    opts, args = cmd.OptionParser.parse_args([])
+    opts.this_manifest_only = True
+    opts.local_only = False
+
+    superproject_logging_data = {}
+    with mock.patch.object(
+        cmd, "ManifestList", return_value=[manifest]
+    ), mock.patch.object(
+        cmd, "_ConfigureSuperproject", return_value=False
+    ), mock.patch.object(
+        sync.git_superproject, "UseSuperproject", return_value=True
+    ):
+        cmd._UpdateProjectsRevisionId(
+            opts, args, superproject_logging_data, manifest
+        )
+
+    assert superproject_logging_data == {
+        "superproject": True,
+        "haslocalmanifests": False,
+        "hassuperprojecttag": True,
+        "updatedrevisionid": False,
+    }
+
+
+def test_sync_update_projects_revision_id_logs_without_superproject_tag(
+    tmp_path: Path,
+) -> None:
+    """Test that sync state is recorded when the manifest has no superproject.
+
+    SyncAnalysisState never removes keys, so values from an earlier sync would
+    otherwise persist in the config.
+    """
+    manifest = _create_manifest_with_groups(tmp_path)
+    cmd = sync.Sync()
+    cmd.manifest = manifest
+
+    opts, args = cmd.OptionParser.parse_args([])
+    opts.this_manifest_only = True
+    opts.local_only = False
+
+    superproject_logging_data = {}
+    cmd._UpdateProjectsRevisionId(
+        opts, args, superproject_logging_data, manifest
+    )
+
+    assert superproject_logging_data == {
+        "superproject": False,
+        "haslocalmanifests": False,
+        "hassuperprojecttag": False,
+    }
+
+
 @pytest.mark.parametrize(
     "generate_manpages, expected_default",
     [