abeltje / Test-Smoke

The Perl5 Core Smoke framework
6 stars 15 forks source link

win32/smoke.mk no longer sets `USE_MINGW_ANSI_STDIO` #70

Closed bram-perl closed 1 year ago

bram-perl commented 2 years ago

When using Test::Smoke on Windows (10) with gcc/gmake the build fails at t/op/sprintf2.t. Doing a manual build (of the same commit) does cause t/op/sprintf2.t to succeed.

Comparing win32/GNUMakefile with win32/smoke.mk;

Looking at the difference:

diff --git a/win32/GNUmakefile b/win32/smoke.mk
index 81ded707b9..7f7929b91f 100644
--- a/win32/GNUmakefile
+++ b/win32/smoke.mk
@@ -137,17 +137,17 @@ USE_PERLIO        := define
 #
 # Comment this out if you want to build perl without __USE_MINGW_ANSI_STDIO defined.
 # (If you're building perl with USE_LONG_DOUBLE defined then
 # __USE_MINGW_ANSI_STDIO will be defined whether or not this is uncommented.)
 # The advantage of defining __USE_MINGW_ANSI_STDIO is that it provides correct
 # (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

I'm guessing this was changed in commit aae8f4a0ca96b836233e338595b211db876d6df7

Because USE_MINGW_ANSI_STDIO is not defined it causes the test to fail.[^1]. In win32/GNUmakefile the option is enabled by default, In win32/smoke.mk it's disabled. Why?

[^1]: For the test failure I've (also) created https://github.com/Perl/perl5/issues/20214 but even if that gets fixed (or the test skipped) that still leaves the question on why Test::Smoke is now building without USE_MINGW_ANSI_STDIO.

abeltje commented 2 years ago

Hi Bram,

Thanks for your excellent detection again! Change [#aae8f] (https://github.com/abeltje/Test-Smoke/commit/aae8f4a0ca96b836233e338595b211db876d6df7) fixes the Configure_win32() in general for gmake, now that that is in order, we can see what's changed over time and add/remove defaults.

My suggestion would be to enable it by default and have a -UUSE_MINGW_ANSI_STDIO switch for the brave:

diff --git a/lib/Test/Smoke/Util.pm b/lib/Test/Smoke/Util.pm
index 094b735..920dd87 100644
--- a/lib/Test/Smoke/Util.pm
+++ b/lib/Test/Smoke/Util.pm
@@ -205,6 +205,7 @@ sub Configure_win32 {
         "-UWIN64"                    => "WIN64",
         "-Uusethreads"               => "USE_ITHREADS",
         "-Uuseithreads"              => "USE_ITHREADS",
+        "-UUSE_MINGW_ANSI_STDIO"     => "USE_MINGW_ANSI_STDIO",
         "-DDEBUGGING"                => "USE_DEBUGGING",
         "-DINST_DRV"                 => "INST_DRV",
         "-DINST_TOP"                 => "INST_TOP",
@@ -243,6 +244,7 @@ sub Configure_win32 {
         WIN64                    => 1,
         USE_SITECUST             => 0,
         DEFAULT_INC_EXCLUDES_DOT => 1,
+        USE_MINGW_ANSI_STDIO     => 1,
         USE_DEBUGGING            => 0,
         INST_DRV                 => undef,
         INST_TOP                 => undef,
bram-perl commented 2 years ago

The -UUSE_MINGW_ANSI_STDIO might not work for mingw-w64 v8+.

See: https://github.com/Perl/perl5/issues/20214#issuecomment-1236030361

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.

(I can't test that tho; I only have a gcc with an old mingw)

abeltje commented 2 years ago

I don't think that is a problem, having a -Umake_proper_working_perl switch shouldn't be our priority. As I understand it, the underlying problem cannot be fixed, or the fix is __USE_MINGW_ANSI_STDIO, if that will be the default (or even mandatory) that's fine - one less problem to fuzz about.

abeltje commented 1 year ago

Anyway we have this:

commit 7a426fffd3e0a46759ae98c7356be6635ba06772
Author: Abe Timmerman <abeltje@cpan.org>
Date:   Fri Sep 2 11:53:18 2022 +0200

    Add USE_MINGW_ANSI_STDIO as default for Configure_win32()

This is in 1.79_07 and will be in 1.80