gnucobol icon indicating copy to clipboard operation
gnucobol copied to clipboard

Ci adjustments for `gcos4gnucobol-3.x`

Open nberth opened this issue 1 year ago • 5 comments

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

nberth avatar Aug 20 '24 07:08 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?

nberth avatar Aug 20 '24 08:08 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?

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.

ddeclerck avatar Aug 20 '24 08:08 ddeclerck

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 avatar Aug 20 '24 20:08 GitMensch

ci changes from #172 should go in here as well

GitMensch avatar Aug 22 '24 15:08 GitMensch

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).

ddeclerck avatar Aug 22 '24 17:08 ddeclerck

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.

nberth avatar Aug 23 '24 08:08 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.

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 avatar Aug 23 '24 09:08 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.

GitMensch avatar Aug 23 '24 11:08 GitMensch

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 :laughing:

nberth avatar Aug 23 '24 13:08 nberth

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.

GitMensch avatar Aug 23 '24 16:08 GitMensch

@GitMensch Note: it appears that https://www.itl.nist.gov/div897/ctg/suites/newcob.val.Z is now an invalid link redirected to a generic web-page). I've changed the workflows for GC3 to use the tar.gz that's on sourceforge, but I think there are other places where a similar change is requires (for GC4 probably, and maybe on appveyor?).

nberth avatar Aug 27 '24 10:08 nberth

Please revert that part as we explicitly don't support C++ comments and a falling CI ID therefore the current thing if used.

GitMensch avatar Aug 27 '24 14:08 GitMensch

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.

Am 27. August 2024 12:09:17 MESZ schrieb Nicolas Berthier @.***>:

@GitMensch Note: it appears that https://www.itl.nist.gov/div897/ctg/suites/newcob.val.Z is now a dead link. I've changed the workflows for GC3 to use the tar.gz that's on sourceforge, but I think there are other places where a similar change is requires (for GC4 probably, and maybe on appveyor?).

-- Reply to this email directly or view it on GitHub: https://github.com/OCamlPro/gnucobol/pull/170#issuecomment-2312107632 You are receiving this because you were mentioned.

Message ID: @.***>

GitMensch avatar Aug 27 '24 14:08 GitMensch

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.

nberth avatar Aug 27 '24 14:08 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).

GitMensch avatar Aug 27 '24 19:08 GitMensch

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).

nberth avatar Aug 27 '24 21:08 nberth

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)

nberth avatar Aug 28 '24 11:08 nberth

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 https://github.com/actions/upload-artifact/issues/485 I'd be tempted to downgrade to V3 until this is fixed...

ddeclerck avatar Aug 28 '24 12:08 ddeclerck

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 avatar Aug 28 '24 19:08 GitMensch

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

GitMensch avatar Aug 28 '24 19:08 GitMensch