Now, two things have happened since this. The first, I mentioned in my post on the latest release of MooseX::AttributeCloner, about the fact that 'passthrough' is being deprecated as an option for dist producing a Makefile.PL.
I won't go into that one now, but the second is potentially more disastrous.
Before I go any further, I would like to make it clear that this is not an attempt to slag off the people who write/maintain Module::Build. The tool is extremely useful, and I got a very helpful response from David Golden about my RT ticket. I would like to say Thankyou for producing the tool. The below is possibly more our abuse of it than their failings with it.
We use Test::Perl::Critic to monitor our coding standards. (OK, I accept that it is a set of guidelines, and is in no way compulsory, but it is a start.) Perlcritic wants $VERSION to be a constant, and to do this within the code, we use (as for all constants)
Readonly::Scalar our $VERSION => do { my ($r) = q$LastChangedRevision: 8362 $ =~ /(\d+)/mxs; $r; };
(Again, yes, taking the version number from a version control system is supposedly 'not good', but it works for us, and others I know)
When I run perl Makefile.PL, I get the following:
(note, I use the passthrough or small Makefile.PL and perl Makefile.PL to run Build.PL)
Creating new 'MYMETA.yml' with configuration results
Error evaling version line 'BEGIN { q# Hide from _packages_inside()
#; package Module::Build::ModuleInfo::_version::p56;
use Module::Build::Version;
no strict;
local $VERSION;
$VERSION=undef;
$vsub = sub {
Readonly::Scalar our $VERSION => do { my ($r) = q$LastChangedRevision: 8362 $ =~ /(\d+)/mxs; $r; };;
$VERSION
};
}' in /this/package/lib/module.pm: syntax error at (eval 92) line 9, near "Readonly::Scalar our "
BEGIN not safe after errors--compilation aborted at (eval 92) line 11,
failed to build version sub for /this/package/lib/module.pm at /Users/ajb/dev/perl/5.10.1/lib/5.10.1/Module/Build/ModuleInfo.pm line 332,
WARNING: Possible missing or corrupt 'MANIFEST' file.
Nothing to enter for 'provides' field in metafile.
Creating new 'Build' script for 'my_project' version '8391.'
OK, in this case, it's a warning, but the Build file is created, and I can do what I need to.
However, updating the Build.PL 'requires' to include some other package libs, the problem becomes more serious.
Error evaling version line 'BEGIN { q# Hide from _packages_inside()
#; package Module::Build::ModuleInfo::_version::p5;
use Module::Build::Version;
no strict;
local $VERSION;
$VERSION=undef;
$vsub = sub {
Readonly::Scalar our $VERSION => do { my ($r) = q$LastChangedRevision: 8212 $ =~ /(\d+)/mxs; $r; };;
$VERSION
};
}' in /another/package/lib/module.pm: syntax error at (eval 28) line 9, near "Readonly::Scalar our "
BEGIN not safe after errors--compilation aborted at (eval 28) line 11,
failed to build version sub for /another/package/lib/module.pm at /Users/ajb/dev/perl/5.10.1/lib/5.10.1/Module/Build/ModuleInfo.pm line 332,
Couldn't run Build.PL: No such file or directory at /Users/ajb/dev/perl/5.10.1/lib/5.10.1/Module/Build/Compat.pm line 335.
Here, because it can't identify the version of the 'external' module, it has croaked out.
I submitted a bug report, and David Golden (Big thanks to him for responding) suggested that before the line could perhaps be made
use Readonly; Readonly::Scalar our $VERSION => do { my ($r) = q$LastChangedRevision: 8212 $ =~ /(\d+)/mxs; $r; };
since the problem is in the eval block.
This is fine to do for internal modules, but a problem for anything we install centrally from CPAN (maintenance of code, root access, etc).
I tried using Module::Install as an alternative. This carps errors in both cases, but in both cases doesn't cause the Makefile not to be created.
This post therefore comes down to 2 things.
1) Information to anyone who really cares or reads this blog.
2) Are we the only people who use Readonly::Scalar to declare 'our $VERSION'? (in which case, MooseX::AttributeCloner is possibly the only module on CPAN which does this)
Thoughts and comments welcome, although I would appreciate people not calling us Idiots (or stronger) for using Readonly::Scalar to 'constant'ify the variable (or at least not without good reason). We have looked at use version;, but it doesn't seem to be right with using a version control value.
Again, thanks to the authors/maintainers of Module::Build and Module::Install for these wonderful tools.
5 comments:
I've read most of PBP and the reasoning for using Readonly over constant really doesn't apply in a case like this; that is, Readonly is for when you want to have a scalar that's readonly till it goes out of scope, whereas constant makes the scalar constant and not go out of scope. Your version shouldn't go out of scope anyway, so why not just use constant?
@fREW, the argument in PBP wasn't for scoping really, but that using the constant module makes constants without sigils (while Readonly does).
Hi fREW, Thanks for responding!
The problem is that this seems to be one of those corner case times when use constant might be more applicable, except that Perl::Critic has a further test that says not to use constant (Perl::Critic::Policy::ValuesAndExpressions::ProhibitConstantPragma) which then needs turning off, which we are trying to avoid.
Perhaps it is one of those times when we should.
BTW, for now, I am adopting putting the $VERSION stuff on the same line as the use Readonly; I can't say I like it (looks untidy code) but I feel the need to be pragmatic about it all, and I would prefer that to use constant.
The CPAN (and hence toolchain) standard is that the line where you declare our $VERSION should be self-contained - i.e. amenable to being eval'ed on its own. if that line on its own can't set the version then it's broken.
Part of me is tempted to say that Module::Install should warn much more loudly or even die on this unless you set the version yourself in the file.
The alternative you could do is:
our $VERSION = '0.001';
Readonly $VERSION;
Having moved it to a separate line, things should all be happy since the val won't evan the Readonly.
Thanks Matt. We were unaware that was what would be expected of CPAN/toolchain modules.
We may well be submitting a patch to Perl::Critic with a policy to check this, so that we can easily apply to our code.
Cheers, and thanks to all who have responded.
Post a Comment