Merge pull request #771 from viash-io/r-treat-warnings-as-errors

treat warnings as errors

CHANGELOG.md

@@ -6,10 +6,14 @@ TODO add summary
 
 * `viash-hub`: Change the url for viash-hub Git access to packages.viash-hub.com (PR #774).
 
+* `RRequirements`: Allow single quotes to be used again in the `.script` field (PR #771).
+
 ## BUG FIXES
 
 * `config build`: Fix a bug where a missing main script would cause a stack trace instead of a proper error message (PR #776).
   The error message showed the path of the missing resource but it was easy to miss given the stack trace, besides it shouldn't have been a stack trace anyway.
+
+* `RRequirements`: Treat warnings as errors when installing R dependencies in Docker engines (PR #771).
 
 # Viash 0.9.0 (2024-09-03): Restructure platforms into runners and engines
 

src/main/scala/io/viash/engines/requirements/RRequirements.scala

@@ -87,15 +87,24 @@ case class RRequirements(
   @example("bioc_force_install: false", "yaml")
   @default("False")
   bioc_force_install: Boolean = false,
+
+  @description("Specifies whether to treat warnings as errors. Default: true.")
+  @example("warnings_as_errors: true", "yaml")
+  @default("True")
+  warnings_as_errors: Boolean = true,
 
   `type`: String = "r"
 ) extends Requirements {
-  assert(script.forall(!_.contains("'")), "R requirement '.script' field contains a single quote ('). This is not allowed.")
-
   def installCommands: List[String] = {
+    val prefix = if (warnings_as_errors) "options(warn = 2); " else ""
+
+    def runRCode(code: String): String = {
+      s"""Rscript -e '${prefix}${code.replaceAll("'", "'\"'\"'")}'"""
+    }
+
     val installRemotes =
       if ((packages ::: cran ::: git ::: github ::: gitlab ::: bitbucket ::: svn ::: url).nonEmpty) {
-        List("""Rscript -e 'if (!requireNamespace("remotes", quietly = TRUE)) install.packages("remotes")'""")
+        List(runRCode("""if (!requireNamespace("remotes", quietly = TRUE)) install.packages("remotes")"""))
       } else {
         Nil
       }
@@ -112,17 +121,17 @@ case class RRequirements(
 
     val installBiocManager =
       if (bioc.nonEmpty) {
-        List("""Rscript -e 'if (!requireNamespace("BiocManager", quietly = TRUE)) install.packages("BiocManager")'""")
+        List(runRCode("""if (!requireNamespace("BiocManager", quietly = TRUE)) install.packages("BiocManager")"""))
       } else {
         Nil
       }
     val installBioc =
       if (bioc.nonEmpty) {
         if (bioc_force_install) {
-          List(s"""Rscript -e 'BiocManager::install(c("${bioc.mkString("\", \"")}"))'""")
+          List(runRCode(s"""BiocManager::install(c("${bioc.mkString("\", \"")}"))"""))
         } else {
           bioc.map { biocPackage =>
-            s"""Rscript -e 'if (!requireNamespace("$biocPackage", quietly = TRUE)) BiocManager::install("$biocPackage")'"""
+            runRCode(s"""if (!requireNamespace("$biocPackage", quietly = TRUE)) BiocManager::install("$biocPackage")""")
           }
         }
       } else {
@@ -132,13 +141,13 @@ case class RRequirements(
     val installers = remotePairs.flatMap {
       case (_, Nil) => None
       case (str, list) =>
-        Some(s"""Rscript -e 'remotes::install_$str(c("${list.mkString("\", \"")}"), repos = "https://cran.rstudio.com")'""")
+        Some(runRCode(s"""remotes::install_$str(c("${list.mkString("\", \"")}"), repos = "https://cran.rstudio.com")"""))
     }
 
     val installScript =
       if (script.nonEmpty) {
         script.map { line =>
-          s"""Rscript -e '$line'"""
+          runRCode(line)
         }
       } else {
         Nil

src/test/scala/io/viash/auxiliary/MainBuildAuxiliaryDockerRequirements.scala

@@ -446,17 +446,38 @@ class MainBuildAuxiliaryDockerRequirementsR extends AbstractMainBuildAuxiliaryDo
     assert(output.output.contains("/usr/local/lib/R/site-library/glue/R/glue doesn't exist."))
   }
 
-  test("setup; check for a descriptive message when .script contains a single quote", DockerTest) { f =>
+  test("setup; check .script contains a single quote", DockerTest) { f =>
     val newConfigFilePath = deriveEngineConfig(Some("""[{ "type": "r", "script": "print('hello world')" }]"""), None, "r_script_single_quote")
 
-    val testOutput = TestHelper.testMainException[ConfigParserException](
+    val testOutput = TestHelper.testMain(
+      "build",
+      "-o", tempFolStr,
+      "--setup", "build",
+      newConfigFilePath
+    )
+
+    println(s"testOutput: ${testOutput}")
+
+    assert(TestHelper.checkDockerImageExists(dockerTag))
+    assert(executableRequirementsFile.exists)
+    assert(executableRequirementsFile.canExecute)
+
+    assert(testOutput.exitCode == Some(0))
+  }
+
+  test("setup; check installing a missing package returns an error", DockerTest) { f =>
+    val newConfigFilePath = deriveEngineConfig(Some("""[{ "type": "r", "packages": ["non-existing-package"] }]"""), None, "r_non_existing_package")
+
+    val testOutput = TestHelper.testMain(
       "build",
       "-o", tempFolStr,
       "--setup", "build",
       newConfigFilePath
     )
 
-    assert(testOutput.exceptionText == Some("assertion failed: R requirement '.script' field contains a single quote ('). This is not allowed."))
+    assert(testOutput.exitCode == Some(1))
+    assert(testOutput.stdout.contains("Error: Failed to install 'non-existing-package' from CRAN"))
+    assert(testOutput.stdout.contains("ERROR: failed to solve"))
   }
 }