fix installation using clang-17

.github/workflows/R-CMD-check.yaml

@@ -26,7 +26,8 @@ jobs:
           # jobs (mainly test-coverage) to run on every commit in PRs so as to not slow down dev.
           # GHA does run these jobs concurrently but even so reducing the load seems like a good idea.
           - {os: windows-latest, r: 'devel'}
-          # - {os: macOS-latest, r: 'release'}    # test-coverage.yaml uses macOS
+          - {os: macos-14, r: 'release'}    
+          - {os: macos-15, r: 'release'}    
           # TODO(remotes>2.5.0): Use 24.04[noble?]
           - {os: ubuntu-22.04, r: 'release', rspm: "https://packagemanager.rstudio.com/cran/__linux__/jammy/latest"}
           # - {os: ubuntu-22.04,   r: 'devel', rspm: "https://packagemanager.rstudio.com/cran/__linux__/jammy/latest", http-user-agent: "R/4.1.0 (ubuntu-22.04) R (4.1.0 x86_64-pc-linux-gnu x86_64 linux-gnu) on GitHub Actions" }
@@ -68,6 +69,21 @@ jobs:
             eval sudo $cmd
           done < <(Rscript -e 'writeLines(remotes::system_requirements("ubuntu", "22.04"))')
 
+      - name: Install R Package Build Dependencies on MacOS, from https://github.com/stan-dev/cmdstanr/pull/1072/files
+        if: runner.os == 'macOS'
+        uses: r-hub/actions/setup-r-sysreqs@v1
+        with:
+          type: 'minimal'
+
+      - name: Intall and configure OpenMP runtime
+        if: runner.os == 'macOS'
+        run: |
+          if clang --version | grep 'clang version 17'; then
+            curl -O https://mac.r-project.org/openmp/openmp-19.1.0-darwin20-Release.tar.gz
+            sudo tar fvvxz openmp-19.1.0-darwin20-Release.tar.gz -C /
+            rm -f openmp-19.1.0-darwin20-Release.tar.gz
+          fi # otherwise R-bundled runtime is fine
+
       - name: Install dependencies
         run: |
           remotes::install_deps(dependencies = TRUE)

NEWS.md

@@ -248,6 +248,8 @@
 
 19. New rolling functions, `frollmin` and `frollprod`, have been implemented, towards [#2778](https://github.com/Rdatatable/data.table/issues/2778). Thanks to @jangorecki for implementation.
 
+20. Enhanced tests for OpenMP support, detecting incompatibilities such as R-bundled runtime _vs._ newer Xcode and testing for a manually installed runtime from <https://mac.r-project.org/openmp>, [#6622](https://github.com/Rdatatable/data.table/issues/6622). Thanks to @dvg-p4 for initial report and testing, @twitched for the pointers, @tdhock and @aitap for the fix.
+
 ### BUG FIXES
 
 1. `fread()` no longer warns on certain systems on R 4.5.0+ where the file owner can't be resolved, [#6918](https://github.com/Rdatatable/data.table/issues/6918). Thanks @ProfFancyPants for the report and PR.

configure

@@ -68,89 +68,79 @@ fi
 # Aside: ${SHLIB_OPENMP_CFLAGS} does not appear to be defined at this point according to Matt's testing on
 # Linux, and R CMD config SHLIB_OPENMP_CFLAGS also returns 'no information for variable'. That's not
 # inconsistent with R-exts$1.2.1.1, though, which states it's 'available for use in Makevars' (so not
-# necessarily here in configure). Hence use -fopenmp directly for this detection step.
+# necessarily here in configure).
 # printf not echo to pass checkbashisms w.r.t. to the \n
+# Specifically use schedule(dynamic) to distinguish incompatibilities in OpenMP runtime on macOS, #7318
 
 cat <<EOF > test-omp.c
-#include <omp.h>
-int main() {
-  return omp_get_num_threads();
+void test_openmp(int * result) {
+ int sum = 0;
+#ifdef _OPENMP
+ #pragma omp parallel for reduction(+:sum) num_threads(2) schedule(dynamic)
+ for (int i = 1; i <= 2; ++i) sum += i;
+#endif
+ *result = sum;
 }
 EOF
 
-detect_openmp () {
-
-  if [ "$(uname)" = "Linux" ]; then
-
-    printf "%s" "* checking if R installation supports OpenMP without any extra hints... "
-    if "${R_HOME}/bin/R" CMD SHLIB test-omp.c >> config.log 2>&1; then
-      echo "yes"
-      export R_OPENMP_ENABLED=1
-      return
-    else
-      echo "no"
-    fi
-
+test_openmp_variant () {
+  cflags="${PKG_CFLAGS:+${PKG_CFLAGS} }$1"
+  libs="${PKG_LIBS:+${PKG_LIBS} }$2"
+  printf "%s" "* checking if OpenMP works with CFLAGS='$cflags' LIBS='$libs'... "
+  if ! PKG_CFLAGS="$cflags" PKG_LIBS="$libs" "${R_HOME}"/bin/R CMD SHLIB --preclean --clean test-omp.c >>config.log 2>&1; then
+    echo "no"
+    return 1
+  fi
 
-    printf "%s" "* checking if R installation supports openmp with \"-fopenmp\" flag... "
-    if ${CC} ${CFLAGS} -fopenmp test-omp.c >> config.log 2>&1; then
-      echo "yes"
-      export PKG_CFLAGS="${PKG_CFLAGS} -fopenmp"
-      export R_OPENMP_ENABLED=1
-      return
-    else
-      echo "no"
-    fi
-  fi # uname=Linux
+  if ! "${R_HOME}"/bin/Rscript -e '
+dll <- paste0("test-omp", .Platform$dynlib.ext)
+dyn.load(dll)
+ans <- .C("test_openmp", ans = integer(1))$ans
+stopifnot(identical(ans, 3L))
+' >> config.log 2>&1; then
+    echo "no"
+    return 1
+  fi
 
-  if [ "$(uname)" = "Darwin" ]; then
+  export PKG_CFLAGS="${PKG_CFLAGS} ${1}"
+  export PKG_LIBS="${PKG_LIBS} ${2}"
+  export R_OPENMP_ENABLED=1
+  echo "yes"
+  return 0
+}
 
+detect_openmp () {
+  # OpenMP flags provided in environment variables or specified when configuring R? Does -fopenmp work?
+  test_openmp_variant "" "" \
+  || test_openmp_variant '$(SHLIB_OPENMP_CFLAGS)' '$(SHLIB_OPENMP_CFLAGS)' \
+  || test_openmp_variant -fopenmp -fopenmp \
+  && return
+
+  case "$(uname)" in
+  Darwin)
     # https://mac.r-project.org/openmp
-    printf "%s" "* checking if R installation supports OpenMP with \"-Xclang -fopenmp\" ... "
-    if CPPFLAGS="${CPPFLAGS} -Xclang -fopenmp" PKG_LIBS="-lomp" "${R_HOME}/bin/R" CMD SHLIB test-omp.c >> config.log 2>&1; then
-      echo "yes"
-      export PKG_CFLAGS="${PKG_CFLAGS} -Xclang -fopenmp"
-      export PKG_LIBS="${PKG_LIBS} -lomp"
-      export R_OPENMP_ENABLED=1
-      return
-    else
-      echo "no"
-    fi
-
-    # https://github.com/Rdatatable/data.table/issues/6409
-    printf "%s" "* checking if R installation supports OpenMP with \"-fopenmp\" ... "
-    if CPPFLAGS="${CPPFLAGS} -fopenmp" "${R_HOME}/bin/R" CMD SHLIB test-omp.c >> config.log 2>&1; then
-      echo "yes"
-      export PKG_CFLAGS="${PKG_CFLAGS} -fopenmp"
-      export PKG_LIBS="${PKG_LIBS} -fopenmp"
-      export R_OPENMP_ENABLED=1
-      return
-    else
-      echo "no"
-    fi
+    test_openmp_variant "-Xclang -fopenmp" "-lomp" && return
 
     if [ "$(uname -m)" = "arm64" ]; then
       HOMEBREW_PREFIX=/opt/homebrew
     else
       HOMEBREW_PREFIX=/usr/local
     fi
-
-    if [ -e "${HOMEBREW_PREFIX}/opt/libomp" ]; then
-      printf "%s" "* checking if libomp installation at ${HOMEBREW_PREFIX}/opt/libomp can be used... "
-      LIBOMP_INCLUDE="-I${HOMEBREW_PREFIX}/opt/libomp/include -Xclang -fopenmp"
-      LIBOMP_LINK="-L${HOMEBREW_PREFIX}/opt/libomp/lib -lomp"
-      if ${CC} ${CFLAGS} ${LIBOMP_INCLUDE} ${LIBOMP_LINK} test-omp.c >> config.log 2>&1; then
-        echo "yes"
-        export PKG_CFLAGS="${PKG_CFLAGS} ${LIBOMP_INCLUDE}"
-        export PKG_LIBS="${PKG_LIBS} ${LIBOMP_LINK}"
-        export R_OPENMP_ENABLED=1
-        return
-      else
-        echo "no"
-      fi
+    test -e "${HOMEBREW_PREFIX}/opt/libomp" ] \
+    && test_openmp_variant \
+      "-I${HOMEBREW_PREFIX}/opt/libomp/include -Xclang -fopenmp" \
+      "-L${HOMEBREW_PREFIX}/opt/libomp/lib -lomp" \
+    && return
+
+    if "${CC}" --version | grep -q 'Apple clang 17'; then
+      test -e /usr/local/lib/libomp.dylib \
+      && test_openmp_variant "-Xclang -fopenmp" "/usr/local/lib/libomp.dylib" \
+      && return
+      echo "*** All OpenMP runtime tests failed and you're running Apple clang 17."
+      echo "*** Do you need a new OpenMP runtime from <https://mac.r-project.org/openmp/>?"
     fi
-
-  fi # uname=Darwin
+    ;;
+  esac
 
   # No support for OpenMP available
   export R_OPENMP_ENABLED=0
@@ -168,14 +158,10 @@ if [ "${R_OPENMP_ENABLED}" = "0" ]; then
   echo "***   https://github.com/Rdatatable/data.table/wiki/Installation"
   echo "*** Continuing installation without OpenMP support..."
   echo "***"
-  sed -e "s|@openmp_cflags@||" src/Makevars.in > src/Makevars
-else
-  sed -e "s|@openmp_cflags@|\$(SHLIB_OPENMP_CFLAGS)|" src/Makevars.in > src/Makevars
 fi
 
 # retain user supplied PKG_ env variables, #4664. See comments in Makevars.in too.
-sed -e "s|@PKG_CFLAGS@|$PKG_CFLAGS|" src/Makevars > src/Makevars.tmp && mv src/Makevars.tmp src/Makevars
-sed -e "s|@PKG_LIBS@|$PKG_LIBS|" src/Makevars > src/Makevars.tmp && mv src/Makevars.tmp src/Makevars
+sed -e "s|@PKG_CFLAGS@|$PKG_CFLAGS|" -e "s|@PKG_LIBS@|$PKG_LIBS|" src/Makevars.in > src/Makevars
 
 # optional dependency on zlib
 if [ "$NOZLIB" = "1" ]; then

src/Makevars.in

@@ -1,5 +1,5 @@
-PKG_CFLAGS = @PKG_CFLAGS@ @openmp_cflags@ @zlib_cflags@
-PKG_LIBS = @PKG_LIBS@ @openmp_cflags@ @zlib_libs@
+PKG_CFLAGS = @PKG_CFLAGS@ @zlib_cflags@
+PKG_LIBS = @PKG_LIBS@ @zlib_libs@
 # See WRE $1.2.1.1. But retain user supplied PKG_* too, #4664.
 # WRE states ($1.6) that += isn't portable and that we aren't allowed to use it.
 # Otherwise we could use the much simpler PKG_LIBS += @openmp_cflags@ -lz.