tuxji commented on a change in pull request #545: URL: https://github.com/apache/daffodil/pull/545#discussion_r623515706
########## File path: README.md ########## @@ -43,7 +43,7 @@ For more information about Daffodil, see <https://daffodil.apache.org/>. * JDK 8 or higher * SBT 0.13.8 or higher * C compiler C99 or higher -* Mini-XML Version 3.2 or higher +* Mini-XML Version 3.0 or higher Review comment: Yes, the rationale for lowering the Mini-XML dependency is so that people won't have to build Mini-XML 3.2 from source if they already have 3.1 on their system (Steve's Fedora 33 had 3.1 although Fedora 34 has 3.2 now). I looked at the changes between 3.0, 3.1, and 3.2 carefully and determined that there were only fairly minor changes, nothing that would make someone regret running 3.1 or 3.0 instead of 3.2. I do agree with scala-steward's philosophy of moving to newer library versions as soon as they're available and stable but the preferred definition of "available" for a C developer may be whatever is available in a distribution's repositories. ########## File path: daffodil-runtime2/src/main/resources/org/apache/daffodil/runtime2/c/libruntime/errors.c ########## @@ -17,34 +17,109 @@ #include "errors.h" #include <assert.h> // for assert -#include <error.h> // for error #include <inttypes.h> // for PRId64 #include <stdbool.h> // for bool, false, true -#include <stdio.h> // for NULL, feof, ferror, FILE, size_t -#include <stdlib.h> // for EXIT_FAILURE +#include <stdio.h> // for fprintf, stderr, NULL, feof, ferror, FILE, size_t, stdout +#include <stdlib.h> // for exit, EXIT_FAILURE -// error_message - return an internationalized error message +// eof_or_error - get pointer to error if stream has eof or error indicator set + +const Error * +eof_or_error(FILE *stream) +{ + if (feof(stream)) + { + static Error error = {ERR_STREAM_EOF, {NULL}}; + return &error; + } + else if (ferror(stream)) + { + static Error error = {ERR_STREAM_ERROR, {NULL}}; + return &error; + } + else + { + return NULL; + } +} + +// get_diagnostics - get pointer to validation diagnostics + +Diagnostics * +get_diagnostics(void) +{ + static Diagnostics diagnostics; + return &diagnostics; +} + +// add_diagnostic - add a new error to validation diagnostics + +bool +add_diagnostic(Diagnostics *diagnostics, const Error *error) +{ + if (diagnostics && error) + { + if (diagnostics->length < LIMIT_DIAGNOSTICS) + { + Error *err = &diagnostics->array[diagnostics->length++]; + err->code = error->code; + err->s = error->s; + return true; + } + } + return false; +} + +// error_message - get an internationalized error message static const char * error_message(enum ErrorCode code) Review comment: Yeah, I originally had these CLI messages in daffodil_getopt.c and thought upon it for a while before deciding it was easier to bundle all the messages in one place for easier internationalization and flow of control (daffodil_parse_cli needs only return statements, not printf and exit calls too). However, if we don't care about hardcoding 'daffodil' in the help, daffodil_parse_cli could pass the help message in err->s and both CLI_HELP_USAGE and CLI_PROGRAM_VERSION then would use the same format, "%s\n". Would you prefer that? ########## File path: daffodil-runtime2/src/main/resources/org/apache/daffodil/runtime2/c/libcli/daffodil_main.c ########## @@ -15,13 +15,13 @@ * limitations under the License. */ -#include <stdio.h> // for NULL, perror, FILE, fclose, fflush, fopen, stdin, stdout -#include <string.h> // for strcmp -#include "daffodil_argp.h" // for daffodil_parse, daffodil_parse_cli, daffodil_unparse, daffodil_unparse_cli, daffodil_cli, parse_daffodil_cli, DAFFODIL_PARSE, DAFFODIL_UNPARSE -#include "errors.h" // for continue_or_exit, Error, print_diagnostics, PState, UState, ERR_FILE_CLOSE, ERR_FILE_FLUSH, ERR_FILE_OPEN, ERR_INFOSET_READ, ERR_INFOSET_WRITE -#include "infoset.h" // for walkInfoset, InfosetBase, rootElement, ERD, VisitEventHandler -#include "xml_reader.h" // for xmlReaderMethods, XMLReader -#include "xml_writer.h" // for xmlWriterMethods, XMLWriter +#include <stdio.h> // for NULL, FILE, perror, fclose, fopen, stdin, stdout +#include <string.h> // for strcmp +#include "daffodil_getopt.h" // for daffodil_cli, parse_daffodil_cli, daffodil_parse, daffodil_parse_cli, daffodil_unparse, daffodil_unparse_cli, DAFFODIL_PARSE, DAFFODIL_UNPARSE Review comment: Which would you prefer? 1. Truncate to 100 columns (losing information) 2. Wrap to 100 columns 3. Keep as is (remember these comments are machine generated) ########## File path: BUILD.md ########## @@ -31,26 +31,32 @@ the latest [SBT] version following their websites' instructions or install them using your operating system's package manager. Since Daffodil now has a C backend as well as a Scala backend, you -will need a C compiler supporting the [C99] standard or later, the -[Mini-XML] library, and possibly the GNU [argp] library if your -system's C library doesn't include it already. You can install either -[gcc] or [clang] using your operating system's package manager. If -you can't install the [Mini-XML] library using your operating system's -package manager, you'll have to build it from source. We'll tell you -how to do that on Windows but the commands should work on other -operating systems too. - -You can set your environment variables "CC" and "AR" to the correct +will need a C compiler supporting the [C99] standard or later and the +[Mini-XML] library. You can install either [gcc] or [clang] using +your operating system's package manager. If you can't install the +[Mini-XML] library using your operating system's package manager, +you'll have to build it from source. We'll tell you how to do that on +Windows but the commands should work on other operating systems too. + +You can set your environment variables `CC` and `AR` to the correct commands (or set them to `true` to disable C compilation altogether) if you don't want `sbt compile` to call your C compiler with `cc` and `ar` as the default commands. -## Fedora 33 +## Fedora 34 -You can use the `dnf` package manager to install most of the build -requirements needed to build Daffodil: +You can use the `dnf` package manager to install most of the tools +needed to build Daffodil: - sudo dnf install gcc git java-11-openjdk-devel make mxml-devel pkgconf + sudo dnf install clang git java-11-openjdk-devel llvm make mxml-devel pkgconf + +If you want to use clang instead of gcc, you'll have to set your Review comment: The default compiler for sbt and make is cc, which is an alias for gcc. All I'm doing is showing how to install clang (which also installs gcc, FYI) and define the environment variables CC and AR to use clang instead of gcc. I have set up the CI workflow in main.yml to use clang by default, but I can't set up your own environment variables for you. You'll still have to set up CC and AR in your development environment if you want to use clang instead of gcc. Do you want me to make these instructions clearer? ########## File path: BUILD.md ########## @@ -61,10 +67,18 @@ commands you type will be able to call the C compiler. ## Ubuntu 20.04 -You can use the `apt` package manager to install most of the build -requirements needed to build Daffodil: +You can use the `apt` package manager to install most of the tools +needed to build Daffodil: + + sudo apt install build-essential clang-10 clang-format-10 default-jdk git libmxml-dev - sudo apt install build-essential default-jdk git libmxml-dev +If you want to use clang instead of gcc, you'll have to set your Review comment: FYI, both the apt install here and the pacman -S below actually install gcc too even though it's no longer specified on the command line. But the default system compiler is still gcc since it's hard to change that without running some extra commands as root or defining your CC environment variable. -- 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]
