Merge pull request #318 from vimc/vimc-6484

Allow control over default branch

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: orderly
 Title: Lightweight Reproducible Reporting
-Version: 1.4.8
+Version: 1.4.9
 Description: Order, create and store reports from R.  By defining a
     lightweight interface around the inputs and outputs of an
     analysis, a lot of the repetitive work for reproducible research

R/config.R

@@ -298,7 +298,9 @@ config_validate_remote <- function(remote, filename) {
     d <- remote[[name]]
     check_fields(d, sprintf("%s:remote:%s", filename, name),
                  c("driver", "args"),
-                 c("url", "slack_url", "teams_url", "primary", "master_only"))
+                 c("url", "slack_url", "teams_url", "primary",
+                   "master_only", "default_branch_only",
+                   "default_branch"))
     field_name <- function(nm) {
       sprintf("%s:remote:%s:%s", filename, name, nm)
     }
@@ -318,10 +320,36 @@ config_validate_remote <- function(remote, filename) {
     } else {
       assert_scalar_logical(d$primary, field_name("primary"))
     }
-    if (is.null(d$master_only)) {
-      d$master_only <- FALSE
+
+    if (!is.null(d$master_only)) {
+      if (!is.null(d$default_branch_only)) {
+        msg <- c("Can't specify both 'master_only' and 'default_branch_only': ",
+                 sprintf("see %s:remote:%s", filename, name))
+        stop(msg)
+      }
+      msg <- c("The 'master_only' field (used in",
+               sprintf("%s:remote:%s", filename, name),
+               "is deprecated and replaced with 'default_branch_only'",
+               "and will be dropped in a future version of",
+               "orderly.  Please rename it in your orderly_config.yml")
+      orderly_warning(flow_text(msg))
+      d$default_branch_only <- d$master_only
+      d$master_only <- NULL
+    }
+
+    if (is.null(d$default_branch_only)) {
+      d$default_branch_only <- FALSE
+    } else {
+      assert_scalar_logical(d$default_branch_only,
+                            field_name("default_branch_only"))
+    }
+
+    ## use '[[' not '$' here to avoid partial matches.
+    if (is.null(d[["default_branch"]])) {
+      d[["default_branch"]] <- "master"
     } else {
-      assert_scalar_logical(d$master_only, field_name("master_only"))
+      assert_scalar_character(d[["default_branch"]],
+                              field_name("default_branch"))
     }
 
     d$driver <- check_symbol_from_str(d$driver, field_name("driver"))

R/remote.R

@@ -241,7 +241,7 @@ orderly_push_archive <- function(name, id = "latest", root = NULL,
 ##'   be included.
 ##'
 ##' @param ref Optional reference, indicating which branch should be
-##'   used.  This cannot be used if the remote has `master_only`
+##'   used.  This cannot be used if the remote has `default_branch_only`
 ##'   set.
 ##'
 ##' @param instance Select instance of the source database to be used,

R/testing.R

@@ -234,7 +234,7 @@ build_git_demo <- function() {
   archive
 }
 
-unzip_git_demo <- function(path = tempfile()) {
+unzip_git_demo <- function(path = tempfile(), default_branch = "master") {
   tmp <- tempfile()
   dir.create(tmp, FALSE, TRUE)
   demo <- getOption("orderly.server.demo", build_git_demo())
@@ -244,14 +244,23 @@ unzip_git_demo <- function(path = tempfile()) {
              no.. = TRUE)
   file_copy(src, path, recursive = TRUE)
   unlink(tmp, recursive = TRUE)
+  if (default_branch != "master") {
+    ## If git changes it's mind about what the default branch is called,
+    ## this will fail, we should probably detect this, but it's likely
+    ## that will break build_git_demo too
+    gert::git_branch_move("master", default_branch, force = TRUE,
+                          repo = path)
+  }
   path
 }
 
 prepare_orderly_git_example <- function(path = tempfile(), run_report = FALSE,
-                                        branch = "master") {
+                                        branch = default_branch,
+                                        default_branch = "master") {
   path_upstream <- file.path(path, "upstream")
-  unzip_git_demo(path)
-  unzip_git_demo(path_upstream)
+  unzip_git_demo(path, default_branch)
+  unzip_git_demo(path_upstream, default_branch)
+
   git_checkout_branch(branch, root = path)
   git_checkout_branch(branch, root = path_upstream)
 

tests/testthat/test-config.R

@@ -126,7 +126,7 @@ test_that("support declaring api server", {
                 main = list(
                   driver = "orderly::orderly_remote_path",
                   primary = TRUE,
-                  master_only = TRUE,
+                  default_branch_only = TRUE,
                   args = list(root = path)),
                 other = list(
                   driver = "orderly::orderly_remote_path",
@@ -145,7 +145,8 @@ test_that("support declaring api server", {
   expect_null(cfg$remote$other$identity)
   expect_equal(
     cfg$server_options(),
-    list(primary = TRUE, master_only = TRUE, name = "main"))
+    list(primary = TRUE, default_branch_only = TRUE,
+         default_branch = "master", name = "main"))
 
   cfg <- withr::with_envvar(
     c("ORDERLY_API_SERVER_IDENTITY" = "other"),
@@ -154,7 +155,8 @@ test_that("support declaring api server", {
   expect_true(cfg$remote$other$identity)
   expect_equal(
     cfg$server_options(),
-    list(primary = FALSE, master_only = FALSE, name = "other"))
+    list(primary = FALSE, default_branch_only = FALSE,
+         default_branch = "master", name = "other"))
 
   cfg <- withr::with_envvar(
     c("ORDERLY_API_SERVER_IDENTITY" = NA),
@@ -203,11 +205,11 @@ test_that("remote parse check", {
                   driver = "orderly::orderly_remote_path",
                   primary = TRUE,
                   args = list(root = path),
-                  master_only = "yeah")))
+                  default_branch_only = "yeah")))
   writeLines(yaml::as.yaml(dat), path_orderly_config_yml(path))
   expect_error(
     orderly_config_$new(path),
-    "'orderly_config.yml:remote:myhost:master_only' must be logical")
+    "'orderly_config.yml:remote:myhost:default_branch_only' must be logical")
 })
 
 test_that("no global folder", {
@@ -555,3 +557,55 @@ test_that("adding new fields in new versions gives good errors", {
     orderly_config_$new(path),
     "Orderly version '9.9.9' is required, but only '.+' installed")
 })
+
+
+test_that("can configure default branch for a remote", {
+  path <- tempfile()
+  dir.create(path)
+  dat <- list(remote = list(
+                main = list(
+                  driver = "orderly::orderly_remote_path",
+                  primary = TRUE,
+                  default_branch_only = TRUE,
+                  default_branch = "main",
+                  args = list(root = path))))
+  writeLines(yaml::as.yaml(dat), path_orderly_config_yml(path))
+  cfg <- orderly_config_$new(path)
+  expect_true(cfg$remote$main$default_branch_only)
+  expect_equal(cfg$remote$main$default_branch, "main")
+})
+
+
+test_that("migrate master_only", {
+  path <- tempfile()
+  dir.create(path)
+  dat <- list(remote = list(
+                main = list(
+                  driver = "orderly::orderly_remote_path",
+                  primary = TRUE,
+                  master_only = TRUE,
+                  args = list(root = path))))
+  writeLines(yaml::as.yaml(dat), path_orderly_config_yml(path))
+  expect_warning(
+    cfg <- orderly_config_$new(path),
+    "The 'master_only' field.*deprecated")
+  expect_true(cfg$remote$main$default_branch_only)
+  expect_equal(cfg$remote$main$default_branch, "master")
+})
+
+
+test_that("disallow both master_only and default_branch_only", {
+  path <- tempfile()
+  dir.create(path)
+  dat <- list(remote = list(
+                main = list(
+                  driver = "orderly::orderly_remote_path",
+                  primary = TRUE,
+                  master_only = TRUE,
+                  default_branch_only = TRUE,
+                  args = list(root = path))))
+  writeLines(yaml::as.yaml(dat), path_orderly_config_yml(path))
+  expect_error(
+    orderly_config_$new(path),
+    "Can't specify both 'master_only' and 'default_branch_only'")
+})