Skip to content

Commit

Permalink
Merge branch 'develop_0_8' into develop
Browse files Browse the repository at this point in the history
  • Loading branch information
Grifs committed Apr 24, 2024
2 parents dea2f5b + fea35e9 commit 3075bc6
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 7 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
3 changes: 1 addition & 2 deletions src/main/scala/io/viash/Main.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
5 changes: 4 additions & 1 deletion src/main/scala/io/viash/ViashNamespace.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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")
Expand Down
16 changes: 15 additions & 1 deletion src/main/scala/io/viash/config/Config.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -747,7 +748,7 @@ object Config extends Logging {
attrs.isRegularFile
})

scriptFiles.map { file =>
val allConfigs = scriptFiles.map { file =>
try {
val rmos = new ReplayableMultiOutputStream()

Expand Down Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/io/viash/helpers/DependencyResolver.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/io/viash/wrapper/BashWrapper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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\""
}
Expand Down
90 changes: 90 additions & 0 deletions src/test/scala/io/viash/e2e/ns_build/MainNSBuildNativeSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit 3075bc6

Please sign in to comment.