diff --git a/CHANGELOG.md b/CHANGELOG.md index 30b2a3c54..14e1d70ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,6 +110,8 @@ Additionally changes are made to sanitize the built config output and include ad * `viash test` and `viash ns test`: Add a hidden `--deterministic_working directory` argument to use a fixed directory path (PR #683). +* `component names`: Verify that component namespace and name combinations are unique (PR #685). + ## BUG FIXES * `NextflowPlatform`: Fix publishing state for output arguments with `multiple: true` (#638, PR #639). diff --git a/src/main/scala/io/viash/Main.scala b/src/main/scala/io/viash/Main.scala index 69e28a413..cfff18bac 100644 --- a/src/main/scala/io/viash/Main.scala +++ b/src/main/scala/io/viash/Main.scala @@ -412,8 +412,7 @@ object Main extends Logging { Some(ViashNamespace.targetOutputPath( targetDir = td, runnerId = ex.id, - namespace = appliedConfig.config.namespace, - name = appliedConfig.config.name + config = appliedConfig.config )) case _ => None } diff --git a/src/main/scala/io/viash/ViashNamespace.scala b/src/main/scala/io/viash/ViashNamespace.scala index ce8b30405..ec7e3df3f 100644 --- a/src/main/scala/io/viash/ViashNamespace.scala +++ b/src/main/scala/io/viash/ViashNamespace.scala @@ -52,6 +52,9 @@ object ViashNamespace extends Logging { list.foreach(f) } + def targetOutputPath(targetDir: String, runnerId: String, config: Config): String = + targetOutputPath(targetDir, runnerId, config.namespace, config.name) + def targetOutputPath( targetDir: String, runnerId: String, @@ -88,7 +91,7 @@ object ViashNamespace extends Logging { if (flatten) { target } else { - targetOutputPath(target, runnerId, ns, funName) + targetOutputPath(target, runnerId, ac.config) } val nsStr = ns.map(" (" + _ + ")").getOrElse("") infoOut(s"Exporting $funName$nsStr =$runnerId=> $out") diff --git a/src/main/scala/io/viash/config/Config.scala b/src/main/scala/io/viash/config/Config.scala index 2a94a42f8..40a190541 100644 --- a/src/main/scala/io/viash/config/Config.scala +++ b/src/main/scala/io/viash/config/Config.scala @@ -27,6 +27,7 @@ import io.viash.helpers.{Git, GitInfo, IO, Logging} import io.viash.helpers.circe._ import io.viash.helpers.{status => BuildStatus}; import io.viash.helpers.Yaml +import io.viash.ViashNamespace.targetOutputPath import java.net.URI @@ -747,7 +748,7 @@ object Config extends Logging { attrs.isRegularFile }) - scriptFiles.map { file => + val allConfigs = scriptFiles.map { file => try { val rmos = new ReplayableMultiOutputStream() @@ -799,6 +800,19 @@ object Config extends Logging { AppliedConfig(Config("failed"), None, Nil, Some(BuildStatus.ParseError)) } } + + // Verify that all configs except the disabled ones are unique + // Even configs disabled by the query should be unique as they might be picked up as a dependency + // Only allowed exception are configs where the status is set to disabled + val allEnabledConfigs = allConfigs.collect{ case ac if ac.config.isEnabled => ac.config } + val uniqueConfigs = allEnabledConfigs.groupBy(c => targetOutputPath("", "", c)) + val duplicateConfigs = uniqueConfigs.filter(_._2.size > 1) + if (duplicateConfigs.nonEmpty) { + val duplicateNames = duplicateConfigs.keys.map(_.dropWhile(_ == '/')).toSeq.sorted.mkString(", ") + throw new RuntimeException(s"Duplicate component name${ if (duplicateConfigs.size == 1) "" else "s" } found: $duplicateNames") + } + + allConfigs } val reservedParameters = List("-h", "--help", "--version", "---v", "---verbose", "---verbosity") diff --git a/src/main/scala/io/viash/helpers/DependencyResolver.scala b/src/main/scala/io/viash/helpers/DependencyResolver.scala index b3cf01c01..dec00b797 100644 --- a/src/main/scala/io/viash/helpers/DependencyResolver.scala +++ b/src/main/scala/io/viash/helpers/DependencyResolver.scala @@ -161,7 +161,7 @@ object DependencyResolver extends Logging { case "nextflow" => "main.nf" case _ => c.name } - Paths.get(ViashNamespace.targetOutputPath("", rid, c.namespace, c.name), executableName).toString() + Paths.get(ViashNamespace.targetOutputPath("", rid, c), executableName).toString() } val info = c.build_info.get.copy( executable = executable diff --git a/src/main/scala/io/viash/runners/nextflow/NextflowHelper.scala b/src/main/scala/io/viash/runners/nextflow/NextflowHelper.scala index 00bdf8195..c2f3c096c 100644 --- a/src/main/scala/io/viash/runners/nextflow/NextflowHelper.scala +++ b/src/main/scala/io/viash/runners/nextflow/NextflowHelper.scala @@ -222,7 +222,7 @@ object NextflowHelper { def renderDependencies(config: Config): String = { // TODO ideally we'd already have 'thisPath' precalculated but until that day, calculate it here // The name of the runner doesn't really matter here as it is just used to generate the relative location, and we don't have access to it anyway. - val thisPath = Paths.get(ViashNamespace.targetOutputPath("", "invalid_runner_name", config.namespace, config.name)) + val thisPath = Paths.get(ViashNamespace.targetOutputPath("", "invalid_runner_name", config)) val depStrs = config.dependencies.map{ dep => NextflowHelper.renderInclude(dep, thisPath) diff --git a/src/main/scala/io/viash/wrapper/BashWrapper.scala b/src/main/scala/io/viash/wrapper/BashWrapper.scala index 5f5186c40..0faefc48f 100644 --- a/src/main/scala/io/viash/wrapper/BashWrapper.scala +++ b/src/main/scala/io/viash/wrapper/BashWrapper.scala @@ -235,7 +235,7 @@ object BashWrapper { val localDependenciesStrings = localDependencies.map{ d => // relativize the path of the main component to the local dependency // TODO ideally we'd already have 'thisPath' precalculated but until that day, calculate it here - val thisPath = ViashNamespace.targetOutputPath("", "invalid_runner_name", config.namespace, config.name) + val thisPath = ViashNamespace.targetOutputPath("", "invalid_runner_name", config) val relativePath = Paths.get(thisPath).relativize(Paths.get(d.configInfo.getOrElse("executable", ""))) s"${d.VIASH_DEP}=\"$$VIASH_META_RESOURCES_DIR/$relativePath\"" } diff --git a/src/test/scala/io/viash/e2e/ns_build/MainNSBuildNativeSuite.scala b/src/test/scala/io/viash/e2e/ns_build/MainNSBuildNativeSuite.scala index 7501b6356..a3c4a857d 100644 --- a/src/test/scala/io/viash/e2e/ns_build/MainNSBuildNativeSuite.scala +++ b/src/test/scala/io/viash/e2e/ns_build/MainNSBuildNativeSuite.scala @@ -115,6 +115,96 @@ class MainNSBuildNativeSuite extends AnyFunSuite with BeforeAndAfterAll{ } } + test("Check uniqueness of component names, same name, different namespace") { + val compStr = + """functionality: + | name: comp + | namespace: %s + |""".stripMargin + val tempSrcDir = IO.makeTemp("viash_ns_build_check_uniqueness_src") + IO.write(compStr.format("ns1"), tempSrcDir.resolve("config1.vsh.yaml")) + IO.write(compStr.format("ns2"), tempSrcDir.resolve("config2.vsh.yaml")) + + val tempTargetDir = IO.makeTemp("viash_ns_build_check_uniqueness_target") + + val testOutput = TestHelper.testMain( + "ns", "build", + "-s", tempSrcDir.toString(), + "-t", tempTargetDir.toString() + ) + + assert(testOutput.exitCode == Some(0)) + assert(testOutput.stderr.contains("All 2 configs built successfully")) + } + + test("Check uniqueness of component names, different name, same namespace") { + val compStr = + """functionality: + | name: %s + | namespace: ns + |""".stripMargin + val tempSrcDir = IO.makeTemp("viash_ns_build_check_uniqueness_src") + IO.write(compStr.format("comp1"), tempSrcDir.resolve("config1.vsh.yaml")) + IO.write(compStr.format("comp2"), tempSrcDir.resolve("config2.vsh.yaml")) + + val tempTargetDir = IO.makeTemp("viash_ns_build_check_uniqueness_target") + + val testOutput = TestHelper.testMain( + "ns", "build", + "-s", tempSrcDir.toString(), + "-t", tempTargetDir.toString() + ) + + assert(testOutput.exitCode == Some(0)) + assert(testOutput.stderr.contains("All 2 configs built successfully")) + } + + test("Check uniqueness of component names, same name, same namespace") { + val compStr = + """functionality: + | name: %s + | namespace: ns + |""".stripMargin + val tempSrcDir = IO.makeTemp("viash_ns_build_check_uniqueness_src") + IO.write(compStr.format("comp"), tempSrcDir.resolve("config1.vsh.yaml")) + IO.write(compStr.format("comp"), tempSrcDir.resolve("config2.vsh.yaml")) + + val tempTargetDir = IO.makeTemp("viash_ns_build_check_uniqueness_target") + + val testOutput = TestHelper.testMainException[RuntimeException]( + "ns", "build", + "-s", tempSrcDir.toString(), + "-t", tempTargetDir.toString() + ) + + assert(!testOutput.stderr.contains("All 2 configs built successfully")) + assert(testOutput.exceptionText.contains("Duplicate component name found: ns/comp")) + } + + test("Check uniqueness of component names, same name, same namespace - multiple duplicates") { + val compStr = + """functionality: + | name: %s + | namespace: ns + |""".stripMargin + val tempSrcDir = IO.makeTemp("viash_ns_build_check_uniqueness_src") + IO.write(compStr.format("comp1"), tempSrcDir.resolve("config1.vsh.yaml")) + IO.write(compStr.format("comp1"), tempSrcDir.resolve("config2.vsh.yaml")) + IO.write(compStr.format("comp2"), tempSrcDir.resolve("config3.vsh.yaml")) + IO.write(compStr.format("comp2"), tempSrcDir.resolve("config4.vsh.yaml")) + + val tempTargetDir = IO.makeTemp("viash_ns_build_check_uniqueness_target") + + val testOutput = TestHelper.testMainException[RuntimeException]( + "ns", "build", + "-s", tempSrcDir.toString(), + "-t", tempTargetDir.toString() + ) + + assert(!testOutput.stderr.contains("All 2 configs built successfully")) + assert(testOutput.exceptionText.contains("Duplicate component names found: ns/comp1, ns/comp2")) + } + override def afterAll(): Unit = { IO.deleteRecursively(temporaryFolder) }