CI adjustments for gcos4gnucobol-3.x

[nberth]

• Upgrade versions of github actions (some are being deprecated);
• Perform tests before install when doing the opposite is not very well justified.

[nberth]

@ddeclerck @GitMensch Some Windows workflows appear to hang during tests (but not always). Do you know if that's an "expected" state of affairs?

[ddeclerck]

@ddeclerck @GitMensch Some Windows workflows appear to hang during tests (but not always). Do you know if that's an "expected" state of affairs?

Yeah, this is a known problem with MSVC Debug targets. Under a proper machine with a GUI you'd get a warning in a popup window, but in a CI, without a GUI, it just hangs.

It was working before though. Now it gets stuck on 132: PROGRAM COLLATING SEQUENCE. Seems this is caused by a recent commit on SVN: 5310.

[GitMensch]

For the changes done: LGTM; we may want to add some more pieces though - see review comments.

[GitMensch]

Oh, and one general thing - to be easier to review failures, please use, after make check a || make check TESTSUITEFLAGS="--recheck --verbose" in all environments, this will show the output nicely on the CI output with most details.

[GitMensch]

ci changes from #172 should go in here as well

[ddeclerck]

ci changes from #172 should go in here as well

For @nberth: it just amounts to removing the whole "Adjust testsuite for Debug target" step in MSVC workflows, and adding continue-on-error: true to the "Run testsuite" step (or maybe something like continue-on-error: ${{ matrix.target == 'Debug' }} - until we resolve all the problems).

[nberth]

https://github.com/OCamlPro/gnucobol/actions/runs/10522831045/job/29156289216?pr=170#step:8:3908

-pedantic -std=c89 was a bit bold; won't be fixed in this PR though.

[GitMensch]

https://github.com/OCamlPro/gnucobol/actions/runs/10522831045/job/29156289216?pr=170#step:8:3908

-pedantic -std=c89 was a bit bold; won't be fixed in this PR though.

Yes, and it wouldn't be reasonable to look for fixing all of those either.
The following should:

 In file included from ../../cobc/scanner.l:100:
../../cobc/cobc.h:108:23: warning: comma at end of enumerator list [-Wpedantic]
  108 |         CB_FORMAT_AUTO,         /* Auto-detect format */
      |                       ^
../../cobc/cobc.h:272:28: warning: comma at end of enumerator list [-Wpedantic]
  272 |         CB_SUB_CHECK_RECORD,    /* PENDING */
      |                            ^
In file included from ../../cobc/scanner.l:100:
../../cobc/config.def:91:21: warning: ISO C forbids forward references to ‘enum’ types [-Wpedantic]
   91 | CB_CONFIG_ANY (enum cb_assign_type, cb_assign_type_default, "assign-clause",
      |                     ^~~~~~~~~~~~~~
../../cobc/cobc.h:551:8: note: in definition of macro ‘CB_CONFIG_ANY’
  551 | extern type                     var;
      |        ^~~~
In file included from ../../cobc/scanner.l:101:
../../cobc/tree.h:1263:30: warning: comma at end of enumerator list [-Wpedantic]
 1263 |         BOP_SHIFT_RC    = 'd',  /* ( x >> y circular-shift ) */
      |                              ^
../../cobc/tree.h:1645:36: warning: comma at end of enumerator list [-Wpedantic]
 1645 |         CB_REPLACE_TRAILING     = 2,
      |                                    ^
../../cobc/tree.h:2802:25: warning: comma at end of enumerator list [-Wpedantic]
 2802 |         CB_COLSEQ_EBCDIC,
      |                         ^
../../cobc/scanner.l:149:22: warning: comma at end of enumerator list [-Wpedantic]
  149 |         CB_LITERAL_NC,
      |                      ^
../../cobc/scanner.l: In function ‘scan_ebcdic_char’:
../../cobc/scanner.l:1370:14: error: C++ style comments are not allowed in ISO C90

and I'm looking to fix that upstream now.

[GitMensch]

Just to share some finding which was new to me: bison generates code like the following:

  case 3291: /* _dot_or_else_area_a: "Identifier (Area A)"  */
#line 20470 "../../cobc/parser.y"
  {
	/* No need to raise the error for *_IN_AREA_A tokens */
	(void) cb_verify (cb_missing_period, _("optional period"));
	cobc_repeat_last_token = 1;
  }
#line 34469 "parser.c"
    break;

which is the reason that gcc -std=c89 -pedantic raises the following error:

../../cobc/parser.y:20475:7: warning: line number out of range
20475 | ;
      |       ^    

the "computed" line number 20475 is the line from the generated code above

#line 34469 "parser.c"

and the message is because C89 only allows SHORT_MAX there:

You will notice that you only get the warning with -pedantic. gcc is warning because C89 has a limit of 32767.

According to C99,

# line digit-sequence new-line

causes the implementation to behave as if the following sequence of source lines begins with a source line that has a line number as specified by the digit sequence (interpreted as a decimal integer). The digit sequence shall not specify zero, nor a number greater than 2147483647.

[...] C99 raised the limit.

The only way to get around that would be to move ~2000 lines out of parser.y (reducing this huge file is a goal in general).
The "original" source parser.y is the biggest we have, currently ~ 20600 LOC, and then of course the generated C file is even bigger.

The easiest way to get the 2000 lines out is to move nearly everything in %{ }% to a separate "header" file, like parserc.h.
After doing that we may move longer code blocks to more static functions in there as well.
... or, more useful. move the functions into a separate C file, having only the defines and the function declarations in this header which then also speeds up building (often we only adjust the content within those functions, which now triggers a regeneration of parser.c (.h is already restored if not different by the build system), which triggers a recompile of that beast. Moving the functions out will tame the beast a bit and for the case of only changing the functions we'd drop that completely.

Note that GnuCOBOL generated functions can easily get > 100.000 LOC, so this may raise additional compatibility issues with generated sources on ancient environments.

[nberth]

Note that GnuCOBOL generated functions can easily get > 100.000 LOC, so this may raise additional compatibility issues with generated sources on ancient environments.

Slipping in a s/\n\([^#]\)/\1/g (sort of) could help with LOC 😆

[GitMensch]

After several adjustments there are now only 3 error in the testsuite with a compile of gcc -std=c89 -pedantic were 3 errors that are now fixed as well:

544. run_fundamental.at:3527: testing Entry point visibility (1) ...
../../tests/run_fundamental.at:3552: $COMPILE prog.cob
../../tests/run_fundamental.at:3553: $COMPILE_MODULE module.cob
--- /dev/null   2024-08-23 10:21:01.764081026 +0000
+++ /workspace/gnucobol/build/tests/testsuite.dir/at-groups/544/stderr  2024-08-23 16:08:20.093913497 +0000
@@ -0,0 +1,4 @@
+/tmp/cob289710_0.c: In function 'module_module_init':
+/tmp/cob289710_0.c:235:34: warning: ISO C forbids conversion of object pointer to function pointer type [-Wpedantic]
+  235 |   module->module_entry.funcptr = (void *(*)())module;
+      |                                  ^
544. run_fundamental.at:3527:  FAILED (run_fundamental.at:3553)

783. run_misc.at:4772: testing CALL C with callback, PROCEDURE DIVISION EXTERN ...
../../tests/run_misc.at:4826: $COMPILE -Wno-unfinished -o prog prog.cob cprog.c
--- /dev/null   2024-08-23 10:21:01.764081026 +0000
+++ /workspace/gnucobol/build/tests/testsuite.dir/at-groups/783/stderr  2024-08-23 16:08:20.365913248 +0000
@@ -0,0 +1,11 @@
+cprog.c: In function 'cprog':
+cprog.c:5:8: warning: ISO C does not support omitting parameter names in function definitions before C2X [-Wpedantic]
+    5 | cprog (void *)
+      |        ^~~~~~
+cprog.c:12:33: error: expected ')' before 'cb'
+   12 |    (int (*)(char *, int, char *)cb)(p1, p2, p3);
+      |    ~                            ^~
+      |                                 )
+cprog.c:12:4: warning: ISO C forbids conversion of object pointer to function pointer type [-Wpedantic]
+   12 |    (int (*)(char *, int, char *)cb)(p1, p2, p3);
+      |    ^
../../tests/run_misc.at:4826: exit code was 1, expected 0
783. run_misc.at:4772:  FAILED (run_misc.at:4826)

784. run_misc.at:4841: testing CALL C with callback, ENTRY-CONVENTION EXTERN ...
../../tests/run_misc.at:4902: $COMPILE -Wno-unfinished -o prog prog.cob cprog.c
--- /dev/null   2024-08-23 10:21:01.764081026 +0000
+++ /workspace/gnucobol/build/tests/testsuite.dir/at-groups/784/stderr  2024-08-23 16:08:20.465913157 +0000
@@ -0,0 +1,4 @@
+cprog.c: In function 'cprog':
+cprog.c:13:5: warning: ISO C forbids conversion of object pointer to function pointer type [-Wpedantic]
+   13 |    ((int (*)(char *, int, char *))cb)(p1, p2, p3);
+      |     ^
784. run_misc.at:4841:  FAILED (run_misc.at:4902)

and these are not depending on -std=c89 at all.

libcob has only a long long warning on c89 (we commonly use something else in that case);
cobc has the amount of line in parser.c + some strange warnings on stdio.h not included in generated flex sources (which is included in its generation itself).

This project did help me to understand why we had - but only in one place - a "split memcpy" generation to 509 bytes - that is because since C89 there is only the guarantee for strings supported with this size.
I've adjusted this code and applied something similar also to other places, but coverage shows that not all of those were even tested in make checkall. configure now checks for a supported length of 2048, and if this isn't given (which is likely embedded where GMP is hard to get or very ancient or ... pedantic) stays below 509 chars.

and just FYI: json.h uses variadic macros, so for those environments above cjson.h is a necessary falback.

I also added some testcases for previous untested scenarios in that area and should commit that until Sunday (Changelogs missing one of the tests fail).

The split of parser.y is planned since at least 2020 - https://sourceforge.net/p/gnucobol/feature-requests/371 and a user report of

#20099-D: Exceeding compiler resource limits, routine: yyparse; some optimizations skipped. Use +Onolimit if override desired"

on HP-UX.

[nberth]

The CI should have no download, only the cache of newcob.val - "make test" will do the download and unpacking as needed, using multiple URLs if necessary.

The issue is that the URL is not dead per se: the download operation (via `wget -t1 -T5) does not fail and brings and HTML page instead (which then ends up in the workflow cache). This is why I think it is better to use the sourceforge address, at least while this link is not fixed.

That's really bad. Please switch URLs to SF fir in the Makefile (upstream).

Ok I just did. The parts on newcob.val.Z are just commented, in case we find a fixed URL for that file (if that matters at all).

[ddeclerck]

Ouch https://github.com/OCamlPro/gnucobol/actions/runs/10595691899/job/29362006666?pr=170#step:14:22 @ddeclerck @GitMensch would you have any idea how to solve that kind of issue? (it's MSYS2)

Seems to be an open issue on V4 of upload-artifact. Cf actions/upload-artifact#485
I'd be tempted to downgrade to V3 until this is fixed...

[GitMensch]

This is fine so I pull that in. Note that the new failure is because of C++ comments which will be fixed with my changes that are current "under work" (which will also fix most of C89 pedantic).

[GitMensch]

@nberth Any chance to rebase and work on #109?