Raise exception when repo is in a detached HEAD state (#626)

* Directly exit when detached HEAD is detected
* Add test for detached head exception
* Remove unused import in test_cli.py
* Make pytests less verbose
* Add sweep test to test_setup.py
* Catch SystemExit exception
* Add get_branch method + test. Replaced direct call to active_branch in branch.py
* Fix type hint from review
---------
Co-authored-by: jo-basevi <[email protected]>

Author: aidanheerdegen

.github/workflows/CI.yml

@@ -87,7 +87,7 @@ jobs:
 
       - name: Run tests
         run: |
-          PYTHONPATH=. pytest --cov=payu -s test;
+          PYTHONPATH=. pytest --cov=payu test;
 
       - name: Coveralls
         uses: AndreMiras/coveralls-python-action@65c1672f0b8a201702d86c81b79187df74072505

payu/branch.py

@@ -442,7 +442,7 @@ def list_branches(config_path: Optional[Path] = None,
     control_path = get_control_path(config_path)
     git_repo = GitRepository(control_path)
 
-    current_branch = git_repo.repo.active_branch
+    current_branch = git_repo.get_branch()
     print(f"* Current Branch: {current_branch.name}")
     print_branch_metadata(current_branch, verbose)
 

payu/git_utils.py

@@ -5,6 +5,7 @@
 
 import warnings
 from pathlib import Path
+import sys
 from typing import Optional, Union, List, Dict
 
 import git
@@ -57,17 +58,39 @@ def __init__(self,
             repo = get_git_repository(repo_path, catch_error=catch_error)
         self.repo = repo
 
-    def get_branch_name(self) -> Optional[str]:
+    def get_branch(self) -> Optional[git.Head]:
         """Return the current git branch or None if repository path is
         not a git repository"""
         if self.repo:
-            return str(self.repo.active_branch)
+            if self.repo.head.is_detached:
+                sys.exit("\nRepo is in a detached HEAD state.\n"
+                         "Before running again checkout a branch using\n\n"
+                         "    payu checkout <branch>\n\n")
+            else:
+                return self.repo.active_branch
+        else:
+            return None
+
+    def get_branch_name(self) -> Optional[str]:
+        """Return the current git branch or None if repository path is
+        not a git repository"""
+
+        branch = self.get_branch()
+
+        if branch is not None:
+            return branch.name
+        else:
+            return None
 
     def get_hash(self) -> Optional[str]:
         """Return the current git commit hash or None if repository path is
           not a git repository"""
         if self.repo:
-            return self.repo.active_branch.object.hexsha
+            branch = self.repo.get_branch()
+            if branch is not None:
+                return branch.object.hexsha
+            else:
+                return None
 
     def get_origin_url(self) -> Optional[str]:
         """Return url of remote origin if it exists"""

test/common.py

@@ -10,7 +10,7 @@
 # Namespace clash if import setup_cmd.runcmd as setup. For
 # consistency use payu_ prefix for all commands
 from payu.subcommands.init_cmd import runcmd as payu_init
-from payu.subcommands.setup_cmd import runcmd as payu_setup_orignal
+from payu.subcommands.setup_cmd import runcmd as payu_setup_original
 from payu.subcommands.sweep_cmd import runcmd as payu_sweep
 
 ctrldir_basename = 'ctrl'
@@ -127,7 +127,7 @@ def payu_setup(model_type=None,
                        hard_sweep=False,
                        lab_path=str(labdir),
                        metadata_off=False)
-        payu_setup_orignal(model_type,
+        payu_setup_original(model_type,
                            config_path,
                            lab_path,
                            force_archive,

test/test_cli.py

@@ -1,4 +1,3 @@
-import pdb
 import pytest
 import shlex
 

test/test_git_utils.py

@@ -29,7 +29,7 @@ def setup_and_teardown():
 
 def create_new_repo(repo_path):
     """Helper function to initialise a repo and create first commit"""
-    repo = git.Repo.init(repo_path)
+    repo = git.Repo.init(repo_path, initial_branch='main')
     init_file = repo_path / "init.txt"
     add_file_and_commit(repo, init_file)
     return repo
@@ -90,6 +90,14 @@ def test_get_git_user_info_config_set():
     assert value == 'TestUserName'
 
 
+def test_get_branch():
+    # Setup
+    repo_path = tmpdir / 'test_repo_get_branch'
+    new_repo = GitRepository(repo_path, repo=create_new_repo(repo_path))
+
+    assert new_repo.get_branch_name() == 'main'
+
+
 @pytest.mark.parametrize("ref", ["branch", "hash", None])
 def test_git_checkout_new_branch_from_remote_ref(ref):
     # Setup
@@ -228,3 +236,32 @@ def test_git_checkout_missing_origin_repo():
 
         # No remote branches found
         assert repo.remote_branches_dict() == {}
+
+
+def test_git_get_branch_detached_head():
+    """In Issue #625 there was an unhelpful exception message
+    when payu sweep was run from a repo with a detached HEAD.
+    Now throw exception will be caught in the caller."""
+
+    # Setup
+    detached_repo_path = tmpdir / 'detachedRepo'
+    create_new_repo(detached_repo_path)
+
+    detached = GitRepository(detached_repo_path)
+
+    assert detached.get_branch_name() == "main"
+
+    # Checkout HEAD commit to make detached state
+    detached.repo.git.checkout(detached.repo.commit("HEAD"))
+
+    assert detached.repo.head.is_detached
+
+    expected_msg = ("\nRepo is in a detached HEAD state.\n"
+                    "Checkout a branch before running again.\n")
+
+    expected_msg = ("\nRepo is in a detached HEAD state.\n"
+                    "Before running again checkout a branch using"
+                    "\n\n    payu checkout <branch>\n\n")
+
+    with pytest.raises(SystemExit, match=expected_msg):
+        detached.get_branch_name()

test/test_setup.py

@@ -12,7 +12,7 @@
 
 from .common import cd, make_random_file, get_manifests
 from .common import tmpdir, ctrldir, labdir, workdir
-from .common import payu_init, payu_setup
+from .common import payu_init, payu_setup, sweep_work
 from .common import config as config_orig
 from .common import write_config
 from .common import make_exe, make_inputs, make_expt_archive_dir
@@ -189,6 +189,18 @@ def test_setup():
     assert workdir.is_symlink and workdir.is_dir()
 
 
+def test_sweep():
+    """Test sweeping ephemeral work directory functions
+    correctly"""
+    run_payu_setup(create_inputs=True, create_config_files=True)
+
+    assert workdir.is_symlink and workdir.is_dir()
+
+    sweep_work()
+
+    assert not workdir.exists()
+
+
 @pytest.mark.parametrize(
     "current_version, min_version",
     [