From 2a149b96111f0ca7a40ce646da389d4ec7d0a4bc Mon Sep 17 00:00:00 2001
From: Paul Lietar <pl2113@ic.ac.uk>
Date: Thu, 27 Jun 2024 17:22:18 +0100
Subject: [PATCH] Improve handling of empty or detached Git repositories.

When orderly runs a report from a Git repository, it records information
about the current state of the repository in the packet metadata. This
includes the current commit hash and the name of the current branch.

It is possible for either of these to not exist, in which case we need
to make sure orderly behaves gracefully. The commit hash will be missing
on a brand new repository which hasn't had any commits yet (or
equivalently, an existing repository where `git checkout --orphan` was
used. The branch name would be missing if the repository is in a
"detached HEAD" state, in other words the user checked out a specific
commit hash instead of a named branch.

The first situation, of an empty repository, was not supported by
orderly at all. It would throw an error and fail to run the report
entirely. In the second case, when on a detached HEAD, the Git library
we use reports the branch name as "HEAD", and that is what we were
recording in the metadata. This is a bit misleading since it is not an
actual branch name.

Both cases have been modified to work without errors and to omit the
missing values from the metadata.
---
 R/outpack_misc.R                  | 12 +++++++--
 tests/testthat/test-outpack-git.R | 42 +++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/R/outpack_misc.R b/R/outpack_misc.R
index 58871c2b..3c3be020 100644
--- a/R/outpack_misc.R
+++ b/R/outpack_misc.R
@@ -44,8 +44,16 @@ git_info <- function(path) {
   if (is.null(repo)) {
     return(NULL)
   }
-  list(sha = gert::git_commit_id(repo = repo),
-       branch = gert::git_branch(repo = repo),
+
+  branch <- gert::git_branch(repo = repo)
+  if (identical(branch, "HEAD")) {
+    # HEAD isn't a valid branch name, and instead is what gets returned when a
+    # detached head was checked out.
+    branch <- NULL
+  }
+
+  list(sha = ignore_errors(gert::git_commit_id(repo = repo)),
+       branch = branch,
        url = gert::git_remote_list(repo = repo)$url)
 }
 
diff --git a/tests/testthat/test-outpack-git.R b/tests/testthat/test-outpack-git.R
index 97cc3860..bdc3ae9c 100644
--- a/tests/testthat/test-outpack-git.R
+++ b/tests/testthat/test-outpack-git.R
@@ -68,3 +68,45 @@ test_that("store no information into packet, if no git found", {
   expect_true("git" %in% names(meta))
   expect_null(meta$git)
 })
+
+
+test_that("store git information when on a detached HEAD", {
+  root <- create_temporary_root()
+  path_src <- create_temporary_simple_src()
+  git_info <- helper_add_git(path_src)
+
+  ## gert has no API to checkout a particular commit, only named branches
+  ## https://github.com/r-lib/gert/issues/147
+  system2("git", c("-C", path_src, "checkout", "-q", git_info$sha))
+
+  suppressMessages({
+    p <- outpack_packet_start(path_src, "example", root = root)
+    id <- p$id
+    outpack_packet_run(p, "script.R")
+    outpack_packet_end(p)
+  })
+
+  meta <- orderly_metadata(id, root = root$path)
+  expect_mapequal(meta$git,
+                  list(sha = git_info$sha,
+                       url = git_info$url))
+})
+
+
+test_that("handle empty git repository correctly", {
+  root <- create_temporary_root()
+  path_src <- create_temporary_simple_src()
+
+  gert::git_init(path_src)
+  gert::git_remote_add("https://example.com/git", repo = path_src)
+
+  suppressMessages({
+    p <- outpack_packet_start(path_src, "example", root = root)
+    id <- p$id
+    outpack_packet_run(p, "script.R")
+    outpack_packet_end(p)
+  })
+
+  meta <- orderly_metadata(id, root = root$path)
+  expect_mapequal(meta$git, list(url = "https://example.com/git"))
+})