Skip to content

Conversation

@hakonhagland
Copy link
Contributor

@hakonhagland
Copy link
Contributor Author

Not sure why this test fails:

Use of uninitialized value $codepage in concatenation (.) or string at D:\a\ExtUtils-MakeMaker\ExtUtils-MakeMaker\blib\lib/ExtUtils/MakeMaker.pm line 229.
Unknown encoding 'cp' at D:\a\ExtUtils-MakeMaker\ExtUtils-MakeMaker\blib\lib/ExtUtils/MakeMaker.pm line 229.
# Looks like you planned 11 tests but ran 9.
# Looks like your test exited with 255 just after 9.
t/prompt.t ................ 

The test does not fail on my laptop..

@jkeenan
Copy link
Contributor

jkeenan commented Sep 19, 2023

Not sure why this test fails:

Use of uninitialized value $codepage in concatenation (.) or string at D:\a\ExtUtils-MakeMaker\ExtUtils-MakeMaker\blib\lib/ExtUtils/MakeMaker.pm line 229.
Unknown encoding 'cp' at D:\a\ExtUtils-MakeMaker\ExtUtils-MakeMaker\blib\lib/ExtUtils/MakeMaker.pm line 229.
# Looks like you planned 11 tests but ran 9.
# Looks like your test exited with 255 just after 9.
t/prompt.t ................ 

The test does not fail on my laptop..

Perhaps in your p.r. at line 223 the return value was an empty string. That would be a defined value, but then at 229 the first argument to decode would only be cp.

@hakonhagland
Copy link
Contributor Author

Perhaps in your p.r. at line 223 the return value was an empty string

@jkeenan Added a test for empty string, see last commit

@Leont
Copy link
Member

Leont commented Sep 19, 2023

I'm not sure if we can use Encode here like that; MakeMaker is part of perl's bootstrap which means it's used before Encode is built. ExtUtils::MakeMaker::Locale is some prior art in this area.

@hakonhagland
Copy link
Contributor Author

I'm not sure if we can use Encode here like that

Then as an alternative, would it be possible to apply this patch to e.g. IO::Prompt::Tiny instead and patch consumers like CPAN::Shell to use IO::Prompt::Tiny::prompt as a replacement for ExtUtils::MakeMaker::prompt ?

@Leont
Copy link
Member

Leont commented Sep 19, 2023

Then as an alternative, would it be possible to apply this patch to e.g. IO::Prompt::Tiny instead and patch consumers like CPAN::Shell to use IO::Prompt::Tiny::prompt as a replacement for ExtUtils::MakeMaker::prompt ?

No. CPAN::Shell is core so can't use any module outside of core.

And I didn't say you can't use Encode at all, I'm just saying it should handle its absence gracefully.

@hakonhagland
Copy link
Contributor Author

ExtUtils::MakeMaker::Locale is some prior art in this area.

Yes, nice one! For reference here is a link to the PR from 2014: #125

@hakonhagland
Copy link
Contributor Author

hakonhagland commented Sep 19, 2023

MakeMaker is part of perl's bootstrap which means it's used before Encode is built.

@Leont But why then can ExtUtils::MakeMaker::Locale use Encode itself? See line 15:

@Leont
Copy link
Member

Leont commented Sep 19, 2023

@Leont But why then can ExtUtils::MakeMaker::Locale use Encode itself? See line 15:

Because this

@hakonhagland
Copy link
Contributor Author

I'm not sure if we can use Encode here like that

Trying with ExtUtils::MakeMaker::Locale instead, see latest commit. Also added a test case.

To fix a test failure, I am checking if it is caused by
Win32::GetConsoleCP()
returning an empty string
Use bundled local module to determine windows codepage instead.
Forgot to increase test number in prompt.t
@mohawk2
Copy link
Member

mohawk2 commented Feb 27, 2025

I've rebased this against master and force-pushed so it gets run against current CI. I didn't touch the commits, but I'd suggest they should be squashed. If the CI likes it, this looks good to me and I'd say it should be merged.

@mohawk2 mohawk2 added the Win32 label Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

registration failed: Internal Server Error at Metabase/Client/Simple.pm line 195.

4 participants