tuxji commented on a change in pull request #465:
URL: https://github.com/apache/incubator-daffodil/pull/465#discussion_r537741532
##########
File path:
daffodil-runtime2/src/test/scala/org/apache/daffodil/runtime2/TestCodeGenerator.scala
##########
@@ -78,35 +91,29 @@ class TestCodeGenerator {
val cg = pf.forLanguage("c")
// Generate code from the test schema successfully
- val outputDir = cg.generateCode(None, "./generateCode_tmp")
+ val outputDir = cg.generateCode(None, s"$tmpOutputDir")
Review comment:
Nice idea, although I've simply added calls to os.temp.dir() so each
test method creates its own temp directory and kept using `@After` to remove
the temp directory. (The os-lib documentation says the directory created by
os.temp.dir() is supposed to be removed when the JVM exits, but I've never
found that to happen in practice, bummer.)
##########
File path: build.sbt
##########
@@ -57,13 +57,15 @@ lazy val runtime2 = Project("daffodil-runtime2",
file("daffodil-runtime2
Compile / ccTargets := ListSet(runtime2CFiles),
Compile / cSources := Map(
runtime2CFiles -> (
- ((Compile / resourceDirectory).value / "c"
* GlobFilter("*.c")).get() ++
+ ((Compile / resourceDirectory).value / "c"
/ "libcli" * GlobFilter("*.c")).get() ++
+ ((Compile / resourceDirectory).value / "c"
/ "libruntime" * GlobFilter("*.c")).get() ++
((Compile / resourceDirectory).value /
"examples" * GlobFilter("*.c")).get()
)
),
Compile / cIncludeDirectories := Map(
runtime2CFiles -> Seq(
- (Compile / resourceDirectory).value / "c",
+ (Compile / resourceDirectory).value / "c"
/ "libcli",
+ (Compile / resourceDirectory).value / "c"
/ "libruntime",
(Compile / resourceDirectory).value /
"examples"
Review comment:
I wish sbt was easier to understand. According to
<https://www.scala-sbt.org/1.x/docs/Globs.html>, I should be able to use
sbt.io.DirectoryFilter (or its converse, sbt.io.RegularFileFilter) to make
cIncludeDirectories above find and list each directory under resourceDirectory
as an includeDirectory. However, the necessary changes aren't obvious to me.
##########
File path: build.sbt
##########
@@ -57,13 +57,15 @@ lazy val runtime2 = Project("daffodil-runtime2",
file("daffodil-runtime2
Compile / ccTargets := ListSet(runtime2CFiles),
Compile / cSources := Map(
runtime2CFiles -> (
- ((Compile / resourceDirectory).value / "c"
* GlobFilter("*.c")).get() ++
+ ((Compile / resourceDirectory).value / "c"
/ "libcli" * GlobFilter("*.c")).get() ++
+ ((Compile / resourceDirectory).value / "c"
/ "libruntime" * GlobFilter("*.c")).get() ++
Review comment:
Thanks, I appreciate the tip and I've used ** to get recursive globs
inside both "c" and "examples". I considered doing a recursive glob on
resourceDirectory, but I kept some explicitness for consistency since I
couldn't figure out how to make cIncludeDirectories below less explicit.
To answer your next question, yes, all object files from c/libcli,
c/libruntime, and examples end up in the same "libruntime2.a" archive created
by runtime2CFiles. We are using runtime2CFiles only as a build target to
ensure sbt test-builds every one of the C source files on developers' machines
(we don't want the C source files to bit-rot). We never distribute the .a file
or even use it in our tests. CodeGenerator compiles and links the C source
files into an executable in a single call to the C compiler on users' machines,
so it doesn't build or use any .a/so files either at this time.
If we start building .a/so files on user machines later, then yes, we'd want
to build separate libcli and libruntime .a/so files. We also might not drop
generated_code.c into libruntime if we had a large set of libruntime files
because every schema would end up generating its own libruntime.a file.
Regarding the examples files, we're building them only to test that they
compile. We don't even test that they can link together with libcli and
libruntime because we're going to expand the number of example files over time
for different schemas and different example files probably will collide with
each other. Example files are essentially copies of the "generated_code.[ch]"
files written by CodeGenerator for different schemas.
##########
File path:
daffodil-runtime2/src/test/scala/org/apache/daffodil/runtime2/TestCodeGenerator.scala
##########
@@ -26,18 +26,31 @@ import org.apache.daffodil.compiler.Compiler
import org.apache.daffodil.util.Misc
import org.apache.daffodil.util.SchemaUtils
import org.apache.daffodil.util.TestUtils
+import org.junit.After
import org.junit.Assert.assertArrayEquals
+import org.junit.Before
import org.junit.Test
/**
* Checks that we can create a [[CodeGenerator]] and call its methods.
* The value of this test is to debug the call path from [[Compiler]]
- * to [[CodeGenerator]] for one very simple DFDL schema. Running TDML
- * tests with daffodil-runtime2 is a more effective way to test the
- * functionality of CodeGenerator's generated code for as many DFDL
- * schemas as you could want.
+ * to [[CodeGenerator]] for a single test DFDL schema. Running TDML
+ * tests with daffodil-runtime2 is a more effective way to check that
+ * CodeGenerator can generate appropriate code for as many DFDL schemas
+ * as you could want.
*/
class TestCodeGenerator {
+ // Ensure all tests remove tmpOutputDir after using it
+ val tmpOutputDir: os.Path = os.pwd/"generate_tmp"
Review comment:
Yes, I also noticed that one of the Windows builds failed because this
directory couldn't be deleted, probably because both the daffodil-cli and
daffodil-runtime2 tests were running at the same time and trying to use this
directory at the same time. Since I've been using os-lib calls for all my file
operations, I've replaced the os.pwd/"generate_tmp" with calls to os.temp.dir()
in the test methods.
----------------------------------------------------------------
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]
