Bug #35047 for Data-SExpression: [PATCH] Makefile.PL portability suggestion

> > Mon Apr 14 23:02:59 2008: Request 35047 was acted upon. > Transaction: Ticket created by ANDK > Queue: Data-SExpression > Subject: [PATCH] Makefile.PL protability suggestion > Broken in: 0.36 > Severity: Normal > Owner: Nobody > Requestors: ANDK@cpan.org > Status: new > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=35047 > > > > In my environment the Makefile.PL fails because it doesn't find yapp > although Parse::Yapp is installed. > > May I suggest to replace the requires_external_bin line and calculate > the path to yapp manually? > > This ensures that the perl that builds the Module and the one that tests > it are the same. And it helps to find our own yapp even if not in the path. > > I've left the fallback to the requires_external_bin line in in case we > find no yapp in the expected directory. > > The configure_requires promises that the yapp already is there when > Makefile.PL is executed because installers are bound to read the META.yml. > > What do you think? > > Here is the patch: > > --- Makefile.PL~ 2008-04-10 00:37:39.000000000 +0200 > +++ Makefile.PL 2008-04-15 05:00:18.000000000 +0200 > @@ -1,19 +1,27 @@ > use inc::Module::Install; > > +use Config; > +use File::Spec; > + > name('Data-SExpression'); > license('Perl'); > > version_from('lib/Data/SExpression.pm'); > > requires('Class::Accessor::Fast'); > -requires('Parse::Yapp'); > +configure_requires('Parse::Yapp'); > > build_requires('Test::More'); > build_requires('Test::Deep'); > > -requires_external_bin('yapp'); > +# > +my $yapp = File::Spec->catfile($Config{scriptdirexp},"yapp"); > +unless (-e $yapp) { > + requires_external_bin('yapp'); > + $yapp = "yapp"; > +} > > -system('yapp -m Data::SExpression::Parser -o > lib/Data/SExpression/Parser.pm lib/Data/SExpression/Parser.yp') > +system("$yapp -m Data::SExpression::Parser -o > lib/Data/SExpression/Parser.pm lib/Data/SExpression/Parser.yp") > and die("Failed: Unable to create Data::SExpression::Parser (Is > Parse::Yapp appropriately installed on your system?)"); > > WriteAll; > > > > > Thanks, >

> This is interesting. So your system has `yapp' installed outside of > $PATH?

> It seems like this should perhaps be a patch to M::I so that > requires_external_bin also looks in $Config{scriptdirexp}.

> I'm > hesitant to trawl specific paths for binaries myself, since I feel > that the infrastructure should know how to do that.

> In any case, the current system of running yapp during Makefile.PL is > poor because > (1) `make' does not cause changes to Parser.yp > to be reflected in Parser.pm, and

> (2) It breaks automatic dependency following, because if Parse::Yapp > is uninstalled, the build fails, rather than M::I trying to > install it. The config_requires change may fix that latter, > however.

> Anyways, I was thinking of replacing the requires_external_bin and > system() lines with something like:

> postamble(<<"END_MAKEFILE"); > lib/Data/SExpression/Parser.pm: lib/Data/SExpression/Parser.yp > \tyapp -m Data::SExpression::Parser -o \$@ \$< > END_MAKEFILE

> which causes a rule to get written to Makefile to generate Parser.pm.

> I take it that would also not work on your system, however, due to > yapp not being in $PATH?