stevedlawrence commented on a change in pull request #525: URL: https://github.com/apache/daffodil/pull/525#discussion_r613461800
########## File path: README.md ########## @@ -45,94 +45,46 @@ For more information about Daffodil, see the [Website]. * C compiler C99 or higher * Mini-XML Version 3.2 or higher -Since Daffodil has a DFDL to C backend, you will need a C compiler -([gcc] or [clang]), the [Mini-XML] library, and possibly the GNU -[argp] library if your system's C library doesn't include it. You can -install gcc and libmxml as system packages on most Unix based -platforms with distribution-specific packager commands such as (Debian -and Ubuntu): - - # Just mentioning all other packages you might need too - sudo apt install build-essential curl git libmxml-dev - -You will need the Java Software Development Kit ([JDK]) and the Scala -Build Tool ([SBT]) to build Daffodil, run all tests, create packages, -and more. [SDK] offers an easy and uniform way to install both java -and sbt on any Unix based platform: - - curl -s "https://get.sdkman.io"; | bash - sdk install java - sdk install sbt - -You can edit the Compile / cCompiler setting in build.sbt if you don't -want sbt to call your C compiler with "cc" as the driver command. - -On Windows, the easiest way to install gcc and libargp is to install -[MSYS2]'s collection of free tools and libraries although MSYS2 has no -package for libmxml which you'll need to build from source. First -install [MSYS2] following its website's installation instructions, -then run the following commands in a "MSYS2 MSYS" window: - - pacman -S gcc git libargp-devel make pkgconf - git clone https://github.com/michaelrsweet/mxml.git - cd mxml - ./configure --prefix=/usr --disable-shared --disable-threads - make - make install - -You also need to install [JDK} and [SBT] from their Windows -installation packages and define an environment variable using -Windows' control panel for editing environment variables. Define an -environment variable with the name `MSYS2_PATH_TYPE` and the value -`inherit`. Now when you open a new "MSYS2 MSYS" window from the Start -Menu, you will be able to type your sbt commands in the MSYS2 window -and both sbt and daffodil will be able to call the C compiler. +See [BUILD.md](BUILD.md) for more details. ## Getting Started -Below are some of the more common commands used for Daffodil development. +[SBT] is the officially supported tool to build Daffodil. Here are +some of the more commonly used commands for Daffodil development. -### Compile +* Compile source code: Review comment: I personally perfer the ### headings over the bullets. It makes it easier to skim over and find the command you might need, rather than having the full sentences. ########## File path: BUILD.md ########## @@ -0,0 +1,63 @@ +# Build Requirements + +Daffodil's build requirements include: + +* JDK 8 or higher +* SBT 0.13.8 or higher +* C compiler C99 or higher +* Mini-XML Version 3.2 or higher + +You will need the Java Software Development Kit ([JDK]) and the Scala +Build Tool ([SBT]) to build Daffodil, run all tests, create packages, +and more. The recommended way is to install the latest [Java 11 +LTS][JDK] version and the latest [SBT] version directly from their +websites. A package manager on your operating system may be a more +convenient way, but it may install an older version than what you'd +prefer. + +Since Daffodil now has a C backend as well as a Scala backend, you +will need a C compiler ([gcc] or [clang]), the [Mini-XML] library, and +possibly the GNU [argp] library if your system's C library doesn't +include it already. The recommended way is to install them using your +operating system's package manager (for example, `apt` on Debian or +Ubuntu): + + sudo apt install build-essential curl git libmxml-dev + # Just listing other packages you might need too + +Some operating systems don't package the [Mini-XML] library, so you +may have to build and install it from source on those operating +systems. See the Windows commands below for how to do so. + +You can set your environment variable "CC" to the appropriate driver +command (or set it to `true` to disable C compilation altogether) if +you don't want `sbt compile` to call your C compiler with `cc` as the +driver command. + +On Windows, the easiest way to install gcc and libargp is to install +[MSYS2]'s collection of free tools and libraries although MSYS2 has no +package for libmxml which you'll need to build from source. First +install [MSYS2] following its website's installation instructions, +then run the following commands in a "MSYS2 MSYS" window: + + pacman -S gcc git libargp-devel make pkgconf + git clone https://github.com/michaelrsweet/mxml.git Review comment: Should we include a specific tag here, e.g. ``` git clone -t v3.2 https://github.com/michaelrsweet/mxml.git ``` I don't really like the idea of reccommending people use the master branch of a repo we don't control. ########## File path: BUILD.md ########## @@ -0,0 +1,63 @@ +# Build Requirements + +Daffodil's build requirements include: + +* JDK 8 or higher +* SBT 0.13.8 or higher +* C compiler C99 or higher +* Mini-XML Version 3.2 or higher + +You will need the Java Software Development Kit ([JDK]) and the Scala +Build Tool ([SBT]) to build Daffodil, run all tests, create packages, +and more. The recommended way is to install the latest [Java 11 +LTS][JDK] version and the latest [SBT] version directly from their +websites. A package manager on your operating system may be a more +convenient way, but it may install an older version than what you'd +prefer. + +Since Daffodil now has a C backend as well as a Scala backend, you +will need a C compiler ([gcc] or [clang]), the [Mini-XML] library, and +possibly the GNU [argp] library if your system's C library doesn't +include it already. The recommended way is to install them using your +operating system's package manager (for example, `apt` on Debian or +Ubuntu): + + sudo apt install build-essential curl git libmxml-dev Review comment: Is libxml-dev MiniXML in ubuntu? On fedora libxml is something completely different. This is why I would prefer that we have step by step build instructions for common operating systems to avoid this kind of confusion. Build instructions like "you might need this because your OS my not provide it can be confusing for new users. By giving specific instructions, it makes it easier to figure out exactly what steps to follow, and if you'r on an varient, likely commands that need to be run. It's also a bit confusing because Windows and no windows things are interleaved. If I'm not on windows, I want to just ignored anything windows related to quickly get set up. For example, I'm on Fedora, there are git clone commands to clone and install mxml. I don't know if I need to do that or not. Turns out, Fedora does have mxml via ``sudo dnf instal mxml-devel`` but that isn't anywhere in the instructions. And that version is only 3.1, so I assume it won't work for this, and I still need to install via source? ########## File path: daffodil-core/src/main/scala/org/apache/daffodil/dsom/ElementBase.scala ########## @@ -352,6 +353,38 @@ trait ElementBase } else DataValue.NoValue } + /** + * Is either None or Some(primTypeValue). Review comment: This comment isn't accurent. This is always a pimitive, but might be the special NoValue. ########## File path: build.sbt ########## @@ -68,8 +68,8 @@ lazy val runtime2 = Project("daffodil-runtime2", file("daffodil-runtime2 .settings(commonSettings) .settings(publishArtifact in (Compile, packageDoc) := false) .settings( - Compile / cCompiler := "cc", - Compile / ccArchiveCommand := "ar", + Compile / cCompiler := sys.env.getOrElse("CC", "cc"), + Compile / ccArchiveCommand := sys.env.getOrElse("AR", "ar"), Review comment: Is it worth mentiong the AR enironment variable in the BUILD setup readme? ########## File path: daffodil-runtime2/src/main/resources/c/libruntime/errors.h ########## @@ -18,8 +18,16 @@ #ifndef ERRORS_H #define ERRORS_H -#include <stdio.h> // for FILE, size_t +#include <stdbool.h> // for bool #include <stdint.h> // for int64_t +#include <stdio.h> // for FILE, size_t + +enum Limits +{ + LIMIT_DIAGNOSTICS = 100, // limits how many diagnostics can accumulate + LIMIT_NAME_LENGTH = 9999, // limits how long infoset names can become Review comment: This limit seems pretty huge. I would think we could make this smaller and reduce memory usage? ########## File path: daffodil-runtime2/src/main/scala/org/apache/daffodil/runtime2/generators/BinaryAbstractCodeGenerator.scala ########## @@ -0,0 +1,70 @@ +/* + * 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.generators + +import org.apache.daffodil.dsom.ElementBase +import org.apache.daffodil.schema.annotation.props.gen.BitOrder +import org.apache.daffodil.schema.annotation.props.gen.ByteOrder +import org.apache.daffodil.schema.annotation.props.gen.OccursCountKind + +trait BinaryAbstractCodeGenerator { + + def binaryAbstractGenerateCode(e: ElementBase, initialValue: String, prim: String, + parseArgs: String, unparseArgs: String, cgState: CodeGeneratorState): Unit = { + + // For the time being this is a very limited back end. + // So there are some restrictions to enforce. + e.schemaDefinitionUnless(e.bitOrder eq BitOrder.MostSignificantBitFirst, "Only dfdl:bitOrder 'mostSignificantBitFirst' is supported.") + e.schemaDefinitionUnless(e.byteOrderEv.isConstant, "Runtime dfdl:byteOrder expressions not supported.") + e.schemaDefinitionUnless(e.elementLengthInBitsEv.isConstant, "Runtime dfdl:length expressions not supported.") + + val fieldName = e.namedQName.local + val byteOrder = e.byteOrderEv.constValue + val conv = if (byteOrder eq ByteOrder.BigEndian) "be" else "le" + val arraySize = if (e.occursCountKind == OccursCountKind.Fixed) e.maxOccurs else 0 + val fixed = e.xml.attribute("fixed") + val fixedValue = if (fixed.isDefined) fixed.get.text else "" + + def addStatements(deref: String): Unit = { + val initStatement = s" instance->$fieldName$deref = $initialValue;" + val parseStatement = + s""" parse_${conv}_$prim(&instance->$fieldName$deref, $parseArgs); + | if (pstate->error) return;""".stripMargin + val unparseStatement = + s""" unparse_${conv}_$prim(instance->$fieldName$deref, $unparseArgs); + | if (ustate->error) return;""".stripMargin + cgState.addSimpleTypeStatements(initStatement, parseStatement, unparseStatement) + + if (fixedValue.nonEmpty) { + val init2 = "" + val parse2 = + s""" parse_validate_fixed(instance->$fieldName$deref == $fixedValue, "$fieldName", pstate); Review comment: I think this only supports fixed values where the primitive is a numeric? Are string primitives supported yet, or do we need a check here that SDE's if hasFixedValue but primitive type is not a numeric? -- 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]
