Fix local dependencies of dependencies of local dependencies (#870)

* fix local dependencies of dependencies of local dependencies

* add changelog

* move some prints to trace

* remove `logger.` from debug or trace calls

* commit missing changes

* Update CHANGELOG.md

Co-authored-by: Robrecht Cannoodt <robrecht@data-intuitive.com>

* fix passing relative paths causing issues with dependency resolution

* add local dependencies in the mix for the testbench

* add helper methods to reduce boilerplate code

* remove debug code

---------

Co-authored-by: Robrecht Cannoodt <robrecht@data-intuitive.com>

Author: hcannoodt

CHANGELOG.md

@@ -7,6 +7,8 @@
 * `Nextflowrunner`: fix publishing of directories when the output file name template contains a trailing slash (PR #867).
 
 * `Logging`: Reduce log level for fetching repositories from info to debug (PR #871).
+* 
+* `Dependencies`: Fix an edge case during dependency resolving (PR #870). Local dependencies of dependencies of local dependencies were resolved incorrectly and resulted in a wrongly resolved destination path.
 
 # Viash 0.9.6 (2025-10-10): Hotfix for dependency path resolution
 

src/main/scala/io/viash/config/dependencies/Dependency.scala

@@ -97,7 +97,7 @@ case class Dependency(
 
   @internalFunctionality
   internalDependencyTargetScope: ScopeEnum = ScopeEnum.Public
-) {
+) extends Logging {
   if (alias.isDefined) {
     // check functionality name
     require(alias.get.matches("^[A-Za-z][A-Za-z0-9_]*$"), message = f"alias '${alias.get}' must begin with a letter and consist only of alphanumeric characters or underscores.")
@@ -124,12 +124,14 @@ case class Dependency(
     if (isLocalDependency) {
       // Local dependency so it will only exist once the component is built.
       // TODO improve this, for one, the runner id should be dynamic
+      debug("getRelativePath: isLocalDependency is true, using ViashNamespace to determine the path")
       Some(ViashNamespace.targetOutputPath("", "executable", internalDependencyTargetScope, None, name))
     } else {
       // Previous existing dependency. Use the location of the '.build.yaml' to determine the relative location.
       val relativePath = Dependency.getRelativePath(fullPath, Paths.get(workRepository.get.localPath))
       if (relativePath.isEmpty)
         throw new MissingBuildYamlException(fullPath, this)
+      debug(s"getRelativePath: relativePath: $relativePath, workRepository: ${workRepository.get}")
       relativePath.flatMap(rp => workRepository.map(r => Paths.get(r.subOutputPath).resolve(rp).toString()))
     }
   }
@@ -154,22 +156,25 @@ object Dependency extends Logging {
     * @param mainDependency Top level dependency for which optionally dependencies of dependencies are being resolved. Used to relativize paths
     * @return Tuple with source and destination paths, relativized to current repository locations, ready to be copied
     */
-  def getSourceAndDestinationFromWrittenPath(dependencyPath: String, output: Path, repoPath: Path, mainDependency: Dependency, remoteLocalDependencyResolver: Option[(Path, Path)]): (Path, Path) = {
+  def getSourceAndDestinationFromWrittenPath(dependencyPath: String, output: Path, repoPath: Path, mainDependency: Dependency, remoteLocalDependencyResolver: Option[(Path, Path, Path)]): (Path, Path) = {
     import scala.jdk.CollectionConverters._
 
+    trace(s"getSourceAndDestinationFromWrittenPath: dependencyPath: $dependencyPath, output: $output, repoPath: $repoPath, mainDependency: $mainDependency, remoteLocalDependencyResolver: $remoteLocalDependencyResolver")
+
     val defaultSourcePath = repoPath.resolve(dependencyPath)
-    val sourcePath = if (defaultSourcePath.toFile().exists()) {
+    val (sourcePath, relativePath) = if (defaultSourcePath.toFile().exists()) {
       // If the dependencyPath is a valid path, use it as source
-      defaultSourcePath
+      debug(s"defaultSourcePath exists, use that as sourcePath: $defaultSourcePath")
+      (defaultSourcePath, None)
     } else if (remoteLocalDependencyResolver.isDefined) {
       // This is empty if we're resolving the first level of dependencies.
       // For the most part we should not end up here, but there is an edge case where we have to resolve a local dependency for a dependency of a dependency.
       // In this case, we don't have the context of the location where the dependency is stored under the dependency folder, however we know where the dependant is stored,
       // so we can use the remote local dependency resolver to find the source path.
-      logger.debug(s"Couldn't find sourcePath, using remote local dependency resolver for $dependencyPath")
-      logger.debug(s"Remote local dependency resolver: $remoteLocalDependencyResolver")
+      debug(s"Couldn't find sourcePath, using remote local dependency resolver for $dependencyPath")
+      debug(s"Remote local dependency resolver: $remoteLocalDependencyResolver")
       val alternativeSourcePath = remoteLocalDependencyResolver.get._1 // This is the path where the dependant is located
-      val alternativeTargetPath = remoteLocalDependencyResolver.get._2 // This is the relative path of the dependant in the target folder
+      val alternativeTargetPath = remoteLocalDependencyResolver.get._3 // This is the relative path of the dependant in the target folder
 
       // strips alternativeTargetPath from dependencyPath
       // ie. `target` from `target/foo/bar` so that it can be added to alternativeSourcePath as it doesn't contain the `target` folder anymore at this point
@@ -179,10 +184,10 @@ object Dependency extends Logging {
         case (depPart, targetPart) => depPart == targetPart
       }
       val relativePath = zipped.flatMap(_._1).reduce((p1, p2) => p1.resolve(p2))
-      logger.debug(s"relativePath: $relativePath")
+      debug(s"relativePath: $relativePath")
       val res = alternativeSourcePath.resolve(relativePath)
-      logger.debug(s"Using alternative source path: $res")
-      res
+      debug(s"Using alternative source path: $res")
+      (res, Some(relativePath))
     } else {
       // Otherwise, throw an error. We shouldn't end up here.
       throw new MissingBuildYamlException(defaultSourcePath, mainDependency)
@@ -194,12 +199,18 @@ object Dependency extends Logging {
       // Drop the other "target" folder from the found path. This can be multiple folders too
       val relativePath = Dependency.getRelativePath(sourcePath, repoPath)
         .fold(throw new MissingBuildYamlException(sourcePath, mainDependency))(identity)
+      trace(s"destinationPath case 1: output: $output, relativePath: $relativePath")
       output.resolve(relativePath)
+    } else if (remoteLocalDependencyResolver.isDefined && relativePath.isDefined) {
+      trace(s"destinationPath case 2: remoteLocalDependencyResolver._2: ${remoteLocalDependencyResolver.get._2}, relativePath: $relativePath")
+      remoteLocalDependencyResolver.get._2.resolve(relativePath.get)
     } else {
       val subPath = mainDependency.getRelativePath(sourcePath)
         .fold(throw new MissingBuildYamlException(sourcePath, mainDependency))(identity)
+      trace(s"destinationPath case 3: output: $output, subPath: $subPath")
       output.resolve("dependencies").resolve(subPath)
     }
+    debug(s"Determined destination path: $destinationPath")
 
     (sourcePath, destinationPath)
   }

src/main/scala/io/viash/helpers/DependencyResolver.scala

@@ -141,7 +141,7 @@ object DependencyResolver extends Logging {
         IO.copyFolder(dependencyRepoPath, dependencyOutputPath)
         // Copy dependencies of dependencies, all the way down
         // Parse new location of the copied dependency. That way that path can be used to determine the new location of namespace dependencies
-        recurseBuiltDependencies(Paths.get(output), Paths.get(dep.workRepository.get.localPath), dependencyOutputPath.toString(), dep)
+        recurseBuiltDependencies(Paths.get(output).toAbsolutePath(), Paths.get(dep.workRepository.get.localPath), dependencyOutputPath.toString(), dep)
         // Store location of the copied files
         dep.copy(writtenPath = Some(dependencyOutputPath.toString()))
       }
@@ -300,7 +300,7 @@ object DependencyResolver extends Logging {
   }
 
   // Handle dependencies of dependencies. For a given already built component, get their dependencies, copy them to our new target folder and recurse into these.
-  def recurseBuiltDependencies(output: Path, repoPath: Path, builtDependencyPath: String, dependency: Dependency, dependencySourcePath: Option[Path] = None, depth: Int = 0): Unit = {
+  def recurseBuiltDependencies(output: Path, repoPath: Path, builtDependencyPath: String, dependency: Dependency, dependencySourceDestPath: Option[(Path, Path)] = None, depth: Int = 0): Unit = {
     import scala.jdk.CollectionConverters._
 
     // Limit recursion depth to prevent infinite loops in e.g. cross dependencies (TODO)
@@ -309,7 +309,7 @@ object DependencyResolver extends Logging {
 
     // this returns paths relative to `repoPath` of dependencies to be copied to `output`
     val (dependencyPaths, relativeOutput) = getSparseDependencyInfo(builtDependencyPath + "/.config.vsh.yaml")
-    logger.debug(s"Paths to relativize: dependencySourcePath: $dependencySourcePath, relativeOutput: $relativeOutput")
+    debug(s"\nPaths to relativize: dependencySourcePath: $dependencySourceDestPath, relativeOutput: $relativeOutput")
 
     // remove the trailing path parts as far as relativeOutputPath matches the dependencySourcePath
     // dependencySourcePath: a/b/c/d/e
@@ -319,18 +319,21 @@ object DependencyResolver extends Logging {
     //   - left: the the path where the dependency is stored down to common root, matching to 'target'
     //   - right: the original 'target' folder name
     // this is needed to relativize paths correctly when resolving a local dependency of this dependency
-    val dependencySourceParts = dependencySourcePath.map { dsp =>
+    val dependencySourceParts = dependencySourceDestPath.map { (dsp, ddp) =>
       val dspParts = dsp.iterator().asScala.toList.map(p => Some(p)).reverse
+      val ddpParts = ddp.iterator().asScala.toList.map(p => Some(p)).reverse
       val relativeOutputPath = Paths.get(relativeOutput).iterator().asScala.toList.map(p => Some(p)).reverse
       // Find the first part that is not in the relative output path
-      val commonParts = dspParts.zipAll(relativeOutputPath, None, None).dropWhile{ case (a, b) => a == b }
+      val commonSourceParts = dspParts.zipAll(relativeOutputPath, None, None).dropWhile{ case (a, b) => a == b }
+      val commonDestParts = ddpParts.zipAll(relativeOutputPath, None, None).dropWhile{ case (a, b) => a == b }
 
-      val leftPath = commonParts.flatMap(_._1).reverse.fold(dsp.getRoot())((p1, p2) => p1.resolve(p2))
-      val rightPath = commonParts.flatMap(_._2).reverse.reduceOption((p1, p2) => p1.resolve(p2)).getOrElse(Paths.get("")) // if there is no right part, use empty path
+      val leftSourcePath = commonSourceParts.flatMap(_._1).reverse.fold(dsp.getRoot())((p1, p2) => p1.resolve(p2))
+      val leftDestPath = commonDestParts.flatMap(_._1).reverse.fold(ddp.getRoot())((p1, p2) => p1.resolve(p2))
+      val rightPath = commonSourceParts.flatMap(_._2).reverse.reduceOption((p1, p2) => p1.resolve(p2)).getOrElse(Paths.get("")) // if there is no right part, use empty path
 
-      (leftPath, rightPath)
+      (leftSourcePath, leftDestPath, rightPath)
     }
-    logger.debug(s"dependencySourceParts: $dependencySourceParts")
+    debug(s"dependencySourceParts: $dependencySourceParts")
 
     for (dp <- dependencyPaths) {
       // Get the source & destination path for the dependency, functionality depends whether it was a previous dependency or not.
@@ -346,7 +349,7 @@ object DependencyResolver extends Logging {
       IO.copyFolder(sourcePath, destPath)
 
       // Check for more dependencies
-      recurseBuiltDependencies(output, repoPath, destPath.toString(), dependency, Some(sourcePath), depth + 1)
+      recurseBuiltDependencies(output, repoPath, destPath.toString(), dependency, Some((sourcePath, destPath)), depth + 1)
     }
   }