Michael G Schwern wrote:
> That's where it falls down. As mentioned before, two reasons.
>
> 1) _verify_att() is currently case insensitive. This is because I was lazy
> and found "hash" easier to write than "HASH" by hand. This means if you just
> add in a key of 'version' it will happily accept VERSION, version and VerSioN
> objects. So that has to be fixed.
This turns out to be a trivial change, so a revised patch is attached. I'll
generate a new test file (since it needs to skip all if version.pm is not
installed).
> 2) _verify_att() currently uses ref() because it has only had to deal with
> references so far. This means any subclass instances of 'version' will be
> rejected and that would be impolite. It should be fixed to use isa().
isa() would normally be correct from an OO point of view, but there are
limitations that mean that the more strict ref() is the correct choice. The
_only_ non-numeric initializers for $VERSION that are supported by PAUSE and
CPAN are version.pm objects. version.pm objects *always* return a ref() of
'version' (whether implemented by XS or pure Perl code). Even though I uploaded
version::AlphaBeta to CPAN (as a proof of concept), that subclass of version.pm
is not supported by PAUSE, so any module using that class would not be
indexable.
To this end, it would be entirely appropriate that EU::MM *would* complain about
a subclass of version.pm being used instead of version.pm itself. No subclass
of version.pm is supported by the entire toolchain, which is all we are trying
to accomplish here. isa() is too general for this case; we really do want to
use ref() and only accept 'version' objects (in addition to scalars).
Does this make any more sense? I'm trying to make the most minimal extension to
the allowed signatures, to prevent mistakes from being made. If there was a way
to extend the basic types of Perl (e.g. ARRAY, CODE, HASH) other than through
object classes, I would have gone that route with version.pm objects and created
a first class 'version' type. We do not want to support any random subclass of
version.pm - we only want to support true version.pm objects. And those will
always return ref() eq 'version'.
John
--
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4501 Forbes Blvd
Suite H
Lanham, MD 20706
301-459-3366 x.5010
fax 301-429-5747
Index: t/writemakefile_args.t
===================================================================
--- t/writemakefile_args.t (revision 3955)
+++ t/writemakefile_args.t (working copy)
@@ -53,7 +53,7 @@
};
is( $warnings, <<VERIFY );
-WARNING: MAN3PODS takes a hash reference not a string/number.
+WARNING: MAN3PODS takes a HASH reference not a string/number.
Please inform the author.
VERIFY
@@ -67,7 +67,7 @@
};
is( $warnings, <<VERIFY );
-WARNING: AUTHOR takes a string/number not a code reference.
+WARNING: AUTHOR takes a string/number not a CODE reference.
Please inform the author.
VERIFY
@@ -105,7 +105,7 @@
};
# We'll get warnings about the bogus libs, that's ok.
- like( $warnings, qr{^WARNING: LIBS takes a array reference or string/number not a hash reference}m );
+ like( $warnings, qr{^WARNING: LIBS takes a ARRAY reference or string/number not a HASH reference}m );
$warnings = '';
Index: lib/ExtUtils/MakeMaker.pm
===================================================================
--- lib/ExtUtils/MakeMaker.pm (revision 3955)
+++ lib/ExtUtils/MakeMaker.pm (working copy)
@@ -68,41 +68,42 @@
# scalar.
my %Att_Sigs;
my %Special_Sigs = (
- C => 'array',
- CONFIG => 'array',
- CONFIGURE => 'code',
- DIR => 'array',
- DL_FUNCS => 'hash',
- DL_VARS => 'array',
- EXCLUDE_EXT => 'array',
- EXE_FILES => 'array',
- FUNCLIST => 'array',
- H => 'array',
- IMPORTS => 'hash',
- INCLUDE_EXT => 'array',
- LIBS => ['array',''],
- MAN1PODS => 'hash',
- MAN3PODS => 'hash',
- PL_FILES => 'hash',
- PM => 'hash',
- PMLIBDIRS => 'array',
- PMLIBPARENTDIRS => 'array',
- PREREQ_PM => 'hash',
- SKIP => 'array',
- TYPEMAPS => 'array',
- XS => 'hash',
+ C => 'ARRAY',
+ CONFIG => 'ARRAY',
+ CONFIGURE => 'CODE',
+ DIR => 'ARRAY',
+ DL_FUNCS => 'HASH',
+ DL_VARS => 'ARRAY',
+ EXCLUDE_EXT => 'ARRAY',
+ EXE_FILES => 'ARRAY',
+ FUNCLIST => 'ARRAY',
+ H => 'ARRAY',
+ IMPORTS => 'HASH',
+ INCLUDE_EXT => 'ARRAY',
+ LIBS => ['ARRAY',''],
+ MAN1PODS => 'HASH',
+ MAN3PODS => 'HASH',
+ PL_FILES => 'HASH',
+ PM => 'HASH',
+ PMLIBDIRS => 'ARRAY',
+ PMLIBPARENTDIRS => 'ARRAY',
+ PREREQ_PM => 'HASH',
+ SKIP => 'ARRAY',
+ TYPEMAPS => 'ARRAY',
+ XS => 'HASH',
+ VERSION => ['version',''],
_KEEP_AFTER_FLUSH => '',
- clean => 'hash',
- depend => 'hash',
- dist => 'hash',
- dynamic_lib=> 'hash',
- linkext => 'hash',
- macro => 'hash',
- postamble => 'hash',
- realclean => 'hash',
- test => 'hash',
- tool_autosplit => 'hash',
+ clean => 'HASH',
+ depend => 'HASH',
+ dist => 'HASH',
+ dynamic_lib=> 'HASH',
+ linkext => 'HASH',
+ macro => 'HASH',
+ postamble => 'HASH',
+ realclean => 'HASH',
+ test => 'HASH',
+ tool_autosplit => 'HASH',
);
@Att_Sigs{keys %Recognized_Att_Keys} = ('') x keys %Recognized_Att_Keys;
@@ -120,7 +121,7 @@
}
my @sigs = ref $sig ? @$sig : $sig;
- my $given = lc ref $val;
+ my $given = ref $val;
unless( grep $given eq $_, @sigs ) {
my $takes = join " or ", map { $_ ne '' ? "$_ reference"
: "string/number"
