diff --git a/subcmds/sync.py b/subcmds/sync.py index fe1b14318..6946735c5 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py @@ -827,13 +827,42 @@ later is required to fix a server side protocol bug. or opt.current_branch_only ) + def _GetSuperprojectRevision( + self, opt: optparse.Values, manifest: XmlManifest + ) -> Optional[str]: + """Returns the --superproject-revision to apply to manifest. + + The option only applies to the outer manifest. + + Args: + opt: Program options returned from optparse. See _Options(). + manifest: The manifest to check. + + Returns: + The requested superproject revision, or None if manifest's + superproject should not be pinned. + """ + if manifest == self.outer_manifest: + return opt.superproject_revision + return None + def _ConfigureSuperproject( - self, - opt: optparse.Values, - manifest, - revision: Optional[str] = None, + self, opt: optparse.Values, manifest: XmlManifest ) -> bool: - """Configure superproject with options.""" + """Configure superproject with options. + + The configuration lives on the manifest's Superproject object, which + is discarded whenever the manifest is unloaded (see _ReloadManifest + and XmlManifest.Unload). Callers must therefore configure the + superproject again after every manifest reload. + + Args: + opt: Program options returned from optparse. See _Options(). + manifest: The manifest whose superproject should be configured. + + Returns: + Whether superproject messages should be printed. + """ if not manifest.superproject: return False manifest.superproject.SetQuiet(not opt.verbose) @@ -841,6 +870,7 @@ later is required to fix a server side protocol bug. opt.use_superproject, manifest ) manifest.superproject.SetPrintMessages(print_messages) + revision = self._GetSuperprojectRevision(opt, manifest) if revision: manifest.superproject.SetRevisionId(revision) return print_messages @@ -2171,7 +2201,7 @@ later is required to fix a server side protocol bug. mp: the manifestProject to query. manifest_name: Manifest file to be reloaded. """ - if opt.superproject_revision and mp.manifest == self.outer_manifest: + if self._GetSuperprojectRevision(opt, mp.manifest): self._SyncToSuperprojectRev( opt, mp.manifest, mp, manifest_name, errors ) @@ -2383,18 +2413,19 @@ later is required to fix a server side protocol bug. def _SyncToSuperprojectRev( self, opt: optparse.Values, - manifest, + manifest: XmlManifest, mp: Project, manifest_name: Optional[str], errors: List[Exception], ) -> None: - """Sync to a specific superproject commit.""" + """Sync to a specific superproject commit. + + Only valid for the outer manifest; see _GetSuperprojectRevision. + """ if not manifest.superproject: raise SyncError("superproject not defined in manifest") - self._ConfigureSuperproject( - opt, manifest, revision=opt.superproject_revision - ) + self._ConfigureSuperproject(opt, manifest) sync_result = manifest.superproject.Sync(self.git_event_log) if not sync_result.success: diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py index 580389a26..172c8d15d 100644 --- a/tests/test_subcmds_sync.py +++ b/tests/test_subcmds_sync.py @@ -254,6 +254,66 @@ def test_sync_update_projects_revision_id_populates_logging_data( } +@pytest.mark.parametrize( + "superproject_revision, is_outer_manifest, expected_revision", + [ + ("deadbeef", True, "deadbeef"), + ("deadbeef", False, None), + (None, True, None), + ], + ids=["outer_manifest", "not_outer_manifest", "no_superproject_revision"], +) +def test_sync_update_projects_revision_id_pins_superproject_revision( + tmp_path: Path, + superproject_revision: Optional[str], + is_outer_manifest: bool, + expected_revision: Optional[str], +) -> None: + """Test that --superproject-revision is applied to a reloaded manifest. + + _SyncToSuperprojectRev pins the superproject, but updating the manifest + project unloads the manifest, so _UpdateProjectsRevisionId must pin the + freshly loaded superproject again before using it. + """ + manifest = _create_manifest_with_groups(tmp_path) + cmd = sync.Sync() + cmd.manifest = manifest + cmd.outer_manifest = manifest if is_outer_manifest else mock.MagicMock() + + # Simulate the state after a manifest reload: a new, unpinned superproject. + superproject = mock.MagicMock() + superproject.UpdateProjectsRevisionId.return_value = mock.MagicMock( + manifest_path=None, fatal=False + ) + manifest._superproject = superproject + + opts, args = cmd.OptionParser.parse_args([]) + opts.verbose = False + opts.this_manifest_only = True + opts.local_only = False + opts.superproject_revision = superproject_revision + + with mock.patch.object( + cmd, "ManifestList", return_value=[manifest] + ), mock.patch.object( + sync.git_superproject, "UseSuperproject", return_value=True + ), mock.patch.object( + sync.git_superproject, "PrintMessages", return_value=False + ): + cmd._UpdateProjectsRevisionId(opts, args, {}, manifest) + + if expected_revision is not None: + superproject.SetRevisionId.assert_called_once_with(expected_revision) + # The pin must be in place before the superproject is used. + method_calls = [name for name, _, _ in superproject.mock_calls] + assert method_calls.index("SetRevisionId") < method_calls.index( + "UpdateProjectsRevisionId" + ) + else: + superproject.SetRevisionId.assert_not_called() + superproject.UpdateProjectsRevisionId.assert_called_once() + + def test_sync_update_projects_revision_id_logs_without_superproject_tag( tmp_path: Path, ) -> None: