Merge branch 'master' into macroRemoval

Author: badasahog

.ci/README.md

@@ -1,6 +1,6 @@
 # data.table continuous integration and deployment
 
-On each Pull Request opened in GitHub we run GitHub Actions test jobs to provide prompt feedback about the status of PR. Our more thorough main CI pipeline runs nightly on GitLab CI. GitLab repository automatically mirrors our GitHub repository and runs pipeline on `master` branch every night. It tests more environments and different configurations. It publishes a variety of artifacts such as our [homepage](https://rdatatable.gitlab.io/data.table/) and [CRAN-like website for dev version](https://rdatatable.gitlab.io/data.table/web/packages/data.table/index.html), including windows binaries for the dev version.
+On each Pull Request opened in GitHub we run GitHub Actions test jobs to provide prompt feedback about the status of PR. Our more thorough main CI pipeline runs nightly on GitLab CI. In addition to branches pushed directly, the GitLab repository automatically mirrors our GitHub repository and runs pipeline on the `master` branch every night. It tests more environments and different configurations. It publishes a variety of artifacts such as our [homepage](https://rdatatable.gitlab.io/data.table/) and [CRAN-like website for dev version](https://rdatatable.gitlab.io/data.table/web/packages/data.table/index.html), including windows binaries for the dev version.
 
 ## Environments
 
@@ -13,7 +13,8 @@ Test jobs:
 - `test-lin-dev-gcc-strict-cran` - `--as-cran` on Linux, `r-devel` built with `-enable-strict-barrier --disable-long-double`, test for compilation warnings, test for new NOTEs/WARNINGs from `R CMD check`.
 - `test-lin-dev-clang-cran` - same as `gcc-strict` job but R built with `clang` and  no `--enable-strict-barrier --disable-long-double` flags.
 - `test-lin-ancient-cran` - Stated R dependency version (currently 3.4.0) on Linux.
-- `test-lin-dev-san` - `r-devel` on Linux built with `clang -fsanitize=address,undefined` (including LeakSanitizer), test for sanitizer output in tests and examples.
+- `test-lin-dev-clang-san` - `r-devel` on Linux built with `clang -fsanitize=address,undefined` (including LeakSanitizer), test for sanitizer output in tests and examples.
+- `test-lin-dev-gcc-san` - `r-devel` on Linux built with `gcc -fsanitize=address,undefined` (including LeakSanitizer), test for sanitizer output in tests and examples.
 - `test-win-rel` - `r-release` on Windows.
 - `test-win-dev` - `r-devel` on Windows.
 - `test-win-old` - `r-oldrel` on Windows.
@@ -49,6 +50,23 @@ Base R implemented helper script, [originally proposed to base R](https://svn.r-
 
 Base R implemented helper script to orchestrate generation of most artifacts and to arrange them nicely. It is being used only in [_integration_ stage in GitLab CI pipeline](./../.gitlab-ci.yml).
 
+### [`lint.R`](./lint.R)
+
+Base R runner for the manual (non-`lintr`) lint checks to be run from GitHub Actions during the code quality check. The command line arguments are as follows:
+1. Path to the directory containing files defining the linters. A linter is a function that accepts one argument (typically the path to the file) and signals an error if it fails the lint check.
+2. Path to the directory containing files to check.
+3. A regular expression matching the files to check.
+
+One of the files in the linter directory may define the `.preprocess` function, which must accept one file path and return a value that other linter functions will understand. The function may also return `NULL` to indicate that the file must be skipped.
+
+Example command lines:
+
+```sh
+Rscript .ci/lint.R .ci/linters/c src '[.][ch]$'
+Rscript .ci/lint.R .ci/linters/po po '[.]po$'
+Rscript .ci/lint.R .ci/linters/md . '[.]R?md$'
+```
+
 ## GitLab Open Source Program
 
 We are currently part of the [GitLab for Open Source Program](https://about.gitlab.com/solutions/open-source/). This gives us 50,000 compute minutes per month for our GitLab CI. Our license needs to be renewed yearly (around July) and is currently managed by @ben-schwen.

.ci/ci.R

@@ -155,10 +155,11 @@ function(pkgs,
   db <- utils::available.packages(repos.url, type = type)
   allpkgs <- c(pkgs, unlist(tools::package_dependencies(unique(pkgs), db, which, recursive = TRUE), use.names = FALSE))
   except <- c("R", unlist(tools:::.get_standard_package_names()[except.priority], use.names = FALSE))
-  ## do not re-download existing packages, ignore version
+  ## do not re-download existing packages with the right version
   if (length(except.repodir) && file.exists(file.path(contrib.url(except.repodir, type = type, ver = binary.ver), "PACKAGES"))) {
     except.curl <- contrib.url(file.path("file:", normalizePath(except.repodir)), type = type, ver = binary.ver)
-    except <- c(except, rownames(utils::available.packages(except.curl, type = type, fields = "Package")))
+    except.db <- utils::available.packages(except.curl, type = type, fields = "Package")
+    except <- c(except, merge(db, except.db, by = c("Package", "Version", "MD5sum"))[,"Package"])
   }
   newpkgs <- setdiff(allpkgs, except)
   if (!all(availpkgs<-newpkgs %in% rownames(db))) {
@@ -174,6 +175,13 @@ function(pkgs,
                     "source" = "tar.gz",
                     "mac.binary" = "tgz",
                     "win.binary" = "zip")
+  ## clean up stale package files for which new versions will be downloaded
+  if (file.exists(file.path(destdir, "PACKAGES"))) {
+    repo.db <- utils::available.packages(file.path("file:", normalizePath(destdir)), type = type)
+    oldver <- repo.db[repo.db[, "Package"] %in% newpkgs, c("Package", "Version"), drop=FALSE]
+    oldfiles <- file.path(destdir, sprintf("%s_%s.%s", oldver[,"Package"], oldver[,"Version"], pkgsext))
+    unlink(oldfiles[file.exists(oldfiles)])
+  }
   pkgsver <- db[db[, "Package"] %in% newpkgs, c("Package", "Version"), drop=FALSE]
   dlfiles <- file.path(destdir, sprintf("%s_%s.%s", pkgsver[,"Package"], pkgsver[,"Version"], pkgsext))
   unlink(dlfiles[file.exists(dlfiles)])

.ci/lint.R

@@ -0,0 +1,36 @@
+#!/usr/bin/Rscript
+# Runner for the manual lint checks in .ci/linters
+args = commandArgs(TRUE)
+if (identical(args, '--help')) {
+  writeLines(c(
+    'Usage: Rscript .ci/lint.R .ci/linters/<KIND> <WHERE> <WHAT> [PREPROCESS]',
+    'KIND must name the directory containing the *.R files defining the linter functions.',
+    'WHERE must name the directory containing the files to lint, e.g. "po", or "src".',
+    "WHAT must contain the regular expression matching the files to lint, e.g., '[.]po$', or '[.][ch]$'.",
+  ))
+  q('no')
+}
+stopifnot(`Invalid arguments, see .ci/lint.R --help` = length(args) == 3)
+
+linter_env = list2env(list(.preprocess = identity))
+for (f in list.files(args[[1]], full.names=TRUE)) sys.source(f, linter_env)
+if (!length(ls(linter_env))) stop(
+  "No linters found after sourcing files in ", dQuote(args[[1]])
+)
+
+sources = list.files(args[[2]], pattern = args[[3]], full.names = TRUE, recursive = TRUE)
+if (!length(sources)) stop(
+  "No files to lint found in directory ", dQuote(args[[2]]), " for mask ", dQuote(args[[3]])
+)
+sources = Filter(Negate(is.null), lapply(setNames(nm = sources), linter_env$.preprocess))
+
+okay = TRUE
+for (src in names(sources))
+for (linter in ls(linter_env)) tryCatch(
+  linter_env[[linter]](sources[[src]]),
+  error = function(e) {
+    message('Source file ', dQuote(src), ' failed lint check ', dQuote(linter), ': ', conditionMessage(e))
+    okay <<- FALSE
+  }
+)
+stopifnot(`Please fix the issues above.` = okay)

.ci/linters/c/00preprocess.R

@@ -0,0 +1,7 @@
+.preprocess = function (f) list(
+  c_obj = f, lines = readLines(f),
+  preprocessed = system2(
+    "gcc", shQuote(c("-fpreprocessed", "-E", f)),
+    stdout = TRUE, stderr = FALSE
+  )
+)

.ci/linters/md/heading_id_linter.R

@@ -1,8 +1,8 @@
-any_mismatch = FALSE
-
 # ensure that ids are limited to alphanumerics and dashes
 # (in particular, dots and underscores break the links)
 check_header_ids = function(md) {
+  if (!grepl('[.]Rmd$', md)) return(invisible())
+  md = readLines(md)
   # A bit surprisingly, some headings don't start with a letter.
   # We're interested in those that set an id to link to, i.e., end with {#id}.
   heading_captures = regmatches(md, regexec("^#+ \\S.*[{]#([^}]*)[}]$", md))
@@ -14,13 +14,5 @@ check_header_ids = function(md) {
     "On line %d, bad heading id '%s':\n%s\n",
     line, heading_captures[[line]][2], heading_captures[[line]][1]
   ))
-  !all(good_ids)
+  stopifnot('Please fix the vignette issues above' = all(good_ids))
 }
-
-any_error = FALSE
-for (vignette in list.files('vignettes', pattern = "[.]Rmd$", recursive = TRUE, full.name = TRUE)) {
-  cat(sprintf("Checking vignette file %s...\n", vignette))
-  rmd_lines = readLines(vignette)
-  any_error = check_header_ids(rmd_lines) || any_error
-}
-if (any_error) stop("Please fix the vignette issues above.")

.ci/linters/md/news_linter.R

@@ -1,13 +1,13 @@
-any_mismatch = FALSE
-
 # ensure that numbered list in each section is in sequence
 check_section_numbering = function(news) {
+  if (!grepl("NEWS", news)) return(invisible())
+  news = readLines(news)
   # plain '#' catches some examples; 'd' for 'data.table'
   sections = grep("^#+ [A-Zd]", news)
   entries = grep("^[0-9]+[.]", news)
   entry_value = as.integer(gsub("^([0-9]+)[.].*", "\\1", news[entries]))
   section_id = findInterval(entries, sections)
-  
+
   any_mismatch = FALSE
   for (id in unique(section_id)) {
     section_entries = entry_value[section_id == id]
@@ -22,11 +22,12 @@ check_section_numbering = function(news) {
       paste0("  [", section_entries[!matched], " --> ", intended_value[!matched], "]", collapse="\n")
     ))
   }
-  return(any_mismatch)
+  stopifnot("Please fix the NEWS issues above" = !any_mismatch)
 }
 
 # ensure that GitHub link text & URL actually agree
 check_gh_links = function(news) {
+  news = readLines(news)
   gh_links_info = gregexpr(
     "\\[#(?<md_number>[0-9]+)\\]\\(https://github.com/Rdatatable/data.table/(?<link_type>[^/]+)/(?<link_number>[0-9]+)\\)",
     news,
@@ -48,14 +49,5 @@ check_gh_links = function(news) {
     "In line %d, link pointing to %s %s is written #%s\n",
     line_number, link_type, link_number, md_number
   )))
-  return(TRUE)
-}
-
-any_error = FALSE
-for (news in list.files(pattern = "NEWS")) {
-  cat(sprintf("Checking NEWS file %s...\n", news))
-  news_lines = readLines(news)
-  any_error = check_section_numbering(news_lines) || any_error
-  any_error = check_gh_links(news_lines) || any_error
+  stop("Please fix the NEWS issues above.")
 }
-if (any_error) stop("Please fix the NEWS issues above.")

.ci/linters/po/00preprocess.R

@@ -0,0 +1,4 @@
+.preprocess = function (f) {
+  diff_v_master = system2('git', c('diff', 'master', f), stdout=TRUE)
+  if (length(diff_v_master)) f
+}

.ci/linters/rd/options_doc_check.R

@@ -0,0 +1,51 @@
+# Ensure that data.table options in code match documentation
+options_documentation_linter = function(rd_file) {
+  if (!grepl("\\name{data.table-options}", readChar(rd_file, 100L), fixed = TRUE)) return(invisible())
+
+  # Find options in R code
+  walk_r_ast_for_options = function(expr) {
+    if (is.call(expr) && length(expr) >= 2L && identical(expr[[1L]], quote(getOption)) && is.character(e2 <- expr[[2L]]) && startsWith(e2, "datatable.")) {
+      e2
+    } else if (is.recursive(expr)) {
+      unlist(lapply(expr, walk_r_ast_for_options))
+    }
+  }
+
+  # Find options in documentation
+  walk_rd_ast_for_options = function(rd_element) {
+    if (!is.list(rd_element)) return(character())
+
+    result = character()
+    if (isTRUE(attr(rd_element, "Rd_tag") == "\\code") && length(rd_element) >= 1L) {
+      content = rd_element[[1L]]
+      if (is.character(content) && startsWith(content, "datatable.")) {
+        result = content
+      }
+    }
+    c(result, unlist(lapply(rd_element, walk_rd_ast_for_options)))
+  }
+
+  code_opts = list.files("R", pattern = "\\.R$", full.names = TRUE) |>
+    lapply(\(f) lapply(parse(f), walk_r_ast_for_options)) |>
+    unlist() |>
+    unique() |>
+    setdiff("datatable.nomatch") # ignore deprecated option(s)
+
+  doc_opts = rd_file |>
+    tools::parse_Rd() |>
+    walk_rd_ast_for_options() |>
+    unique()
+
+  miss_in_doc = setdiff(code_opts, doc_opts)
+  miss_in_code = setdiff(doc_opts, code_opts)
+
+  if (length(miss_in_doc) > 0L || length(miss_in_code) > 0L) {
+    if (length(miss_in_doc) > 0L) {
+      cat(sprintf("Options in code but missing from docs: %s\n", toString(miss_in_doc)))
+    }
+    if (length(miss_in_code) > 0L) {
+      cat(sprintf("Options in docs but not in code: %s\n", toString(miss_in_code)))
+    }
+    stop("Please sync man/data.table-options.Rd with code options")
+  }
+}

.devcontainer/r-ancient-gcc/Dockerfile

@@ -1,4 +1,4 @@
-FROM registry.gitlab.com/jangorecki/dockerfiles/r-3.4.0
+FROM registry.gitlab.com/rdatatable/dockerfiles/r-3.4.0
 
 RUN apt-get -qq update \
   && apt-get install -y --no-install-recommends git

.devcontainer/r-devel-gcc/Dockerfile

@@ -1,4 +1,4 @@
-FROM registry.gitlab.com/jangorecki/dockerfiles/r-devel-gcc
+FROM registry.gitlab.com/rdatatable/dockerfiles/r-devel-gcc
 
 RUN apt-get -qq update \
   && apt-get install -y --no-install-recommends git