-
Notifications
You must be signed in to change notification settings - Fork 2
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
Makefile.PL: check Tcl/Tk installation via Tcl.pm #19
base: trunk
Are you sure you want to change the base?
Conversation
my $i; | ||
my $tk_patchLevel; | ||
|
||
sub _die ($) { | ||
# CPAN smokers report FAIL if Makefile.PL dies, it should exit with status 0 | ||
my $err = shift; | ||
warn $err; | ||
# Cleanly exit Tcl/Tk if it was loaded | ||
$i->Eval('destroy .') if ($tk_patchLevel); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid issues related to #14, if Tk is loaded then the root Tk window is destroyed before Makefile.PL exits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As currently proposed it leaves Makefile.PL kind of bloated. Lots of repetitive code to check each optional Tcl/Tk extension. Maybe the check should at least be moved to another file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is suggested to use eval { …; 1; } or do { my $error = $@ || 'unknown error'; … }
instead of eval { … }; if ($@) {…}
.
https://perlmaven.com/fatal-errors-in-external-modules
f066c9f
to
fa71eec
Compare
This idea has since been implemented for Tkx (although that was simpler due to not checking for any Tk extensions): chrstphrchvz/tkx@5d0230d |
d83b200
to
87428a0
Compare
8.4 already required since Tcl.pm 0.81 (released 2004).
An idea I brought up on perl.tcltk: https://www.nntp.perl.org/group/perl.tcltk/2020/07/msg840.html
The goal is to use Tcl.pm to check the Tcl/Tk installation rather than find/ask for and run tclsh. This avoids the possibility of checking a Tcl/Tk installation other than the one Tcl.pm is configured to use.