John Peacock wrote:
>> I also noted what will have to happen in order for the problem to really be
>> fixed. Your trivial fix is too trivial, I'm sorry, and had to be rejected.
>> I
>> await the slightly less trivial patch. If you have questions about how to
>> implement that I'll be happy to help.
>
> OK, here's the problem. ExtUtils::MakeMaker is written to assume only certain
> hash values (for the keys that EU::MM accepts as parameters) can be anything
> but
> scalars. That's what %Special_Sigs is used for. EU::MM has decided /a
> priori/
> that anything not in that hash must be a scalar or a warning is thrown.
> EU::MM
> is validating hash values based on certain assumptions, which in this case are
> no longer true.
I agree with all that, no question. It sure would be handy if there was a way
to extend %Special_Sigs. That would also allay my misgivings about special
casing a particular module.
> Once upon a time, $VERSION contained only a number (or a string that looked
> like
> a number with underscores). That isn't true any more. $VERSION can now
> contain
> a version object. So my trivial patch simply adds the VERSION key and the
> possible values of ['version',''] (version object or scalar).
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.
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().
A patch for version objects would have to make those two changes.
>> I still lack an explanation about
>> how to trigger this bug and I'm not going to spend time guessing when you can
>> just attach a tarball with the failing code instead of feeding me a few
>> out-of-context lines at a time.
>>
>
> This doesn't have anything really to do with Module::Install. Here's how to
> reproduce it. Download the SVK tarball. Unpack it and type
>
> perl Makefile.PL
>
> After you answer any questions about installing missing (or optional) modules,
> you get the warning. Here's the backtrace from that point:
>
> . = ExtUtils::MakeMaker::_verify_att(ref(HASH)) called from file
> `/usr/lib/perl5/5.8.8/ExtUtils/MakeMaker.pm' line 57
> $ = ExtUtils::MakeMaker::WriteMakefile('test', ref(HASH), 'NAME', 'SVK',
> 'DISTNAME', 'SVK', 'AUTHOR', 'Chia-liang Kao <[EMAIL PROTECTED]>', 'DIR',
> ref(ARRAY), 'dist', ref(HASH), 'clean', ref(HASH), 'NO_META', 1, 'ABSTRACT',
> 'A
> decentralized version control system', 'PL_FILES', ref(HASH), 'EXE_FILES',
> ref(ARRAY), 'VERSION', ref(version), 'PREREQ_PM', ref(HASH)) called from file
> `inc/Module/Install/Makefile.pm' line 147
> . = Module::Install::Makefile::write(ref(Module::Install::Makefile)) called
> from
> file `inc/Module/Install/WriteAll.pm' line 37
That explains why it happens with a version_from() in Module::Install and not
with a VERSION_FROM in MakeMaker. Module::Install is pulling the version out
itself and passing it into WriteMakefile() as VERSION.
Now all is clear.
> . = Module::Install::WriteAll::WriteAll(undef, undef, undef, 'sign', 1) called
> from file `Makefile.PL' line 142
>
> I *should* be able to use the exact code you cited:
>
> use ExtUtils::MakeMaker;
> use version;
>
> WriteMakefile(
> NAME => "Foo",
> VERSION => qv(2.0.0),
> );
>
> and not have EU::MM throw a warning.
We're having a violent agreement.
> I think you are overthinking this into
> imagining you need to generically support random objects everywhere. I'm just
> saying that there is now a legitimate way to initialize $VERSION that uses an
> object, instead of a scalar, and it would be reasonable for EU::MM to support
> that...
I'm only vaguely worried about making a single class special. However, as it
will be special in 5.10 I'm not so worried. Allowing the ability to extend
what keys and types WriteMakefile() will take would be nice, but I'm not
asking you to do that.
I'll take a patch to support version objects, but it has to be done right and
there's two things in _verify_att() that have to change for that to happen.
Some tests would be spiffy.
