Read output log without mutating the output repository

cadetrdm/logging.py

@@ -76,6 +76,12 @@ def fulfils_environment(self, environment: Environment):
             Instance of Environment class, with requirements as key: value pairs.
         :return:
         """
+        # Environment matching is opt-in. Without requirements to check against, the
+        # recorded environment is not read at all, so loading results does not depend
+        # on the run_history files being present in the working tree.
+        if environment is None:
+            return True
+
         if self._environment is None:
             self._load_environment()
 
@@ -137,6 +143,31 @@ def n_entries(self) -> int:
         """int: Number of results stored in the repository."""
         return len(self.entries)
 
+    @classmethod
+    def from_string(cls, content: str, filepath=None):
+        """
+        Create an OutputLog from the raw contents of a log.tsv file.
+
+        Used to read the log out of a git ref without checking that ref out. The
+        filepath is not read from, but is retained so that LogEntry can resolve the
+        run_history files next to it.
+
+        :param content:
+            Raw tab-separated contents of a log.tsv file.
+        :param filepath:
+            Optional path the contents belong to.
+        """
+        instance = cls()
+        instance._filepath = filepath
+
+        lines = [line.split("\t") for line in content.splitlines() if line]
+        if not lines:
+            return instance
+
+        instance._entry_list = lines
+        instance.entries: dict[str, LogEntry] = instance._entries_from_entry_list(instance._entry_list)
+        return instance
+
     @classmethod
     def from_list(cls, entry_list: list[list[str]]):
         instance = cls()

cadetrdm/repositories.py

@@ -1095,6 +1095,9 @@ def update_output_main_logs(
         Dumps all the metadata information about the project repositories state and
         the commit hash and branch name of the ouput repository into the main branch of
         the output repository.
+        This is the write path for the output log: it intentionally checks out the
+        output repository's main branch, updates the run history, commits it, and then
+        returns to the result branch.
         :param output_dict:
         Dictionary containing key-value pairs to be added to the log.
         """
@@ -1565,11 +1568,21 @@ def output_log_file_path(self):
 
     @property
     def output_log(self):
-        if self.has_uncomitted_changes:
-            self._reset_hard_to_head(force_entry=True)
-        if not self.active_branch == self.main_branch:
-            self.checkout(self.main_branch)
-        return OutputLog(filepath=self.output_log_file_path)
+        """
+        OutputLog: The run history recorded on the main branch.
+
+        Read directly from the main branch ref, so that inspecting the log neither
+        checks out that branch nor touches the working tree. Reading the log used to
+        discard uncommitted changes and check out the main branch, which made loading
+        results a destructive operation.
+        """
+        try:
+            log_content = self._git.show(f"{self.main_branch}:log.tsv")
+        except git.GitCommandError:
+            # No log.tsv on the main branch yet, e.g. in a freshly initialized repo.
+            return OutputLog()
+
+        return OutputLog.from_string(log_content, filepath=self.path / "log.tsv")
 
     def print_output_log(self):
         self.checkout(self.main_branch)

tests/test_read_only_loading.py

@@ -0,0 +1,99 @@
+"""Regression tests pinning that reading results does not mutate git state.
+
+Loading archived results must not move either repository. These tests snapshot the
+observable git state of both repositories, perform a read, and assert the snapshot is
+unchanged.
+"""
+
+from pathlib import Path
+
+import pytest
+
+from cadetrdm import ProjectRepo, initialize_repo
+
+
+def git_state(repo):
+    """Snapshot the observable git state of a repository.
+
+    Covers everything a read operation must leave alone: the checked out commit and
+    branch, the set of local branches, and whether the working tree is dirty.
+    """
+    return {
+        "commit": repo.current_commit_hash,
+        "branch": str(repo.active_branch),
+        "branches": sorted(head.name for head in repo._git_repo.heads),
+        "is_dirty": repo._git_repo.is_dirty(untracked_files=True),
+    }
+
+
+@pytest.fixture
+def repo_with_results(tmp_path):
+    """A project repo holding one recorded result, left on the result branch.
+
+    Leaving the output repo on the result branch rather than on main is what makes the
+    mutation visible: a read that checks out main would change the active branch.
+    """
+    path_to_repo = tmp_path / "project"
+    initialize_repo(path_to_repo, "results")
+
+    repo = ProjectRepo(path_to_repo)
+    with repo.track_results(results_commit_message="Add result"):
+        (repo.output_path / "result.csv").write_text("1,2,3\n")
+
+    assert str(repo.output_repo.active_branch) != repo.output_repo.main_branch
+
+    return repo
+
+
+def test_reading_output_log_leaves_output_repo_untouched(repo_with_results):
+    output_repo = repo_with_results.output_repo
+    state_before = git_state(output_repo)
+
+    output_repo.output_log
+
+    assert git_state(output_repo) == state_before
+
+
+def test_reading_output_log_preserves_uncommitted_changes(repo_with_results):
+    output_repo = repo_with_results.output_repo
+    scratch_file = Path(output_repo.path) / "uncommitted.txt"
+    scratch_file.write_text("work in progress\n")
+
+    output_repo.output_log
+
+    assert scratch_file.exists(), "reading the log discarded uncommitted work"
+    assert scratch_file.read_text() == "work in progress\n"
+
+
+def test_output_log_read_without_checkout_lists_the_result_branch(repo_with_results):
+    output_repo = repo_with_results.output_repo
+    result_branch = str(output_repo.active_branch)
+
+    entries = output_repo.output_log.entries
+
+    assert result_branch in entries
+    assert entries[result_branch].project_repo_commit_hash == repo_with_results.current_commit_hash
+
+
+def test_matching_without_an_environment_does_not_read_run_history(repo_with_results):
+    """Without requirements to match, recorded environments must not be read.
+
+    run_history is only populated on the main branch, so on a result branch the
+    conda_environment.yml a LogEntry would load is not in the working tree. Reading it
+    used to work only because reading the log checked out main first.
+    """
+    output_repo = repo_with_results.output_repo
+    assert not (Path(output_repo.path) / "run_history").exists()
+
+    entry = output_repo.output_log.entries[str(output_repo.active_branch)]
+
+    assert entry.fulfils_environment(None) is True
+
+
+def test_output_log_is_empty_when_no_results_are_recorded(tmp_path):
+    path_to_repo = tmp_path / "project"
+    initialize_repo(path_to_repo, "results")
+
+    repo = ProjectRepo(path_to_repo)
+
+    assert repo.output_repo.output_log.n_entries == 0