perlancar / perl-Getopt-Long-More

1 stars 1 forks source link

Avoid inadvertently hijacking GoL's legacy "default destinations" #9

Closed tabulon closed 5 years ago

tabulon commented 5 years ago

BACKGROUND

<Getopt::Long> (GoL) has a legacy feature that it describes as below :

When no destination is specified for an option, GetOptions will store the resultant value in a global variable named opt_XXX, where XXX is the primary name of this option.

More of the GoL doc quoted at the bottom of this description.

SUMMARY of the issue

GLM (Getopt:Long::More) inadvertenly "hijacks" what GoL (Getopt:Long) calls legacy "default destinations".

GoL ends up creating and populating GLOBAL VARIABLES that are named like $Getopt:Long::More::opt_XXX where XXX is the original option name.

Judging from the code, the issue appears to be unrelated to any recent change; and that the problem probably existed from the the very beginings of GLM.

What's worse, the issue occurs quite silently :

The only thing that might alert the GLM consumer, happens when they use warnigs is a meager warning about a global variable used only once... which they could easily think to be coming from the way GoL works...

STEPS TO REPRODUCE

Easily reproducible with 3-4 lines of code... Below is a bit verbose, just for fun.

EXAMPLE SCRIPT


#!perl

#  Invoke this script twice like below with perl -M
#    perl -M 'Getopt::Long'       <THIS_SCRIPT>  --foo hello
#    perl -M 'Getopt::Long::More' <THIS_SCRIPT>  --foo hello
#
#  To get the equivalent effects of either of the following:
#use Getopt::Long;
#use Getopt::Long::More;

our $opt_foo;   # <<< this is where <Getopts::Long> is expected to put the value for option 'foo'
my $res = GetOptions( 'foo=s' );

my $status = $res ? 'SUCESS' : 'FAILURE';
print <<"EOF"

 Getopts() call status         : '$status'

 But where did my 'foo' go ?

  \$opt_foo                     : '$opt_foo'
  \$main::opt_foo               : '$main::opt_foo'
  \$Getopt::Long::More::opt_foo : '$Getopt::Long::More::opt_foo'

EOF
;
if (defined $Getopt::Long::More::opt_foo) { # and this demonstrates the CULPRIT
  print  "In fact, I have just HIT UPON A STRANGE BUG \n";
  print  "   -- that I am not even aware of. \n"    if $res;
}

# Below lies code in deep PSYCHOSIS :-)
# ...
#

EXPECTED OUTCOME

Note : The below can readily be obtained by passing -M'Getopt::Long' to perl, instead of -M'Getopt::Long::More'

perl -M'Getopt::Long::More' <<script-above>>  --foo hello

 Getopts() call status         : 'SUCESS'

 But where did my 'foo' go ?

  $opt_foo                     : 'hello'
  $main::opt_foo               : 'hello'
  $Getopt::Long::More::opt_foo : ''

As of today, the above can be obtained by passing -M'Getopt::Long' to perl, instead of -M'Getopt::Long::More', like so:

perl -M'Getopt::Long' <<script-above>>  --foo hello

CURRENT (BUGGY) OUTCOME

perl -M'Getopt::Long::More' <> --foo hello


 Getopts() call status         : 'SUCESS'

 But where did my 'foo' go ?

  $opt_foo                     : ''
  $main::opt_foo               : ''
  $Getopt::Long::More::opt_foo : 'hello'

In fact, I have just HIT UPON A STRANGE BUG
   -- that I am not even aware of.

DISCUSSION & PROPOSED SOLUTION

Looking at GoL source, it appears that GoL has a package global variable named $caller which seems to have been conceived exactly for this problem, by judging from the way it is employed further down in that module source code.

So, it seems to be just a matter of doing the following, in various places:

$Getopt::Long::caller ||= (caller)[0]

This appears to be cleanest solution avaliable, without any kind of witchcraft involved.

The above conditional assignment would normally have to happen at the earliest occasion when pretty much any of GLM's front-facing routines are invoked, which would then naturally left as is (thanks to the ||= ) on its way down to GoL.


The only initial concern is that it's marked as one of the # Official invisible variables, whatever that signifies, but I like the word offical in there :-)

# Official invisible variables.
use vars qw($genprefix $caller $gnu_compat $auto_help $auto_version $longprefix);

In any case, GLM already makes use of at least two other variables in there ( $auto_help and $auto_version), probably another shot of "invisible" shouldn't hurt, especially if it is "# Official", right?

This approach would benefit the scenarios where GLM itself is wrapped.

Things should continue to work when the consumer, or some one along the line, has already set $caller before GLM is invooked.

REFERENCES

Below is a quote form the GoL documentation that describes GoL's Default destinations, under the Legacy section.

Legacy

The earliest development of newgetopt.pl started in 1990, with Perl version 4. As a result, its development, and the development of Getopt::Long, has gone through several stages. Since backward compatibility has always been extremely important, the current version of Getopt::Long still supports a lot of constructs that nowadays are no longer necessary or otherwise unwanted. This section describes briefly some of these 'features'.

Default destinations

When no destination is specified for an option, GetOptions will store the resultant value in a global variable named opt_XXX, where XXX is the primary name of this option. When a program executes under use strict (recommended), these variables must be pre-declared with our() or use vars.

our $opt_length = 0; GetOptions ('length=i'); # will store in $opt_length

To yield a usable Perl variable, characters that are not part of the syntax for variables are translated to underscores. For example, --fpp-struct-return will set the variable $opt_fpp_struct_return. Note that this variable resides in the namespace of the calling program, not necessarily main. For example:

GetOptions ("size=i", "sizes=i@");

with command line "-size 10 -sizes 24 -sizes 48" will perform the equivalent of the assignments

$opt_size = 10; @opt_sizes = (24, 48);