feat: Checkout tags as well as branches when cloning into scratch

helm/blueapi/config_schema.json

@@ -430,9 +430,9 @@
                     "title": "Remote Url",
                     "type": "string"
                 },
-                "branch": {
-                    "description": "Branch of repo to check out - defaults to remote's default when cloning and the existing branch when the repo already exists",
-                    "title": "Branch",
+                "target_revision": {
+                    "description": "Revision (branch or tag) to check out when cloning - defaults to remote's HEAD. If a tag is used, the repo will be left in a 'detached head' state.",
+                    "title": "Target Revision",
                     "type": "string"
                 }
             },

helm/blueapi/values.schema.json

@@ -852,11 +852,6 @@
             "title": "ScratchRepository",
             "type": "object",
             "properties": {
-                "branch": {
-                    "title": "Branch",
-                    "description": "Branch of repo to check out - defaults to remote's default when cloning and the existing branch when the repo already exists",
-                    "type": "string"
-                },
                 "name": {
                     "title": "Name",
                     "description": "Unique name for this repository in the scratch directory",
@@ -868,6 +863,11 @@
                     "description": "URL to clone from",
                     "default": "https://github.com/example/example.git",
                     "type": "string"
+                },
+                "target_revision": {
+                    "title": "Target Revision",
+                    "description": "Revision (branch or tag) to check out when cloning - defaults to remote's HEAD. If a tag is used, the repo will be left in a 'detached head' state.",
+                    "type": "string"
                 }
             },
             "additionalProperties": false

src/blueapi/cli/scratch.py

@@ -48,12 +48,12 @@ def setup_scratch(
             )
     for repo in config.repositories:
         local_directory = config.root / repo.name
-        ensure_repo(repo.remote_url, local_directory, repo.branch)
+        ensure_repo(repo.remote_url, local_directory, repo.target_revision)
         scratch_install(local_directory, timeout=install_timeout)
 
 
 def ensure_repo(
-    remote_url: str, local_directory: Path, branch: str | None = None
+    remote_url: str, local_directory: Path, target_revision: str | None = None
 ) -> None:
     """
     Ensure that a repository is checked out for use in the scratch area.
@@ -69,29 +69,29 @@ def ensure_repo(
 
     if not local_directory.exists():
         LOGGER.info(f"Cloning {remote_url}")
-        repo = Repo.clone_from(remote_url, local_directory)
+        Repo.clone_from(remote_url, local_directory, branch=target_revision)
         LOGGER.info(f"Cloned {remote_url} -> {local_directory}")
     elif local_directory.is_dir():
         repo = Repo(local_directory)
-        LOGGER.info(f"Found {local_directory}")
+        head = repo.head.commit
+        LOGGER.info("Found %s @ %s", local_directory, head.name_rev)
+        if target_revision:
+            if target_revision in repo.refs:
+                if repo.refs[target_revision].commit != head:
+                    LOGGER.warning(
+                        "Repository %s not at target revision: %r instead of %r",
+                        local_directory.name,
+                        head.name_rev,
+                        target_revision,
+                    )
+            else:
+                LOGGER.warning("Target revision %r not found", target_revision)
     else:
         raise KeyError(
-            f"Unable to open {local_directory} as a git repository because it is a file"
+            f"Unable to open {local_directory} as a git repository because it is not a "
+            "directory"
         )
 
-    if branch:
-        if not (local := getattr(repo.heads, branch, None)):
-            origin = repo.remotes[0]
-            origin.fetch()
-            LOGGER.info(
-                "Creating branch '%s' to track remote '%s'", branch, origin.refs[branch]
-            )
-            local = repo.create_head(branch, origin.refs[branch])
-            local.set_tracking_branch(origin.refs[branch])
-
-        LOGGER.info("Checking out branch '%s'", branch)
-        local.checkout()
-
 
 def scratch_install(path: Path, timeout: float = _DEFAULT_INSTALL_TIMEOUT) -> None:
     """

src/blueapi/config.py

@@ -12,6 +12,7 @@
 import yaml
 from bluesky_stomp.models import BasicAuthentication
 from pydantic import (
+    AliasChoices,
     AnyUrl,
     BaseModel,
     Field,
@@ -191,11 +192,13 @@ class ScratchRepository(BlueapiBaseModel):
         description="URL to clone from",
         default="https://github.com/example/example.git",
     )
-    branch: str | SkipJsonSchema[None] = Field(
+    target_revision: str | SkipJsonSchema[None] = Field(
         description=(
-            "Branch of repo to check out - defaults to remote's default when "
-            "cloning and the existing branch when the repo already exists"
+            "Revision (branch or tag) to check out when cloning - defaults to "
+            "remote's HEAD. If a tag is used, the repo will be left in a "
+            "'detached head' state."
         ),
+        validation_alias=AliasChoices("branch", "tag", "target_revision"),
         exclude_if=lambda f: f is None,
         # using default_factory instead of default means the schema doesn't
         # include an invalid value
@@ -455,5 +458,5 @@ def generate(self, schema, mode="validation"):
             return json_schema
 
     return ApplicationConfig.model_json_schema(
-        schema_generator=_GenerateJsonSchema, ref_template="{model}"
+        by_alias=False, schema_generator=_GenerateJsonSchema, ref_template="{model}"
     )

tests/unit_tests/cli/test_cli.py

@@ -1055,7 +1055,22 @@ def test_init_scratch_calls_setup_scratch(mock_setup_scratch: Mock, runner: CliR
             ScratchRepository(
                 name="dodal",
                 remote_url="https://github.com/DiamondLightSource/dodal.git",
-            )
+            ),
+            ScratchRepository(
+                name="with_target",
+                remote_url="https://github.com/DiamondLightSource/dodal.git",
+                target_revision="demo",
+            ),
+            ScratchRepository(
+                name="with_branch",
+                remote_url="https://github.com/DiamondLightSource/dodal.git",
+                target_revision="demo_branch",
+            ),
+            ScratchRepository(
+                name="with_tag",
+                remote_url="https://github.com/DiamondLightSource/dodal.git",
+                target_revision="demo_tag",
+            ),
         ],
     )
 

tests/unit_tests/cli/test_scratch.py

@@ -123,7 +123,7 @@ def test_repo_cloned_if_not_found_locally(
     ensure_repo("http://example.com/foo.git", nonexistant_path)
     mock_repo.assert_not_called()
     mock_repo.clone_from.assert_called_once_with(
-        "http://example.com/foo.git", nonexistant_path
+        "http://example.com/foo.git", nonexistant_path, branch=None
     )
 
 
@@ -140,7 +140,7 @@ def write_repo_files():
         with file_path.open("w") as stream:
             stream.write("foo")
 
-    mock_repo.clone_from.side_effect = lambda url, path: write_repo_files()
+    mock_repo.clone_from.side_effect = lambda url, path, branch=None: write_repo_files()
 
     ensure_repo("http://example.com/foo.git", repo_root)
     assert file_path.exists()
@@ -162,26 +162,23 @@ def test_cloned_repo_changes_to_new_branch(mock_repo, directory_path: Path):
 
     ensure_repo("http://example.com/foo.git", directory_path / "demo_branch", "demo")
 
-    mock_repo.clone_from.assert_called_once_with("http://example.com/foo.git", ANY)
-    repo.create_head.assert_called_once_with("demo", ANY)
-    repo.create_head().checkout.assert_called_once()
+    mock_repo.clone_from.assert_called_once_with(
+        "http://example.com/foo.git", ANY, branch="demo"
+    )
 
 
 @patch("blueapi.cli.scratch.Repo")
-def test_existing_repo_changes_to_existing_branch(mock_repo, directory_path: Path):
+def test_existing_repo_not_changed_to_existing_branch(mock_repo, directory_path: Path):
     (directory_path / "demo_branch").mkdir()
-    repo = Mock(spec=Repo)
-    mock_repo.return_value = repo
 
     ensure_repo("http://example.com/foo.git", directory_path / "demo_branch", "demo")
 
+    mock_repo.assert_called_once_with(directory_path / "demo_branch")
     mock_repo.clone_from.assert_not_called()
-    repo.create_head.assert_not_called()
-    repo.heads.demo.checkout.assert_called_once()
 
 
 @patch("blueapi.cli.scratch.Repo")
-def test_existing_repo_changes_to_new_branch(mock_repo, directory_path: Path):
+def test_existing_repo_not_changed_to_new_branch(mock_repo, directory_path: Path):
     (directory_path / "demo_branch").mkdir()
     repo = MagicMock(name="ExistingRepo", spec=Repo)
     repo.heads.demo = None
@@ -190,8 +187,43 @@ def test_existing_repo_changes_to_new_branch(mock_repo, directory_path: Path):
     ensure_repo("http://example.com/foo.git", directory_path / "demo_branch", "demo")
 
     mock_repo.clone_from.assert_not_called()
-    repo.create_head.assert_called_once_with("demo", repo.remotes[0].refs["demo"])
-    repo.create_head().checkout.assert_called_once()
+    repo.create_head.assert_not_called()
+
+
+@patch("blueapi.cli.scratch.Repo")
+@patch("blueapi.cli.scratch.LOGGER")
+def test_existing_repo_state_checked(
+    mock_logger: MagicMock, mock_repo: MagicMock, directory_path: Path
+):
+    repo = mock_repo.return_value
+    repo.head.commit.name_rev = "current"
+    repo.refs = {"demo": Mock()}
+
+    ensure_repo("http://example.com/foo.git", directory_path, "demo")
+
+    mock_logger.warning.assert_called_once_with(
+        "Repository %s not at target revision: %r instead of %r",
+        directory_path.name,
+        repo.head.commit.name_rev,
+        "demo",
+    )
+
+
+@patch("blueapi.cli.scratch.Repo")
+@patch("blueapi.cli.scratch.LOGGER")
+def test_existing_repo_unknown_revision(
+    mock_logger: MagicMock, mock_repo: MagicMock, directory_path: Path
+):
+    repo = mock_repo.return_value
+    repo.head.commit.name_rev = "current"
+    repo.refs = {}
+
+    ensure_repo("http://example.com/foo.git", directory_path, "demo")
+
+    mock_logger.warning.assert_called_once_with(
+        "Target revision %r not found",
+        "demo",
+    )
 
 
 def test_setup_scratch_fails_on_nonexistant_root(
@@ -292,14 +324,11 @@ def test_setup_scratch_iterates_repos(
     config = ScratchConfig(
         root=directory_path_with_sgid,
         repositories=[
-            ScratchRepository(
-                name="foo",
-                remote_url="http://example.com/foo.git",
-            ),
+            ScratchRepository(name="foo", remote_url="http://example.com/foo.git"),
             ScratchRepository(
                 name="bar",
                 remote_url="http://example.com/bar.git",
-                branch="demo",
+                target_revision="demo",
             ),
         ],
     )

tests/unit_tests/example_yaml/scratch.yaml

@@ -3,3 +3,12 @@ scratch:
   repositories:
     - name: dodal
       remote_url: https://github.com/DiamondLightSource/dodal.git
+    - name: with_target
+      remote_url: https://github.com/DiamondLightSource/dodal.git
+      target_revision: demo
+    - name: with_branch
+      remote_url: https://github.com/DiamondLightSource/dodal.git
+      branch: demo_branch
+    - name: with_tag
+      remote_url: https://github.com/DiamondLightSource/dodal.git
+      tag: demo_tag

tests/unit_tests/test_helm_chart.py

@@ -84,7 +84,7 @@
                     ScratchRepository(
                         name="bar",
                         remote_url="https://example.git",
-                        branch="bar_branch",
+                        target_revision="bar_branch",
                     ),
                 ],
             ),