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

Makefile.PL: check Tcl/Tk installation via Tcl.pm #19

Draft
wants to merge 4 commits into
base: trunk
Choose a base branch
from

Conversation

chrstphrchvz
Copy link
Owner

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.

Comment on lines +6 to +14
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);
Copy link
Owner Author

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.

Copy link
Owner Author

@chrstphrchvz chrstphrchvz left a 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.

Copy link
Owner Author

@chrstphrchvz chrstphrchvz left a 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

@chrstphrchvz
Copy link
Owner Author

This idea has since been implemented for Tkx (although that was simpler due to not checking for any Tk extensions): chrstphrchvz/tkx@5d0230d

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.

1 participant