diff --git a/CHANGELOG.md b/CHANGELOG.md index 5891d1644..10624ee3f 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/main/scala/io/viash/engines/requirements/RRequirements.scala b/src/main/scala/io/viash/engines/requirements/RRequirements.scala index f306099d2..bf4bb5aee 100644 --- a/src/main/scala/io/viash/engines/requirements/RRequirements.scala +++ b/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 diff --git a/src/test/scala/io/viash/auxiliary/MainBuildAuxiliaryDockerRequirements.scala b/src/test/scala/io/viash/auxiliary/MainBuildAuxiliaryDockerRequirements.scala index a3f117432..a2567bfcf 100644 --- a/src/test/scala/io/viash/auxiliary/MainBuildAuxiliaryDockerRequirements.scala +++ b/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")) } }