You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "arosien (via GitHub)" <gi...@apache.org> on 2023/07/19 18:45:34 UTC
[GitHub] [daffodil] arosien opened a new pull request, #1057: Tdml gen app
arosien opened a new pull request, #1057:
URL: https://github.com/apache/daffodil/pull/1057
(no comment)
--
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
[GitHub] [daffodil] stevedlawrence commented on pull request #1057: Tdml gen app
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#issuecomment-1693350557
> Some runs printed their error messages in foreign languages (I wonder how that happened?)
For a while we had a few tests that expected error messages that were generated and internationalized by the OS (e.g. "could not open file"). But the tests expected english error messages so would fail on systems that defaulted to another language. These tests were fixed to not look for these specific error messages, but to avoid new tests that might do something similar, a couple of github action jobs were updated to run with LANG set to de_DE and ja_JP.
https://github.com/apache/daffodil/blob/main/.github/workflows/main.yml#L61-L68
--
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] Tdml gen app [daffodil]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1484500882
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/ScalaxbTests.scala:
##########
@@ -0,0 +1,23 @@
+package org.apache.daffodil.tdml
+
+import org.apache.daffodil.tdml.scalaxb.TestSuite
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite =
+ _root_.scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+}
Review Comment:
The frontend is typescript. The backend that communicates with Daffodil is Scala.
--
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] Tdml gen app [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1484381139
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/ScalaxbTests.scala:
##########
@@ -0,0 +1,23 @@
+package org.apache.daffodil.tdml
+
+import org.apache.daffodil.tdml.scalaxb.TestSuite
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite =
+ _root_.scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+}
Review Comment:
> but if you are rewriting the file this stuff has to be preserved
I don't think jaxb/scalaxb has ways to preserve comments/cdata etc. It's all converted to Java objects that don't retain that information.
I think this just needs to be a limitation for people wanting to use jaxb/scalaxb, they need to strip comments prior to unmarshalling, and roundtripping is not guaranteed when marshalling.
--
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] Tdml gen app [daffodil]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#issuecomment-2018784700
> Since I'm re-reading this ticket.... I had a thought. Rather than making the VSCode read/write TDML, which we can see using the scalaxb approach, is hard to make compatible with human authored stuff like CDATA bracketing, comments, etc,, why don't we define this to be different from HA (Human Authored) TDML, can explicitly call it MG (Machine Generated) TDML. This is really just a shift in perspective, but we're giving ourselves the freedom to simply be incompatible with Human-Authored TDML, and we can add features like an explicit 'tdml:comment.' element to overcome the XML-Comments issue.
This is more or less how it is now. Our use cases currently assume MG TDML files, and we've been testing with MG files. I think we've been trying to get basic parsing working with HA files because, in theory, we should be able to load both in. We should be able append/execute any TDML as well, but, to my knowledge, we haven't tested with that yet.
--
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
[GitHub] [daffodil] arosien commented on pull request #1057: Tdml gen app
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#issuecomment-1715917424
FYI the SbtXjcPlugin unfortunately installs itself globally, so it applies to all projects that have xsd schemas in them, hence the need for the `disablePlugins(SbtXjcPlugin)` expressions sprinkled around everywhere except the tdml-lib project.
--
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
[GitHub] [daffodil] mbeckerle commented on a diff in pull request #1057: Tdml gen app
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1327532352
##########
build.sbt:
##########
@@ -211,6 +214,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compliation
Review Comment:
We could have tdml-core.xsd and tdml.xsd which includes tdml-core.xsd.
The jaxb could use only the core, and the full tdml.xsd could add in the missing stuff like defineSchema and provide the root tdml:testSuite element.
--
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] Tdml gen app [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1483348540
##########
project/plugins.sbt:
##########
@@ -26,3 +26,5 @@ addSbtPlugin("org.scoverage" % "sbt-scoverage" % "2.0.9")
addSbtPlugin("com.github.sbt" % "sbt-unidoc" % "0.5.0")
addSbtPlugin("org.scalameta" % "sbt-scalafmt" % "2.5.2")
+
+addSbtPlugin("org.scalaxb" % "sbt-scalaxb" % "1.12.0")
Review Comment:
Does anyone know about licenses for generated code?
It looks like this plugin doesn't add any new dependencies (except for build dependencies, which ASF requires be compatible but do not need to be mentioned in any license/notice files).
But scalaxb does generate code in `daffodil-tdml-lib/src_managed/`. I can't find anything that ASF says about generated code licenses, and scalaxb doesn't mention the license of the code it generates that I can find.
I imagine scalaxb cannot make license claims about the code it generates, since it relies on input which may be licensed differently, so it's probably fine for us to can claim the generated code is ALv2? Maybe one edge case is that the `src_managed/scalaxb.scala` file looks it is "generated" from this file:
https://github.com/eed3si9n/scalaxb/blob/develop/cli/src/main/resources/scalaxb.scala.template
That file isn't really generated at all--it is pretty much a straight copy out of the scalaxb jar, with one minor template variable. I'm not sure you could make the same argument, and maybe this file has the same license as scalaxb, which is MIT. Which is still ASF compatible, but would need a mention in the daffodil-tdml-lib META-INF/LICENSE.
Or can we claim everything it outputs is generated code, regardless of how much changes are made from the templates, and that we can assign whatever license we want? Or should we get clarification from scalaxb about the license of their generated code?
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/ScalaxbTests.scala:
##########
@@ -0,0 +1,23 @@
+package org.apache.daffodil.tdml
+
+import org.apache.daffodil.tdml.scalaxb.TestSuite
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite =
+ _root_.scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+}
Review Comment:
It might be useful to have some more extensive tests that show how to access certain pieces of information (e.g test names, infosets, data, etc.). It would confirm that they can be accessed in reasonable way, and also act as examples on how to use the classes it generates. This would be especially useful to confirm that it has the capabilities that the VSCode plugin will need, since I think that is the use case for this?
For example, you said the `dfdlinfoset` is translated into this:
```scala
case class DfdlInfosetType(var mixed: Seq[scalaxb.DataRecord[Any]] = Nil,
var attributes: Map[String, scalaxb.DataRecord[Any]] = Map.empty) {
def typeValue = attributes("@type").as[Type]
def typeValue_=(_value: Type)(implicit evidence: scalaxb.CanWriteXML[Type]) = attributes += "@type" -> scalaxb.DataRecord(_value)
}
```
That doesn't seem like it has a way to actually access the infoset. Maybe the `Any` record needs to be type cast into something. Showing how that works seems useful and makes sure if we ever change the tdml file in some way that causes changes to the scalaxb code we'll know about it.
--
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] Tdml gen app [daffodil]
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#issuecomment-1930452219
> With the changes to scalaxb, is this ready for re-review?
Yes! See some [infoset translation](https://github.com/apache/daffodil/pull/1057#discussion_r1480298851) and here is the top-level test suite:
```scala
case class TestSuite(var testSuiteChoices: Seq[scalaxb.DataRecord[Any]] = Nil,
var attributes: Map[String, scalaxb.DataRecord[Any]] = Map.empty) {
def suiteName = attributes.get("@suiteName") map { _.as[String]}
def suiteName_=(_value: Option[String])(implicit evidence: scalaxb.CanWriteXML[Option[String]]) = attributes += "@suiteName" -> scalaxb.DataRecord(_value)
def ID = attributes.get("@ID") map { _.as[String]}
def ID_=(_value: Option[String])(implicit evidence: scalaxb.CanWriteXML[Option[String]]) = attributes += "@ID" -> scalaxb.DataRecord(_value)
def description = attributes.get("@description") map { _.as[String]}
def description_=(_value: Option[String])(implicit evidence: scalaxb.CanWriteXML[Option[String]]) = attributes += "@description" -> scalaxb.DataRecord(_value)
def defaultRoundTrip = attributes.get("@defaultRoundTrip") map { _.as[RoundTripType]}
def defaultRoundTrip_=(_value: Option[RoundTripType])(implicit evidence: scalaxb.CanWriteXML[Option[RoundTripType]]) = attributes += "@defaultRoundTrip" -> scalaxb.DataRecord(_value)
def defaultValidation = attributes.get("@defaultValidation") map { _.as[ValidationType]}
def defaultValidation_=(_value: Option[ValidationType])(implicit evidence: scalaxb.CanWriteXML[Option[ValidationType]]) = attributes += "@defaultValidation" -> scalaxb.DataRecord(_value)
def defaultConfig = attributes.get("@defaultConfig") map { _.as[String]}
def defaultConfig_=(_value: Option[String])(implicit evidence: scalaxb.CanWriteXML[Option[String]]) = attributes += "@defaultConfig" -> scalaxb.DataRecord(_value)
def defaultImplementations = attributes.get("@defaultImplementations") map { _.as[Seq[ImplementationItem]]}
def defaultImplementations_=(_value: Option[Seq[ImplementationItem]])(implicit evidence: scalaxb.CanWriteXML[Option[Seq[ImplementationItem]]]) = attributes += "@defaultImplementations" -> scalaxb.DataRecord(_value)
}
```
Users of these data structures will need to build up, for example, `ParserTestCaseType` values, serialize them via `scalaxb.toXML`, and push them into the `testSuiteChoices` member.
--
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
[GitHub] [daffodil] michael-hoke commented on a diff in pull request #1057: Tdml gen app
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1327536811
##########
build.sbt:
##########
@@ -211,6 +214,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compliation
Review Comment:
This change looks like it would work for the extension. There was a change related to namespaces that I'm not thrilled about, but that can more than likely be fixed with a couple more code changes...
We are not planning to support internal schemas in the extension. One of the features we are looking to implement is to provide the ability to package the TDML schema and the DFDL schema in a zip archive, which directly conflicts with having an internal schema, so I don't think we'd be looking at that even for the longer term.
We currently don't support configs in the extension at all, as far as I know... Even if we were to add them in, there's no reason for us to not use them externally, as we're already using the schema.
--
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
[GitHub] [daffodil] stevedlawrence commented on a diff in pull request #1057: Tdml gen app
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1338491517
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
Review Comment:
Can you combine these into a single setting, i.e.:
```scala
xjcCommandLine ++= Seq(
"-nv",
"-p",
"org.apache.daffodil.tdml",
),
```
That's the standard for things like this, especially since it makes it more clear the order is important.
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ file("daffodil-lib/src/main/resources/org/apache/daffodil/xsd/tdml-core.xsd"),
+ ),
+ Compile / doc / sources := Seq(file("")),
Review Comment:
Is this necessary? I think this effectively disables scala/javadocs and the -doc.jar for projects that use this setting (i.e. tdml-lib currently). Not sure why doc sources would affect xjc.
##########
daffodil-lib/src/main/resources/org/apache/daffodil/xsd/tdml-core.xsd:
##########
@@ -0,0 +1,325 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+
+<xsd:schema xmlns:xsd="http://www.w3.org/2001/XMLSchema"
+ xmlns:xs="http://www.w3.org/2001/XMLSchema"
+ xmlns="http://www.w3.org/2001/XMLSchema"
+ targetNamespace="http://www.ibm.com/xmlns/dfdl/testData"
+ xmlns:tns="http://www.ibm.com/xmlns/dfdl/testData"
+ elementFormDefault="unqualified"
+ xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/"
+ xmlns:daf="urn:ogf:dfdl:2013:imp:daffodil.apache.org:2018:ext"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
+
+ <xs:attribute name="tutorialInclude" type="xs:string" fixed="no"/>
+
+<!--
+ This element's type is a stub that satisfies the need for xhtml5 validation as part of TDML files.
+
+ It is just a wildcard with skip validation in it. So it's not really validating anything.
+
+ Users of TDML creating tutorials should paste in known-correct xhtml5 as the contents of the
+ tutorial elements.
+-->
+ <xs:element name="tutorial" substitutionGroup="tns:testSuiteChoices">
+ <xs:complexType mixed="true">
+ <xs:sequence>
+ <xs:any namespace="##any" processContents="skip" minOccurs="0" maxOccurs="unbounded"/>
+ </xs:sequence>
+ <xs:anyAttribute namespace="##any" processContents="skip"/>
+ </xs:complexType>
+ </xs:element>
+
+ <!-- IBM uses this namespace http://www.ibm.com/xmlns/dfdl/testData -->
Review Comment:
This comment makes it looks like it's talking about testSuiteChoices but it's not. I think it was intended to be about the testSuite element below, which is in the tdml namespace?
Also, can you also add a comment explaining testSuiteChoices, substituion groups, and why we need it?
My understanding is anything that references this element as a substituionGroup can appear where this element is referenced? And those elements are `tutorial`, `parserTestCase`, and `unparserTest` case in the context of `tdml-core.xsd`, but additionally `defineSchema` and `defineConfig` in the context of `tdml.xsd`, which includes this file?
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
Review Comment:
Can we spend some time trying to figure out the reproducible builds issue with jaxb generated files that we saw with vscode? Daffodil currently has reproducible builds for jars (the other zip/tar/etc release artifacts have timestamp issues) and I would like to keep it that way. There's some discussions in the ASF about automating things like release signing, but that requires reproducible builds.
I see there's a `-no-header` xjc option that prevents a header comment containing a timestamp from being added. I wouldn't expect a comment to affect the .class file, so I'm not optimistic it would fix it, but at least it's one less piece of randomness to consider.
##########
daffodil-lib/src/main/resources/org/apache/daffodil/xsd/tdml-core.xsd:
##########
@@ -0,0 +1,325 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+
+<xsd:schema xmlns:xsd="http://www.w3.org/2001/XMLSchema"
+ xmlns:xs="http://www.w3.org/2001/XMLSchema"
+ xmlns="http://www.w3.org/2001/XMLSchema"
+ targetNamespace="http://www.ibm.com/xmlns/dfdl/testData"
+ xmlns:tns="http://www.ibm.com/xmlns/dfdl/testData"
+ elementFormDefault="unqualified"
+ xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/"
+ xmlns:daf="urn:ogf:dfdl:2013:imp:daffodil.apache.org:2018:ext"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
+
+ <xs:attribute name="tutorialInclude" type="xs:string" fixed="no"/>
+
+<!--
+ This element's type is a stub that satisfies the need for xhtml5 validation as part of TDML files.
+
+ It is just a wildcard with skip validation in it. So it's not really validating anything.
+
+ Users of TDML creating tutorials should paste in known-correct xhtml5 as the contents of the
+ tutorial elements.
+-->
+ <xs:element name="tutorial" substitutionGroup="tns:testSuiteChoices">
+ <xs:complexType mixed="true">
+ <xs:sequence>
+ <xs:any namespace="##any" processContents="skip" minOccurs="0" maxOccurs="unbounded"/>
+ </xs:sequence>
+ <xs:anyAttribute namespace="##any" processContents="skip"/>
+ </xs:complexType>
+ </xs:element>
+
+ <!-- IBM uses this namespace http://www.ibm.com/xmlns/dfdl/testData -->
+ <element name="testSuiteChoices" abstract="true"/>
+
+ <element name="testSuite">
+ <complexType>
+ <sequence>
+ <xsd:element minOccurs="0" maxOccurs="unbounded" ref="tns:testSuiteChoices"></xsd:element>
Review Comment:
For consistency with the rest of the file, can you remove the xsd prefix and close with `/>`
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
Review Comment:
Yeah, please define these in `project/Dependencies.scala`.
Also do we need all of these for Daffodil, or are some only needed for generating the .java files? I'd assume we just need impl, and the other two are for generating XML? But maybe that's not the case?
Also, if we are adding new dependencies, we must update the LICENSE/NOTICE files (assuming these dependencies are ASF compatible).
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
Review Comment:
I would suggest adding this logic directly to the `xjcJvmOpts` below, e.g
```scala
xjcJvmSettings ++= {
if (scala.util.Properties.isJavaAtLeast("17"))
Seq(
"--add-opens",
"java.base/java.lang=ALL-UNNAMED",
)
else Seq()
},
```
Otherwise it could be easily confused that these options are added to a JVM doing compilation or running tests or something, which isn't the case. These are only for running the xjc code generator.
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
Review Comment:
Actually, looking a the plugin, the `xjcLibs` aren't compile or test. They are actually added to a custom "xjc-tool" ivy configuration that is used to download all the jars and fork xjc with a main class and arguments. So this is actually correct.
That said, the plugin defines a default value of xjcLibs that is the same as this, so this setting is just duplication and can be removed. This is only needed if we want to use a different version of xjc.
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
Review Comment:
I think it depends on where the reflection is happening. If it's only when executing xjc to to generate the java files, then it should be fine to only be here. If the reflection also happens when parsing XML to instantiate the Java classes it's generated, then it probably is needed at runtime. From looking at the xjcPlugin, my guess is that these options are only needed to during build, so should be fine.
That said, we should add tests that actually try to parse a TDML file the fields so we know this works on different JVMs without additional optinos.
--
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
[GitHub] [daffodil] arosien commented on a diff in pull request #1057: Tdml gen app
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1339294952
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
Review Comment:
I also accidentally type "optinos" all the time :)
--
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] Tdml gen app [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1342851003
##########
project/Dependencies.scala:
##########
@@ -63,4 +63,17 @@ object Dependencies {
lazy val exi = Seq(
"com.siemens.ct.exi" % "exificient" % "1.0.7",
)
+
+ lazy val xjc =
Review Comment:
The xjc dependencies are fine, but CDDL is category B we do need to mention CDDL so needs to be mentioned in a readme.
We already depend on scala-xml and scala-parser-combinators. LGPL is category X and is not allowed by ASF, but I don't think it's a mandatory dependency. Dispatch http looks like an http communication library library that scalaxb can generate code to support, but it looks optional to me. For example, I think it can be disabled with:
```scala
Compile / scalaxb / scalaxbGenerateDispatchClient := false
```
Similarly scalaxb also supports http4s and gigahorse, which I think are also optional with similar keys to disabled (disabled by default). I'm also not sure where scalaxb actually uses the jaxb-api. I don't see it mentioned in the generated code. Maybe that's some an optional dependency as well?
But either way I think we're fine, we just need to make sure to update LICENSE/NOTICE appropriately if CDDL is included.
--
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
[GitHub] [daffodil] tuxji commented on pull request #1057: Tdml gen app
Posted by "tuxji (via GitHub)" <gi...@apache.org>.
tuxji commented on PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#issuecomment-1693334746
I saw that this PR needed a maintainer's approval to run CI on it, so I looked over the changes and let CI run but unfortunately the changes don't seem to work on GitHub runner machines. Some runs printed their error messages in foreign languages (I wonder how that happened?), but one run's English [errors](https://github.com/apache/daffodil/actions/runs/5729863413/job/16213034094?pr=1057) seem to indicate that DFDL additions to XML Schema confuse xjc. Did you run xjc successfully on your local machine?
--
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] Tdml gen app [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1484450953
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/ScalaxbTests.scala:
##########
@@ -0,0 +1,23 @@
+package org.apache.daffodil.tdml
+
+import org.apache.daffodil.tdml.scalaxb.TestSuite
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite =
+ _root_.scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+}
Review Comment:
@arosien what is the use case here. I had thought the purpose of this scalaxb stuff was to enable adding TDML support to the VSCode extension, and that would mean both reading and writing out TDML.
--
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] Tdml gen app [daffodil]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1484667731
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/ScalaxbTests.scala:
##########
@@ -0,0 +1,23 @@
+package org.apache.daffodil.tdml
+
+import org.apache.daffodil.tdml.scalaxb.TestSuite
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite =
+ _root_.scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+}
Review Comment:
> it should be the case that someone with a pre-existing non-generated TDML file would want to add a generated test case to the file, in which case the app using this lib should either raise an error if there are comments, or warn that we're going to trash them.
Agreed that we should, but the immediate use case is for generated files. The hope is that people with custom non-generated files that don't work out of the box create an issue and we can deal with the edge cases that way. I think having some sort of message/warning if comments (or anything else that we can't currently handle) are detected is a really good starting point that we're not currently doing - a ticket to track this would probably be useful.
> We currently launch the daffodil debugger to create and append TDML files
This isn't quite as true as it was before. With the GUI implemented, we've integrated the TDML generate/append more into the actual DFDL parse. By default, a TDML file is generated for every parse - it's not a separate action anymore. The generate/append actions work off of the last parse that was ran instead of requiring the user to do another parse.
So, when the user hits the generate/append button, it's all handled on the typescript side now. Even though that's not the complete picture, it's a step towards decoupling the TDML from the backend if that's a direction we eventually move to... Although, I don't see how we can do that as a TDML test case (or at least for how we're using it) is implicitly tied to a DFDL parse. Maybe there's a path to take if we add a way to have the backend send information back to the frontend after a parse?
--
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
[GitHub] [daffodil] michael-hoke commented on a diff in pull request #1057: Tdml gen app
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1326475164
##########
build.sbt:
##########
@@ -211,6 +214,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compliation
Review Comment:
I just did a build, removing every xsd file other than tdml.xsd and removing the reference to bindings.xjb. I get a lot of undefined element messages. The XJC plugin seems to require every file whose definitions we import to go through the JAXB process as well.
--
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
[GitHub] [daffodil] mbeckerle commented on a diff in pull request #1057: Tdml gen app
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1337710486
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
Review Comment:
Most of our dependencies are in project/Dependencies.scala.
Should these be there as well for consistency?
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
Review Comment:
We do compile daffodil but then run it with a variety of JVM versions, so it's possible this needs to be a runtime thing.
E.g., are these options even allowed on earlier JVMs?
@stevedlawrence please look this over.
##########
build.sbt:
##########
@@ -241,6 +279,10 @@ lazy val commonSettings = Seq(
resourceManaged := baseDirectory.value / "resource_managed",
libraryDependencies ++= Dependencies.common,
testOptions += Tests.Argument(TestFrameworks.JUnit, "-q", "--verbosity=1"),
+ // SbtXjcPlugin is enabled globally, and will, be default, attempt to build xsd schemas if any exist in a project
Review Comment:
typo "by" default
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
Review Comment:
Please label which of these are the runtime dependencies, and which are the compile-time dependencies. (I'm assuming there is that separation).
--
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
[GitHub] [daffodil] arosien commented on a diff in pull request #1057: Tdml gen app
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1339295822
##########
build.sbt:
##########
@@ -241,6 +279,10 @@ lazy val commonSettings = Seq(
resourceManaged := baseDirectory.value / "resource_managed",
libraryDependencies ++= Dependencies.common,
testOptions += Tests.Argument(TestFrameworks.JUnit, "-q", "--verbosity=1"),
+ // SbtXjcPlugin is enabled globally, and will, be default, attempt to build xsd schemas if any exist in a project
Review Comment:
```suggestion
// SbtXjcPlugin is enabled globally, and will, by default, attempt to build xsd schemas if any exist in a project
```
--
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
[GitHub] [daffodil] arosien commented on pull request #1057: Tdml gen app
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#issuecomment-1741266255
Yay, all tests pass @NolanMatt. Now on to linting, etc.
--
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] Tdml gen app [daffodil]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1484479897
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/ScalaxbTests.scala:
##########
@@ -0,0 +1,23 @@
+package org.apache.daffodil.tdml
+
+import org.apache.daffodil.tdml.scalaxb.TestSuite
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite =
+ _root_.scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+}
Review Comment:
The use case for the extension is to enable reading and writing of **generated** TDML files, so the comments aren't an immediate issue for us. We don't expect users to have or use non-generated TDML files in order to use the features in the extension.
--
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] Tdml gen app [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1484660294
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/ScalaxbTests.scala:
##########
@@ -0,0 +1,23 @@
+package org.apache.daffodil.tdml
+
+import org.apache.daffodil.tdml.scalaxb.TestSuite
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite =
+ _root_.scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+}
Review Comment:
> We currently launch the daffodil debugger to create and append TDML files, which is IMO rather excessive, given that we could alternatively read and write XML files from the typescript extension itself.
Yeah, a pure typescript implementation has some clear advantages. That might be a better approach.
Another thought, there have been some discussions about an alternative DFDL syntax since there's a lot of dislike of XML. Maybe it's also worth considering a different test format? Maybe json (or something similar) would be sufficient, which I believe typescript has much better support for. That probably requires an even longer discussion and is maybe a more long term thing, if even done, but possibly something worth considering.
--
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] Tdml gen app [daffodil]
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1538226635
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/scalaxb/ScalaxbTests.scala:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.tdml.scalaxb
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReadingNoTests(): Unit = {
+ val testSuite =
+ scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+
+ // currently failing, not sure why scalaxb isn't handling comments. is the xsd too strict and somehow excludes allowing comments?
Review Comment:
Also @mbeckerle helped in a previous comment https://github.com/apache/daffodil/pull/1057#discussion_r1483645379 🙇
--
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
[GitHub] [daffodil] mbeckerle commented on a diff in pull request #1057: Tdml gen app
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1332196155
##########
build.sbt:
##########
@@ -211,6 +212,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compilation
+ file("daffodil-lib/src/main/resources/org/apache/daffodil/xsd/tdml.xsd"),
Review Comment:
ok. Specifically tag me for re-review then when you've done that. Thx.
--
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
[GitHub] [daffodil] mbeckerle commented on a diff in pull request #1057: Tdml gen app
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1332168592
##########
build.sbt:
##########
@@ -211,6 +212,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compilation
+ file("daffodil-lib/src/main/resources/org/apache/daffodil/xsd/tdml.xsd"),
Review Comment:
Use this trick to make the lines less redundant
```
val daflib = "daffodil-lib/src/main/resources/org/apache/daffodil/xsd/"
file(daflib + "tdml.xsd")
```
--
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
[GitHub] [daffodil] stevedlawrence commented on a diff in pull request #1057: Tdml gen app
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1327458858
##########
build.sbt:
##########
@@ -211,6 +214,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compliation
Review Comment:
The content of defineSchema is used for testing schemas emedded in the TDML file itself.
I guess excluding that from jaxb would be fine if the extension only supports external schemas, which is how most real schema projects are used. So maybe that's a fine limitation? Same probably goes for he defineConfig section?
The most important stuff is probably the test cases with inputs and expected outputs.
Is there a plan so we don't have to maintain two separate tdml.xsd files that are 90% the same except for this small patch?
--
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
[GitHub] [daffodil] arosien commented on a diff in pull request #1057: Tdml gen app
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1326188730
##########
build.sbt:
##########
@@ -241,6 +305,11 @@ lazy val commonSettings = Seq(
resourceManaged := baseDirectory.value / "resource_managed",
libraryDependencies ++= Dependencies.common,
testOptions += Tests.Argument(TestFrameworks.JUnit, "-q", "--verbosity=1"),
+ // SbtXjcPlugin is enabled globally, even though we only explictly enable it for the daffodil-tdml project
+ // It will, by default, attempt to build xsd schemas if any exist in a project. These lines overwrite that
+ // behavior and tell it that there are no xsd schemas to build.
Review Comment:
```suggestion
// SbtXjcPlugin is enabled globally, and will, by default, attempt to build xsd schemas if any exist in a project.
// These lines overwrite that and tell it that there are no xsd schemas to build.
```
--
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
[GitHub] [daffodil] mbeckerle commented on a diff in pull request #1057: Tdml gen app
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1323206772
##########
build.sbt:
##########
@@ -60,6 +61,7 @@ lazy val macroLib = Project("daffodil-macro-lib", file("daffodil-macro-lib"))
.disablePlugins(OsgiCheckPlugin)
lazy val propgen = Project("daffodil-propgen", file("daffodil-propgen"))
+ .disablePlugins(SbtXjcPlugin)
Review Comment:
Can this disable just become part of commonSettings, then the only clutter would be the one enableSettings for tdml?
--
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
[GitHub] [daffodil] michael-hoke commented on a diff in pull request #1057: Tdml gen app
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1340443578
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
Review Comment:
If two items in all imported schemas (elements, attributes, etc.) share a name, JAXB will attempt to create a binding for them at the same Java file location, which, obviously, won't work, so there needs to be a way to tell it how to rename one of them. With the SbtXjcPlugin, that is the bindings.xjb file. With the scalaxb plugin, I could not find a way to do this outside of refactoring the schemas so that everything has a unique name or by submitting a patch to the plugin to support a bindings file... Although, the tdml-core refactor also resolves this as long as we only want JAXB bindings for that subset of the TDML schema.
If you are going to eventually integrate the JAXB bindings into the TDMLRunner, I imagine that would require generating bindings for the base DFDL schemas, which do have clashing names in a few places. I'm not suggesting that we make any changes at this point - just want to point it out so that this isn't a surprise if/when you get to this point.
--
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] Tdml gen app [daffodil]
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1484828158
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/ScalaxbTests.scala:
##########
@@ -0,0 +1,23 @@
+package org.apache.daffodil.tdml
+
+import org.apache.daffodil.tdml.scalaxb.TestSuite
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite =
+ _root_.scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+}
Review Comment:
> So, when the user hits the generate/append button, it's all handled on the typescript side now.
ah, i forgot about that, thanks @michael-hoke
--
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] Tdml gen app [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1484498937
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/ScalaxbTests.scala:
##########
@@ -0,0 +1,23 @@
+package org.apache.daffodil.tdml
+
+import org.apache.daffodil.tdml.scalaxb.TestSuite
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite =
+ _root_.scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+}
Review Comment:
Can VS Code extensions use Java/Scala libraries? I thought everything was typescript?
--
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
[GitHub] [daffodil] NolanMatt commented on pull request #1057: Tdml gen app
Posted by "NolanMatt (via GitHub)" <gi...@apache.org>.
NolanMatt commented on PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#issuecomment-1660478307
@arosien I updated it to only apply to the daffodil-tdml-lib portion of the build, but still getting errors, my new thought is that on the xjcCommandLine it's referencing org.apache.daffodil.tdml which seems to me like a bit of a circular reference that might not resolve since this is the project we're building it in. I'm not sure though, just seems a bit off to me. Any ideas?
--
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
[GitHub] [daffodil] michael-hoke commented on a diff in pull request #1057: Tdml gen app
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1326291623
##########
build.sbt:
##########
@@ -211,6 +214,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compliation
Review Comment:
Double checked - it clashes a lot with XMLSchema_for_DFDL.xsd - about 50 additional entries into the bindings.xjb file would be required.
--
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
[GitHub] [daffodil] mbeckerle commented on a diff in pull request #1057: Tdml gen app
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1326462641
##########
build.sbt:
##########
@@ -211,6 +214,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compliation
Review Comment:
I think you should only need to generate objects for:
tdml.xsd
The challenge is that some TDML files embed mini-schemas inside using tdml:defineSchema.
That can hold anything one would find inside a DFDL schema, so.... not sure how to cope with that without dragging all of XMLSchema_for_DFDL.xsd
These other schema files should not be needed nor used:
dafint.xsd
dafext.xsd
dfdlx.xsd
datatypes.dtd
xml.xsd
XMLSchema.dtd
XMLSchema.xsd
And hopefully not these depending on the tdml:defineSchema issue:
XMLSchema_for_DFDL.xsd
DFDL_part?_*.xsd - 3 files match this (used by XMLSchema_for_DFDL.xsd)
--
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
[GitHub] [daffodil] arosien commented on a diff in pull request #1057: Tdml gen app
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1326190308
##########
build.sbt:
##########
@@ -17,6 +17,7 @@
import scala.collection.immutable.ListSet
+import com.github.retronym.sbtxjc.SbtXjcPlugin
Review Comment:
```suggestion
```
--
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
[GitHub] [daffodil] arosien commented on a diff in pull request #1057: Tdml gen app
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1326180475
##########
build.sbt:
##########
@@ -143,7 +144,9 @@ lazy val sapi = Project("daffodil-sapi", file("daffodil-sapi"))
lazy val tdmlLib = Project("daffodil-tdml-lib", file("daffodil-tdml-lib"))
.dependsOn(macroLib % "compile-internal", lib, io, io % "test->test", slf4jLogger % "test")
+ .enablePlugins(SbtXjcPlugin)
Review Comment:
```suggestion
```
Since the plugin applies globally, this isn't needed.
--
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
[GitHub] [daffodil] mbeckerle commented on a diff in pull request #1057: Tdml gen app
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1326652042
##########
build.sbt:
##########
@@ -211,6 +214,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compliation
Review Comment:
I suggest maybe this is an approach that could work:
- make a variant of tdml.xsd which omits the tdml:defineSchema element.
- plan to add back some manual implementation for that object.
So you need to:
Comment out all the includes/imports.
Comment out line 64 (element ref to tns:defineSchema).
Comment out all other uses of "defineSchema" and the definition of it.
Will your tool to generate code still work then? That *should* eliminate pulling in all that other stuff and all the excess definitions.
--
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
[GitHub] [daffodil] michael-hoke commented on a diff in pull request #1057: Tdml gen app
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1332174417
##########
build.sbt:
##########
@@ -211,6 +212,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compilation
+ file("daffodil-lib/src/main/resources/org/apache/daffodil/xsd/tdml.xsd"),
Review Comment:
I'd like to give the splitting out of the tdml schema into tdml + tdml-core a try before we merge this. I'll leave it as a separate commit until after you and Steve have a chance to review it so we can easily back it out if you decide it's something you don't want.
--
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
[GitHub] [daffodil] michael-hoke commented on pull request #1057: Tdml gen app
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#issuecomment-1732126495
@mbeckerle @stevedlawrence
I refactored the local parts of the TDML schema into tdml-core.xsd. Other than what I showed earlier in the patch file, the only other notable change was to replace the `choice` in `testSuite` with a `substitutionGroup`.
Everything appears to have been properly separated - the generated JAXB classes don't allow the internal schema/configs in `testSuite`, but the compile/test process is still working for me locally.
I will squash once we decide whether this is a direction we want to continue in.
--
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
[GitHub] [daffodil] arosien commented on a diff in pull request #1057: Tdml gen app
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1326327683
##########
build.sbt:
##########
@@ -211,6 +214,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compliation
Review Comment:
Ah I see, re: `xs:include` wonkiness. Yeah I was hoping to use the default `Compile / xjc / sources` (https://github.com/sbt/sbt-xjc/blob/master/src/main/scala/com/github/retronym/sbtxjc/SbtXjcPlugin.scala#L64).
The only downside is that if there are any file-level xsd additions or deletions then the build file needs to be adjusted.
--
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
[GitHub] [daffodil] arosien commented on a diff in pull request #1057: Tdml gen app
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1339338337
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
Review Comment:
I thought the plugin defined the same set of libs, but the activation jar was [only added recently](https://github.com/sbt/sbt-xjc/commit/2178c47e50020dd09ad55781c932282250b8d752) (2021!) but hasn't been released yet. (Latest release is 0.10, which doesn't even have a tag.)
I'm testing reducing the additional library mentions in this settings block.
--
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
[GitHub] [daffodil] stevedlawrence commented on a diff in pull request #1057: Tdml gen app
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1340414791
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
Review Comment:
> duplicate names for items in the schemas.
Can you clarify what this means? I'm not sure I understand.
I'm also not against the tdml-core split. It would just be a plus if we didn't need it with scalaxb, but I guess we do.
Regardless of xjc vs scalaxb, I prefer tdml-core over bindings.xjb.
--
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
[GitHub] [daffodil] stevedlawrence commented on a diff in pull request #1057: Tdml gen app
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1340036645
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
Review Comment:
Oof, I didn't notice it that change is two years old without a release. I guess we could just do this in that case to just fix the issue:
```scala
xjcLibs += "javax.activation" % "activation" % "1.1.1",
```
Keeps things a little less verbose. And I doubt we have to worry about he plugin being updated and breaking things. Doubt there will be another release anytime soon.
Alternatively, has anyone looked at [scalaxb](https://github.com/eed3si9n/scalaxb)? It looks like it's created by the main SBT maintainer, so imagine it will continue to be maintained for a while and with SBT best practices.
I made changes this PR to use scalaxb instead of xjc and it was pretty straightforward. This was all I had to change to this PR:
```scala
lazy val tdmlLib = Project("daffodil-tdml-lib", file("daffodil-tdml-lib"))
.enablePlugins(ScalaxbPlugin)
.dependsOn(macroLib % "compile-internal", lib, io, io % "test->test", slf4jLogger % "test")
.settings(commonSettings)
.settings(scalaxbSettings)
lazy val scalaxbSettings = Seq(
Compile / scalaxb / scalaxbPackageName := "org.apache.daffodil.tdml.scalaxb",
Compile / scalaxb / sources := Seq(
(lib / Compile / resourceDirectory).value / "org/apache/daffodil/xsd/tdml-core.xsd",
)
)
```
Nicely, it doesn't auto enable so it can be enabled for only the one project we need without the `sources := Seq()` hack.
The only issue I ran into is the generator adds imports that aren't used, which Daffodil treats as an error. But disabling fatal warnings, the generated code compiles fine. There's probably a way to fix this without disabling warnings. And if not, I imagine scalaxb would accept a patch and get a release out quickly.
I also noticed it generates immuatable case classes. But it does have a `scalaxbGenerateMutable` setting to make the case members `var`s if that's preferred
Plus, I imagine it is probably more straightforward to move our existing TDML runner functionality into those generated classes since it's already scala code, which would be nice. It would probably reduce boilerplate in our existing TDML runner lib, and make it so you could run tests without having to serialize wih saxb and then reload with the TDML runner. That's a far off goal, but scalaxb makes that more possible I think.
--
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] Tdml gen app [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1342948977
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
Review Comment:
I am actually not able to reproduce the non-reproducable-build issue, either with or without the `-no-header` flag. So I'm not convinced that was the actual cause (which I think makes sense--I think we agree that comments shouldn't affect the class file).
I do see the different header dates and sometimes different ordering of `@links`, but none of that seems to cause the class files to be different for me. Seems like there must be something else going on. I can't reproduce it so I'm not sure what the cause would be...
The fact that sometimes `@links` show up in a different order or in different files makes me feel like maybe there is some source of randomness or ordering that *sometimes* does cause a change to the class file. Maybe no-header fixes that as a side-effect or maybe it's just not always reproducible, like I'm having trouble with now?
--
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] Tdml gen app [daffodil]
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1532873371
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/scalaxb/ScalaxbTests.scala:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.tdml.scalaxb
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReadingNoTests(): Unit = {
+ val testSuite =
+ scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+
+ // currently failing, not sure why scalaxb isn't handling comments. is the xsd too strict and somehow excludes allowing comments?
Review Comment:
This test fails due to XML comments in the TDML file. It seems fairly normal to have comments in XML, so this seems like a bad blocker for the use of scalaxb. But as noted, perhaps this is due to an overly strict XSD file? Is that possible?
--
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] Tdml gen app [daffodil]
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1484639668
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/ScalaxbTests.scala:
##########
@@ -0,0 +1,23 @@
+package org.apache.daffodil.tdml
+
+import org.apache.daffodil.tdml.scalaxb.TestSuite
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite =
+ _root_.scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+}
Review Comment:
> The use case for the extension is to enable reading and writing of generated TDML files, so the comments aren't an immediate issue for us. We don't expect users to have or use non-generated TDML files in order to use the features in the extension.
I don't quite think this is true. We happen to most often handle completely generated TDML files, but it should be the case that someone with a pre-existing non-generated TDML file would want to add a generated test case to the file, in which case the app using this lib should either raise an error if there are comments, or warn that we're going to trash them.
> @arosien what is the use case here. I had thought the purpose of this scalaxb stuff was to enable adding TDML support to the VSCode extension, and that would mean both reading and writing out TDML.
It's both reading and writing in the daffodil-vscode project, but I don't think that matters for the changes in daffodil proper. We can document the limitation with comments somewhere and downstream projects can choose to raise an error when encountering comments, filter them out, etc.
--
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] Tdml gen app [daffodil]
Posted by "shanedell (via GitHub)" <gi...@apache.org>.
shanedell commented on PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#issuecomment-2018671523
@stevedlawrence I know there was a discuss out for releasing Daffodil 3.7.0. Would this possibly be a PR we could get in before the actual release or where do you stand on what still needs done for this to meet requirements?
--
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
[GitHub] [daffodil] arosien commented on a diff in pull request #1057: Tdml gen app
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1326182294
##########
build.sbt:
##########
@@ -211,6 +214,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compliation
Review Comment:
```suggestion
// Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compilation
```
speeling
--
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
[GitHub] [daffodil] michael-hoke commented on a diff in pull request #1057: Tdml gen app
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1326332161
##########
build.sbt:
##########
@@ -211,6 +214,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compliation
Review Comment:
Right. Unless there's a way to use the default and then explicitly exclude XMLSchema.xsd but that isn't perfect either... I'd be okay with either way.
--
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] Tdml gen app [daffodil]
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1483625832
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/ScalaxbTests.scala:
##########
@@ -0,0 +1,23 @@
+package org.apache.daffodil.tdml
+
+import org.apache.daffodil.tdml.scalaxb.TestSuite
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite =
+ _root_.scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+}
Review Comment:
While trying to update the tests, I tried to parse IPv4.tdml from another module's tests and discovered scalaxb does not properly ignore XML comments when parsing, failing to parse. This is likely a bug in the generated parsing code of scalaxb, and I'll file an issue, but it may be a blocker for us. What do you think if this is the case @stevedlawrence @mbeckerle?
--
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] Tdml gen app [daffodil]
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#issuecomment-1934915794
> I think this could just use a bit more extensive test, and maybe some license questions?
>
> Also, it looks like you squashed into a single commit, but it still needs to be rebased to fix conflicts.
>
> Also, can you update the commit message to include a bug reference, right now it has a broken link. I think [DAFFODIL-2830](https://issues.apache.org/jira/browse/DAFFODIL-2830) is the right bug?
I don't know why it says build.sbt has conflicts, locally I don't see them. I'll poke around more.
Will update the bug reference.
--
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] Tdml gen app [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#issuecomment-1930057971
With the changes to scalaxb, is this ready for re-review?
--
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] Tdml gen app [daffodil]
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1480298851
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/JaxbTests.scala:
##########
@@ -0,0 +1,20 @@
+package org.apache.daffodil.tdml
+
+import org.junit.Assert._
+import org.junit.Test
+
+import javax.xml.bind.JAXBContext
+
+class JaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite = JAXBContext
+ .newInstance(classOf[TestSuite])
+ .createUnmarshaller()
+ .unmarshal(getClass.getClassLoader().getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"))
+ .asInstanceOf[TestSuite]
+
+ assertNotNull(testSuite)
+ assertEquals("dpaext", testSuite.suiteName)
Review Comment:
`dfdlinfoset` is translated into:
```scala
case class DfdlInfosetType(var mixed: Seq[scalaxb.DataRecord[Any]] = Nil,
var attributes: Map[String, scalaxb.DataRecord[Any]] = Map.empty) {
def typeValue = attributes("@type").as[Type]
def typeValue_=(_value: Type)(implicit evidence: scalaxb.CanWriteXML[Type]) = attributes += "@type" -> scalaxb.DataRecord(_value)
}
```
--
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] Tdml gen app [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1532973954
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/scalaxb/ScalaxbTests.scala:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.tdml.scalaxb
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReadingNoTests(): Unit = {
+ val testSuite =
+ scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+
+ // currently failing, not sure why scalaxb isn't handling comments. is the xsd too strict and somehow excludes allowing comments?
Review Comment:
I'mt not sure XSD can allow or disallow comments, I think they're just baked into XML. So it probably means scalaxb just can't handle comments.
Ideally, we could open a ticket with scalaxb to see if they could add the ability to ignored comments.
Alternatively, and maybe an interim solution until scalaxb ignores comments, users of `scalaxb.fromXML[TestSuite](...)` could filter the NodeSeq to remove comments before passing it to scalaxb. Scala-xml has a [Transformer API](https://github.com/scala/scala-xml/tree/main/shared/src/main/scala/scala/xml/transform) that should make it not too painful. It's definitely annoying to have to worry about that, but I'm not sure it's necessarily a blocker as long as we document it. E.g.
```scala
val node = scala.xml.XML.load(...)
val noComments = CustomRemoveCommentsTransformer.transform(node)
val testSuite = scalaxb.fromXML[TestSuite](noComments)
```
--
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] Tdml gen app [daffodil]
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1538102297
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/scalaxb/ScalaxbTests.scala:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.tdml.scalaxb
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReadingNoTests(): Unit = {
+ val testSuite =
+ scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+
+ // currently failing, not sure why scalaxb isn't handling comments. is the xsd too strict and somehow excludes allowing comments?
+ @Test def testReadingComments(): Unit =
+ scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/IPv4-with-comments.tdml"),
+ )
+ )
+
+ @Test def testReading(): Unit = {
+ val testSuite =
+ scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/IPv4.tdml"),
+ )
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(None, testSuite.suiteName)
+
+ val testcase =
+ scalaxb.fromXML[ParserTestCaseType](
+ <tdml:parserTestCase name="IPv4_1" root="IPv4Header" model="org/apache/daffodil/layers/IPv4.dfdl.xsd" roundTrip="none"/>
+ )
+ val doc =
+ scalaxb.fromXML[DocumentType](
+ <tdml:document xmlns:tdml="http://www.ibm.com/xmlns/dfdl/testData">
+ <tdml:documentPart type="byte">{scala.xml.PCData("4500 0073 0000 4000 4011 b861 c0a8 0001 c0a8 00c7")}</tdml:documentPart>
+ </tdml:document>
+ )
+ val infoset =
+ scalaxb.fromXML[InfosetType](
+ <tdml:infoset xmlns:tdml="http://www.ibm.com/xmlns/dfdl/testData">
+ <tdml:dfdlInfoset>
+ <ipv4:IPv4Header>
+ <Version>4</Version>
+ <IHL>5</IHL>
+ <DSCP>0</DSCP>
+ <ECN>0</ECN>
+ <Length>115</Length>
+ <Identification>0</Identification>
+ <Flags>2</Flags>
+ <FragmentOffset>0</FragmentOffset>
+ <TTL>64</TTL>
+ <Protocol>17</Protocol>
+ <Checksum>47201</Checksum>
+ <IPSrc>C0A80001</IPSrc>
+ <IPDest>C0A800C7</IPDest>
+ <FakeData>115</FakeData>
+ <ComputedChecksum>47201</ComputedChecksum>
+ </ipv4:IPv4Header>
+ </tdml:dfdlInfoset>
+ </tdml:infoset>
+ )
+ val expectedParserTestCase =
+ scalaxb.fromXML[ParserTestCaseType](
+ <tdml:parserTestCase name="IPv4_1" root="IPv4Header" model="org/apache/daffodil/layers/IPv4.dfdl.xsd"
+ roundTrip="none" xmlns:tdml="http://www.ibm.com/xmlns/dfdl/testData" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/"
+ xmlns:xs="http://www.w3.org/2001/XMLSchema"
+ xmlns:fn="http://www.w3.org/2005/xpath-functions"
+ xmlns:dfdlx="http://www.ogf.org/dfdl/dfdl-1.0/extensions"
+ xmlns:ipv4="urn:org.apache.daffodil.layers.IPv4"
+ xmlns:chksum="urn:org.apache.daffodil.layers.IPv4Checksum"
+ xmlns:ex="http://example.com"
+ xmlns:tns="http://example.com">
+ <tdml:document>
+ <tdml:documentPart type="byte">{scala.xml.PCData("4500 0073 0000 4000 4011 b861 c0a8 0001 c0a8 00c7")}</tdml:documentPart>
+ </tdml:document>
+ <tdml:infoset>
+ <tdml:dfdlInfoset>
+ <ipv4:IPv4Header>
+ <Version>4</Version>
+ <IHL>5</IHL>
+ <DSCP>0</DSCP>
+ <ECN>0</ECN>
+ <Length>115</Length>
+ <Identification>0</Identification>
+ <Flags>2</Flags>
+ <FragmentOffset>0</FragmentOffset>
+ <TTL>64</TTL>
+ <Protocol>17</Protocol>
+ <Checksum>47201</Checksum>
+ <IPSrc>C0A80001</IPSrc>
+ <IPDest>C0A800C7</IPDest>
+ <FakeData>115</FakeData>
+ <ComputedChecksum>47201</ComputedChecksum>
+ </ipv4:IPv4Header>
+ </tdml:dfdlInfoset>
+ </tdml:infoset>
+ </tdml:parserTestCase>
+ )
+ val actualParserTestCase: ParserTestCaseType =
+ testSuite
+ .testSuiteChoices
+ .headOption
+ .map(_.as[ParserTestCaseType])
+ .getOrElse(throw new AssertionError("first test case is missing")) // fail() returns Unit so mimic JUnit
+ assertEquals(expectedParserTestCase, actualParserTestCase)
Review Comment:
This test case also fails, but it is going to take some time to figure out what is different. I suspect it some kind of namespace problem....
--
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] Tdml gen app [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#issuecomment-2018729820
Since I'm re-reading this ticket.... I had a thought. Rather than making the VSCode read/write TDML, which we can see using the scalaxb approach, is hard to make compatible with human authored stuff like CDATA bracketing, comments, etc,, why don't we define this to be different from HA (Human Authored) TDML, can explicitly call it MG (Machine Generated) TDML. This is really just a shift in perspective, but we're giving ourselves the freedom to simply be incompatible with Human-Authored TDML, and we can add features like an explicit '<tdml:comment>.' element to overcome the XML-Comments issue.
--
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
[GitHub] [daffodil] arosien commented on a diff in pull request #1057: Tdml gen app
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1326185239
##########
build.sbt:
##########
@@ -211,6 +214,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compliation
Review Comment:
But actually I don't understand this comment. More than these xsd files are compiled into Java, but we don't want that?
--
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
[GitHub] [daffodil] michael-hoke commented on pull request #1057: Tdml gen app
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#issuecomment-1734602025
One more (hopefully, the last) push. It's been squashed and should pass the formatting check as well.
This also is tracked in JIRA - [DAFFODIL-2830](https://issues.apache.org/jira/browse/DAFFODIL-2830).
--
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
[GitHub] [daffodil] mbeckerle commented on a diff in pull request #1057: Tdml gen app
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1335022709
##########
daffodil-lib/src/main/resources/org/apache/daffodil/xsd/tdml.xsd:
##########
@@ -31,86 +31,13 @@
<xsd:import namespace="http://www.ogf.org/dfdl/dfdl-1.0/"/>
<xsd:import namespace="http://www.ogf.org/dfdl/dfdl-1.0/extensions"/>
- <xs:attribute name="tutorialInclude" type="xs:string" fixed="no"/>
-
-<!--
- This element's type is a stub that satisfies the need for xhtml5 validation as part of TDML files.
-
- It is just a wildcard with skip validation in it. So it's not really validating anything.
-
- Users of TDML creating tutorials should paste in known-correct xhtml5 as the contents of the
- tutorial elements.
--->
- <xs:element name="tutorial">
- <xs:complexType mixed="true">
- <xs:sequence>
- <xs:any namespace="##any" processContents="skip" minOccurs="0" maxOccurs="unbounded"/>
- </xs:sequence>
- <xs:anyAttribute namespace="##any" processContents="skip"/>
- </xs:complexType>
- </xs:element>
-
- <!-- IBM uses this namespace http://www.ibm.com/xmlns/dfdl/testData -->
-
- <element name="testSuite">
- <complexType>
- <sequence>
- <choice maxOccurs="unbounded" minOccurs="0">
- <element ref="tns:tutorial"/>
- <element ref="tns:parserTestCase"/>
- <!-- This is an extension to the IBM TDML language. We allow schemas
- to be directly embedded inside the TDML file. A TDML file that contains all
- the schemas it needs is a "self contained" TDML file. -->
- <element ref="tns:defineSchema" minOccurs="0"/>
-
- <element ref="tns:defineConfig" minOccurs="0"/>
-
- <element ref="tns:unparserTestCase"/>
- </choice>
- </sequence>
- <attribute name="suiteName" type="xs:token" use="optional"/>
- <attribute name="ID" type="xs:token" use="optional"/>
- <attribute name="description" type="xs:string" use="optional"/>
- <attribute name="defaultRoundTrip" type="tns:roundTripType" use="optional"/>
- <attribute name="defaultValidation" type="tns:validationType" use="optional"/>
- <attribute name="defaultConfig" type="xs:string" use="optional"/>
- <attribute name="defaultImplementations" type="tns:implementationsType" use="optional"/>
- </complexType>
- <unique name="unique-parserTestCase-name">
- <selector xpath="parserTestCase"/>
- <field xpath="@name"/>
- </unique>
- <unique name="unique-unparserTestCase-name">
- <selector xpath="unparserTestCase"/>
- <field xpath="@name"/>
- </unique>
- <unique name="unique-embeddedSchema-name">
- <selector xpath="defineSchema"/>
- <field xpath="@name"/>
- </unique>
- <unique name="unique-embeddedConfig-name">
- <selector xpath="defineConfig"/>
- <field xpath="@name"/>
- </unique>
- </element>
-
- <simpleType name="implementationsType">
- <list itemType="tns:implementationItem"/>
- </simpleType>
-
- <simpleType name="implementationItem">
- <restriction base="xs:token">
- <enumeration value="daffodil"/>
- <enumeration value="daffodilC"/>
- <enumeration value="ibm"/>
- </restriction>
- </simpleType>
+ <xsd:include schemaLocation="tdml-core.xsd"/>
<!-- We want to allow an xsd:schema to be named and directly embedded in
the TDML thereby allowing a fully-self-contained single file test case as
an exchange medium for tests. -->
- <element name="defineSchema" type="tns:defineSchemaType"/>
+ <element name="defineSchema" type="tns:defineSchemaType" substitutionGroup="tns:testSuiteChoices"/>
Review Comment:
Wow. I learned what a substitution group is really for. Cool.
--
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
[GitHub] [daffodil] stevedlawrence commented on a diff in pull request #1057: Tdml gen app
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1340139109
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
Review Comment:
FYI, I found that setting `Compile / scalaxb / scalaxbHttpClientStyle := HttpClientStyle.Sync` fixes the import issue.
Also, there is a `Compile / scalaxb / scalaxbIgnoreUnknown` setting. I haven't tested that, but it might allow us to revert the `tdml.xsd`/`tdml-core.xsd` split and have it just ignore things like defineSchema/Config that it doesn't know about in `tdml.xsd`. Not a big deal, but removing tdml.xsd complexity would be nice.
--
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
[GitHub] [daffodil] michael-hoke commented on a diff in pull request #1057: Tdml gen app
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1340368579
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
Review Comment:
Re: scalaxb, I had initially attempted to implement the JAXB bindings with that plugin but was unsuccessful. I could not find a way to handle duplicate names for items in the schemas. In the SbtXjc plugin, we handled that with the bindings.xjb file, but I could not find a way to provide anything like that in scalaxb. We won't run into that issue with tdml-core.xsd, but if you want to eventually go back to importing all of the schemas in JAXB, I don't believe scalaxb would currently be able to handle that.
--
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] Tdml gen app [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1342630389
##########
build.sbt:
##########
@@ -211,6 +214,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Dependencies.xjc,
+ xjcCommandLine ++= Seq(
+ "-nv",
+ "-p", "org.apache.daffodil.tdml",
+ "-no-header"),
+ xjcLibs ++= Dependencies.xjc,
+ xjcJvmOpts ++=
+ /* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll need to provide documentation about this.
+ */
+ (if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()),
+ Compile / xjc / sources := Seq(
+ file("daffodil-lib/src/main/resources/org/apache/daffodil/xsd/tdml-core.xsd"),
+ ),
+ // Auto-generated Java files have invalid Scaladoc :(
+ Compile / doc / sources ~= (_ filterNot (_.getName endsWith ".java")),
+ // Tests use JAXB, which uses reflection, which has restrictions above JVM 17.
+ Test / fork := true,
Review Comment:
I'm generally not a fan of forking tests, we've seen forking cause things to go much slower. But I guess in this specific case it's only for tdml-lib, which is small enough that it's not really noticeable. So should be fine. Just a heads up to make sure we don't do this globally.
But another win for scalaxb, no need for these Java 17 reflection options (which users running Daffodil on Java 17 would also need to add?), no need to fork tests, no Java conditional dependencies, and no need to mess with doc or xjc sources. Everything just works the way it's expected to.
If xjc is the right technology to use I guess this is fine, but there sure are a lot of hacks being added to get it working that I think just go away if we switch to scalaxb. And with the tdml-core change, the changes are very straightforward. I was able to get it working with just a few settings:
```scala
lazy val scalaxbSettings = Seq(
Compile / scalaxb / scalaxbPackageName := "org.apache.daffodil.tdml.scalaxb",
Compile / scalaxb / scalaxbGenerateMutable := true,
Compile / scalaxb / sources := Seq(
(lib / Compile / resourceDirectory).value / "org/apache/daffodil/xsd/tdml.xsd",
),
// fixes issue with unused imports
Compile / scalaxb / scalaxbHttpClientStyle := HttpClientStyle.Sync,
)
```
Granted, I didn't create any tests to see how marshalling/unmarshalling worked, but I imagine it's straightforward.
##########
project/Dependencies.scala:
##########
@@ -63,4 +63,17 @@ object Dependencies {
lazy val exi = Seq(
"com.siemens.ct.exi" % "exificient" % "1.0.7",
)
+
+ lazy val xjc =
Review Comment:
This `xjc` dependency list is assigned to both `xjcLibs` and `libraryDependencies`. This is fine for `xjcLibs` since that setting only defines dependencies needed to run the xjc source generator. But I don't think we can have dependencies added to `libraryDependencies` that are conditional on the java version, since the version of Java we build with might be different than the version of Java a used at runtime.
In fact, since releases are built and published with Java 8, these won't be listed as actual dependencies for our official releases. So people using Java 11 or later won't have them as a dependency.
But maybe that's okay and none of these should be added to `libraryDependencies` in the first place? Maybe we document (or maybe it's obvious to JAXB users?) that if a user wants to use these xjc generated files they need to depend on those xjc libraries? Or maybe we just always require them regardless of java version?
Is this the result of https://github.com/apache/daffodil/pull/1057#discussion_r1339338337
Note, another win for scalaxb--I don't think it has any dependency complications like 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] Tdml gen app [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1342632068
##########
project/Dependencies.scala:
##########
@@ -63,4 +63,17 @@ object Dependencies {
lazy val exi = Seq(
"com.siemens.ct.exi" % "exificient" % "1.0.7",
)
+
+ lazy val xjc =
Review Comment:
Also, this adds new dependencies. Have you verified that the license are ASF compatible and if changes are needed to NOTICE/LICENSE?
--
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] Tdml gen app [daffodil]
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1484649978
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/ScalaxbTests.scala:
##########
@@ -0,0 +1,23 @@
+package org.apache.daffodil.tdml
+
+import org.apache.daffodil.tdml.scalaxb.TestSuite
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite =
+ _root_.scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+}
Review Comment:
> Can VS Code extensions use Java/Scala libraries? I thought everything was typescript?
> The frontend is typescript. The backend that communicates with Daffodil is Scala.
Oh, I forgot, this is my favorite part... We currently launch the daffodil debugger to create and append TDML files, which is IMO rather excessive, given that we could alternatively read and write XML files from the typescript extension itself. (There are a number of typescript-based XSD XML parser generator libraries out there.) But we can perhaps discuss that in a different forum.
--
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] Tdml gen app [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1538105304
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/scalaxb/ScalaxbTests.scala:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.tdml.scalaxb
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReadingNoTests(): Unit = {
+ val testSuite =
+ scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+
+ // currently failing, not sure why scalaxb isn't handling comments. is the xsd too strict and somehow excludes allowing comments?
Review Comment:
XSD cannot talk about comments in the XML document. They're orthogonal to schema validity.
--
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
[GitHub] [daffodil] michael-hoke commented on a diff in pull request #1057: Tdml gen app
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1326198819
##########
build.sbt:
##########
@@ -211,6 +214,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compliation
Review Comment:
I don't think that's the case - We set `Compile / xjc / sources := Seq()` in `commonSettings` and overwrite it with this - these should be the only files that we generate JAXB for. Potentially, any imports we would also grab, but I believe we have all of the imports explicitly defined here anyways.
I can double check this, but I'm almost positive that either setting `Compile / xjc / sources` to contain the folders or adding XMLSchema won't build because XMLSchema prevents the build from passing (If I'm remembering correctly, there are a lot of clashes, and we would need a lot more definitions in the bindings.xjb file). I also don't see any `xs:include` lines for it, so I don't think it's strictly necessary to include this file for the TDML JAXB bindings to be complete.
--
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
[GitHub] [daffodil] michael-hoke commented on a diff in pull request #1057: Tdml gen app
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1323328479
##########
build.sbt:
##########
@@ -60,6 +61,7 @@ lazy val macroLib = Project("daffodil-macro-lib", file("daffodil-macro-lib"))
.disablePlugins(OsgiCheckPlugin)
lazy val propgen = Project("daffodil-propgen", file("daffodil-propgen"))
+ .disablePlugins(SbtXjcPlugin)
Review Comment:
We did something similar - set the Seq containing xsd schemas to run through JAXB to empty in commonSettings - it allows us to remove all of these disablePlugins lines. It builds for me locally, so, hopefully, the CI tests will build now as well.
--
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
[GitHub] [daffodil] michael-hoke commented on a diff in pull request #1057: Tdml gen app
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1327443529
##########
build.sbt:
##########
@@ -211,6 +214,67 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcBindings += "daffodil-tdml-lib/src/main/resources/bindings.xjb",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ ),
+ xjcJvmOpts ++= extraJvmOptions,
+ Compile / xjc / sources := Seq(
+ // Need to refer to the individual files because we do not want XMLSchema to go through the JAXB compliation
Review Comment:
I ended up having to take out a bit more than just that, but I was able to generate the XSDs without a bindings.xjb file and only having tdml.xsd listed as a source file. Here's the output from `git diff` in a patch file. If you would like me to upload the entire changed document, let me know.
I will go back and attempt to see if this works with our extension, but it looks like it should work for us since we only use a very minimal subset of the TDML schema for now.
[tdml-xsd-update.patch](https://github.com/apache/daffodil/files/12620820/tdml-xsd-update.patch)
--
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
[GitHub] [daffodil] stevedlawrence commented on a diff in pull request #1057: Tdml gen app
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1340461983
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
+ xjcLibs := Seq(
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
Review Comment:
I see, thanks for the explanation. Sounds like the tdml-core split is a better solution.
If we did integrate the TDMLRunner into the JAXB bindings, I'm not sure we would even want bindings for the stuff we ignore now. For example, embedded DFDL schemas just want to be treated as raw strings or XML nodes or something. Those schemas are just written to a file and compiled with Daffodil, so converting them to JAXB bindings just to serialize back to XML is probably unnecessary.
But I don't think there's a plan to integrate the TDML runner into these bindings, at least in the near term, so whatever the solution is for duplicate identifiers, it can be dealt with then.
--
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] Tdml gen app [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1342828127
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/JaxbTests.scala:
##########
@@ -0,0 +1,20 @@
+package org.apache.daffodil.tdml
+
+import org.junit.Assert._
+import org.junit.Test
+
+import javax.xml.bind.JAXBContext
+
+class JaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite = JAXBContext
+ .newInstance(classOf[TestSuite])
+ .createUnmarshaller()
+ .unmarshal(getClass.getClassLoader().getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"))
+ .asInstanceOf[TestSuite]
+
+ assertNotNull(testSuite)
+ assertEquals("dpaext", testSuite.suiteName)
Review Comment:
I'm curious what the `dfdInfoset` part unmarshalls to, since that can contain random XML (or a string for `type="file"`) that isn't defined by the schema, it's just an XSD `any` element. Is it made available as a string, some sort of XML DOM, or is it just ignored? If ignored, that's likely a problem since users of this will probably want access to the infoset part of a TDML file.
It might be useful to modify this test to examine some of the things we think we'll need (like `dfdlnfoset`) to make sure they are unmarshalling in usable ways. And maybe also act as a reference for using this.
A related thought, if XSD `any` does parse to something useful like a string or DOM, could XSD `any` also be used to support the `defineSchema` and `defineConfig` elements? For example, what if we added a new `tdml-jaxb.xsd` file that did something like:
```xml
...
<xsd:include schemaLocation="tdml-core.xsd"/>
<element name="defineSchema" type="tns:defineSchemaType" substitutionGroup="tns:testSuiteChoices"/>
<complexType name="defineSchemaType">
<sequence>
<any namespace="##any" processContents="lax" minOccurs="0" maxOccurs="unbounded"/>
</sequence>
</comlexType>
...
```
So very similar to how `dfdlInfoset` is already defined. We can configure xjc to generate code based on this new jaxb specific tdml file, and the `defineSchema` just becomes a string or DOM or however it treats XSD `any`, just like `dfdlInfoset` does.
Doesn't mean we necessarily have to support embedded configs and schemas in the vscode tool where this is planned to be used, but it at least makes it possible. And also makes it more possible to combin the existing TDML runner with the new jaxb stuff.
--
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] Tdml gen app [daffodil]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1342825027
##########
project/Dependencies.scala:
##########
@@ -63,4 +63,17 @@ object Dependencies {
lazy val exi = Seq(
"com.siemens.ct.exi" % "exificient" % "1.0.7",
)
+
+ lazy val xjc =
Review Comment:
I believe we will need to make updates to the NOTICE/LICENSE files.
SbtXjcPlugin is BSD-3
jaxb-xjc and jaxb-impl are EDL 1.0
activation is CDDL 1.0 - this is the most restrictive, but I think we're okay because we're open source.
On the other hand, if we switch to scalaxb, that uses MIT.
However, there are dependencies listed on the github page that I don't see in Steve's code snippet. In case we need those:
org.dispatchhttp.dispatch-core - LGPL 3.0
javax.xml.bind.jaxb-api - CDDL 1.0
org.scala-lang.modules.scala-xml - Apache 2.0
org.scala-lang.scala-parser-combinators - Apache 2.0.
--
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] Tdml gen app [daffodil]
Posted by "michael-hoke (via GitHub)" <gi...@apache.org>.
michael-hoke commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1342897901
##########
build.sbt:
##########
@@ -211,6 +212,43 @@ lazy val testStdLayout = Project("daffodil-test-stdLayout", file("test-stdLayout
.dependsOn(tdmlProc % "test")
.settings(commonSettings, nopublish)
+/* Workaround: certain reflection (used by JAXB) isn't allowed by default in JDK 17:
+ * https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B
+ *
+ * While we can handle this JVM quirk at build time, at runtime we won't know
+ * a user's JVM version. We'll provide documentation and an extension setting
+ * to add these flags to the extension-launched debugger backend.
+ */
+lazy val extraJvmOptions: Seq[String] =
+ if (scala.util.Properties.isJavaAtLeast("17"))
+ Seq(
+ "--add-opens",
+ "java.base/java.lang=ALL-UNNAMED",
+ )
+ else Seq()
+
+lazy val xjcSettings =
+ Seq(
+ libraryDependencies ++= Seq(
+ "com.sun.xml.bind" % "jaxb-impl" % "2.2.11",
+ "javax.activation" % "activation" % "1.1.1",
+ "org.glassfish.jaxb" % "jaxb-xjc" % "2.2.11",
+ ),
+ xjcCommandLine += "-nv",
+ xjcCommandLine += "-p",
+ xjcCommandLine += "org.apache.daffodil.tdml",
Review Comment:
It looks like the `-no-header` flag solves the issue for the .class and .jar files, but I'm not exactly sure why... I did confirm that all that flag does is strip out a comment header at the beginning of the file, so I don't know why that matters for .class files... I did confirm that the build is not reproducible without the `-no-header` flag. I also found out that there is a .class decompiler in VSCode (I think it might even be native). I decompiled both versions of a class that the `diff` command reported as different and did not see a change.
That being said, even with the `-no-header` flag, I notice differences in the generated .java files. These differences are the order of {@link ...} comments, so they also do not affect the actual code... Completely unsure as to why these comments don't matter and the header's do. The list of files that differ also varies, at least slightly, with every build.
If you can confirm that the current code on this branch creates a reproducible build for you, we'll leave `-no-header` in. If/When we attempt to switch over to scalaxb, we'll verify that we have a reproducible build.
--
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] Tdml gen app [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#issuecomment-1930491749
Can you please rebase to the latest main branch and resolve the conflicts?
--
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] Tdml gen app [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1483645379
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/ScalaxbTests.scala:
##########
@@ -0,0 +1,23 @@
+package org.apache.daffodil.tdml
+
+import org.apache.daffodil.tdml.scalaxb.TestSuite
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite =
+ _root_.scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+}
Review Comment:
We have various tools in daffodil-misc/xml/XMLUtils.scala that strip out comments, strip out processing instructions, etc.
You may need to do that pass first in order to be able to read in TDML.
This is a really common thing in XML processing that people don't accommodate comments, which are part of the XML Infoset. As are EntityRefs, etc. Somewhere in the daffodil tests we have a DFDL schema that has a comment everwhere that a comment is allowed. To make sure we tolerate them in all the places they can appear.
Are you parsing TDML, or reading and writing it back?
Reading one can strip out this stuff, but if you are rewriting the file this stuff has to be preserved. TDML is not an object model. Ex: reading and writing this:
```
<str><![CDATA[big long string with
significant whitespace
inside it.
]]></str>
```
And getting it to round trip with that CDATA bracketing in it, all whitespace preserved, etc. .... that's going to be tricky.
--
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] Tdml gen app [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1484698839
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/ScalaxbTests.scala:
##########
@@ -0,0 +1,23 @@
+package org.apache.daffodil.tdml
+
+import org.apache.daffodil.tdml.scalaxb.TestSuite
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReading(): Unit = {
+ val testSuite =
+ _root_.scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+}
Review Comment:
> It's both reading and writing in the [daffodil-vscode](https://issues.apache.org/jira/browse/DAFFODIL-vscode) project, but I don't think that matters for the changes in daffodil proper. We can document the limitation with comments somewhere and downstream projects can choose to raise an error when encountering comments, filter them out, etc.
So when scalaxb writes out an infoset, strings in the infoset are likely to contain "<", ">", "&", etc. So they need XML escapifying when output. I would assume this would happen automatically.
But note that if you read in TDML and an infoset is embedded and uses `<![CDATA[...]]>` stuff, that it will write out as the string contents (without the CDATA bracketing) but escapified.
I'm ok with that. It's just fragile if XML tooling decides it is ok to mess with the whitespace of the TDML file. E.x., if you output the XML back to a file by pretty printing it, then data might be corrupted by whitespace changes.
This is just a really painful area of experience with XML. It's just not a great data language because it was designed to be a human authored markup language. Hence, the whitespace insensitivity, line-ending changes, etc.
--
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] Tdml gen app [daffodil]
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1538180239
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/scalaxb/ScalaxbTests.scala:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.tdml.scalaxb
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReadingNoTests(): Unit = {
+ val testSuite =
+ scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+
+ // currently failing, not sure why scalaxb isn't handling comments. is the xsd too strict and somehow excludes allowing comments?
Review Comment:
@stevedlawrence, you from the past helped me find the problem! https://stackoverflow.com/questions/52984363/scala-xml-child-method-from-an-xml-node-gets-trailling-white-space
This fixes the reading-comments issue, along with my [comparisons-failing issue](https://github.com/apache/daffodil/pull/1057#discussion_r1538102297).
--
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] Tdml gen app [daffodil]
Posted by "arosien (via GitHub)" <gi...@apache.org>.
arosien commented on code in PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#discussion_r1538096954
##########
daffodil-tdml-lib/src/test/scala/org/apache/daffodil/tdml/scalaxb/ScalaxbTests.scala:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.tdml.scalaxb
+
+import org.junit.Assert._
+import org.junit.Test
+
+class ScalaxbTests {
+
+ @Test def testReadingNoTests(): Unit = {
+ val testSuite =
+ scalaxb.fromXML[TestSuite](
+ scala.xml.XML.load(
+ getClass
+ .getClassLoader()
+ .getResourceAsStream("test-suite/ibm-contributed/dpaext1-2.tdml"),
+ ),
+ )
+
+ assertNotNull(testSuite)
+ assertEquals(Some("dpaext"), testSuite.suiteName)
+ }
+
+ // currently failing, not sure why scalaxb isn't handling comments. is the xsd too strict and somehow excludes allowing comments?
Review Comment:
I tested the [example xsd](https://scalaxb.org/running-scalaxb) with some XML with various comments inside it, and the scalaxb generated code handled the comments just fine, so I suspect there's something more complicated going on with the TDML xsd. Ick.
--
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] Tdml gen app [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on PR #1057:
URL: https://github.com/apache/daffodil/pull/1057#issuecomment-2018716359
> @stevedlawrence I know there was a discuss out for releasing Daffodil 3.7.0. Would this possibly be a PR we could get in before the actual release or where do you stand on what still needs done for this to meet requirements?
Based on @arosien's comments, it sounds like there are still some open issues to resolve with this PR? There's also merge conflicts and a potential licensing issue to figure out. So it might be best to push to the next release. Were thinking we might try to get 3.8.0 out quicker than usual with the new layer API. We could try to get this in that release so you wouldn't have to wait too long.
--
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