Skip to content

Conversation

@KES777
Copy link
Contributor

@KES777 KES777 commented Sep 30, 2025

GH #23780
Perl installation fails on minimized systems.

Here we just add another test that man is available and SKIP perldoc section if it is not.

After the fix output looks like this:

ok 123 - Test that the debugger chdirs to the initial directory after a restart.
ok 124 # skip No man. This system has been minimized...
ok 125 - Test that the a command does not emit warnings on program exit.

@KES777 KES777 force-pushed the fix_perl_installation_via_brew_at_docker branch from b357d5c to 65a30ae Compare September 30, 2025 22:58
Copy link
Contributor

@tonycoz tonycoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did consider whether testing man perlrules matched the "No manpage found" text would be better, but that could lead to silent skips when we should still be performing the test.

lib/perl5db.t Outdated
local $ENV{LC_ALL} = "C";
my $out = `/usr/bin/man --version`;
$out !~ /^This system has been minimized/
or skip "No man. This system has been minimized...", 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 3 lines would be better placed 3 lines higher, i.e., just after the other skip statements and before the 3 local $ENV{} statements.

I think the last 2 lines would be more quickly understood if they were expressed in the positive, rather than in the negative, i.e.,

    $out =~ /^This system has been minimized/
        and skip "No man. This system has been minimized...", 1;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the locale is non-English moving this above the environment changes may produce non-English messages, breaking the test (though I don't know if whatever produces the message pays the space cost of localizing the message.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it probably should come after the localizations, but I do agree with @jkeenan that the negative match doesn't help readability here. How about

if ($out =~ /^This system has been minimized/) {
    skip "No man. This system has been minimized...", 1;
}

@KES777 KES777 force-pushed the fix_perl_installation_via_brew_at_docker branch from 65a30ae to f43f6b5 Compare October 2, 2025 21:39
@KES777 KES777 force-pushed the fix_perl_installation_via_brew_at_docker branch from f43f6b5 to 82e0a32 Compare October 2, 2025 21:41
@KES777 KES777 requested review from Leont and jkeenan October 2, 2025 21:42
Copy link
Contributor

@jkeenan jkeenan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming neither @tonycoz nor @Leont objects, I am okay with this change.

@tonycoz tonycoz merged commit e9d9a70 into Perl:blead Oct 3, 2025
33 checks passed
@KES777 KES777 deleted the fix_perl_installation_via_brew_at_docker branch October 3, 2025 21:09
@KES777
Copy link
Contributor Author

KES777 commented Oct 3, 2025

thanks everyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants