stevedlawrence commented on a change in pull request #422:
URL: https://github.com/apache/incubator-daffodil/pull/422#discussion_r498289830
##########
File path:
daffodil-core/src/main/scala/org/apache/daffodil/grammar/GrammarTerm.scala
##########
@@ -51,7 +52,8 @@ import org.apache.daffodil.dsom.Term
abstract class Gram(contextArg: SchemaComponent)
extends OOLAGHostImpl(contextArg)
with BasicComponent
- with GramRuntime1Mixin {
+ with GramRuntime1Mixin
+ with GramRuntime2Mixin {
Review comment:
I think it's worth having a discussion to figure out how to separate the
runtime logic from the compile logic. Having to modify Grammar/compiler logic
everytime we want to add a new runtime is pretty painful. I'm not sure what
that would look like, but having a generic solution would be nice.
##########
File path:
daffodil-core/src/main/scala/org/apache/daffodil/runtime2/Runtime2DataProcessor.scala
##########
@@ -0,0 +1,211 @@
+/*
+ * 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.runtime2
+
+import java.io.File
+import java.io.InputStream
+import java.io.OutputStream
+
+import org.apache.daffodil.api.DFDL
+import org.apache.daffodil.api.DaffodilTunables
+import org.apache.daffodil.api.DataLocation
+import org.apache.daffodil.api.ValidationMode
+import org.apache.daffodil.externalvars.Binding
+import org.apache.daffodil.processors.Failure
+import org.apache.daffodil.processors.ProcessorResult
+import org.apache.daffodil.processors.Success
+import org.apache.daffodil.processors.VariableMap
+import org.apache.daffodil.processors.WithDiagnosticsImpl
+import org.apache.daffodil.processors.parsers.ParseError
+import org.apache.daffodil.processors.unparsers.UnparseError
+import org.apache.daffodil.util.Maybe
+import org.apache.daffodil.util.Maybe.Nope
+import os.Pipe
+
+/**
+ * Effectively a scala proxy object that does its work via the underlying
C-code.
+ * Will need to consider how to use features of underlying C-code to get
infoset,
+ * walk infoset, generate XML for use by TDML tests.
+ */
+class Runtime2DataProcessor(executableFile: os.Path) extends
DFDL.DataProcessorBase {
+ /**
+ * Returns a data processor with all the same state, but the validation mode
changed to that of the argument.
+ *
+ * Note that the default validation mode is "off", that is, no validation is
performed.
+ */
+ override def withValidationMode(mode: ValidationMode.Type):
DFDL.DataProcessor = ???
+
+ override def withTunable(name: String, value: String): DFDL.DataProcessor =
???
+
+ override def withTunables(tunables: Map[String, String]): DFDL.DataProcessor
= ???
+
+ override def withExternalVariables(extVars: Map[String, String]):
DFDL.DataProcessor = ???
+
+ override def withExternalVariables(extVars: File): DFDL.DataProcessor = ???
+
+ override def withExternalVariables(extVars: Seq[Binding]):
DFDL.DataProcessor = ???
+
+ override def validationMode: ValidationMode.Type = ???
+
+ override def getTunables(): DaffodilTunables = ???
+
+ override def save(output: DFDL.Output): Unit = ???
+
+ override def variableMap: VariableMap = ???
+
+ override def setValidationMode(mode: ValidationMode.Type): Unit = ???
+
+ override def setExternalVariables(extVars: Map[String, String]): Unit = ???
+
+ override def setExternalVariables(extVars: File): Unit = ???
+
+ override def setExternalVariables(extVars: File, tunable: DaffodilTunables):
Unit = ???
+
+ override def setExternalVariables(extVars: Seq[Binding]): Unit = ???
+
+ override def setTunable(tunable: String, value: String): Unit = ???
+
+ override def setTunables(tunables: Map[String, String]): Unit = ???
+
+ /**
+ * Returns an object which contains the result, and/or diagnostic
information.
+ */
+ def parse(input: InputStream): ParseResult = {
Review comment:
This feels odd to me. I would expect the runtime2 to not actually have a
parse method. It would just generate an executable/library and users would have
to know how to use that. So the API sort of changes depending on what backends
are made avialable. We may need to rethink how things like the API and CLI
work. For example, if this runtime2 backend were on the class path, you might
expect something like this to be available:
daffodil runtime2 -s schema.dfdl.xsd -o schema.bin
This would generate a schema.bin executable that could then be used to do
the actual parse unparse. So the concept of a DataProcessor in runtime2 migh
not exist. And it would be purely a runtime1 concept?
The fact that so many of these methods are unimplemented says to be me need
to put some thought into how these backends will work in practice.
##########
File path: daffodil-cli/build.sbt
##########
@@ -42,6 +42,18 @@ mappings in Universal ++= Seq(
baseDirectory.value / "README.md" -> "README.md",
)
+mappings in Universal ++= Seq(
+ baseDirectory.value / ".." / "daffodil-runtime2" / "src" / "main" / "c" /
"common_runtime.h" -> "include/common_runtime.h",
+ baseDirectory.value / ".." / "daffodil-runtime2" / "src" / "main" / "c" /
"daffodil_argp.h" -> "include/daffodil_argp.h",
+ baseDirectory.value / ".." / "daffodil-runtime2" / "src" / "main" / "c" /
"stack.h" -> "include/stack.h",
+ baseDirectory.value / ".." / "daffodil-runtime2" / "src" / "main" / "c" /
"xml_reader.h" -> "include/xml_reader.h",
+ baseDirectory.value / ".." / "daffodil-runtime2" / "src" / "main" / "c" /
"xml_writer.h" -> "include/xml_writer.h",
+)
+
+mappings in Universal ++= Seq(
+ baseDirectory.value / ".." / "daffodil-runtime2" / "target" / "streams" /
"compile" / "ccTargetMap" / "_global" / "streams" / "compile" / "sbtcc.Library"
/ "libruntime2.a" -> "lib/libruntime2.a",
Review comment:
The mappings in Universal determines how we package the convenience
binaries that we release in zip/tar/rpm form. So this is saying to install the
header files from the runtime2 directory into ``include/`` and the compiled
static library into ``lib/``. You're right that explicitly mentioning target is
uncommon in an sbt file. I would expect the cc plugin to make some property
available to get access to the statically lib. Also, like above, we should be
able to use a glob for these headers.
##########
File path: build.sbt
##########
@@ -43,6 +46,32 @@ lazy val runtime1 = Project("daffodil-runtime1",
file("daffodil-runtime1
.dependsOn(io, lib % "test->test", udf, macroLib
% "compile-internal, test-internal")
.settings(commonSettings, usesMacros)
+val runtime2StaticLib = Library("libruntime2.a")
+lazy val runtime2 = Project("daffodil-runtime2",
file("daffodil-runtime2")).configs(IntegrationTest)
Review comment:
Not an issue with this change per se, but I'm wondering if we should
think about better names for our runtimes, as well as for all our subprojects.
For example, the difference between daffodil-core and daffodil-lib is not
obvious. Perhaps core should be something like "daffodil-schema-compiler".
Likewise "runtime1" is not clear at all. Perhaps this should be
"daffodil-runtime-scala-parser". It's verbose, but much more obvious from
newcomers.
I would even argue perhaps "runtime" isn't that good of a name. Especially
in the context of this new code generator, where the runtime is the generated
code, not this code generator. Maybe we something more generic like "backend".
So projects might be daffodil-backend-scala-parser,
daffodil-backend-scala-unparser, daffodil-backend-generator-c, etc. And
eventually we will have the capability to plugin different backends to our
compiler.
##########
File path:
daffodil-core/src/main/scala/org/apache/daffodil/runtime2/GeneratedCodeCompiler.scala
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.runtime2
+
+import org.apache.commons.io.FileUtils
+import org.apache.daffodil.compiler.ProcessorFactory
+import org.apache.daffodil.dsom.SchemaDefinitionError
+import org.apache.daffodil.runtime2.generators.CodeGeneratorState
+import org.apache.daffodil.util.Misc
+import os.Pipe
+
+class GeneratedCodeCompiler(pf: ProcessorFactory) {
Review comment:
I would expect all this logic to be in the runtime2 subproject. So
runtime is essentially made up of jar that generates C code and compiles it
with the static library. Perhaps this library could even live inside the jar
and extracted for the during compilation process? So there would be no need to
actually distribute the static lib in the zip/tar/rpm since it's all part of
the jar?
##########
File path: build.sbt
##########
@@ -43,6 +46,32 @@ lazy val runtime1 = Project("daffodil-runtime1",
file("daffodil-runtime1
.dependsOn(io, lib % "test->test", udf, macroLib
% "compile-internal, test-internal")
.settings(commonSettings, usesMacros)
+val runtime2StaticLib = Library("libruntime2.a")
+lazy val runtime2 = Project("daffodil-runtime2",
file("daffodil-runtime2")).configs(IntegrationTest)
+ .enablePlugins(CcPlugin)
+ .dependsOn(tdmlProc)
+ .settings(commonSettings)
+ .settings(publishArtifact in (Compile,
packageDoc) := false)
+ .settings(
+ Compile / ccTargets :=
ListSet(runtime2StaticLib),
+ Compile / cSources := Map(
+ runtime2StaticLib -> Seq(
+ baseDirectory.value / "src" / "main" / "c"
/ "common_runtime.c",
Review comment:
Scala has a way to essentially do a glob listing of a directory, we
probably want something like that.
##########
File path:
daffodil-core/src/main/scala/org/apache/daffodil/runtime2/GeneratedCodeCompiler.scala
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.runtime2
+
+import org.apache.commons.io.FileUtils
+import org.apache.daffodil.compiler.ProcessorFactory
+import org.apache.daffodil.dsom.SchemaDefinitionError
+import org.apache.daffodil.runtime2.generators.CodeGeneratorState
+import org.apache.daffodil.util.Misc
+import os.Pipe
+
+class GeneratedCodeCompiler(pf: ProcessorFactory) {
+ private var executableFile: os.Path = _
+ private lazy val isWindows =
System.getProperty("os.name").toLowerCase().startsWith("windows")
+
+ /**
+ * Compiles and links generated C code with runtime2 library to
+ * build an executable file.
+ */
+ def compile(rootElementName: String, codeGeneratorState:
CodeGeneratorState): Unit = {
+ val compiler = "cc"
+ val location = Option(this.getClass.getProtectionDomain.getCodeSource)
flatMap (x => Option(x.getLocation))
+ val wd = if (os.exists(os.pwd/"daffodil-runtime2"))
+ os.pwd
+ else if (os.exists(os.pwd/os.up/"daffodil-runtime2"))
+ os.pwd/os.up
+ else if (location.isDefined)
+ os.Path(FileUtils.toFile(location.get))/os.up/os.up
+ else
+ os.pwd
+ val includeDir = if (os.exists(wd/"include"))
+ wd/"include"
+ else
+ wd/"daffodil-runtime2"/"src"/"main"/"c"
+ val libDir = if (os.exists(wd/"lib"))
+ wd/"lib"
+ else
+
wd/"daffodil-runtime2"/"target"/"streams"/"compile"/"ccTargetMap"/"_global"/"streams"/"compile"/"sbtcc.Library"
Review comment:
The sbt-cc plugin doesn't know about these generated c files. I think
the generator doesn't even necessarily have to run in the context of sbt. So
this new runtime needs to know how to compile things at runtime using some
other compiler. We almost certainly don't want to have this sbt path in here,
with the expectation that the static library is made available somehow when the
runtime2 is used.
##########
File path: daffodil-runtime2/src/main/c/.gitignore
##########
@@ -0,0 +1,24 @@
+# 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.
+
+# Ignore in case we compile the C files by hand
+*.a
+*.o
+daffodil
+
+# Ignore since these files are not fit for source control
+GPATH
+GRTAGS
+GTAGS
Review comment:
I generally prefer we only modify the root .gitignore file. It's too
easy to forget about .gitignore files in subdirectories which I think can
sometimes lead to confusion.
What are the daffodil a G* files? It also feels like all these files should
be built in a ``target`` directory so that they should be ignored by default.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
