Repository: alan-turing-institute/repro-catalogue

Issue:
git commit by catalogue engage gives error if no previous commits have been made
Something of a niche bug - I came across this one in a quick test this afternoon.

I created a new git repository with `git init`, saved a file in the code folder and then tried to run `catalogue engage ...`. The option to commit all uncommitted changes was presented, and after selecting `y`, the following error appeared:
```
gitdb.exc.BadName: Ref 'HEAD' did not resolve to an object
```

I'd assume this is because repro-catalogue tries to create a new branch for this commit, but as there are no prior commits it isn't possible to create a branch yet. We should check for this and either
1. Give the option to commit directly to master
2. Exit and make the user manually handle the first commit

Option 2. requires a little more effort on the user's part, but would mean that repro-catalogue won't change the master branch which I think is what we want here.

Discussion:

Patch:
diff --git a/catalogue/engage.py b/catalogue/engage.py
index 91895e5..03f6f30 100644
--- a/catalogue/engage.py
+++ b/catalogue/engage.py
@@ -1,161 +1,164 @@
-
-import os
-import json
-import git
-from git import InvalidGitRepositoryError
-
-from . import catalogue as ct
-from .compare import compare_hashes, print_comparison
-from .utils import create_timestamp, check_paths_exists, prune_files
-
-
-def git_query(repo_path, catalogue_dir, commit_changes=False):
-    """
-    Check status of a git repository
-
-    Checks the git status of the repository on the provided path
-    - if clean, returns true
-    - if there are uncommitted changes (including untracked files other than
-        those held in `catalogue_dir`), and the `commit_changes` flag is
-        `True`, offer the user to stage and commit all tracked (and untracked)
-        files and continue (returns `True`)
-        otherwise, returns `False`
-
-    If the `commit_changes` flag is `True` and the user accepts the offer
-    to commit changes, a new branch is created with the name
-    `"catalogue-%Y%m%d-%H%M%S"` and all tracked files that have been
-    changed & any untracked files will be staged and committed.
-
-    Parameters:
-    ------------
-    repo_path : str
-        path to the code directory
-    catalogue_dir : str
-        path to directory with catalogue output files
-    commit_changes : bool
-        boolean indicating if the user should be prompted to stage and commit changes
-        (optional, default is False)
-
-    Returns:
-    ---------
-    Boolean indicating if git directory is clean
-    """
-
-    try:
-        repo = git.Repo(repo_path, search_parent_directories=True)
-    except InvalidGitRepositoryError:
-        raise InvalidGitRepositoryError("provided code directory is not a valid git repository")
-
-    untracked = prune_files(repo.untracked_files, catalogue_dir)
-
-    if repo.is_dirty() or (len(untracked) != 0):
-        if commit_changes:
-            print("Working directory contains uncommitted changes.")
-            print("Do you want to stage and commit all changes? (y/[n])")
-            user_choice = input().strip().lower()
-            if user_choice == "y" or user_choice == "yes":
-                timestamp = create_timestamp()
-                new_branch = repo.create_head("catalogue-" + timestamp)
-                new_branch.checkout()
-                changed_files = [ item.a_path for item in repo.index.diff(None) ]
-                changed_files.extend(untracked)
-                repo.index.add(changed_files)
-                repo.index.commit("repro-catalogue tool auto commit at " + timestamp)
-                return True
-            elif user_choice == "n" or user_choice == "no" or user_choice == "":
-                return False
-            else:
-                print("Unrecognized response, leaving repository unchaged")
-                return False
-        else:
-            return False
-    else:
-        return True
-
-
-def engage(args):
-    """
-    The `catalogue engage` command.
-
-    The engage command is used prior to running an analysis. The `args` contain
-    paths to `input_data` and `code` (which must be a git repo).
-
-    The engage command:
-        - does a `git_query()` check of the `code` repo
-            (engage does not proceed unless the repo is clean)
-        - gets hashes for the input_data and code (from `construct_dict()`)
-        - saves the hashes to a `.lock` file
-
-    Once engaged (a `.lock` file exists), the command cannot be run again until
-    `disengage` has been run.
-
-    Parameters:
-    ------------
-    args : obj
-        Command line input arguments (argparse.Namespace).
-
-    Returns:
-    ---------
-    None
-    """
-    assert check_paths_exists(args), 'Not all provided filepaths exist.'
-
-    if git_query(args.code, args.catalogue_results, True):
-        try:
-            assert not os.path.exists(os.path.join(args.catalogue_results, ".lock"))
-        except AssertionError:
-            print("Already engaged (.lock file exists). To disengage run 'catalogue disengage...'")
-            print("See 'catalogue disengage --help' for details")
-        else:
-            hash_dict = ct.construct_dict(create_timestamp(), args)
-            ct.store_hash(hash_dict, "", args.catalogue_results, ext="lock")
-            print("'catalogue engage' succeeded. Proceed with analysis")
-
-def disengage(args):
-    """
-    The `catalogue disengage` command.
-
-    The disengage command is run just after finishing an analysis. It cannot be run
-    unless `engage` was run first.
-
-    The `args` contain paths to `input_data`, `code` (must be a git repo) and `output_data`.
-
-    The disengage command:
-        - reads hashes stored in the `.lock` file created during `engage`
-        - gets hashes for the `input_data`, `code` and `output_data` (from `construct_dict()`)
-        - compares the two sets of hashes
-            (if `input_data` and `code` hashes match, saves the hashes to a file)
-        - prints the results of the comparison
-
-    Parameters:
-    ------------
-    args : obj
-        Command line input arguments (argparse.Namespace).
-
-    Returns:
-    ---------
-    None
-    """
-    assert check_paths_exists(args), 'Not all provided filepaths exist.'
-
-    timestamp = create_timestamp()
-    try:
-        LOCK_FILE_PATH = os.path.join(args.catalogue_results, ".lock")
-        lock_dict = ct.load_hash(LOCK_FILE_PATH)
-        os.remove(LOCK_FILE_PATH)
-    except FileNotFoundError:
-        print("Not currently engaged (could not find .lock file). To engage run 'catalogue engage...'")
-        print("See 'catalogue engage --help' for details")
-    else:
-        hash_dict = ct.construct_dict(timestamp, args)
-        compare = compare_hashes(hash_dict, lock_dict)
-        # check if 'input_data' and 'code' were in matches
-        assert "matches" in compare.keys(), "Error in constructing comparison dictionary"
-        if 'input_data' in compare["matches"] and 'code' in compare["matches"]:
-            # add engage timestamp to hash_dict
-            hash_dict["timestamp"].update({"engage": lock_dict["timestamp"]["engage"]})
-            if (args.csv is not None) and (os.path.splitext(args.csv)[1] == '.csv'):
-                ct.save_csv(hash_dict, timestamp, os.path.join(args.catalogue_results, args.csv))
-            else:
-                ct.store_hash(hash_dict, timestamp, args.catalogue_results)
-        print_comparison(compare)
+import os
+import json
+import git
+from git import InvalidGitRepositoryError, BadName
+
+from . import catalogue as ct
+from .compare import compare_hashes, print_comparison
+from .utils import create_timestamp, check_paths_exists, prune_files
+
+
+def git_query(repo_path, catalogue_dir, commit_changes=False):
+    """
+    Check status of a git repository
+
+    Checks the git status of the repository on the provided path
+    - if clean, returns true
+    - if there are uncommitted changes (including untracked files other than
+        those held in `catalogue_dir`), and the `commit_changes` flag is
+        `True`, offer the user to stage and commit all tracked (and untracked)
+        files and continue (returns `True`)
+        otherwise, returns `False`
+
+    If the `commit_changes` flag is `True` and the user accepts the offer
+    to commit changes, a new branch is created with the name
+    `"catalogue-%Y%m%d-%H%M%S"` and all tracked files that have been
+    changed & any untracked files will be staged and committed.
+
+    Parameters:
+    ------------
+    repo_path : str
+        path to the code directory
+    catalogue_dir : str
+        path to directory with catalogue output files
+    commit_changes : bool
+        boolean indicating if the user should be prompted to stage and commit changes
+        (optional, default is False)
+
+    Returns:
+    ---------
+    Boolean indicating if git directory is clean
+    """
+
+    try:
+        repo = git.Repo(repo_path, search_parent_directories=True)
+    except InvalidGitRepositoryError:
+        raise InvalidGitRepositoryError("provided code directory is not a valid git repository")
+
+    untracked = prune_files(repo.untracked_files, catalogue_dir)
+
+    if repo.is_dirty() or (len(untracked) != 0):
+        if commit_changes:
+            print("Working directory contains uncommitted changes.")
+            print("Do you want to stage and commit all changes? (y/[n])")
+            user_choice = input().strip().lower()
+            if user_choice == "y" or user_choice == "yes":
+                timestamp = create_timestamp()
+                try:
+                    new_branch = repo.create_head("catalogue-" + timestamp)
+                except BadName:
+                    print("\nCannot create a branch for this commit as there are no existing commits.\nMake a commit manually, then run catalogue engage again.\n")
+                    return False
+                new_branch.checkout()
+                changed_files = [ item.a_path for item in repo.index.diff(None) ]
+                changed_files.extend(untracked)
+                repo.index.add(changed_files)
+                repo.index.commit("repro-catalogue tool auto commit at " + timestamp)
+                return True
+            elif user_choice == "n" or user_choice == "no" or user_choice == "":
+                return False
+            else:
+                print("Unrecognized response, leaving repository unchaged")
+                return False
+        else:
+            return False
+    else:
+        return True
+
+
+def engage(args):
+    """
+    The `catalogue engage` command.
+
+    The engage command is used prior to running an analysis. The `args` contain
+    paths to `input_data` and `code` (which must be a git repo).
+
+    The engage command:
+        - does a `git_query()` check of the `code` repo
+            (engage does not proceed unless the repo is clean)
+        - gets hashes for the input_data and code (from `construct_dict()`)
+        - saves the hashes to a `.lock` file
+
+    Once engaged (a `.lock` file exists), the command cannot be run again until
+    `disengage` has been run.
+
+    Parameters:
+    ------------
+    args : obj
+        Command line input arguments (argparse.Namespace).
+
+    Returns:
+    ---------
+    None
+    """
+    assert check_paths_exists(args), 'Not all provided filepaths exist.'
+
+    if git_query(args.code, args.catalogue_results, True):
+        try:
+            assert not os.path.exists(os.path.join(args.catalogue_results, ".lock"))
+        except AssertionError:
+            print("Already engaged (.lock file exists). To disengage run 'catalogue disengage...'")
+            print("See 'catalogue disengage --help' for details")
+        else:
+            hash_dict = ct.construct_dict(create_timestamp(), args)
+            ct.store_hash(hash_dict, "", args.catalogue_results, ext="lock")
+            print("'catalogue engage' succeeded. Proceed with analysis")
+
+def disengage(args):
+    """
+    The `catalogue disengage` command.
+
+    The disengage command is run just after finishing an analysis. It cannot be run
+    unless `engage` was run first.
+
+    The `args` contain paths to `input_data`, `code` (must be a git repo) and `output_data`.
+
+    The disengage command:
+        - reads hashes stored in the `.lock` file created during `engage`
+        - gets hashes for the `input_data`, `code` and `output_data` (from `construct_dict()`)
+        - compares the two sets of hashes
+            (if `input_data` and `code` hashes match, saves the hashes to a file)
+        - prints the results of the comparison
+
+    Parameters:
+    ------------
+    args : obj
+        Command line input arguments (argparse.Namespace).
+
+    Returns:
+    ---------
+    None
+    """
+    assert check_paths_exists(args), 'Not all provided filepaths exist.'
+
+    timestamp = create_timestamp()
+    try:
+        LOCK_FILE_PATH = os.path.join(args.catalogue_results, ".lock")
+        lock_dict = ct.load_hash(LOCK_FILE_PATH)
+        os.remove(LOCK_FILE_PATH)
+    except FileNotFoundError:
+        print("Not currently engaged (could not find .lock file). To engage run 'catalogue engage...'")
+        print("See 'catalogue engage --help' for details")
+    else:
+        hash_dict = ct.construct_dict(timestamp, args)
+        compare = compare_hashes(hash_dict, lock_dict)
+        # check if 'input_data' and 'code' were in matches
+        assert "matches" in compare.keys(), "Error in constructing comparison dictionary"
+        if 'input_data' in compare["matches"] and 'code' in compare["matches"]:
+            # add engage timestamp to hash_dict
+            hash_dict["timestamp"].update({"engage": lock_dict["timestamp"]["engage"]})
+            if (args.csv is not None) and (os.path.splitext(args.csv)[1] == '.csv'):
+                ct.save_csv(hash_dict, timestamp, os.path.join(args.catalogue_results, args.csv))
+            else:
+                ct.store_hash(hash_dict, timestamp, args.catalogue_results)
+        print_comparison(compare)


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index acf3ff6..cef7841 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -67,6 +67,19 @@ def git_repo(workspace, fixtures_dir):
     repo.index.commit("Initial commit")
     return workspace.workspace
 
+@pytest.fixture
+def git_repo_no_commits(workspace, fixtures_dir):
+    """
+    Create git repository in temporary workspace (from pytest-shutil).
+    This repo has just been created, so there is no initial commit yet.
+    Copy in files from fixtures.
+    Return path to repo.
+    """
+    repo = git.Repo.init(workspace.workspace)
+    workspace.run("cp -R {} data/".format(fixtures_dir))
+    workspace.run("cp -R {} results/".format(fixtures_dir))
+    return workspace.workspace
+
 @pytest.fixture
 def git_hash(git_repo):
     """
diff --git a/tests/test_engage.py b/tests/test_engage.py
index ffbf2dc..61f5eb7 100644
--- a/tests/test_engage.py
+++ b/tests/test_engage.py
@@ -98,6 +98,24 @@ def test_git_query(git_repo, capsys, workspace, monkeypatch):
     with pytest.raises(TypeError):
         git_query()
 
+def test_git_query_new_repo(git_repo_no_commits, capsys, workspace, monkeypatch):
+    """
+    Test that the engage command exits correctly when the code repo is new and contains uncommitted changes.
+    """
+
+    # Check that git_query correctly determines that the repo is not currently clean
+    assert not git_query(git_repo_no_commits, "catalogue_results")
+
+    # Select "no" when asked whether to commit local changes; repo will still not be clean
+    monkeypatch.setattr('builtins.input', lambda: "n")
+    assert not git_query(git_repo_no_commits, "catalogue_results", True)
+
+    # Select "yes" when asked whether to commit local changes
+    # This should throw a sensible error, as a branch can't be generated yet (no initial commit)
+    monkeypatch.setattr('builtins.input', lambda: "y")
+    assert not git_query(git_repo_no_commits, "catalogue_results", True)
+    captured = capsys.readouterr()
+    assert "Cannot create a branch for this commit as there are no existing commits.\nMake a commit manually, then run catalogue engage again." in captured.out
 
 def test_engage(git_repo, test_args, capsys):
     """