Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.94k stars 553 forks source link

Win32: (gcc) build without `USE_MINGW_ANSI_STDIO` fail at t/op/sprintf2.t #20214

Open bram-perl opened 2 years ago

bram-perl commented 2 years ago

On Windows 10 - using with gcc/gmake - t/op/sprintf2.t fails when build is done without USE_MINGW_ANSI_STDIO:[^1]

# Failed test 1704 - sprintf( "%.54g", 0.3 ) renders correctly at op/sprintf2.t line 1197
#      got "0.29999999999999999"
# expected eq "0.299999999999999988897769753748434595763683319091796875"

Looking at:

I'm guessing this failure is somewhat expected on builds not using USE_MINGW_ANSI_STDIO? Can/should this test be skipped when build is done without USE_MINGW_ANSI_STDIO? Is there an easy way to detect this in the test?

Steps to reproduce

  1. Disable USE_MINGWS_ANSI_STDIO in win32/GNUmakefile, i.e.:

    diff --git a/win32/GNUmakefile b/win32/GNUmakefile
    index 81ded707b9..df5f417978 100644
    --- a/win32/GNUmakefile
    +++ b/win32/GNUmakefile
    @@ -142,7 +142,7 @@ USE_PERLIO  := define
    # (s)printf formatting of numbers, whereas the MS runtime might not.
    # This option has no effect on MSVC builds.
    #
    -USE_MINGW_ANSI_STDIO := define
    +#USE_MINGW_ANSI_STDIO := define
    
    #
    # Comment this out if you want the legacy default behavior of including '.' at
  2. Build
  3. Run t/op/sprintf2.t

Result:

Test failure:

C:\Perl\perl-current\t>.\perl.exe harness op/sprintf2.t
op/sprintf2.t .. 1437/? # Failed test 1704 - sprintf( "%.54g", 0.3 ) renders correctly at op/sprintf2.t line 1197
#      got "0.29999999999999999"
# expected eq "0.299999999999999988897769753748434595763683319091796875"
op/sprintf2.t .. Failed 1/1704 subtests
        (less 30 skipped subtests: 1673 okay)

Expected result

No test failure (/test skipped/...)

[^1]: I noticed this on my Windows 10 smoker due to a recent change in Test::Smoke which now builds without USE_MINGW_ANSI_STDIO.

sisyphus commented 2 years ago

Is there an easy way to detect this in the test?

Yes - we would just need to check $Config{ccflags} for the presence of -D__USE_MINGW_ANSI_STDIO.

Given that we already TODO the test for Cygwin, VMS, and for non-recent MSVC-built Windows perls, I guess it wouldn't hurt to TODO it for Windows perls built without __USE_MINGW_ANSI_STDIO. (The precedent is well-established.)

Perl is just using the underlying runtime's "%g" formatting - so, in that sense, it's not perl's fault. But it's something that's probably never going to be fixed in those runtimes, and (I hope) attempts to fix it via fiddling with the perl source will also not be forthcoming - all of which makes a bit of a mockery of the "TODO" label.

Given that the test runs successfully with MSVC142 (VS 2019), I expect that it will also run successfully (either now, or at some point in the future) on mingw-built perls built without -D__USE_MINGW_ANSI_STDIO. I'll check on that and get back.

What was $Config{gccversion} for your failing perl ?

Cheers, Rob

sisyphus commented 2 years ago

After some testing, I can see that this test will pass even if __USE_MINGW_ANSI_STDIO is not defined, if and only if mingw runtime version (__MINGW64_VERSION_MAJOR) is greater than or equal to 8. So the TODO would only need to be enforced if mingw runtime version is less than 8. (Latest available Strawberry Perl has mingw runtime 6.0.0.)

Unfortunately, AFAIK, the mingw runtime version cannot presently be ascertained during the running of the test script.

I once proposed (https://github.com/Perl/perl5/issues/19495) that we should have a %Config key that provides this value. But I became disheartened when it became clear that the preferred %Config key should be a newly created one - a task that became a bit too messy for my liking. I was initially hoping to use the currently existing (but empty on Windows) gnulibc_version key, mainly because that's so easy to implement. That's still my preferred approach. I've actually been doing that in my own builds of perl-5.36.0 and blead ever since. It works fine, and is even displayed in the perl -V listing.

Cheers, Rob

bram-perl commented 2 years ago

What was $Config{gccversion} for your failing perl ?

Using the toolchain that came with latest Strawberry Perl, so gcc 8.3.0 (so according to your follow up reply that should be mingw runtime 6.0.0)

I once proposed (#19495) that we should have a %Config key that provides this value. But I became disheartened when it became clear that the preferred %Config key should be a newly created one - a task that became a bit too messy for my liking.

Having it in %Config sounds like a good plan; (I'm not sure the comments in #19495 are all correct, verifying some of those now.)

xenu commented 2 years ago

This shouldn't be configurable. Why would anyone want to use the buggy version of printf?

Also, on new mingw-w64 versions that option is always enabled, regardless of what you set in Makefile.

sisyphus commented 2 years ago

Why would anyone want to use the buggy version of printf?

For compatibility with pre-MSVC142 versions of Visual Studio. At least, that's the sort of reasoning I've seen presented in the past ... rather feeble, IMO. That sort of consideration was probably more prevalent last century than it is today.

Also, on new mingw-w64 versions that option is always enabled, regardless of what you set in Makefile.

I hadn't picked up on that - thanks, @xenu, for pointing it out. AFAICS, they first defined __USE_MINGW_ANSI_STDIO in runtime 7, but they didn't define it to a true value so it didn't have the intended affect. (Ooops ? ... or maybe they first wanted to verify that defining the symbol to 0 had no effect.) From runtime 8, they've defined it to 1, which aligns with the behaviour we see.

One can still disable that setting with -D__USE_MINGW_ANSI_STDIO=0, whereby the "buggy" printf behaviour returns. And I think you could achieve that override by hacking the GNUmakefile. But I think @xenu is correct in that the current GNUmakefile "configuration" options don't enable such a thing - and you would have to "hack" that file.

sisyphus commented 2 years ago

I've just (speculatively) created https://github.com/Perl/perl5/pull/20265 which adds mingw runtime version to %Config.