stevedlawrence commented on code in PR #183:
URL: https://github.com/apache/daffodil-sbt/pull/183#discussion_r3284280877
##########
src/main/scala/org/apache/daffodil/DaffodilPlugin.scala:
##########
@@ -63,6 +67,15 @@ object DaffodilPlugin extends AutoPlugin {
val daffodilTdmlUsesPackageBin = settingKey[Boolean](
"Whether or not TDML files use the saved parsers created by
daffodilPackageBin"
)
+ val daffodilFlattenTarget = settingKey[File](
+ "File to write the flattened schemas package to"
+ )
+ val daffodilFlattenSchemas = taskKey[File](
Review Comment:
A thought about crossDaffodilVersions: When building cross versions, we
currently do this for the root project:
```scala
val rootProjectSettings = Seq(
// publish cross versioned artifacts with the root project
artifacts. This is important
// since all projects have the same name, differing only in
classifiers, so we cannot
// publish subprojects separately since most artifact repositories
do not support
// publishing additional artifacts after a jar has already been
published
crossProjects.flatMap { project =>
Seq(
artifacts ++= (project / artifacts).value,
packagedArtifacts ++= (project / packagedArtifacts).value
)
}
).flatten
```
So we copy all of the crossProject artifacts and packagedArtifacts to the
root project so they are all published when the root is published. This
normally works well because all crossProject artifacts have unique classifiers
and so there are no conflicts with the artifacts. But that's not the case with
the flattened artifact. Flattened artifacts just have a "-flat" classifier, so
the artifact is the same for all cross versions. This could lead to duplicate
artifacts, or at the very least duplicate work to create the same artifact for
all subprojects.
I'm not sure the best way to fix this. One option is when creating the
crossProject we can add a setting to the existing crossProjectSettings val that
essentially disable the `daffodilFlattSchema /products` task and disables
publishing it, e.g.:
```scala
// Settings only used for cross version projects
val crossProjectSettings = Seq(
...,
daffodilFlattenSchemas / products := Seq(),
daffodilFlattenSchemas / publishAritfact := false,
)
```
So we don't create them and we don't publish them. That has one issue though
that if you ever run `sbt root_daffodilXYZ/daffodilFlattenSchemas` or if
aggregation is enabled it will fail because there are no products, and
`daffodilFlattanSchemas` task requires a product. Maybe we detect if it's empty
and just return /dev/null? Or maybe we change `daffodilFlattenSchemas` to a
taskKey[Unit], and then change the task to this:
```scala
daffodilFlattenSchemas := {
(daffodilFlattenSchemas / products).value
},
```
So it triggers products to be created if they exist, but if products is
empty it's fine. That feels like normal sbt convention to me.
And it's nice in that it means there's literally only one way to get a
flattened schema and it's via the root project.
##########
src/main/scala/org/apache/daffodil/DaffodilPlugin.scala:
##########
@@ -1034,13 +1038,17 @@ object DaffodilPlugin extends AutoPlugin {
// For each reference replace each instance of it in the
// file with the same reference but with "/" changed to "__"
val resolvedURI = optResolvedURL.toURI
- val relativized = resolvedURI.getScheme match {
- case "jar" =>
+ val relativized = (resolvedURI.getScheme, contextURI.getScheme)
match {
+ case ("jar", _) =>
resolvedURI.toString match {
case jarRegex(_, jarName, path) =>
s"$jarName/${path.tail}"
}
- case _ => {
+ case ("file", "jar") => {
+ val resolvedRoot = getRootPath(resolvedURI)
+ resolvedRoot.relativize(Paths.get(resolvedURI)).toString
Review Comment:
Do we want to always use this logic, regardless if the context is a "jar" or
"file" scheme?
If we have resolved a reference to a "file" URI, then that URI must have
come from one of our resource directories. So in order to find its flattened
relative path we just have to figure what resource directory it is in and
relativize based on that. Which is what this logic does.
I think using conextURI below is just a bug. That will only work when the
found "file" reference is in the same resource directory as the context. That
is the common case, but isn't always true. Even if the contextURI is a file,
the resolved file could be in a different resource directory and the relativize
won't work
So I think we can just remove the following case and stick with this new
one. And we probably don't need to match on contextURI.scheme anymore.
--
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]
