You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "stevedlawrence (via GitHub)" <gi...@apache.org> on 2025/11/10 14:16:07 UTC
[PR] Remove reflection from DaffodilSaver [daffodil-sbt]
stevedlawrence opened a new pull request, #136:
URL: https://github.com/apache/daffodil-sbt/pull/136
The reflection code is messy and hard to maintain. This also is no longer the recommended way to create tools that support multiple versions of Daffodil--instead we should use sbt-projectmatrix to create separate jars with thin shim layers to deal with differences in Daffodil APIs.
This commit does just that. We change our build.sbt to add a new "utils" subproject that contains code that needs cross Daffodil support. This subproject uses sbt-projectmatrix to build three jars, one for Scala 2.12, 2.13, and 3, which are mapped to the various Daffodil versions. This appropriate jar is added as a libraryDependency along with the Daffodil version.
DaffodilSaver is moved into this util subproject and rewritten in Scala. Scala type inference and similarites in the API in different versions of Daffodil allow this code to mostly not need to know anything about the Daffodil version being used. In cases where that is needed, we use a new DaffodilAPI shim class that provides type alases or helper functions that are Scala/Daffodil version speciic.
Closes #135
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove reflection from DaffodilSaver [daffodil-sbt]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #136:
URL: https://github.com/apache/daffodil-sbt/pull/136#discussion_r2518665594
##########
utils/src/main/scala/org/apache/daffodil/DaffodilSaver.scala:
##########
@@ -0,0 +1,119 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License") you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.daffodil
+
+import java.nio.channels.FileChannel
+import java.nio.file.Paths
+import java.nio.file.StandardOpenOption
+import scala.jdk.CollectionConverters._
+
+// When creating saved parsers using packageDaffodilBin, we need to create a saved parser for a
+// different version of Daffodil than is added to libraryDependencies. For this reason, we have
+// this DaffodilSaver class, which can be forked with a Different versionof Daffodil on the
+// classpath. Note that we use sbt-projectmatrix to compile this class into three separate jars
+// for each version of Scala/Daffodil (Scala 2.12 is used for Daffodil 3.10.0 and older, Scala
+// 2.13 is used for Daffodil 3.11.0, and Scala 3.x is used for Daffodil 4.0.0 and newer. WWe
+// expect the sbt plugin to set up the classpath to include the correct jar for the version of
+// Daffodil being used when forking this class.
+object DaffodilSaver {
+
+ /**
+ * Usage: daffodilSaver <apiVersion> <schemaResource> <outputFile> <root> <config>
+ *
+ * If <root> or <config> is unknown/not-provided, they must be the empty string
+ */
+ def main(args: Array[String]): Unit = {
+ try {
+ run(args)
+ } catch {
+ case e: Exception => {
+ e.printStackTrace()
+ System.exit(1)
+ }
+ }
+ }
+
+ private def run(args: Array[String]): Unit = {
+
+ if (args.length != 5) {
+ throw new Exception(
+ "DaffodilPlugin did not provide the correct number of arguments when forking DaffodilSaver"
+ )
+ }
+
+ // the "version" of the Daffodil API to use. Note that this is not the same as the Daffodil
+ // version, but is related. See the "daffodilInternalAPIVersionMapping" in the plugin code
+ // for an explanation of why we have this and what version of Daffodil it represents.
+ val apiVersion = Integer.parseInt(args(0))
+
+ val schemaResource = args(1)
+ val schemaUrl = this.getClass.getResource(schemaResource)
+ if (schemaUrl == null) {
+ System.err.println("failed to find schema resource: " + schemaResource)
+ System.exit(1)
+ }
+
+ val output =
+ FileChannel.open(Paths.get(args(2)), StandardOpenOption.CREATE, StandardOpenOption.WRITE)
+ val root = if (!args(3).isEmpty()) args(3) else null
+ val config = if (!args(4).isEmpty()) args(4) else null
+
+ val tunables =
+ if (config != null) {
+ val configXml = scala.xml.Utility.trim(scala.xml.XML.loadFile(config))
+ (configXml \ "tunables").flatMap { tunablesNode =>
+ tunablesNode.child.map { node => (node.label, node.text) }
+ }.toMap
+ } else {
+ Map.empty[String, String]
+ }
+ val jTunables = new java.util.HashMap[String, String](tunables.asJava)
+
+ val compiler = DaffodilAPI
+ .compiler()
+ .withTunables(jTunables)
+
+ val processorFactory = apiVersion match {
Review Comment:
I actually started down this approach and in this specific case it felt like it actually made things more complex without a whole lot of gain. The DaffodilAPI classes a became complex and more different from one another (it's nice to keep this shim class as small as possible), and all it really got rid of was this one small conditional. I think as sbt-daffodil-utils grows in complexity it's very possible we'll need something like what you describe, but I lean towards holding off on that change until we really need it so we can keep things a simple as possible for now.
For reference here's the diff to move the logic to the DaffodilAPI class:
```diff
diff --git a/utils/src/main/scala-2/org/apache/daffodil/DaffodilAPI.scala b/utils/src/main/scala-2/org/apache/daffodil/DaffodilAPI.scala
index 057bd9b..f9c0e64 100644
--- a/utils/src/main/scala-2/org/apache/daffodil/DaffodilAPI.scala
+++ b/utils/src/main/scala-2/org/apache/daffodil/DaffodilAPI.scala
@@ -22,7 +22,20 @@ package org.apache.daffodil
* compatability with Scala 2.x and Daffodil 3.x
*/
object DaffodilAPI {
+ type Compiler = org.apache.daffodil.japi.Compiler
type Diagnostic = org.apache.daffodil.japi.Diagnostic
def compiler() = org.apache.daffodil.japi.Daffodil.compiler()
+
+ def compileResource(compiler: Compiler, resource: String, root: String, apiVersion: Int) = {
+ apiVersion match {
+ case 1 => {
+ val schemaUrl = this.getClass.getResource(resource)
+ compiler.compileSource(schemaUrl.toURI, root, null)
+ }
+ case 2 => {
+ compiler.compileResource(resource, root, null)
+ }
+ }
+ }
}
diff --git a/utils/src/main/scala-3/org/apache/daffodil/DaffodilAPI.scala b/utils/src/main/scala-3/org/apache/daffodil/DaffodilAPI.scala
index 05c1c69..1ff59a3 100644
--- a/utils/src/main/scala-3/org/apache/daffodil/DaffodilAPI.scala
+++ b/utils/src/main/scala-3/org/apache/daffodil/DaffodilAPI.scala
@@ -23,6 +23,11 @@ package org.apache.daffodil
*/
object DaffodilAPI {
type Diagnostic = org.apache.daffodil.api.Diagnostic
+ type Compiler = org.apache.daffodil.api.Compiler
def compiler() = org.apache.daffodil.api.Daffodil.compiler()
+
+ def compileResource(compiler: Compiler, resource: String, root: String, apiVersion: Int) = {
+ compiler.compileResource(resource, root, null)
+ }
}
diff --git a/utils/src/main/scala/org/apache/daffodil/DaffodilSaver.scala b/utils/src/main/scala/org/apache/daffodil/DaffodilSaver.scala
index 3c66173..be17017 100644
--- a/utils/src/main/scala/org/apache/daffodil/DaffodilSaver.scala
+++ b/utils/src/main/scala/org/apache/daffodil/DaffodilSaver.scala
@@ -88,10 +88,8 @@ object DaffodilSaver {
.compiler()
.withTunables(jTunables)
- val processorFactory = apiVersion match {
- case 1 => compiler.compileSource(schemaUrl.toURI, root, null)
- case 2 => compiler.compileResource(schemaResource, root, null)
- }
+ val processorFactory =
+ DaffodilAPI.compileResource(compiler, schemaResource, root, apiVersion)
if (processorFactory.isError()) {
printDiagnostics(processorFactory.getDiagnostics.asScala.toSeq)
System.exit(1)
```
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove reflection from DaffodilSaver [daffodil-sbt]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on PR #136:
URL: https://github.com/apache/daffodil-sbt/pull/136#issuecomment-3512770230
> A little more complicated on the build configuration side but much simpler than dealing with reflection in the actual code.
Agreed. I imagine over time the `DaffodilAPI` shim could get a bit more complicated, but I doubt that much, and I think it's still a big win as far as maintainability goes
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove reflection from DaffodilSaver [daffodil-sbt]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence merged PR #136:
URL: https://github.com/apache/daffodil-sbt/pull/136
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove reflection from DaffodilSaver [daffodil-sbt]
Posted by "jadams-tresys (via GitHub)" <gi...@apache.org>.
jadams-tresys commented on code in PR #136:
URL: https://github.com/apache/daffodil-sbt/pull/136#discussion_r2511723618
##########
build.sbt:
##########
@@ -15,53 +15,115 @@
* limitations under the License.
*/
-name := "sbt-daffodil"
-
-organization := "org.apache.daffodil"
-
-version := IO.read((ThisBuild / baseDirectory).value / "VERSION").trim
-
-scalaVersion := "2.12.19"
-
-scalacOptions ++= Seq(
- "-Ywarn-unused:imports"
+val commonSettings = Seq(
+ organization := "org.apache.daffodil",
+ version := IO.read((ThisBuild / baseDirectory).value / "VERSION").trim,
+ scalacOptions ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" | "2.13" =>
+ Seq(
+ "-Werror",
+ "-Ywarn-unused:imports"
+ )
+ case "3" =>
+ Seq(
+ "-no-indent",
+ "-Werror",
+ "-Wunused:imports"
+ )
+ }
+ },
+ scmInfo := Some(
+ ScmInfo(
+ browseUrl = url("https://github.com/apache/daffodil-sbt"),
+ connection = "scm:git:https://github.com/apache/daffodil-sbt"
+ )
+ ),
+ licenses := Seq(License.Apache2),
+ homepage := Some(url("https://daffodil.apache.org")),
+ releaseNotesURL := Some(url(s"https://daffodil.apache.org/sbt/${version.value}/"))
)
-scmInfo := Some(
- ScmInfo(
- browseUrl = url("https://github.com/apache/daffodil-sbt"),
- connection = "scm:git:https://github.com/apache/daffodil-sbt"
+lazy val plugin = (project in file("."))
+ .settings(commonSettings)
+ .settings(
+ name := "sbt-daffodil",
+
+ // SBT plugin settings
+ scalaVersion := "2.12.19",
+ crossSbtVersions := Seq("1.8.0"),
+ scriptedLaunchOpts ++= Seq(
+ "-Xmx1024M",
+ "-Dplugin.version=" + version.value
+ ),
+ scriptedDependencies := {
+ // scripted runs publishLocal for the plugin and its dependencies as part of the
+ // scriptedDependencies task. But the utils subprojects aren't actually dependencies (so
+ // won't be locally published). We still need the util jars locally published so the
+ // scripted tests can find the jars at runtime, so we manually run publishLocal for each
+ // of the utils subprojects as part of the scriptedDependencies task
+ publishLocal.all(ScopeFilter(projects = inProjects(utils.projectRefs: _*))).value
+ scriptedDependencies.value
+ },
+ Test / test := {
+ // run all scripted tasks as part of testing
+ (Compile / scripted).toTask("").value
+ (Test / test).value
+ },
+
+ // Rat check settings
+ ratExcludes := Seq(
+ file(".git"),
+ file("VERSION")
+ ),
+ ratFailBinaries := true
)
-)
-
-licenses := Seq(License.Apache2)
-
-homepage := Some(url("https://daffodil.apache.org"))
-
-releaseNotesURL := Some(url(s"https://daffodil.apache.org/sbt/${version.value}/"))
-
-// SBT Plugin settings
-
-enablePlugins(SbtPlugin)
+ .enablePlugins(SbtPlugin)
+ .aggregate(utils.projectRefs: _*)
-crossSbtVersions := Seq("1.8.0")
-
-scriptedLaunchOpts ++= Seq(
- "-Xmx1024M",
- "-Dplugin.version=" + version.value
-)
-
-// Rat check settings
-
-ratExcludes := Seq(
- file(".git"),
- file("VERSION")
-)
-
-ratFailBinaries := true
-
-Test / test := {
- // run all scripted tasks as part of testing
- (Compile / scripted).toTask("").value
- (Test / test).value
-}
+lazy val utils = (projectMatrix in file("utils"))
+ .settings(commonSettings)
+ .settings(
+ name := "sbt-daffodil-utils"
+ )
+ .settings(
+ javacOptions ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" => Seq("-target", "8")
+ case "2.13" => Seq("-target", "8")
+ case "3" => Seq("-target", "17")
+ }
+ },
+ scalacOptions ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" => Seq(s"--target:jvm-8")
+ case "2.13" => Seq(s"--release", "8")
+ case "3" => Seq(s"--release", "17")
+ }
+ },
+ libraryDependencies ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" => {
+ Seq(
+ // scala-steward:off
+ "org.apache.daffodil" %% "daffodil-japi" % "3.10.0" % "provided",
+ // scala-steward:on
+ "org.scala-lang.modules" %% "scala-collection-compat" % "2.14.0"
+ )
+ }
+ case "2.13" => {
+ Seq(
+ // scala-steward:off
+ "org.apache.daffodil" %% "daffodil-japi" % "3.11.0" % "provided"
+ // scala-steward:on
+ )
+ }
+ case "3" => {
+ Seq(
+ "org.apache.daffodil" %% "daffodil-core" % "4.0.0" % "provided"
Review Comment:
Was looking back at these changes and was wondering does this mean that the sbt-daffodil only supports 3.10.0, 3.11.0 and 4.0.0? It no longer supports 3.9.0 and below?
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove reflection from DaffodilSaver [daffodil-sbt]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #136:
URL: https://github.com/apache/daffodil-sbt/pull/136#discussion_r2514145751
##########
build.sbt:
##########
@@ -15,53 +15,115 @@
* limitations under the License.
*/
-name := "sbt-daffodil"
-
-organization := "org.apache.daffodil"
-
-version := IO.read((ThisBuild / baseDirectory).value / "VERSION").trim
-
-scalaVersion := "2.12.19"
-
-scalacOptions ++= Seq(
- "-Ywarn-unused:imports"
+val commonSettings = Seq(
+ organization := "org.apache.daffodil",
+ version := IO.read((ThisBuild / baseDirectory).value / "VERSION").trim,
+ scalacOptions ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" | "2.13" =>
+ Seq(
+ "-Werror",
+ "-Ywarn-unused:imports"
+ )
+ case "3" =>
+ Seq(
+ "-no-indent",
+ "-Werror",
+ "-Wunused:imports"
+ )
+ }
+ },
+ scmInfo := Some(
+ ScmInfo(
+ browseUrl = url("https://github.com/apache/daffodil-sbt"),
+ connection = "scm:git:https://github.com/apache/daffodil-sbt"
+ )
+ ),
+ licenses := Seq(License.Apache2),
+ homepage := Some(url("https://daffodil.apache.org")),
+ releaseNotesURL := Some(url(s"https://daffodil.apache.org/sbt/${version.value}/"))
)
-scmInfo := Some(
- ScmInfo(
- browseUrl = url("https://github.com/apache/daffodil-sbt"),
- connection = "scm:git:https://github.com/apache/daffodil-sbt"
+lazy val plugin = (project in file("."))
+ .settings(commonSettings)
+ .settings(
+ name := "sbt-daffodil",
+
+ // SBT plugin settings
+ scalaVersion := "2.12.19",
+ crossSbtVersions := Seq("1.8.0"),
+ scriptedLaunchOpts ++= Seq(
+ "-Xmx1024M",
+ "-Dplugin.version=" + version.value
+ ),
+ scriptedDependencies := {
+ // scripted runs publishLocal for the plugin and its dependencies as part of the
+ // scriptedDependencies task. But the utils subprojects aren't actually dependencies (so
+ // won't be locally published). We still need the util jars locally published so the
+ // scripted tests can find the jars at runtime, so we manually run publishLocal for each
+ // of the utils subprojects as part of the scriptedDependencies task
+ publishLocal.all(ScopeFilter(projects = inProjects(utils.projectRefs: _*))).value
+ scriptedDependencies.value
+ },
+ Test / test := {
+ // run all scripted tasks as part of testing
+ (Compile / scripted).toTask("").value
+ (Test / test).value
+ },
+
+ // Rat check settings
+ ratExcludes := Seq(
+ file(".git"),
+ file("VERSION")
+ ),
+ ratFailBinaries := true
)
-)
-
-licenses := Seq(License.Apache2)
-
-homepage := Some(url("https://daffodil.apache.org"))
-
-releaseNotesURL := Some(url(s"https://daffodil.apache.org/sbt/${version.value}/"))
-
-// SBT Plugin settings
-
-enablePlugins(SbtPlugin)
+ .enablePlugins(SbtPlugin)
+ .aggregate(utils.projectRefs: _*)
-crossSbtVersions := Seq("1.8.0")
-
-scriptedLaunchOpts ++= Seq(
- "-Xmx1024M",
- "-Dplugin.version=" + version.value
-)
-
-// Rat check settings
-
-ratExcludes := Seq(
- file(".git"),
- file("VERSION")
-)
-
-ratFailBinaries := true
-
-Test / test := {
- // run all scripted tasks as part of testing
- (Compile / scripted).toTask("").value
- (Test / test).value
-}
+lazy val utils = (projectMatrix in file("utils"))
+ .settings(commonSettings)
+ .settings(
+ name := "sbt-daffodil-utils"
+ )
+ .settings(
+ javacOptions ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" => Seq("-target", "8")
+ case "2.13" => Seq("-target", "8")
+ case "3" => Seq("-target", "17")
+ }
+ },
+ scalacOptions ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" => Seq(s"--target:jvm-8")
+ case "2.13" => Seq(s"--release", "8")
+ case "3" => Seq(s"--release", "17")
+ }
+ },
+ libraryDependencies ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" => {
+ Seq(
+ // scala-steward:off
+ "org.apache.daffodil" %% "daffodil-japi" % "3.10.0" % "provided",
+ // scala-steward:on
+ "org.scala-lang.modules" %% "scala-collection-compat" % "2.14.0"
+ )
+ }
+ case "2.13" => {
+ Seq(
+ // scala-steward:off
+ "org.apache.daffodil" %% "daffodil-japi" % "3.11.0" % "provided"
+ // scala-steward:on
+ )
+ }
+ case "3" => {
+ Seq(
+ "org.apache.daffodil" %% "daffodil-core" % "4.0.0" % "provided"
Review Comment:
There was an API change I believe from 3.7.0 to 3.8.0 also (packages all changed) and things like the metadata and data APIs for the Apache Drill integration were added at that time I believe.
Apache Drill code has been updated to use 3.11.0 btw.
I'm not suggesting we do this as part of this change set, but can the approach used here be extended to support even older Daffodil versions like 3.7.0, and even 3.4.0 ?
I guess I'm interested in what are the limitations of this version matrix approach?
##########
utils/src/main/scala/org/apache/daffodil/DaffodilSaver.scala:
##########
@@ -0,0 +1,119 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License") you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.daffodil
+
+import java.nio.channels.FileChannel
+import java.nio.file.Paths
+import java.nio.file.StandardOpenOption
+import scala.jdk.CollectionConverters._
+
+// When creating saved parsers using packageDaffodilBin, we need to create a saved parser for a
+// different version of Daffodil than is added to libraryDependencies. For this reason, we have
+// this DaffodilSaver class, which can be forked with a Different versionof Daffodil on the
+// classpath. Note that we use sbt-projectmatrix to compile this class into three separate jars
+// for each version of Scala/Daffodil (Scala 2.12 is used for Daffodil 3.10.0 and older, Scala
+// 2.13 is used for Daffodil 3.11.0, and Scala 3.x is used for Daffodil 4.0.0 and newer. WWe
+// expect the sbt plugin to set up the classpath to include the correct jar for the version of
+// Daffodil being used when forking this class.
+object DaffodilSaver {
+
+ /**
+ * Usage: daffodilSaver <apiVersion> <schemaResource> <outputFile> <root> <config>
+ *
+ * If <root> or <config> is unknown/not-provided, they must be the empty string
+ */
+ def main(args: Array[String]): Unit = {
+ try {
+ run(args)
+ } catch {
+ case e: Exception => {
Review Comment:
Please add a comment like "// ok to catch Exception since this is a main" to avoid code-quality checkers (human) doing the simple thing of looking for "case e: Exception" won't nag about potentially problematic style.
##########
utils/src/main/scala/org/apache/daffodil/DaffodilSaver.scala:
##########
@@ -0,0 +1,119 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License") you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.daffodil
+
+import java.nio.channels.FileChannel
+import java.nio.file.Paths
+import java.nio.file.StandardOpenOption
+import scala.jdk.CollectionConverters._
+
+// When creating saved parsers using packageDaffodilBin, we need to create a saved parser for a
+// different version of Daffodil than is added to libraryDependencies. For this reason, we have
+// this DaffodilSaver class, which can be forked with a Different versionof Daffodil on the
+// classpath. Note that we use sbt-projectmatrix to compile this class into three separate jars
+// for each version of Scala/Daffodil (Scala 2.12 is used for Daffodil 3.10.0 and older, Scala
+// 2.13 is used for Daffodil 3.11.0, and Scala 3.x is used for Daffodil 4.0.0 and newer. WWe
+// expect the sbt plugin to set up the classpath to include the correct jar for the version of
+// Daffodil being used when forking this class.
+object DaffodilSaver {
+
+ /**
+ * Usage: daffodilSaver <apiVersion> <schemaResource> <outputFile> <root> <config>
+ *
+ * If <root> or <config> is unknown/not-provided, they must be the empty string
+ */
+ def main(args: Array[String]): Unit = {
+ try {
+ run(args)
+ } catch {
+ case e: Exception => {
+ e.printStackTrace()
+ System.exit(1)
+ }
+ }
+ }
+
+ private def run(args: Array[String]): Unit = {
+
+ if (args.length != 5) {
+ throw new Exception(
+ "DaffodilPlugin did not provide the correct number of arguments when forking DaffodilSaver"
+ )
+ }
+
+ // the "version" of the Daffodil API to use. Note that this is not the same as the Daffodil
+ // version, but is related. See the "daffodilInternalAPIVersionMapping" in the plugin code
+ // for an explanation of why we have this and what version of Daffodil it represents.
+ val apiVersion = Integer.parseInt(args(0))
+
+ val schemaResource = args(1)
+ val schemaUrl = this.getClass.getResource(schemaResource)
+ if (schemaUrl == null) {
+ System.err.println("failed to find schema resource: " + schemaResource)
+ System.exit(1)
+ }
+
+ val output =
+ FileChannel.open(Paths.get(args(2)), StandardOpenOption.CREATE, StandardOpenOption.WRITE)
+ val root = if (!args(3).isEmpty()) args(3) else null
+ val config = if (!args(4).isEmpty()) args(4) else null
+
+ val tunables =
+ if (config != null) {
+ val configXml = scala.xml.Utility.trim(scala.xml.XML.loadFile(config))
+ (configXml \ "tunables").flatMap { tunablesNode =>
+ tunablesNode.child.map { node => (node.label, node.text) }
+ }.toMap
+ } else {
+ Map.empty[String, String]
+ }
+ val jTunables = new java.util.HashMap[String, String](tunables.asJava)
+
+ val compiler = DaffodilAPI
+ .compiler()
+ .withTunables(jTunables)
+
+ val processorFactory = apiVersion match {
Review Comment:
An alternative to this would have been for each DaffodilAPI instance to define a compileResource wrapper method which just delegates to the real compileResource for the APIs that have it, and does this code to adapt for those that don't have it.
Right now it's just this one conditional that cares, so I don't care. But it seems over time the DaffodilAPI objects are the cross-version API adapters. So maybe this really belongs there instead?
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove reflection from DaffodilSaver [daffodil-sbt]
Posted by "jadams-tresys (via GitHub)" <gi...@apache.org>.
jadams-tresys commented on code in PR #136:
URL: https://github.com/apache/daffodil-sbt/pull/136#discussion_r2511723618
##########
build.sbt:
##########
@@ -15,53 +15,115 @@
* limitations under the License.
*/
-name := "sbt-daffodil"
-
-organization := "org.apache.daffodil"
-
-version := IO.read((ThisBuild / baseDirectory).value / "VERSION").trim
-
-scalaVersion := "2.12.19"
-
-scalacOptions ++= Seq(
- "-Ywarn-unused:imports"
+val commonSettings = Seq(
+ organization := "org.apache.daffodil",
+ version := IO.read((ThisBuild / baseDirectory).value / "VERSION").trim,
+ scalacOptions ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" | "2.13" =>
+ Seq(
+ "-Werror",
+ "-Ywarn-unused:imports"
+ )
+ case "3" =>
+ Seq(
+ "-no-indent",
+ "-Werror",
+ "-Wunused:imports"
+ )
+ }
+ },
+ scmInfo := Some(
+ ScmInfo(
+ browseUrl = url("https://github.com/apache/daffodil-sbt"),
+ connection = "scm:git:https://github.com/apache/daffodil-sbt"
+ )
+ ),
+ licenses := Seq(License.Apache2),
+ homepage := Some(url("https://daffodil.apache.org")),
+ releaseNotesURL := Some(url(s"https://daffodil.apache.org/sbt/${version.value}/"))
)
-scmInfo := Some(
- ScmInfo(
- browseUrl = url("https://github.com/apache/daffodil-sbt"),
- connection = "scm:git:https://github.com/apache/daffodil-sbt"
+lazy val plugin = (project in file("."))
+ .settings(commonSettings)
+ .settings(
+ name := "sbt-daffodil",
+
+ // SBT plugin settings
+ scalaVersion := "2.12.19",
+ crossSbtVersions := Seq("1.8.0"),
+ scriptedLaunchOpts ++= Seq(
+ "-Xmx1024M",
+ "-Dplugin.version=" + version.value
+ ),
+ scriptedDependencies := {
+ // scripted runs publishLocal for the plugin and its dependencies as part of the
+ // scriptedDependencies task. But the utils subprojects aren't actually dependencies (so
+ // won't be locally published). We still need the util jars locally published so the
+ // scripted tests can find the jars at runtime, so we manually run publishLocal for each
+ // of the utils subprojects as part of the scriptedDependencies task
+ publishLocal.all(ScopeFilter(projects = inProjects(utils.projectRefs: _*))).value
+ scriptedDependencies.value
+ },
+ Test / test := {
+ // run all scripted tasks as part of testing
+ (Compile / scripted).toTask("").value
+ (Test / test).value
+ },
+
+ // Rat check settings
+ ratExcludes := Seq(
+ file(".git"),
+ file("VERSION")
+ ),
+ ratFailBinaries := true
)
-)
-
-licenses := Seq(License.Apache2)
-
-homepage := Some(url("https://daffodil.apache.org"))
-
-releaseNotesURL := Some(url(s"https://daffodil.apache.org/sbt/${version.value}/"))
-
-// SBT Plugin settings
-
-enablePlugins(SbtPlugin)
+ .enablePlugins(SbtPlugin)
+ .aggregate(utils.projectRefs: _*)
-crossSbtVersions := Seq("1.8.0")
-
-scriptedLaunchOpts ++= Seq(
- "-Xmx1024M",
- "-Dplugin.version=" + version.value
-)
-
-// Rat check settings
-
-ratExcludes := Seq(
- file(".git"),
- file("VERSION")
-)
-
-ratFailBinaries := true
-
-Test / test := {
- // run all scripted tasks as part of testing
- (Compile / scripted).toTask("").value
- (Test / test).value
-}
+lazy val utils = (projectMatrix in file("utils"))
+ .settings(commonSettings)
+ .settings(
+ name := "sbt-daffodil-utils"
+ )
+ .settings(
+ javacOptions ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" => Seq("-target", "8")
+ case "2.13" => Seq("-target", "8")
+ case "3" => Seq("-target", "17")
+ }
+ },
+ scalacOptions ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" => Seq(s"--target:jvm-8")
+ case "2.13" => Seq(s"--release", "8")
+ case "3" => Seq(s"--release", "17")
+ }
+ },
+ libraryDependencies ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" => {
+ Seq(
+ // scala-steward:off
+ "org.apache.daffodil" %% "daffodil-japi" % "3.10.0" % "provided",
+ // scala-steward:on
+ "org.scala-lang.modules" %% "scala-collection-compat" % "2.14.0"
+ )
+ }
+ case "2.13" => {
+ Seq(
+ // scala-steward:off
+ "org.apache.daffodil" %% "daffodil-japi" % "3.11.0" % "provided"
+ // scala-steward:on
+ )
+ }
+ case "3" => {
+ Seq(
+ "org.apache.daffodil" %% "daffodil-core" % "4.0.0" % "provided"
Review Comment:
Was looking back at these changes and was wondering does this mean that the sbt-daffodil only supports 3.10.0, 3.11.0 and 4.0.0? It no longer supports 3.9.0 and below?
EDIT: nvm, saw below that it uses binDaffodilVersion to specify which version to use for actually packaging the saved parser. Just a little confused when I took a second look at this.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove reflection from DaffodilSaver [daffodil-sbt]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #136:
URL: https://github.com/apache/daffodil-sbt/pull/136#discussion_r2511793203
##########
build.sbt:
##########
@@ -15,53 +15,115 @@
* limitations under the License.
*/
-name := "sbt-daffodil"
-
-organization := "org.apache.daffodil"
-
-version := IO.read((ThisBuild / baseDirectory).value / "VERSION").trim
-
-scalaVersion := "2.12.19"
-
-scalacOptions ++= Seq(
- "-Ywarn-unused:imports"
+val commonSettings = Seq(
+ organization := "org.apache.daffodil",
+ version := IO.read((ThisBuild / baseDirectory).value / "VERSION").trim,
+ scalacOptions ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" | "2.13" =>
+ Seq(
+ "-Werror",
+ "-Ywarn-unused:imports"
+ )
+ case "3" =>
+ Seq(
+ "-no-indent",
+ "-Werror",
+ "-Wunused:imports"
+ )
+ }
+ },
+ scmInfo := Some(
+ ScmInfo(
+ browseUrl = url("https://github.com/apache/daffodil-sbt"),
+ connection = "scm:git:https://github.com/apache/daffodil-sbt"
+ )
+ ),
+ licenses := Seq(License.Apache2),
+ homepage := Some(url("https://daffodil.apache.org")),
+ releaseNotesURL := Some(url(s"https://daffodil.apache.org/sbt/${version.value}/"))
)
-scmInfo := Some(
- ScmInfo(
- browseUrl = url("https://github.com/apache/daffodil-sbt"),
- connection = "scm:git:https://github.com/apache/daffodil-sbt"
+lazy val plugin = (project in file("."))
+ .settings(commonSettings)
+ .settings(
+ name := "sbt-daffodil",
+
+ // SBT plugin settings
+ scalaVersion := "2.12.19",
+ crossSbtVersions := Seq("1.8.0"),
+ scriptedLaunchOpts ++= Seq(
+ "-Xmx1024M",
+ "-Dplugin.version=" + version.value
+ ),
+ scriptedDependencies := {
+ // scripted runs publishLocal for the plugin and its dependencies as part of the
+ // scriptedDependencies task. But the utils subprojects aren't actually dependencies (so
+ // won't be locally published). We still need the util jars locally published so the
+ // scripted tests can find the jars at runtime, so we manually run publishLocal for each
+ // of the utils subprojects as part of the scriptedDependencies task
+ publishLocal.all(ScopeFilter(projects = inProjects(utils.projectRefs: _*))).value
+ scriptedDependencies.value
+ },
+ Test / test := {
+ // run all scripted tasks as part of testing
+ (Compile / scripted).toTask("").value
+ (Test / test).value
+ },
+
+ // Rat check settings
+ ratExcludes := Seq(
+ file(".git"),
+ file("VERSION")
+ ),
+ ratFailBinaries := true
)
-)
-
-licenses := Seq(License.Apache2)
-
-homepage := Some(url("https://daffodil.apache.org"))
-
-releaseNotesURL := Some(url(s"https://daffodil.apache.org/sbt/${version.value}/"))
-
-// SBT Plugin settings
-
-enablePlugins(SbtPlugin)
+ .enablePlugins(SbtPlugin)
+ .aggregate(utils.projectRefs: _*)
-crossSbtVersions := Seq("1.8.0")
-
-scriptedLaunchOpts ++= Seq(
- "-Xmx1024M",
- "-Dplugin.version=" + version.value
-)
-
-// Rat check settings
-
-ratExcludes := Seq(
- file(".git"),
- file("VERSION")
-)
-
-ratFailBinaries := true
-
-Test / test := {
- // run all scripted tasks as part of testing
- (Compile / scripted).toTask("").value
- (Test / test).value
-}
+lazy val utils = (projectMatrix in file("utils"))
+ .settings(commonSettings)
+ .settings(
+ name := "sbt-daffodil-utils"
+ )
+ .settings(
+ javacOptions ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" => Seq("-target", "8")
+ case "2.13" => Seq("-target", "8")
+ case "3" => Seq("-target", "17")
+ }
+ },
+ scalacOptions ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" => Seq(s"--target:jvm-8")
+ case "2.13" => Seq(s"--release", "8")
+ case "3" => Seq(s"--release", "17")
+ }
+ },
+ libraryDependencies ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" => {
+ Seq(
+ // scala-steward:off
+ "org.apache.daffodil" %% "daffodil-japi" % "3.10.0" % "provided",
+ // scala-steward:on
+ "org.scala-lang.modules" %% "scala-collection-compat" % "2.14.0"
+ )
+ }
+ case "2.13" => {
+ Seq(
+ // scala-steward:off
+ "org.apache.daffodil" %% "daffodil-japi" % "3.11.0" % "provided"
+ // scala-steward:on
+ )
+ }
+ case "3" => {
+ Seq(
+ "org.apache.daffodil" %% "daffodil-core" % "4.0.0" % "provided"
Review Comment:
Yeah, this is a bit confusing, I'll add a comment in the code clarifying this.
But to answer your question, because we add `provided` to the dependencies, it means we only compile the sbt-daffodil-utils jars with a specific version of Daffodil. And we expect something else to put Daffodil jars on the classpath at runtime. In this case, that's the DaffodPlugin and the plugin can put whatever Daffodil version it's wants on the classpath. The only requirement is that whatever jars we put on the classpath have the same classes and methods that the jar expects. And because our API doesn't really change, we can actually put any version of Daffodil 3.10.0 or older on the classpath with sbt-daffodil-utils_2.12 and everything will just work. Java doesn't care that it wasn't compiled with the same jar, it just cares that the jars have the same class names and functions that sbt-daffodil-utils expects.
The one gotcha is Daffodil 3.9.0 and older do not have the `compileResource` function, so if we tried to call that at runtime we would get a NoSuchMethodFound exception. That's the purpose of the `daffodilInternalApiVersionMapping`--that essentially tells DaffodilSaver to use `compileSource` instead of `compileResource` on older version of Daffodil.
So as a rule of thumb, if Daffodil adds a new API method, then we need a new daffodilInternalApiVersionMapping for it to be used so it's not used on old versions.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove reflection from DaffodilSaver [daffodil-sbt]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #136:
URL: https://github.com/apache/daffodil-sbt/pull/136#discussion_r2518580035
##########
build.sbt:
##########
@@ -15,53 +15,115 @@
* limitations under the License.
*/
-name := "sbt-daffodil"
-
-organization := "org.apache.daffodil"
-
-version := IO.read((ThisBuild / baseDirectory).value / "VERSION").trim
-
-scalaVersion := "2.12.19"
-
-scalacOptions ++= Seq(
- "-Ywarn-unused:imports"
+val commonSettings = Seq(
+ organization := "org.apache.daffodil",
+ version := IO.read((ThisBuild / baseDirectory).value / "VERSION").trim,
+ scalacOptions ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" | "2.13" =>
+ Seq(
+ "-Werror",
+ "-Ywarn-unused:imports"
+ )
+ case "3" =>
+ Seq(
+ "-no-indent",
+ "-Werror",
+ "-Wunused:imports"
+ )
+ }
+ },
+ scmInfo := Some(
+ ScmInfo(
+ browseUrl = url("https://github.com/apache/daffodil-sbt"),
+ connection = "scm:git:https://github.com/apache/daffodil-sbt"
+ )
+ ),
+ licenses := Seq(License.Apache2),
+ homepage := Some(url("https://daffodil.apache.org")),
+ releaseNotesURL := Some(url(s"https://daffodil.apache.org/sbt/${version.value}/"))
)
-scmInfo := Some(
- ScmInfo(
- browseUrl = url("https://github.com/apache/daffodil-sbt"),
- connection = "scm:git:https://github.com/apache/daffodil-sbt"
+lazy val plugin = (project in file("."))
+ .settings(commonSettings)
+ .settings(
+ name := "sbt-daffodil",
+
+ // SBT plugin settings
+ scalaVersion := "2.12.19",
+ crossSbtVersions := Seq("1.8.0"),
+ scriptedLaunchOpts ++= Seq(
+ "-Xmx1024M",
+ "-Dplugin.version=" + version.value
+ ),
+ scriptedDependencies := {
+ // scripted runs publishLocal for the plugin and its dependencies as part of the
+ // scriptedDependencies task. But the utils subprojects aren't actually dependencies (so
+ // won't be locally published). We still need the util jars locally published so the
+ // scripted tests can find the jars at runtime, so we manually run publishLocal for each
+ // of the utils subprojects as part of the scriptedDependencies task
+ publishLocal.all(ScopeFilter(projects = inProjects(utils.projectRefs: _*))).value
+ scriptedDependencies.value
+ },
+ Test / test := {
+ // run all scripted tasks as part of testing
+ (Compile / scripted).toTask("").value
+ (Test / test).value
+ },
+
+ // Rat check settings
+ ratExcludes := Seq(
+ file(".git"),
+ file("VERSION")
+ ),
+ ratFailBinaries := true
)
-)
-
-licenses := Seq(License.Apache2)
-
-homepage := Some(url("https://daffodil.apache.org"))
-
-releaseNotesURL := Some(url(s"https://daffodil.apache.org/sbt/${version.value}/"))
-
-// SBT Plugin settings
-
-enablePlugins(SbtPlugin)
+ .enablePlugins(SbtPlugin)
+ .aggregate(utils.projectRefs: _*)
-crossSbtVersions := Seq("1.8.0")
-
-scriptedLaunchOpts ++= Seq(
- "-Xmx1024M",
- "-Dplugin.version=" + version.value
-)
-
-// Rat check settings
-
-ratExcludes := Seq(
- file(".git"),
- file("VERSION")
-)
-
-ratFailBinaries := true
-
-Test / test := {
- // run all scripted tasks as part of testing
- (Compile / scripted).toTask("").value
- (Test / test).value
-}
+lazy val utils = (projectMatrix in file("utils"))
+ .settings(commonSettings)
+ .settings(
+ name := "sbt-daffodil-utils"
+ )
+ .settings(
+ javacOptions ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" => Seq("-target", "8")
+ case "2.13" => Seq("-target", "8")
+ case "3" => Seq("-target", "17")
+ }
+ },
+ scalacOptions ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" => Seq(s"--target:jvm-8")
+ case "2.13" => Seq(s"--release", "8")
+ case "3" => Seq(s"--release", "17")
+ }
+ },
+ libraryDependencies ++= {
+ scalaBinaryVersion.value match {
+ case "2.12" => {
+ Seq(
+ // scala-steward:off
+ "org.apache.daffodil" %% "daffodil-japi" % "3.10.0" % "provided",
+ // scala-steward:on
+ "org.scala-lang.modules" %% "scala-collection-compat" % "2.14.0"
+ )
+ }
+ case "2.13" => {
+ Seq(
+ // scala-steward:off
+ "org.apache.daffodil" %% "daffodil-japi" % "3.11.0" % "provided"
+ // scala-steward:on
+ )
+ }
+ case "3" => {
+ Seq(
+ "org.apache.daffodil" %% "daffodil-core" % "4.0.0" % "provided"
Review Comment:
> I'm not suggesting we do this as part of this change set, but can the approach used here be extended to support even older Daffodil versions like 3.7.0, and even 3.4.0 ?
This approach does support even older versions, but is kindof restricted to using APIs that do not have backwards incompatible changes.
For the example above, if Daffodil 3.10.0 had removed `compileSource` and replaced it with `compileResource`, then this approach wouldn't work and you would have to use reflection. Similarly, if 3.8.0 moves a class to a different package, that is a backwards incompatible change and reflection would be required.
The way the think about this is we have three jars:
* sbt-daffodil-utils_2.12 - for 3.10.0 and older
* sbt-daffodil-utils_2.13 - for 3.11.0
* sbt-daffodil-tusil_3 - for 4.0.0 and newer
The Daffodil API class must be written to work for all versions within one of those ranges. If there are backwards incompatible changes in a range, then reflection must be used, likely with classes to abstract away the reflection. New APIs can be used, but conditional logic must be used to avoid using the new APIs when on older versions.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org