Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/resolved paths in config view #668

Merged
merged 8 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ TODO add summary

* `deprecation & removal warning`: Improve the displayed warning where a deprecated or removed field could display a double '.' when it field was located at the root level (PR #671).

* `resource path`: Don't finalize the `path` field of a resource until it's written as part of building a component (PR #668).


# Viash 0.9.0-RC2 (2024-02-23): Restructure the config and change some default values

The `.functionality` layer has been removed from the config and all fields have been moved to the top layer.
Expand Down
9 changes: 7 additions & 2 deletions src/main/scala/io/viash/config/ConfigMeta.scala
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,23 @@ object ConfigMeta {

// change the config object before writing to yaml:
// * substitute 'text' fields in resources with placeholders
// * set 'path' fields to the resourcePath
// * add more info variables
val toWriteConfig = config.copy(
resources = config.resources.map{ res =>
if (res.text.isDefined) {
val textVal = Some(placeholderMap(res))
res.copyResource(text = textVal, parent = None)
} else {
res.copyResource(parent = None)
res.copyResource(parent = None, path = Some(res.resourcePath))
}
},
test_resources = config.test_resources.map { res =>
res.copyResource(parent = None)
if (res.text.isDefined) {
res.copyResource(parent = None)
} else {
res.copyResource(parent = None, path = Some(res.resourcePath))
}
},
build_info = config.build_info.map(_.copy(
output = buildDir.map(_.toString),
Expand Down
40 changes: 33 additions & 7 deletions src/main/scala/io/viash/config/resources/Resource.scala
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@

// val uri: Option[URI] = path.map(IO.uri)
def uri: Option[URI] = {
path match {
resolvedPath match {
case Some(pat) => {
val patEsc = pat.replaceAll(" ", "%20")
val newPath = parent match {
Expand All @@ -96,6 +96,34 @@
}
}

/**
* Resolve the path of the resource w.r.t. to the package dir.
* Only resolve if the path is relative to the package dir.
*
* @return the resolved path
*/
def resolvedPath: Option[String] = {
Grifs marked this conversation as resolved.
Show resolved Hide resolved
// don't modify if the resource represents a command that should be available in the PATH
if (this.isInstanceOf[Executable]) {
return path
}
// don't modify if the resource doesn't have a PATH or contains a URI
if (path.isEmpty || path.get.contains(":")) {
return path
}

// if the path starts with a /, resolve it w.r.t. to the package dir
val pathStr = path.get
if (pathStr.startsWith("/")) {
if (parent.isEmpty) {
throw new RuntimeException(s"One of the resources is relative to the package root ($path), but no package config file (_viash.yaml) could be found.")

Check warning on line 119 in src/main/scala/io/viash/config/resources/Resource.scala

View check run for this annotation

Codecov / codecov/patch

src/main/scala/io/viash/config/resources/Resource.scala#L119

Added line #L119 was not covered by tests
}
Some(IO.resolvePathWrtURI(pathStr, parent.get))
} else {
path
}
}

private val basenameRegex = ".*/".r
private val getFolderNameRegex = ".*?([^/]+|/)/*$".r

Expand All @@ -109,7 +137,7 @@
if (dest.isDefined) {
dest.get
} else {
getFolderNameRegex.replaceFirstIn(Paths.get(path.get).normalize.toString, "$1")
getFolderNameRegex.replaceFirstIn(Paths.get(resolvedPath.get).normalize.toString, "$1")
}
}
/**
Expand Down Expand Up @@ -160,22 +188,20 @@
if (packageDir.isEmpty) {
throw new RuntimeException(s"One of the resources is relative to the package root ($path), but no package config file (_viash.yaml) could be found.")
}
val pathStr1 = IO.resolvePathWrtURI(pathStr, packageDir.get)
return this.copyResource(
path = Some(pathStr1),
return copyResource(
parent = packageDir
)
}

// if the path is relative (which it probably should be),
// set the directory of the config as the parent of this file.
if (!Paths.get(pathStr).isAbsolute) {
return this.copyResource(
return copyResource(
parent = Some(parent)
)
}

return this
this
}

// TODO: This can probably be solved much nicer.
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/io/viash/helpers/NsExecData.scala
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ object NsExecData {
def apply(configPath: String, config: Config, runner: Option[Runner], engine: Option[Engine]): NsExecData = {
val configPath_ = Paths.get(configPath)
val dirPath = configPath_.getParent()
val mainScript = config.mainScript.flatMap(s => s.path).map(dirPath.resolve(_))
val mainScript = config.mainScript.flatMap(s => s.resolvedPath).map(dirPath.resolve(_))
apply(
configFullPath = configPath,
absoluteConfigFullPath = configPath_.toAbsolutePath.toString,
Expand Down
Empty file.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ resources:
- path: file:/tmp/viash_tmp_resources/resource3.txt
- path: file:/tmp/viash_tmp_resources/resource3.txt
dest: target_folder/relocated_file_4.txt
- path: /resource4.txt

#{path: /foo/abc} -> $meta_resources_dir/abc
#{path: foo/abc} -> $meta_resources_dir/abc
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This file is refered to by absolute path in the config
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import org.scalatest.funsuite.AnyFunSuite

import java.nio.file.{Files, Paths, StandardCopyOption}
import io.viash.ConfigDeriver
import io.viash.packageConfig.PackageConfig

class MainBuildAuxiliaryDockerResourceCopying extends AnyFunSuite with BeforeAndAfterAll {
Logger.UseColorOverride.value = Some(false)
Expand All @@ -16,7 +17,8 @@ class MainBuildAuxiliaryDockerResourceCopying extends AnyFunSuite with BeforeAnd


private val configFile = getClass.getResource("/testbash/auxiliary_resource/config_resource_test.vsh.yaml").getPath
private val config = Config.read(configFile)
private val dummyPackage = Some(PackageConfig(rootDir = Some(Paths.get(configFile).getParent())))
private val config = Config.read(configFile, viashPackage = dummyPackage)
private val executable = Paths.get(tempFolStr, config.name).toFile

private val temporaryConfigFolder = IO.makeTemp(s"viash_${this.getClass.getName}_")
Expand All @@ -37,6 +39,7 @@ class MainBuildAuxiliaryDockerResourceCopying extends AnyFunSuite with BeforeAnd

// generate viash script
TestHelper.testMain(
workingDir = Some(temporaryConfigFolder),
"build",
"--engine", "docker",
"-o", tempFolStr,
Expand All @@ -60,6 +63,7 @@ class MainBuildAuxiliaryDockerResourceCopying extends AnyFunSuite with BeforeAnd
("target_folder/relocated_file_2.txt", "51954bf10062451e683121e58d858417"),
("target_folder/relocated_file_3.txt", ".*"), // turn off checksum match
("resource3.txt", "aa2037b3d308bcb6a78a3d4fbf04b297"),
("resource4.txt", "21cd10137f841da59921aa35de998942"),
("target_folder/relocated_file_4.txt", "aa2037b3d308bcb6a78a3d4fbf04b297")
)

Expand All @@ -72,6 +76,23 @@ class MainBuildAuxiliaryDockerResourceCopying extends AnyFunSuite with BeforeAnd
val hash = TestHelper.computeHash(resourceFile.getPath)
assert(md5sum.r.findFirstMatchIn(hash).isDefined, s"Calculated md5sum doesn't match the given md5sum for $resourceFile")
}

// Check the resources listed in the built .config.vsh.yaml file
// Checked values are relativized paths to the output folder
val builtConfigUri = temporaryFolder.resolve(".config.vsh.yaml")
val builtConfig = Config.read(builtConfigUri.toString)
val resourcePaths = builtConfig.resources.map(
resource => {
assert(resource.path.isDefined, s"Resource path is not defined for $resource")
resource.path.get
}
)

for ((name, _) <- expectedResources) {
// skip tests for resources in 'resource_folder' as it's copied as folder and thus the files won't be listed in the built config
if (!name.contains("resource_folder/"))
assert(resourcePaths.contains(name), s"Could not find $name in the built config")
}
}

test("Check resources with unsupported format") {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package io.viash.auxiliary

import io.viash.{DockerTest, TestHelper}
import io.viash.config.Config
import io.viash.helpers.{IO, Exec, Logger}
import org.scalatest.BeforeAndAfterAll
import org.scalatest.funsuite.AnyFunSuite

import java.nio.file.{Files, Paths, StandardCopyOption}
import io.viash.ConfigDeriver
import io.viash.packageConfig.PackageConfig

class MainConfigViewNativeAuxiliaryResource extends AnyFunSuite with BeforeAndAfterAll {
Logger.UseColorOverride.value = Some(false)
private val temporaryFolder = IO.makeTemp("viash_tester")
private val tempFolStr = temporaryFolder.toString


private val configFile = getClass.getResource("/testbash/auxiliary_resource/config_resource_test.vsh.yaml").getPath
private val dummyPackage = Some(PackageConfig(rootDir = Some(Paths.get(configFile).getParent())))
private val sourceConfig = Config.read(configFile, viashPackage = dummyPackage)

private val temporaryConfigFolder = IO.makeTemp(s"viash_${this.getClass.getName}_")
private val configDeriver = ConfigDeriver(Paths.get(configFile), temporaryConfigFolder)

test("Check resource paths are unchanged in config view") {

// copy some resources to /tmp/viash_tmp_resources/ so we can test absolute path resources
val tmpFolderResourceSourceFile = Paths.get(getClass.getResource("/testbash/auxiliary_resource/resource3.txt").getFile)

val tmpFolderResourceDestinationFolder = Paths.get("/tmp/viash_tmp_resources/").toFile
val tmpFolderResourceDestinationFile = Paths.get(tmpFolderResourceDestinationFolder.getPath, "resource3.txt")

if (!tmpFolderResourceDestinationFolder.exists())
tmpFolderResourceDestinationFolder.mkdir()

Files.copy(tmpFolderResourceSourceFile, tmpFolderResourceDestinationFile, StandardCopyOption.REPLACE_EXISTING)

// generate viash script
val testOutput = TestHelper.testMain(
workingDir = Some(temporaryConfigFolder),
"config", "view",
"--engine", "native",
configFile
)

// Check the resources listed in the built .config.vsh.yaml file
val builtConfigPath = temporaryFolder.resolve(".config.vsh.yaml")
IO.write(testOutput.stdout, builtConfigPath)
val dummyPackage = Some(PackageConfig(rootDir = Some(temporaryFolder)))
val outputConfig = Config.read(builtConfigPath.toString, viashPackage = dummyPackage)

val resourcePaths = outputConfig.resources.map(
resource => {
assert(resource.path.isDefined, s"Resource path is not defined for $resource")
resource.path.get
}
)

for (sourceResource <- sourceConfig.resources) {
assert(resourcePaths.contains(sourceResource.path.get), s"Resource ${sourceResource.path.get} not found in built config")
}

}

override def afterAll(): Unit = {
IO.deleteRecursively(temporaryFolder)
IO.deleteRecursively(temporaryConfigFolder)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,11 @@ class MainTestAuxiliaryDockerResourceCopy extends AnyFunSuite with BeforeAndAfte

// generate viash script
val testOutput = TestHelper.testMain(
workingDir = Some(temporaryConfigFolder),
"test",
"--engine", "docker",
"-k", "true",
configFile
configFile,
)

// basic checks to see if standard test/build was correct
Expand Down Expand Up @@ -62,6 +63,7 @@ class MainTestAuxiliaryDockerResourceCopy extends AnyFunSuite with BeforeAndAfte
("target_folder/relocated_file_2.txt", "51954bf10062451e683121e58d858417"),
("target_folder/relocated_file_3.txt", ".*"), // turn off checksum match, as it changes regularly.
("resource3.txt", "aa2037b3d308bcb6a78a3d4fbf04b297"),
("resource4.txt", "21cd10137f841da59921aa35de998942"),
("target_folder/relocated_file_4.txt", "aa2037b3d308bcb6a78a3d4fbf04b297")
)

Expand Down
5 changes: 3 additions & 2 deletions src/test/scala/io/viash/e2e/build/DockerMeta.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ import io.viash.config.resources.PlainFile
class DockerMeta extends AnyFunSuite with BeforeAndAfterAll {
Logger.UseColorOverride.value = Some(false)
// which config to test
private val configFile = getClass.getResource(s"/testbash/config.vsh.yaml").getPath
private val configFile = getClass.getResource("/testbash/config.vsh.yaml").getPath
private val parent = getClass.getResource("/").toURI()

// parse config from file
private val config = Config.read(configFile)
private def configAndResources = PlainFile(path = Some(configFile.toString)) :: config.resources
private def configAndResources = PlainFile(path = Some("testbash/config.vsh.yaml"), parent = Some(parent)) :: config.resources

test("Get meta data of a docker", DockerTest) {
// Create temporary folder to copy the files to so we can do a git init in that folder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class MainConfigInjectSuite extends AnyFunSuite with BeforeAndAfterAll {

val config = Config.read(configFile.toString())

val scriptFile = destPath.resolve(s"$name/" + config.mainScript.get.path.get) // assume all of these things exist
val scriptFile = destPath.resolve(s"$name/" + config.mainScript.get.resolvedPath.get) // assume all of these things exist
assert(scriptFile.toFile().exists, "Check dest script exists")

// inject script
Expand Down
Loading