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>
diff --git a/subcmds/sync.py b/subcmds/sync.py index 32fd7f0..0739c82 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py
@@ -858,6 +858,11 @@ 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 @@ 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:
diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py index 7203414..efd9ffe 100644 --- a/tests/test_subcmds_sync.py +++ b/tests/test_subcmds_sync.py
@@ -215,6 +215,72 @@ 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", [