stevedlawrence commented on code in PR #183:
URL: https://github.com/apache/daffodil-sbt/pull/183#discussion_r3267206525
##########
src/main/scala/org/apache/daffodil/DaffodilPlugin.scala:
##########
@@ -996,32 +1004,21 @@ object DaffodilPlugin extends AutoPlugin {
// diretory or jar file as the context schema
Some(refPath.toUri.toURL)
} else {
- contextURI.getScheme match {
+ val contextRoot = getRootPath(contextURI)
Review Comment:
contextURI could be a jar here, and getRootPath fails for non-file schemes.
I think `contextRoot` and `relPath` can be moved in to the "file" case, since
they aren't needed for the jar case.
##########
src/main/scala/org/apache/daffodil/DaffodilPlugin.scala:
##########
@@ -996,32 +1004,21 @@ object DaffodilPlugin extends AutoPlugin {
// diretory or jar file as the context schema
Some(refPath.toUri.toURL)
} else {
- contextURI.getScheme match {
+ val contextRoot = getRootPath(contextURI)
+ val relPath = contextRoot.relativize(refPath)
+ val optResolvedRelative = contextURI.getScheme match {
case "file" => {
// Need to check other resource directories
val resolvedRoot = (Compile /
resourceDirectories).value.find(root =>
- Files.exists(Paths.get(root.toURI.resolve(ref)))
+ Files.exists(root.toPath.resolve(relPath))
)
- if (resolvedRoot.isDefined)
- Some(resolvedRoot.get.toURI.resolve(ref).toURL)
- else {
- // Maybe this path is actually absolute, just missing the
- // leading '/', check the classpath
- Option(classLoader.findResource(ref))
- }
+ resolvedRoot.map(_.toURI.resolve(ref).toURL)
Review Comment:
Should this be `.resolve(relPath)` too? Above we found a root where
`root.toPath.resolve(relPath)` exists so I think we want to use the same
resolution with that root here to get the URL?
##########
src/main/scala/org/apache/daffodil/DaffodilPlugin.scala:
##########
@@ -996,32 +1004,21 @@ object DaffodilPlugin extends AutoPlugin {
// diretory or jar file as the context schema
Some(refPath.toUri.toURL)
} else {
- contextURI.getScheme match {
+ val contextRoot = getRootPath(contextURI)
+ val relPath = contextRoot.relativize(refPath)
+ val optResolvedRelative = contextURI.getScheme match {
case "file" => {
// Need to check other resource directories
val resolvedRoot = (Compile /
resourceDirectories).value.find(root =>
- Files.exists(Paths.get(root.toURI.resolve(ref)))
+ Files.exists(root.toPath.resolve(relPath))
)
- if (resolvedRoot.isDefined)
- Some(resolvedRoot.get.toURI.resolve(ref).toURL)
- else {
- // Maybe this path is actually absolute, just missing the
- // leading '/', check the classpath
- Option(classLoader.findResource(ref))
- }
+ resolvedRoot.map(_.toURI.resolve(ref).toURL)
}
- case "jar" => {
- // Maybe this path in the JAR is actually absolute, just
- // missing the leading '/', try resolving it relative to the
- // root of the JAR
- val jarPath = contextPath.getRoot().resolve(ref)
- if (Files.exists(jarPath))
- Some(jarPath.toUri.toURL)
- else
- None
- }
- case _ => None
+ case "jar" =>
+ // Nothing else to check, resolveSibling should have found
it in the same jar
+ None
}
+ optResolvedRelative.orElse(Option(classLoader.findResource(ref)))
Review Comment:
Please add a comment that this is to support the deprecated behavior of
resolving relative paths as if they were absolute.
##########
src/main/scala/org/apache/daffodil/DaffodilPlugin.scala:
##########
@@ -1039,11 +1036,11 @@ object DaffodilPlugin extends AutoPlugin {
case "jar" =>
resolvedURI.toString match {
case jarRegex(_, jarName, path) =>
- s"${jarName}__${path.tail.replaceAll("/", "__")}"
+ s"$jarName/${path.tail}"
}
case _ => {
val resolvedRoot = (Compile /
resourceDirectories).value.find(root =>
- resolvedURI.toString.contains(root.toString)
+ resolvedURI.getPath.startsWith(root.toString)
Review Comment:
Can you use the new getRootPath function here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
