Fix some Windows bugs in 4.x

[ddeclerck]

This PR fixes 19 failing tests in the Windows MSVC CI (8 tests under MSYS2).

[ddeclerck]

@GitMensch Note that at least two more tests could be easily fixed (opinion needed).

• INDEXED undeclared keys

Difference in error message :

-libcob: prog.cob:82: error: unknown file error (status = 39) for file file2 ('./fileX') on OPEN
+libcob: prog.cob:82: error: unknown file error (status = 39) for file file2 ('./fileX' => .\fileX) on OPEN

This is because cob_get_filename_print considers ./fileX and .\fileX to be different.
I see two options here:

• using a dedicated path comparison that ignores separator differences
• normalizing paths to use unix-style spearators

Any preference ?

• trace feature with indexed EXTFH

The error is as follows:

fatal error C1189: #error:  Macro definition of snprintf conflicts with Standard Library function declaration

This should already be taken care of by this definition in common.h:

#if !COB_USE_VC2015_OR_GREATER
/* VC2015+ provides standard function with plain
   name, old posix emulation with underscore */
#define snprintf		_snprintf
#endif

However, COB_USE_VC2015_OR_GREATER is defined in cobinternal.h, which is not included in common.h. Should we move these COB_USE_VC* to common.h ? Or since this is the only occurrence, just inline the definition of COB_USE_VC2015_OR_GREATER in the definition of snprintf ?

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 50.00000% with 8 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (gitside-master@f27c23a). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
libcob/fileio.c	53.84%	1 Missing and 5 partials ⚠️
libcob/cobcapi.c	0.00%	1 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@                Coverage Diff                @@
##             gitside-master     #301   +/-   ##
=================================================
  Coverage                  ?   62.94%           
=================================================
  Files                     ?       40           
  Lines                     ?    72837           
  Branches                  ?    20312           
=================================================
  Hits                      ?    45849           
  Misses                    ?    19799           
  Partials                  ?     7189           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[GitMensch]

Wouldn't be the changes to build_windows be relevant for 3.x already (or did we just not merge them yet)?

Can you please commit https://github.com/OCamlPro/gnucobol/pull/300/changes upstream? I guess that will fix at least part of the MSVC issues here as well, no?

... why don't we have that much failures on MSYS2 also on 3.x? (the fileio parts are clear, these are new in 4.x and not that well tested on Windows)

[GitMensch]

switch this condition, please, same below

[ddeclerck]

What do you mean by "switching" ?

Note that an identical condition already exists in cob_file_close to protect a call to fdcobsync - I merely copied that.

[GitMensch]

		if (f->open_mode != COB_OPEN_INPUT
		 && f->fd >= 0) {

[GitMensch]

If possible at all, we should prefer replacing most dump calls to execute CBL_OC_DUMP instead...

are those tests fixed in 3.x (possibly just in a different place there?

[ddeclerck]

All the tests in 3.x that use C functions correctly use COB_EXT_EXPORT. Some tests specific to 4.x did not, hence the fixes here.

As for CBL_OC_DUMP, note that it "does not work yet" in 4.x (cf tests "LINE SEQUENTIAL" (run_file.at) failing because CBL_OC_DUMP can not be found). I'd have to dig into that, but since I'm going for vacation this afternoon, it's unlikely I'll have time to tackle this issue before.

[ddeclerck]

Wouldn't be the changes to build_windows be relevant for 3.x already (or did we just not merge them yet)?

Yes, I'm already preparing a PR for that - just my focus was on 4.x for now (takes time to context-switch my brain between the two branches). It will also include the fixes that also apply to 3.x (protecting the call to fdcobsync and cob_free in cob_get_buff).

Can you please commit https://github.com/OCamlPro/gnucobol/pull/300/changes upstream? I guess that will fix at least part of the MSVC issues here as well, no?

Yes, will do that right now. And this has to be merged in 4.x too (I'll add it to the GC3/GC4 merge PR).

... why don't we have that much failures on MSYS2 also on 3.x? (the fileio parts are clear, these are new in 4.x and not that well tested on Windows)

Many failures were just because of new code in 4.x. Also, some new tests in 4.x "accidentally" revealed bugs that are present in 3.x but not triggered.

P.S : any suggestion regarding the two bugs mentionned in #301 (comment) ?

[GitMensch]

This is because cob_get_filename_print considers ./fileX and .\fileX to be different. I see two options here:

* using a dedicated path comparison that ignores separator differences

that's reasonable for [_WIN32] - but then this should likely include /c -> [cC]: (I tend to not ignore file case, as FAT32 and NTFS case-sensitive exists on both Windows and Linux)...

This should already be taken care of by this definition in common.h.... in cobinternal.h

OK, that's a "new" bug in GC4 then; I understand Ron's idea with cobinternal.h, but ... am not 100% sure what to do with this and other headers splits. Please think about the general approach and then share your thoughts.

For that specific test where a C test file is compiled: do we need snprintf there in first case?
Isn't there a function in libcob that we could call instead (when then also ensures that the runtime from libcob is used)?

[GitMensch]

Also, some new tests in 4.x "accidentally" revealed bugs that are present in 3.x but not triggered.

I hope we get testcases for those together with the bugfix to 3.x :-)