Fix some runtime2 bugs#545
Conversation
|
The rationale for lowering the dependency to 3.0 for mini-XML is what? That's kind of at cross purposes to all the scala-steward philosphy we use elsewhere now of moving forward to newer library versions as soon as they're available and stable. |
mbeckerle
left a comment
There was a problem hiding this comment.
+1 Just a few questions. Looks good.
tuxji
left a comment
There was a problem hiding this comment.
I still need to fix the Windows MSYS2 test failures. I know CC=clang is causing these test failures but I'm not sure how or why yet. Using the c/Makefile with CC=clang to compile the C files and run a test works fine on Windows MSYS2, so I need to debug the Scala code next.
tuxji
left a comment
There was a problem hiding this comment.
Well, all the checks pass now (yay!). Please let me know whether to make further changes after my responses to the last round of reviewer comments. I've already decided to update main.yml (removing diffutils) and BUILD.md (exporting variables on one line).
|
I've looked at what would happen if we split error codes between libruntime and libcli. It turns out that most of the existing error codes had been used only in libcli too. We would end up with this split between libcli and libruntime: // cli_errors.h - libcli
enum CliCode
{
CLI_FILE_CLOSE,
CLI_FILE_OPEN,
CLI_HELP_USAGE,
CLI_INVALID_COMMAND,
CLI_INVALID_INFOSET,
CLI_INVALID_OPTION,
CLI_MISSING_COMMAND,
CLI_MISSING_VALUE,
CLI_PROGRAM_ERROR,
CLI_PROGRAM_VERSION,
CLI_STACK_EMPTY,
CLI_STACK_OVERFLOW,
CLI_STACK_UNDERFLOW,
CLI_STRTOBOOL,
CLI_STRTOD_ERRNO,
CLI_STRTOI_ERRNO,
CLI_STRTONUM_EMPTY,
CLI_STRTONUM_NOT,
CLI_STRTONUM_RANGE,
CLI_UNEXPECTED_ARGUMENT,
CLI_XML_DECL,
CLI_XML_ELEMENT,
CLI_XML_ERD,
CLI_XML_GONE,
CLI_XML_INPUT,
CLI_XML_LEFT,
CLI_XML_MISMATCH,
CLI_XML_WRITE,
};
// errors.h - libruntime
enum ErrorCode
{
ERR_CHOICE_KEY,
ERR_FIXED_VALUE,
ERR_PARSE_BOOL,
ERR_STREAM_EOF,
ERR_STREAM_ERROR,
};@mbeckerle and @stevedlawrence , do you still want the codes and messages to be split up? I would end up duplicating enum CliCode and enum Error, struct CliError and struct Error, and function cli_continue_or_exit and continue_or_exit along with a couple of static functions. |
|
I like splitting the errors up that way yes. |
|
OK, I've moved everything CLI-related in libruntime out to libcli now. I made the error lookup mechanism pluggable (libcli has a hook to insert its own error lookup routine). I also made changes to help automate clang-format, include-what-you-use, and generated_code.[ch] updates later. |
Parse C executable's command line arguments with getopt instead of
argp (now have to put all options before first non-option argument and
lost ability to say "daffodil parse in.dat -o out.xml", but CLI was
meant for TDML runner rather than users anyway).
Update build instructions and CI workflow to build with clang instead
of gcc. Fix MSYS2 portability problems exposed by change (e.g., no
error() function).
Lower minimum required Mini-XML version from 3.2 to 3.0. Add
missing LICENSE and NOTICE files to daffodil-runtime2 jar.
Move everything CLI-related in libruntime out to libcli. Make error
lookup mechanism pluggable with a hook to let libcli insert its own
error lookup routine. Also make changes and move files to help
automate clang-format, include-what-you-use, and generated_code.[ch]
updates later.
DAFFODIL-2500, DAFFODIL-2505, DAFFODIL-2508
---
main.yml: Build C files with clang instead of gcc, but build mxml with
gcc on MYSYS2 since runtime2 tests break with mxml compiled by
clang. Also need diffutils on MSYS2 to avoid mxml makefile calling
Cygwin's cmp with unnecessary error message.
BUILD.md: Lower Mini-XML version to 3.0. Remove argp and gcc
instructions. Add clang instructions. Show how to set env vars on
one line. On MSYS2, install diffutils to let mxml makefile call cmp.
README.md: Lower Mini-XML version to 3.0.
.clang-format: Add StatementsMacros to allow clang-format to format
parsers.c and unparsers.c without unexpected indentation. Also make
all C files tell clang-format to leave include lines alone to prevent
clang-format from interfering with include-what-you-use (SortIncludes:
false isn't sufficient since clang-format still re-indents comments):
// clang-format off
#include ... // for ...
// clang-format on
LICENSE: Add to daffdil-runtime2 jar.
NOTICE: Add to daffdil-runtime2 jar.
Makefile: Add comments explaining how to use targets. Rename tests to
check target for consistency with GNU make standard. Put options
before non-option arguments in daffodil commands.
All C files: Tell clang-format to leave includes alone so it won't
interfere with include-what-you-use.
cli_errors.[ch]: Move all error codes and messages used by libcli
here. Use lookup tables and pluggable mechanism to allow libruntime
to look up and format libcli messages. Move/rename those CLI-related
enum constants here from libruntime:
- ERR_FILE_CLOSE (all ERR_* renamed to CLI_*)
- ERR_FILE_OPEN
- ERR_INFOSET_READ/WRITE (-> CLI_INVALD_INFOSET)
- ERR_STACK_EMPTY
- ERR_STACK_OVERFLOW
- ERR_STACK_UNDERFLOW
- ERR_STRTOBOOL
- ERR_STRTOD_ERRNO
- ERR_STRTOI_ERRNO
- ERR_STRTONUM_EMPTY
- ERR_STRTONUM_NOT
- ERR_STRTONUM_RANGE
- ERR_XML_DECL
- ERR_XML_ELEMENT
- ERR_XML_ERD
- ERR_XML_GONE
- ERR_XML_INPUT
- ERR_XML_LEFT
- ERR_XML_MISMATCH
- ERR_XML_WRITE
- LIMIT_XML_NESTING
daffodil_argp.[ch]: Rename to daffodil_getopt.[ch].
daffodil_getopt.c: Remove all argp structs and handlers. Simplify to
just a single daffodil_parse_cli function calling getopt and returning
a pointer to Error if any error happens. Note getopt has no portable
way to parse "daffodil [options] command [more options] arguments" so
callers now have to put all options before first non-option argument
("daffodil [options] command arguments").
daffodil_getopt.h: Include "errors.h" and make parse_daffodil_cli
return a pointer to Error so we can use continue_or_exit(error) for
all errors/messages.
daffodil_main.c: Remove fflush_continue_or_exit (no really good reason
to flush a stream right before closing it). Call continue_or_exit
after parse_daffodil_cli to handle any CLi error. Simplify rest of
code in main function. Rename error enumerations (ERR -> CLI) and
initialize c field with 0 instead of s field with NULL since we now
sort Error fields alphabetically (also needed in stack.c,
xml_reader.c, xml_writer.c).
errors.c: Remove <error.h> and replace error calls with fprintf/exit
calls since error's a GNU function which MYSYS2 clang doesn't provide.
Move eof_or_error, get_diagnostics, add_diagnostics up so they come
first in file. Replace error_message function containing switch
statement with error_lookup function indexing lookup table and
returning ErrorLookup structs. Change print_maybe_stop function to
call both error_lookup and cli_error_lookup (pluggable mechanism used
to look up CLI errors) and switch on ErrorField enumerations instead
of ErrorCode enumerations to print formatted messages with appropriate
Error fields. Add check_error_lookup function (after you rearrange
codes or messages or get an assertion failure in error_lookup, call
check_error_lookup from the debugger to check all codes map to
expected messages).
errors.h: Move CLI-related errors to cli_errors.h. Add ERR_ZZZ
enumeration to allow libcli's first error code to be numbered
consecutively following libruntime's last error code without a gap
between them. Add "int c" member to ErrorCode anonymous union for
getopt-related errors. Add ErrorField enumerations and ErrorLookup
struct to allow print_maybe_stop to print libcli's messages without
hardcoding any knowledge about them. Sort Error's fields
alphabetically. Move PState and UState structs back to infoset.h
where they really belong. Declare daffodil_program_version (we were
using argp_program_version before which is no longer available) and
move eof_or_error up before get_diagnostics. Declare cli_error_lookup
pluggable mechanism to allow print_maybe_stop to look up libcli's
messages without knowing anythng about them.
infoset.h: Move PState and UState structs back here.
parsers.c, unparsers.c: Initialize c field with 0 and use explicit .s
identifier to initialize s field since we now sort Error fields
alphabetically.
unparsers.h: Format from 80 to 100 columns with clang-format (missed
this one before).
CodeGenerator.scala: Pass relative/*.c filenames instead of absolute
filenames on Windows to avoid problem running MSYS2 clang compiler on
Windows. Remove "-largp" since we no longer need it on MSYS2.
Reorder pickCompiler's list of compilers to prefer ${CC}, cc, clang,
gcc in that order.
Runtime2DataProcessor.scala: Call executable with -o outfile before
parse or unparse in CLI command lines.
CodeGeneratorState.scala: Initialize c field with 0 since we now sort
Error fields alphabetically. Replace argp_program_version with
daffodil_program_version since we no longer use "argp.h".
NestedUnion.[ch] -> generated_code.[ch]: Move and regenerate with code
generator without renaming or manual editing to enable automated
update of generated_code.[ch] examples with daffodil's C code
generator in future.
ex_nums.[ch] -> generated_code.[ch]: Move and regenerate with code
generator without renaming or manual editing to enable automated
update of generated_code.[ch] examples with daffodil's C code
generator in future.
Rat.scala: Ignore generated_code.[ch] examples since daffodil's C code
generator doesn't include Apache license when generating them.
Remove dependency on argp and use getopt instead. Build with clang
instead of gcc. Lower minimum required Mini-XML version from 3.2 to
3.0. Add missing LICENSE and NOTICE files to daffodil-runtime2 jar.
main.yml: Build with clang and llvm-ar instead of cc and ar.
BUILD.md: Lower Mini-XML version to 3.0. Remove argp and gcc
instructions. Add clang instructions.
README.md: Lower Mini-XML version to 3.0.
LICENSE: Add to daffdil-runtime2 jar.
NOTICE: Add to daffdil-runtime2 jar.
Makefile: Improve comments explaining how to use targets.
daffodil_argp.[ch]: Rename to daffodil_getopt.[ch].
daffodil_getopt.c: Remove all argp structs and handlers. Simplify to
just a single function calling getopt and returning a pointer to Error
if any error happens. Note there is no portable way to parse
"daffodil [options] command [more options] arguments" with getopt. We
will code the simplest way (hope that getopt will move all non-option
arguments to the end) and users will have to put all options before
all arguments if their getopt doesn't support that behavior ("daffodil
[options] [more options] command arguments").
daffodil_getopt.h: Include "errors.h" and make parse_daffodil_cli
return a pointer to Error so we can use continue_or_exit(error) for
all errors/messages.
daffodil_main.c: Remove fflush_continue_or_exit (no really good reason
to flush a stream right before closing it). Call continue_or_exit
after parse_daffodil_cli to handle any CLi error. Simplify rest of
code in main function.
errors.c: Move eof_or_error, get_diagnostics, add_diagnostics up so
they come first in file. Add error_message switch cases to define all
CLI messages. Add print_maybe_stop switch cases to print all CLI
messages, making sure to call printf & exit instead of error for CLI
help & version messages.
errors.h: Add CLI messages to ErrorCode and "int c" member to
ErrorCode anonymous union. Rename ERR_INFOSET_READ/WRITE to
CLI_INVALID_INFOSET since it's a CLI message too. Declare
daffodil_program_version (we were using argp_program_version before
which is no longer available) and move eof_or_error up before
get_diagnostics.
NestedUnion.c, ex_nums.c: Regenerate with code generator.
CodeGenerator.scala: Remove "-largp" since we no longer need it.
CodeGeneratorState.scala: Replace argp_program_version with
daffodil_program_version since we no longer use "argp.h".
DAFFODIL-2500 DAFFODIL-2505 DAFFODIL-2508