Graceful later_fd fallback if later_api.h is newer than installed lat…

…er (#204) * Graceful later_fd fallback if later_api.h is newer than installed later * Add comments, clearer stub function name * GHA test for issue #203 * Intentionally break GHA test to see if it errors * Try to fix GHA test * Fix GHA * FIX GHA * Undo intentional breakage * Add NEWS * Apply suggestions from code review Co-authored-by: Charlie Gao <[email protected]> --------- Co-authored-by: Charlie Gao <[email protected]>
r-lib · Nov 27, 2024 · 69721d5 · 69721d5
1 parent d82ec07
commit 69721d5
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 3 deletions.
diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml
@@ -19,3 +19,32 @@ jobs:
     uses: rstudio/shiny-workflows/.github/workflows/routine.yaml@v1
   R-CMD-check:
     uses: rstudio/shiny-workflows/.github/workflows/R-CMD-check.yaml@v1
+  cpp-version-mismatch:
+    name: cpp-version-mismatch
+    runs-on: ubuntu-latest
+    env:
+      GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+    steps:
+      - name: Checkout GitHub repo
+        uses: rstudio/shiny-workflows/.github/internal/checkout@v1
+
+      - name: Install R, system dependencies, and package dependencies
+        uses: rstudio/shiny-workflows/setup-r-package@v1
+        with:
+          needs: check
+          extra-packages: |
+            any::remotes
+      - name: Compile promises against the newest later
+        run: |
+          print(packageVersion("later"))
+          remotes::install_github("rstudio/promises", force = TRUE)
+        shell: Rscript {0}
+      - name: Downgrade later
+        run: |
+          remotes::install_version('later', '1.3.1')
+        shell: Rscript {0}
+      - name: See if dependent package (built against newer later ver) can still load
+        run: |
+          print(packageVersion("later"))
+          library(promises)
+        shell: Rscript {0}
diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,7 @@
 # later 1.4.0.9000 (development)
 
+* Fixed #203: Resolves an issue where packages that have `LinkingTo: later` (including `promises` and `httpuv`) and were built against `later` 1.4.0, would fail to load on systems that actually had older versions of `later` installed, erroring out with the message "function 'execLaterFdNative' not provided by package 'later'". With this fix, such dependent packages should gracefully deal with older versions at load time, and complain with helpful error messages if newer C interfaces (than are available on the installed `later`) are accessed. (#204)
+
 # later 1.4.0
 
 * Adds `later_fd()` which executes a function when a file descriptor is ready for reading or writing, at some indeterminate time in the future (subject to an optional timeout). This facilitates an event-driven approach to asynchronous or streaming downloads. (@shikokuchuo and @jcheng5, #190)

diff --git a/inst/include/later.h b/inst/include/later.h
@@ -48,10 +48,17 @@ namespace later {
 //
 // int (*dll_api_version)() = (int (*)()) R_GetCCallable("later", "apiVersion");
 // if (LATER_H_API_VERSION != (*dll_api_version)()) { ... }
-#define LATER_H_API_VERSION 2
+#define LATER_H_API_VERSION 3
 #define GLOBAL_LOOP 0
 
 
+// Gets the version of the later API that's provided by the _actually installed_
+// version of later.
+static int apiVersionRuntime() {
+  int (*dll_api_version)(void) = (int (*)(void)) R_GetCCallable("later", "apiVersion");
+  return (*dll_api_version)();
+}
+
 inline void later(void (*func)(void*), void* data, double secs, int loop_id) {
   // This function works by retrieving the later::execLaterNative2 function
   // pointer using R_GetCCallable the first time it's called (per compilation
@@ -96,6 +103,11 @@ inline void later(void (*func)(void*), void* data, double secs) {
   later(func, data, secs, GLOBAL_LOOP);
 }
 
+static void later_fd_version_error(void (*func)(int *, void *), void *data, int num_fds, struct pollfd *fds, double secs, int loop_id) {
+  (void) func; (void) data; (void) num_fds; (void) fds; (void) secs; (void) loop_id;
+  Rf_error("later_fd called, but installed version of the 'later' package is too old; please upgrade 'later' to 1.4.1 or above");
+}
+
 inline void later_fd(void (*func)(int *, void *), void *data, int num_fds, struct pollfd *fds, double secs, int loop_id) {
   // See above note for later()
 
@@ -112,7 +124,13 @@ inline void later_fd(void (*func)(int *, void *), void *data, int num_fds, struc
         "If you're using <later.h>, please switch to <later_api.h>.\n"
       );
     }
-    elfdn = (elfdnfun) R_GetCCallable("later", "execLaterFdNative");
+    if (apiVersionRuntime() >= 3) {
+      // Only later API version 3 supports execLaterFdNative
+      elfdn = (elfdnfun) R_GetCCallable("later", "execLaterFdNative");
+    } else {
+      // The installed version is too old and doesn't offer execLaterFdNative.
+      elfdn = later_fd_version_error;
+    }
   }
 
   // We didn't want to execute anything, just initialize

diff --git a/src/later.h b/src/later.h
@@ -8,7 +8,7 @@
 // This should be kept in sync with LATER_H_API_VERSION in
 // inst/include/later.h. Whenever the interface between inst/include/later.h
 // and the code in src/ changes, these values should be incremented.
-#define LATER_DLL_API_VERSION 2
+#define LATER_DLL_API_VERSION 3
 
 #define GLOBAL_LOOP 0