Fix the Windows-only failures in the v2.2.2 test suite by bigpresh · Pull Request #1845 · PerlDancer/Dancer2

bigpresh · 2026-09-30T21:38:10Z

Fixes the three test files that fail on Windows and only on Windows in the
cpantesters results for v2.2.2
(report).
Each was a platform assumption, in the code or in the test, about the platform
it is running on.

t/unit/logger/format.t

%t and %u are documented as dd/Mon/yyyy hh:mm:ss, and were built by
handing "%d/%b/%Y %H:%M:%S" to strftime. The MinGW C runtime behind
Strawberry Perl renders %b as the month number, so a September log line came
out 29/9/2026 23:59:25 and failed the test's [A-Z][a-z]{2}.

The four date codes are now assembled field by field with sprintf, so they
zero-pad the same way everywhere, and the month falls back to the English
abbreviation only where strftime has no name to give back — so a localised
month name still works where the platform supports it. strftime is still used
for %{...}t, where the format string is the caller's own.

See the follow-up comment: an earlier revision of this PR used a fixed month
table, which would have changed %t/%u for every non-English locale, not just
for the platform that broke.

t/e2e/cli/gen.t

The generated MANIFEST and MANIFEST.SKIP were written on handles opened
without an explicit layer. On Windows that is a text-mode handle, so every
\n became \r\n and the pattern the generator appends to MANIFEST.SKIP
ended in a carriage return (^MyApp-App-\r), which does not match the path it
is there to match. Both files are now written with binmode ':raw', as
_copy_templates already did for every other file it writes.

Separately, the subtest checking that an explicit -d is honoured verbatim
used -d Other::App. A colon cannot appear in a Windows file name at all - it
introduces an alternate data stream - so no Windows file system can hold that
directory and a generator asked to make one there can only fail. That check now
uses a name Windows can hold, and the colon spelling is checked under a SKIP
on Windows, so the assertion still runs everywhere such a directory can exist.
The pre-existing assertions on the -d runs now diag STDERR/STDOUT when the
generator exits non-zero, since "died with status 5632" was not enough to
diagnose it from the report.

t/integration/handler/file.t

The surplus-../ case built a path back down to the fixture from the
filesystem root using $ROOT->relative('/'). On Windows that hands back the
whole path including the drive letter, and a drive letter in the middle of a
request path is not a name any file can have, so the handler's existence check
answered 404 before its containment check was reached - still safe, but the
wrong thing for a subtest about the guard to be exercising. The path is now
built from the fixture's own path with the volume and root stripped, so the
filesystem root is the only thing surplus ../ can collapse at on every
platform.

Verification

Full suite passes: 209 files, 1857 tests.
PERLIO=crlf (which reproduces Windows' text-mode newline translation on
other platforms) shows the generated MANIFEST/MANIFEST.SKIP coming out
with LF after the fix, and CRLF before it - and gen.t failing before and
passing after under the same setting.
The logger output is checked against a strftime patched to return the month
number: the old code reproduces the reported 29/9/2026 23:59:25 and fails
the test's regex, the new code renders 29/Sep/2026 23:59:25 and passes.

Not fixed here

t/config_reader.t also failed in that report, with a syntax error at line 17
(__FILE__() is not parseable on perl 5.14.4). That is an old-perl issue rather
than a Windows one, so it is left alone - worth its own fix, since it will fail
on any 5.14.

Three files fail on Windows and only on Windows in the cpantesters results for 2.2.2 (matrix.perl-magpie.org/results/ac560063-6c5e-1014-9b5f-c5c807c19ce6). Each is a platform assumption, in the code or in the test, about the platform it is running on. t/unit/logger/format.t %t and %u are documented as dd/Mon/yyyy hh:mm:ss and were built by handing "%d/%b/%Y %H:%M:%S" to strftime. The MinGW C runtime behind Strawberry Perl renders %b as the month number, so a September log line came out "29/9/2026 23:59:25" and failed the test's [A-Z][a-z]{2}. The four date codes are now assembled field by field with sprintf, and the month from a fixed Jan..Dec table, so that the documented format is the format produced on every platform. strftime is still used for %{...}t, where the format string is the caller's own and whatever their C library makes of it is what they asked for. t/e2e/cli/gen.t The generated MANIFEST and MANIFEST.SKIP were written on handles opened without an explicit layer. On Windows that is a text-mode handle, so every \n became \r\n, and the pattern the generator appends to MANIFEST.SKIP ended in a carriage return - "^MyApp-App-\r" - which does not match the path it is there to match. Both files are now written with binmode ':raw', as _copy_templates already did for every other file it writes. The subtest that checks an explicit -d is honoured verbatim used "-d Other::App". A colon cannot appear in a Windows file name at all - it introduces an alternate data stream - so no Windows file system can hold that directory and a generator asked to make one there can only fail. The check is now made with a name Windows can hold, and the colon spelling is checked under a skip on Windows, so the assertion still runs where such a directory can exist. t/integration/handler/file.t The surplus-../ case builds a path back down to the fixture from the filesystem root, using $ROOT->relative('/'). On Windows that hands back the whole path including the drive letter, and a drive letter in the middle of a request path is not a name any file can have, so the handler's existence check answered 404 before its containment check was ever reached. That is still safe, but it is the wrong thing for a subtest about the guard to be exercising. The path is now built from the fixture's path with the volume and root stripped, so the filesystem root is the only thing that surplus ../ can collapse at on every platform, and the test is back to testing the guard. Verified: the full suite passes (209 files, 1857 tests); the two generated files come out with LF under PERLIO=crlf, which reproduces Windows' text-mode newline translation elsewhere, before and after; and the logger output is checked against a strftime patched to return the month number, where the old code reproduces the reported failure and the new code does not.

Review feedback, and a correction to my own earlier claim: the fixed Jan..Dec table I reached for first would have changed the output for everyone whose locale is not English, not just on the platform that broke. Perl calls setlocale(LC_ALL, "") at startup, so LANG does reach strftime - with LC_ALL=de_DE.UTF-8 the old code renders March as "Mär" and French as "mars", which is what people reading their own logs expect, and it is behaviour worth keeping. (I had assumed the environment locale would not reach strftime and would not have noticed. It does.) So ask strftime for %b as before, and fall back to the English abbreviation only when the answer is not a name at all - which is the MinGW case, where %b comes back as the month number. Locale-aware where the platform can be, deterministic where it cannot. Verified against de_DE and fr_FR built locally: the output is identical to the old code's under each, and under a strftime patched to return the month number the old code reproduces the reported 29/9/2026 while the new code gives 29/Sep/2026. Full suite passes.

bigpresh · 2026-09-30T21:47:47Z

Follow-up on the t/unit/logger/format.t fix: the month is no longer hardcoded.

@MONTH_NAME as a fixed table was too blunt. It fixed MinGW, but it also changed
%t/%u output for every non-English locale, which is not a change this
needs to make.

Perl calls setlocale(LC_ALL, "") at startup, so the environment locale does
reach strftime — with LC_ALL=de_DE.UTF-8 the old code renders March as
Mär, and fr_FR.UTF-8 as mars. That is the behaviour people reading their
logs on those systems have, and it is worth keeping.

_month_name now asks strftime for %b as before, and falls back to the
English abbreviation only when the answer is not a name — which is exactly
the MinGW case, where %b is the month number.

Verified with de_DE/fr_FR built locally, comparing old and new through the
same strftime:

locale	old	new
`C`	`11/Mar/2026 09:05:07`	`11/Mar/2026 09:05:07`
`de_DE.UTF-8`	`11/Mär/2026 09:05:07`	`11/Mär/2026 09:05:07`
`fr_FR.UTF-8`	`11/mars/2026 09:05:07`	`11/mars/2026 09:05:07`

and with %b patched to return the month number (the reported MinGW behaviour):

old = 29/9/2026 23:59:25     <- reproduces the cpantesters failure
new = 29/Sep/2026 23:59:25   <- documented dd/Mon/yyyy

Full suite passes (209 files, 1857 tests). So the "behaviour change for non-English
locales" noted in the PR description does not apply — the earlier commit and this
one together are locale-preserving everywhere the platform supports it.

One correction to my own reasoning

In the original description I wrote that %b "yields the month number" as though
I had confirmed that from MinGW's sources. I hadn't — the fetch of
mingw-w64-crt/msvcrt/strftime.c 404'd, and I did not retry. The evidence is
behavioural only: the cpantesters report output (29/9/2026 23:59:25) plus the
simulation above, which reproduces the failure exactly. The observation is solid;
the mechanism (which C runtime, and why) is not something I verified. Worth
keeping in mind when reviewing.

The test asserted the month is exactly `[A-Z][a-z]{2}`, which passes in C/en_GB but fails in many locales: e.g. French `mars` or `sept.`, and German `Mär` (non-ASCII). The old code would also have produced those values and the test would have failed on such systems even though the code behaved as documented. Relax the check to require that the month field is not all digits and contains at least one non-slash character — enough to catch the actual MinGW regression where `%b` returned the month number (`29/9/2026`), while not breaking on real locale month names. This matches the locale-preserving fix to the logger itself. All logger tests still pass in C, en_GB, de_DE, and fr_FR locales.

bigpresh · 2026-09-30T22:07:14Z

Second follow-up: the date-codes test itself was locale-sensitive, and is now loosened.

Raised while fixing the month regression, so noting it here rather than opening a
separate PR.

t/unit/logger/format.t asserted the month is exactly [A-Z][a-z]{2}. That holds in
C/en_GB but not elsewhere:

locale	`%b`	old assertion
`en_GB`	`Sep`	passes
`de_DE` (March)	`Mär`	fails — non-ASCII
`fr_FR`	`sept.` / `mars`	fails — lower-case, punctuated

This is a pre-existing failure in v2.2.2, not something the earlier commits
introduced — verified by running the shipped Logger.pm under fr_FR. It's
date-dependent, which makes it worse than a clean always-fail: de_DE passes
today only because German and English abbreviate September the same way, and
fails 12 days a year on the other months.

The assertion now requires that the month field is not all digits (and is not
empty/slash), leaving the numeric fields strict:

my $MONTH = qr{[^\d/][^/]*};

The digit check is what keeps the test meaningful. Every real month name contains
a non-digit; the MinGW %b-returns-a-number bug produces 29/9/2026, which is
still rejected:

29/9/2026 23:59:25     -> NO MATCH (caught)
11/3/2026 09:05:07     -> NO MATCH (caught)
01/12/2026 00:00:00    -> NO MATCH (caught)

29/Sep/2026 23:59:25   -> MATCH
11/Mär/2026 09:05:07   -> MATCH
30/sept./2026 22:57:17 -> MATCH
11/mars/2026 09:05:07  -> MATCH

format.t passes under C, en_GB, de_DE and fr_FR. Full suite passes
(209 files, 1857 tests).

I also checked for the same assumption elsewhere — it appears only in this
subtest. t/time.t and t/unit/core/cookie.t hardcode Jan but those assert
fixed epochs in HTTP-date form, which is correctly locale-independent.

cromedome

Thanks for running these down! The fixes look right, and they should clear the Windows reports. There are still a few things to sort out:

The loosened month regex in format.t still fails under CJK locales. It's failing right now under ja_JP, because October is 10月.
The _month_name fallback is the actual fix, but nothing tests it.
I don't think the MinGW explanation holds up, so I'd rather Changes didn't state it as fact.

The rest are nits. Details inline.

While I was in here I also found some older generator bugs. This PR didn't cause them, so I've filed them separately: #1849, #1850, #1851.

cromedome · 2026-10-14T12:31:45Z


+    [ BUG FIXES ]
+    * %t and %u in log_format render the month as a number on Windows
+      perls built against the MinGW runtime (29/9/2026 where everywhere


I don't think we can pin this on MinGW. A few things from the reports:

All nine failures come from the same tester farm, and it's on UTC+9 (compare the %t and %u times).

The month comes back unpadded (9, 10). If %b were acting like %m, we'd see 09.

The one MSVC-built perl on that farm (5.34.1) passed format.t.

My guess is Windows' Japanese locale data, where the abbreviated month names are just digits. But that MSVC pass says the runtime or perl version matters too, so I can't prove that either. Can we describe what we saw instead? Something like "on some Windows systems, strftime's %b returns the month as a bare number".

cromedome · 2026-10-14T12:31:45Z

+      same way everywhere, and the month falls back to the English
+      abbreviation only where strftime has no name to give back - so a
+      localised month name still works where the platform supports it
+    * dancer2 gen wrote the generated MANIFEST and MANIFEST.SKIP with CRLF


Nit: this overstates it a bit. ExtUtils::Manifest strips the \r when it reads MANIFEST.SKIP, so CRLF never actually broke make manifest for anyone. Only our test noticed, because it reads the file raw. Still worth fixing (mixed line endings aren't great), but I'd reword it so nobody thinks their existing apps package wrong.

cromedome · 2026-10-14T12:31:45Z

    my $manifest_name = path( $dir, 'MANIFEST' );
    open( my $manifest, '>', $manifest_name ) or die $!;
+
+    # MANIFEST and MANIFEST.SKIP are matched line by line, against paths


Nit: open( my $manifest, '>:raw', $manifest_name ) does the same thing in one line, and matches the spew_raw that _copy_templates already uses. The packaging claim in here isn't quite right either (see my Changes comment).

cromedome · 2026-10-14T12:31:45Z

+    # would become \r\n, and the appended pattern - the one naming what
+    # `make manifest` should not pick up - would no longer match the path it
+    # is meant to match.
+    binmode $fh, ':raw';


Same here: open my $fh, '>>:raw', $filename would do it.

One more thing, though. Most of MANIFEST.SKIP is the skeleton file, copied byte for byte, and we don't have a .gitattributes. On a Windows git checkout (Git for Windows defaults to core.autocrlf=true), those skeleton lines will still be CRLF. share/skel/** eol=lf in a .gitattributes would fix that at the source. CPAN installs aren't affected, so this is fine as a follow-up.

cromedome · 2026-10-14T12:31:45Z

+
+# %b is what "dd/Mon/yyyy" has always meant here -- the locale's abbreviated
+# month name -- so it is still asked for. Not every platform can answer,
+# though: the MinGW C runtime behind Strawberry Perl gives %b the month


Same as my comment on Changes: we don't actually know it's the MinGW runtime. I'd describe what we saw ("some Windows perls return the month as a number") and leave the mechanism out.

cromedome · 2026-10-14T12:31:45Z

-        T => sub { POSIX::strftime( "%Y-%m-%d %H:%M:%S", localtime(time) ) },
-        u => sub { POSIX::strftime( "%d/%b/%Y %H:%M:%S", gmtime(time) ) },
-        U => sub { POSIX::strftime( "%Y-%m-%d %H:%M:%S", gmtime(time) ) },
+        t => sub { _date_slashes( localtime(time) ) },


Question, not a blocker: %{%d/%b/%Y:%H:%M:%S}t still hands %b straight to strftime, so the same Windows boxes will still log 29/9/2026 with an Apache-style format. I get the "it's the caller's own format" reasoning. Should %b go through _month_name there too? Happy for that to be a follow-up.

cromedome · 2026-10-14T12:31:45Z

+    # here is one Windows can put on disk, so this runs everywhere; the
+    # colon-bearing spelling of the same thing, which is what makes the point
+    # most sharply, is checked below where such a directory can exist at all.
+    my $res2 = gen( '-a', 'Other::App', '-d', 'Other_App', '--path', $dir );


Nice. Other_App still proves an explicit -d beats the dashed name, and the SKIP keeps the colon case running everywhere it can.

cromedome · 2026-10-14T12:31:45Z

+    # what a nonsense path happens to do.
+    my $from_root = $ROOT->stringify;
+    $from_root =~ s{\\}{/}g;                  # a request path is always '/'-separated
+    $from_root =~ s{\A[A-Za-z]:/+}{/};         # a volume is not a directory name


Nit: this handles drive letters but not UNC paths, so a TEMP on \\server\share would 404 again. File::Spec handles both and replaces all four regexes (needs a use File::Spec; up top):

my ( undef, $dirs ) = File::Spec->splitpath( "$ROOT", 1 ); my @dirs = grep { length } File::Spec->splitdir($dirs); my $from_root = join '/', @dirs;

(Also, the / that line 381 puts back gets stripped right off again by 382.)

cromedome · 2026-10-14T12:31:45Z

+    # collapse, however deep the temporary directory holding the fixture is.
    my $deep = $test->request( HTTP::Request->new(
-        GET => '/' . ( '../' x 20 ) . $ROOT->relative('/') . '/secret.txt' ) );
+        GET => '/' . ( '../' x ( scalar(@dirs) + 5 ) )


Nit: the /etc/passwd case a bit further down still uses '../' x 20. Might as well give it the same @dirs + 5 count, since a deep enough TMPDIR breaks it the same way.

cromedome · 2026-10-14T12:31:45Z

+    # which is what keeps the case that actually broke - the MinGW C runtime,
+    # whose %b returns the month *number*, giving "29/9/2026" for September -
+    # from passing as though it were fine.
+    my $MONTH = qr{[^\d/][^/]*};


This still fails under CJK locales. The comment says the month just has to contain a non-digit, but [^\d/] at the front means it has to start with one. Japanese, Chinese and Korean write October as 10月/10월, so LC_ALL=ja_JP.UTF-8 prove -l t/unit/logger/format.t fails today. (On glibc, zh_CN fails all year: September is 9月.)

This version still rejects 29/9/2026, and I checked it passes under C, ja_JP, ko_KR, zh_CN, de_DE and fr_FR:

my $MONTH = qr{[^/]*[^\d/][^/]*};

@Dirs

Review notes, worked through point by point. Don't name a mechanism we did not confirm. The MinGW runtime was never verified (the msvcrt source fetch 404'd); all we have is the behaviour, so Changes, the Logger comment and the test comment now say "on some Windows systems, strftime's %b returns the month as a bare number", and the PR description is corrected to match. The Changes entry for MANIFEST/MANIFEST.SKIP is reworded too: ExtUtils::Manifest strips the \r when it reads MANIFEST.SKIP, so nothing mis-packaged, and it should not read as though it did. Make the CJK month names pass. qr{[^\d/][^/]*} required the month to *start* with a non-digit, so "10月" was rejected. Adopt the reviewer's qr{[^/]*[^\d/][^/]*}, which still rejects the numeric "29/9/2026". Test the fallback. It only runs where %b is not a name, which is nowhere we run the suite, so it was untested. Fake a numeric %b through local *POSIX::strftime and check %t/%u; assert month_abbrev(0..11) directly as the shape alone cannot catch an off-by-one. Keep the private helpers private. Role::Tiny composes every named sub in the role package onto each logger class, so _month_name/_date_slashes were landing there. They are lexical coderefs now. Share the month table. Dancer2::Core::Time already had the English Jan..Dec names, so hoist them there as month_abbrev and use the one copy. Put %T/%U back. They are all numbers, C guarantees the zero-padding, and they passed in every report; only %t/%u needed the change. Use File::Spec for the fixture path in t/integration/handler/file.t, so a UNC TEMP is handled as well as a drive letter, and give the /etc/passwd case the same @Dirs + 5 count. Write the generated MANIFEST and MANIFEST.SKIP with '>:raw'/'>>:raw' in one line each, as _copy_templates does, and trim the comments' packaging claim. Full suite passes: 209 files, 1858 tests.

cromedome

👍 Pulling this locally, rebasing, and merging. Thanks!

bigpresh added 2 commits September 30, 2026 22:37

cromedome requested changes Oct 14, 2026

View reviewed changes

This was referenced Oct 14, 2026

log_format's %{...}t block code renders the month as a number on some Windows systems #1853

Open

Add .gitattributes so share/skel keeps LF on a Windows git checkout #1854

Open

cromedome approved these changes Oct 18, 2026

View reviewed changes

cromedome closed this Oct 18, 2026

Conversation

bigpresh commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

t/unit/logger/format.t

t/e2e/cli/gen.t

t/integration/handler/file.t

Verification

Not fixed here

Uh oh!

bigpresh commented Sep 30, 2026

One correction to my own reasoning

Uh oh!

bigpresh commented Sep 30, 2026

Uh oh!

cromedome left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cromedome left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bigpresh commented Sep 30, 2026 •

edited

Loading