OCamlPro / gnucobol

A clone of the sourceforge GnuCOBOL compiler from COBOL to C.
https://get-superbol.com
GNU Lesser General Public License v3.0
16 stars 21 forks source link

Auto-detect free format from wrong indicator on first line #81

Closed lefessan closed 1 year ago

lefessan commented 1 year ago

Simpler version than what ws proposed in https://sourceforge.net/p/gnucobol/feature-requests/45/

codecov-commenter commented 1 year ago

Codecov Report

Merging #81 (d5c2389) into gcos4gnucobol-3.x (f550c9b) will decrease coverage by 0.04%. The diff coverage is 93.33%.

@@                  Coverage Diff                  @@
##           gcos4gnucobol-3.x      #81      +/-   ##
=====================================================
- Coverage              65.27%   65.23%   -0.04%     
=====================================================
  Files                     31       31              
  Lines                  56425    56440      +15     
  Branches               14723    14727       +4     
=====================================================
- Hits                   36830    36819      -11     
- Misses                 13807    13832      +25     
- Partials                5788     5789       +1     
Impacted Files Coverage Δ
cobc/cobc.c 71.10% <93.33%> (+0.09%) :arrow_up:
cobc/pplex.l 69.78% <0.00%> (-1.79%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

GitMensch commented 1 year ago

Note: the implicit change should also only be done "per file":

cobc -x main.cob other.cob - if main.cob is "implied fixed" then other.cob may be "implied free", to do so: issue the check in cobc.c (preprocess) after save_source_format = cobc_get_source_format (); or - possibly better - in ppparse() directly.

GitMensch commented 1 year ago

Note: the NEWS entry for -fformat should be adjusted to say something like "if not specified, then the format is tried to be automatically recognized as either fixed or free, depending on column 7 in the first non-space line".

lefessan commented 1 year ago

I will modify the Changelog and NEWS files when leaving the Draft mode.

@GitMensch What's the difference between make check and make distcheck ? On my computer, make check prints no errors on the modified testsuite, whereas make distcheck displays the same errors as the CI. Any idea why ?

Also, what is the canonical way to run only a subset of tests ? I currently use ../tests/testsuite -k cobc, is there something better ?

GitMensch commented 1 year ago

I will modify the Changelog and NEWS files when leaving the Draft mode.

+1, please also adjust gnucobol texi "2.1.3 Source format"

What's the difference between make check and make distcheck ? On my computer, make check prints no errors on the modified testsuite, whereas make distcheck displays the same errors as the CI. Any idea why ?

make check (when executed on the buildroot) runs make all in each directory, then builds the testsuite script if it isn't up-to date (actually part of make all in "tests"), then executes it. make distcheck packages the source tree, then tries to build from there, then runs make check in that. In 98% failures here are related to Makefile.am 's not containing newly added source files. Less likely, but also possible is that somewhere paths are newly hardcoded where they shouldn't, another reason that happens from time to time is that configure does not work as expected (only relevant when changing one of the autotools).

Also, what is the canonical way to run only a subset of tests ? I currently use ../tests/testsuite -k cobc, is there something better ?

Sure: make check TESTSUITEFLAGS="--jobs=6 -k cobc" (or --list or numbers). When doing changes I commonly run make --jobs=4 check TESTSUITEFLAGS="--jobs=6", then see only failures in the internal suite, then doing make check TESTSUITEFLAGS="--recheck --verbose".

lefessan commented 1 year ago

Hum... I didn't add any new file. Actually:

GitMensch commented 1 year ago

As seen in testsuite.log:

| FLAGS="--fixed -debug -Wall ${COBOL_FLAGS} -fno-diagnostics-show-option"
....
../../../tests/syn_misc.at:4747: $COMPILE_ONLY -fformat=auto -std=cobol85 prog.cob

the testsuite is correctly used "as expected".

If I understood you correctly you get the same result locally - make check works, but make distcheck fails - correct?

BTW: Do we need an adjustment in syn_misc.at - if yes, why?

lefessan commented 1 year ago

My understanding for the fixes is that I added --fixed to atlocal.in, so that all tests are called with that flag, which causes errors on a few tests. So, I had to add -fformat=auto to force auto-detection of format.

GitMensch commented 1 year ago

Looks like the tests pass now.

GitMensch commented 1 year ago

Note: I try to get to rc-2 state until tomorrow, so that would be the last possible time for this PR (which is in a "general good state") to be included in 3.2.

GitMensch commented 1 year ago

Still a draft?

I will modify the Changelog and NEWS files when leaving the Draft mode.

That's done.

please also adjust gnucobol texi "2.1.3 Source format"

Ah, that's likely the reason for the "draft" ;-)

GitMensch commented 1 year ago

Feel free to check that into gc3x branch.

Afterwards I'll drop the text-column (and maybe reformat the patch a bit), so that will be solved, too.

GitMensch commented 1 year ago

Just rechecking: is it likely that you check this in tonight or tomorrow?

lefessan commented 1 year ago

I am going to try to do it right now, I have half an hour and it's the first time I will commit in the SVN. FWIW, I am traveling all the week, so I am not very responsive.

lefessan commented 1 year ago

Yeah, I managed to do it ! Just hoping the SVN message is ok.

GitMensch commented 1 year ago

Thanks, I'll post-adjust it a bit, (tomorrow) but that's fine in general. Also congrats for the first svn commit, somehow most people stumble a bit first.

lefessan commented 1 year ago

To be honest, @nberth sent me a small « cheatsheet » on how to do it, a few days ago. It helped me a lot :-)