Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mkheader: Prevent 'print on closed filehandle' messages #18332

Closed
wants to merge 1 commit into from

Conversation

jkeenan
Copy link
Contributor

@jkeenan jkeenan commented Nov 17, 2020

Program 'mkheader' is called from cpan/Unicode-Collate/Makefile.PL,
which was recently revised to run with both strictures and warnings.
Makefile.PL executes mkheader via a 'do'. This was causing the
following statements to be printed during 'make' in the Perl
5 build process:

printf() on closed filehandle $fh_h at .../cpan/ExtUtils-MakeMaker/lib/ExtUtils/MakeMaker.pm line 1233.
print() on closed filehandle $fh_h at .../cpan/ExtUtils-MakeMaker/lib/ExtUtils/MakeMaker.pm line 1234.

Storing the original filehandle during select(), then restoring it at
the end of the loop, clears up those statements.

@Grinnz
Copy link
Contributor

Grinnz commented Nov 17, 2020

This file is upstream cpan: https://metacpan.org/release/Unicode-Collate

@jkeenan
Copy link
Contributor Author

jkeenan commented Nov 17, 2020

This patch will ultimately have to go upstream, as Unicode-Collate is cpan-upstream. However, it took me so long to figure out what the problem was that I would like to get confirmation of my diagnosis here.

I think that the problem arose when a new version of Unicode-Collate that had use strict; use warnings; added to its Makefile.PL. When that program runs, it executes do ./mkheader. Without this patch, it appears that even though the filehandle in question was explicitly closed before the end of a loop, it nonetheless remained the select()-ed filehandle.

At first I thought that this was a problem when a new version of ExtUtils-MakeMaker was synched into blead, so I filed https://rt.cpan.org/Ticket/Display.html?id=133762. However, I now think the problem lies entirely within Unicode-Collate.

Thoughts?

Thank you very much.
Jim Keenan

@Grinnz
Copy link
Contributor

Grinnz commented Nov 17, 2020

The analysis makes sense to me. Personally I think a better change would be to modify every print and printf in that block to reference $fh_h so it does not mess with global state at all, even though it would be more tedious.

@karenetheridge karenetheridge added the cpan-dual-life issues regarding dual-life cpan-first distributions label Nov 19, 2020
@khwilliamson
Copy link
Contributor

@jkeenan Several people think there is a better way of accomplishing this. What do you say?

Program 'mkheader' is called from cpan/Unicode-Collate/Makefile.PL,
which was recently revised to run with both strictures and warnings.
Makefile.PL executes mkheader via a 'do'.  This was causing the
following statements to be printed during 'make' in the Perl
5 build process:

printf() on closed filehandle $fh_h at .../cpan/ExtUtils-MakeMaker/lib/ExtUtils/MakeMaker.pm line 1233.
print() on closed filehandle $fh_h at .../cpan/ExtUtils-MakeMaker/lib/ExtUtils/MakeMaker.pm line 1234.

Storing the original filehandle during select(), then restoring it at
the end of the loop, clears up those statements.
@jkeenan jkeenan force-pushed the jkeenan/unicode-collate-mkheader-warnings branch from 5de3e0d to 4f58a3a Compare December 8, 2020 20:42
@jkeenan
Copy link
Contributor Author

jkeenan commented Dec 8, 2020

I cannot reproduce these unexpected warnings at HEAD in blead. It appears they vanished somewhere between c49e90e and 0aee951. So we can close this p.r. for the time being.

Thank you very much.
Jim Keenan

@jkeenan jkeenan closed this Dec 8, 2020
@haarg
Copy link
Contributor

haarg commented Dec 8, 2020

EUMM added a workaround for the issue. This is still a bug in mkheader that should be fixed.

@haarg haarg reopened this Dec 8, 2020
jkeenan added a commit to jkeenan/Unicode-Collate that referenced this pull request Dec 26, 2020
Unicode-Collate ships with the Perl 5 core distribution and is invoked
during Perl 5's build process.  In this context, program 'mkheader' is
called from cpan/Unicode-Collate/Makefile.PL, which was recently revised
to run with both strictures and warnings.  Makefile.PL executes mkheader
via a 'do'.  This was causing the following statements to be printed
during 'make' in the Perl 5 build process:

printf() on closed filehandle $fh_h at .../cpan/ExtUtils-MakeMaker/lib/ExtUtils/MakeMaker.pm line 1233.
print() on closed filehandle $fh_h at .../cpan/ExtUtils-MakeMaker/lib/ExtUtils/MakeMaker.pm line 1234.

Storing the original filehandle during select(), then restoring it at
the end of the loop, clears up those statements.

This problem was originally reported to Perl 5 in Perl/perl5#18332.  Since the original report was filed, a workaround in ExtUtils::MakeMaker has silenced the statements cited above during the Perl 5 build process, but the problem in Unicode-Collate remains.
@jkeenan jkeenan added sendToCPAN Closable? We might be able to close this ticket, but we need to check with the reporter labels Dec 26, 2020
@jkeenan jkeenan self-assigned this Dec 26, 2020
@jkeenan
Copy link
Contributor Author

jkeenan commented Dec 26, 2020

EUMM added a workaround for the issue. This is still a bug in mkheader that should be fixed.

Bug ticket opened for Unicode-Collate at https://rt.cpan.org/Ticket/Display.html?id=133952. I think this ticket itself is closable.

Thank you very much.
Jim Keenan

@jkeenan
Copy link
Contributor Author

jkeenan commented Jan 1, 2021

EUMM added a workaround for the issue. This is still a bug in mkheader that should be fixed.

Bug ticket opened for Unicode-Collate at https://rt.cpan.org/Ticket/Display.html?id=133952. I think this ticket itself is closable.

No one has argued that this ticket is not closable, so I'm (re-)closing it now.

Thank you very much.
Jim Keenan

@jkeenan jkeenan closed this Jan 1, 2021
@jkeenan jkeenan removed the Closable? We might be able to close this ticket, but we need to check with the reporter label Jan 1, 2021
@jkeenan jkeenan deleted the jkeenan/unicode-collate-mkheader-warnings branch May 8, 2021 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpan-dual-life issues regarding dual-life cpan-first distributions sendToCPAN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants