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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to