Bug #21282 for ExtUtils-MakeMaker: Split metafile generation into user overridable methods.

Sat Sep 02 14:45:01 2006 mschwern [...] cpan.org - Ticket created

Subject:

Split metafile generation into user overridable methods.

Adriano Ferreira wrote: Show quoted text

> The following patch extends EXTRA_META to accept hash refs as well it > already accepts YAML-formatted text. > > This way I could write a Makefile.PL like > > use 5.006; > use ExtUtils::MakeMaker; > > WriteMakefile( > NAME => 'Foo', > VERSION => '42', > EXTRA_META => { > recommends => { > 'Test::Pod' => '1.18', > 'Test::Pod::Coverage' => '1.04', > } > } > > );

This is a much better idea than my "paste this onto the end of the META.yml" which makes my feet itch and I'm going to undocument right now so it doesn't hold up 6.31. Probably even better is to simply expose the META.yml hash generation method and allow the user to override. metafile_data() would simply return the data which MakeMaker then turns into a META.yml file. metafile_file() would take that and turn it into a file. metafile_target would simply do the necessary make juju to paste the metafile_file into the Makefile target. This has the nice side-effect of MakeMaker not having to depend on META.yml. It can continue to use its crappy fake YAML generator and if you write a metafile_data() that does something more complicated you can override metafile_file() to use YAML.pm. I'm not holding up 6.31 for this, but its a very good idea. Patches welcome.

Sat Sep 02 14:45:40 2006 mschwern [...] cpan.org - Cc makemaker@perl.org added

Sat Sep 02 14:45:40 2006 mschwern [...] cpan.org - Requestor a.r.ferreira@gmail.com added

Sat Sep 02 14:49:51 2006 mschwern [...] cpan.org - Correspondence added

Reposting this for Adriano and makemaker@perl.org's benefit. On Sat Sep 02 14:45:01 2006, MSCHWERN wrote: Show quoted text

> Adriano Ferreira wrote:

> > The following patch extends EXTRA_META to accept hash refs as well it > > already accepts YAML-formatted text. > > > > This way I could write a Makefile.PL like > > > > use 5.006; > > use ExtUtils::MakeMaker; > > > > WriteMakefile( > > NAME => 'Foo', > > VERSION => '42', > > EXTRA_META => { > > recommends => { > > 'Test::Pod' => '1.18', > > 'Test::Pod::Coverage' => '1.04', > > } > > } > > > > );

> > This is a much better idea than my "paste this onto the end of the > META.yml" which makes my feet itch and I'm going to undocument right now > so it doesn't hold up 6.31. > > Probably even better is to simply expose the META.yml hash generation > method and allow the user to override. metafile_data() would simply > return the data which MakeMaker then turns into a META.yml file. > metafile_file() would take that and turn it into a file. > metafile_target would simply do the necessary make juju to paste the > metafile_file into the Makefile target. > > This has the nice side-effect of MakeMaker not having to depend on > META.yml. It can continue to use its crappy fake YAML generator and if > you write a metafile_data() that does something more complicated you can > override metafile_file() to use YAML.pm. > > I'm not holding up 6.31 for this, but its a very good idea. Patches > welcome.

Sat Sep 02 14:49:53 2006 mschwern [...] cpan.org - Status changed from 'new' to 'open'

Tue Sep 05 12:36:36 2006 a.r.ferreira [...] gmail.com - Correspondence added

CC:	makemaker [...] perl.org
Subject:	Re: [rt.cpan.org #21282] Split metafile generation into user overridable methods.
Date:	Tue, 5 Sep 2006 13:36:20 -0300
To:	bug-ExtUtils-MakeMaker [...] rt.cpan.org
From:	"Adriano Ferreira" <a.r.ferreira [...] gmail.com>

Ok. That's my first attempt at the subject: "Split metafile generation into user overridable methods." The patch against ExtUtils-MakeMaker-6.30_03 is attached. Some explanations are in order: (1) The patch proposes to change the nature of EXTRA_META from a string into a hash ref - it also may be an array (ref) of pairs, if preserving the order of appearance is desirable. Well, that's not compatible with 6.30_01, _02, and _03 with the "paste this onto the end of the META.yml" thing. But it was experimental and Schwern said that, as it was, it would not pass onto 6.31. (2) There is now a method C<metafile_data> in ExtUtils::MM_Any which spits the data to be dumped to META.yml If EXTRA_META was set, the additional pairs are appended to the data. If EXTRA_META contains keys such as 'license' and 'name' which are already MakeMaker-generated you get a warning and the given values are ignored. The order of the fields in META.yml with respect to the previous EUMM versions are preserved. The additional keys 'installdirs' and 'version_from' that C<metafile_target> documented (for example, in 6.30) and which were missing in 6.30_01, _02, and _03 were restored. I don't know if the removal was intentional or not. I thought it would be a good idea to update the META.yml specification link to: 'meta-spec' => { url => 'http://module-build.sourceforge.net/META-spec-v1.1.html', version => 1.1* }, But I am not sure since 'no_index' is at v1.2. Another doubt is that 6.30_01 dumped the URL as url: <http://module-build.sourceforge.net/META-spec-new.html>; Should it be that way? (3) There is now a method C<metafile_file> in ExtUtils::MM_Any which transforms the metadata (as returned by C<metafile_data>) into (fake) YAML. Both metafile_data and metafile_file were abstracted from the previous code of C<metafile_target> and separated according to the description of RT ticket #21282. The dumper was coded to mimick exactly the previous behavior and extended to support nested hashes and strings of arrays. It was done to cover the basic cases of adding 'recommends' and 'require_build' hashes and 'no_index' fields (which contains arrays of strings). That simulation made the code a bit more complex than it could be. Among the features kept, there are: * the original order of the pairs in the root hash is preserved - so that what left metafile_data pass orderly to YAML. * keys and values in the root hash and 'requires' are aligned: name: Foo version: 1 requires: Test::More: 0 File::Spec: 0 * 'requires' pairs are sorted in a case-insensitive fashion (as it was in previous versions) New feature: * nested hashes sort with { $a cmp $b } (4) By now, I wrote a test script 't/metafile_file.t' which tests the dumper via C<ExtUtils::MM_Any::metafile_file>. Best regards, Adriano Ferreira. On 9/2/06, Michael_G_Schwern via RT <bug-ExtUtils-MakeMaker@rt.cpan.org> wrote: Show quoted text

> > <URL: http://rt.cpan.org/Ticket/Display.html?id=21282 > > > Reposting this for Adriano and makemaker@perl.org's benefit. > > On Sat Sep 02 14:45:01 2006, MSCHWERN wrote:

> > Adriano Ferreira wrote:

> > > The following patch extends EXTRA_META to accept hash refs as well it > > > already accepts YAML-formatted text. > > > > > > This way I could write a Makefile.PL like > > > > > > use 5.006; > > > use ExtUtils::MakeMaker; > > > > > > WriteMakefile( > > > NAME => 'Foo', > > > VERSION => '42', > > > EXTRA_META => { > > > recommends => { > > > 'Test::Pod' => '1.18', > > > 'Test::Pod::Coverage' => '1.04', > > > } > > > } > > > > > > );

> > > > This is a much better idea than my "paste this onto the end of the > > META.yml" which makes my feet itch and I'm going to undocument right now > > so it doesn't hold up 6.31. > > > > Probably even better is to simply expose the META.yml hash generation > > method and allow the user to override. metafile_data() would simply > > return the data which MakeMaker then turns into a META.yml file. > > metafile_file() would take that and turn it into a file. > > metafile_target would simply do the necessary make juju to paste the > > metafile_file into the Makefile target. > > > > This has the nice side-effect of MakeMaker not having to depend on > > META.yml. It can continue to use its crappy fake YAML generator and if > > you write a metafile_data() that does something more complicated you can > > override metafile_file() to use YAML.pm. > > > > I'm not holding up 6.31 for this, but its a very good idea. Patches > > welcome.

> > >

Message body is not shown because sender requested not to inline it.

Mon Sep 11 16:31:12 2006 mschwern [...] cpan.org - Dependency by ticket #17332 added

Mon Sep 11 16:40:25 2006 mschwern [...] cpan.org - Correspondence added

On Tue Sep 05 12:36:36 2006, a.r.ferreira@gmail.com wrote: Show quoted text

> (2) There is now a method C<metafile_data> in ExtUtils::MM_Any > which spits the data to be dumped to META.yml > > If EXTRA_META was set, the additional pairs are appended to > the data. If EXTRA_META contains keys such as 'license' and 'name' > which are already MakeMaker-generated you get a warning and > the given values are ignored.

I think they should override. If they user wants to blow over what MakeMaker generates, let them. Show quoted text

> The additional keys 'installdirs' and 'version_from' that > C<metafile_target> documented (for example, in 6.30) and which > were missing > in 6.30_01, _02, and _03 were restored. I don't know if the > removal was intentional or not.

The removal was intentional. 6.30_01 changed our META.yml to match META spec and dropped those fields. Show quoted text

> I thought it would be a good idea to update the META.yml > specification > link to: > > 'meta-spec' => { > url => > 'http://module-build.sourceforge.net/META-spec-v1.1.html', > version => 1.1* > }, > > But I am not sure since 'no_index' is at v1.2. Another doubt is > that 6.30_01 dumped the URL as > > url: <http://module-build.sourceforge.net/META-spec-new.html>; > > Should it be that way?

At the time that was the URL to the 1.1 spec-to-be. Updating to 1.2 is appreciated. Show quoted text

> (3) There is now a method C<metafile_file> in ExtUtils::MM_Any > which transforms the metadata (as returned by C<metafile_data>) > into (fake) YAML. > > Both metafile_data and metafile_file were abstracted from > the previous code of C<metafile_target> and separated according > to the description of RT ticket #21282. > > The dumper was coded to mimick exactly the previous > behavior and extended to support nested hashes and strings > of arrays. It was done to cover the basic cases of > adding 'recommends' and 'require_build' hashes and > 'no_index' fields (which contains arrays of strings). > > That simulation made the code a bit more complex than > it could be. Among the features kept, there are: > > * the original order of the pairs in the root hash > is preserved - so that what left metafile_data > pass orderly to YAML. > > * keys and values in the root hash and 'requires' > are aligned: > > name: Foo > version: 1 > requires: > Test::More: 0 > File::Spec: 0 > > * 'requires' pairs are sorted in a case-insensitive > fashion (as it was in previous versions)

Excellent work, thank you. Show quoted text

> New feature: > > * nested hashes sort with { $a cmp $b }

Sort them case insensitively, too. Show quoted text

> (4) By now, I wrote a test script 't/metafile_file.t' > which tests the dumper via C<ExtUtils::MM_Any::metafile_file>.

Thank you. As I mentioned, I'm going to hold this until after 6.31.

Mon Sep 11 16:40:26 2006 mschwern [...] cpan.org - Status changed from 'open' to 'stalled'

Mon Sep 11 20:12:02 2006 a.r.ferreira [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #21282] Split metafile generation into user overridable methods.
Date:	Mon, 11 Sep 2006 21:11:42 -0300
To:	bug-ExtUtils-MakeMaker [...] rt.cpan.org
From:	"Adriano Ferreira" <a.r.ferreira [...] gmail.com>

On 9/11/06, Michael_G_Schwern via RT <bug-ExtUtils-MakeMaker@rt.cpan.org> wrote: Show quoted text

> I think they should override. If they user wants to blow over what > MakeMaker generates, let them.

Show quoted text

> The removal was intentional. 6.30_01 changed our META.yml to match META > spec and dropped those fields.

Show quoted text

> At the time that was the URL to the 1.1 spec-to-be. Updating to 1.2 is > appreciated.

Show quoted text

> Excellent work, thank you.

Show quoted text

> As I mentioned, I'm going to hold this until after 6.31.

Thank you too. I will prepare a replacement patch with: (1) EXTRA_META possibly overriding MakeMaker generated fields, (2) no installdirs and version_from, (3) and meta-spec at v 1.2. An additional doubt I had was about the way to dump the URL - I think 6.30 used this Show quoted text

> url: <http://module-build.sourceforge.net/META-spec-new.html>;

but 6.30_01, _02, _03 used this: Show quoted text

> url: http://module-build.sourceforge.net/META-spec-new.html

What then? Do you think I wait for 6.31 to gather together this new patch and send you? Do you think I could make accessible via CPAN a distribution such as ExtUtils-MakeMaker-FERREIRA-6.30_90 so that people could experiment? Or that would be undesirable in general terms (could lend to confusion, etc.)? Adriano.

Mon Sep 11 20:12:02 2006 The RT System itself - Status changed from 'stalled' to 'open'

Mon Sep 11 20:30:36 2006 schwern [...] pobox.com - Correspondence added

Subject:	Re: [rt.cpan.org #21282] Split metafile generation into user overridable methods.
Date:	Mon, 11 Sep 2006 20:30:01 -0400
To:	bug-ExtUtils-MakeMaker [...] rt.cpan.org
From:	Michael G Schwern <schwern [...] gmail.com>

a.r.ferreira@gmail.com via RT wrote: Show quoted text

> Thank you too. I will prepare a replacement patch with: (1) EXTRA_META > possibly overriding MakeMaker generated fields, (2) no installdirs and > version_from, (3) and meta-spec at v 1.2. An additional doubt I had > was about the way to dump the URL - I think 6.30 used this >

>> url: <http://module-build.sourceforge.net/META-spec-new.html>;

> > but 6.30_01, _02, _03 used this: >

>> url: http://module-build.sourceforge.net/META-spec-new.html

> > What then?

Whatever way the META spec says to do (which is the latter). In fact, pull from the repository as I made some small changes to the meta stuff. http://svn.schwern.org/CPAN/ExtUtils-MakeMaker/trunk Show quoted text

> Do you think I wait for 6.31 to gather together this new patch and > send you? Do you think I could make accessible via CPAN a distribution > such as ExtUtils-MakeMaker-FERREIRA-6.30_90 so that people could > experiment? Or that would be undesirable in general terms (could lend > to confusion, etc.)?

You can send the patch now, I'm unlikely to do any major work to the metafile stuff. If you really want to make a release you can, CPAN won't index it. But I don't think its necessary. If you're interested in doing more MakeMaker work I can give you a commit bit to the repo and you can work in a branch there.

Mon Sep 11 21:34:30 2006 a.r.ferreira [...] gmail.com - Correspondence added

CC:	makemaker [...] perl.org
Subject:	Re: [rt.cpan.org #21282] Split metafile generation into user overridable methods.
Date:	Mon, 11 Sep 2006 22:34:12 -0300
To:	bug-ExtUtils-MakeMaker [...] rt.cpan.org, "Michael G Schwern" <mschwern [...] cpan.org>
From:	"Adriano Ferreira" <a.r.ferreira [...] gmail.com>

On 9/11/06, Michael G Schwern via RT <bug-ExtUtils-MakeMaker@rt.cpan.org> wrote: Show quoted text

> If you're interested in doing more MakeMaker work I can give you a commit bit to the repo and you can work in a branch there.

Indeed, I am interested. The metafile stuff was the first one I felt confident to contribute with. But I think I can work on some other issues as well. For a long time, I have also been contemplating your offer to someone to pick up ExtUtils::Manifest and some other modules. Yves took some of them with ExtUtils::Install. Maybe I could do this now, after showing you some patches. But I have to study the code closer yet. Adriano.

Wed Oct 25 03:17:24 2006 mschwern [...] cpan.org - Dependency by ticket #17332 deleted

Fri Mar 02 20:24:01 2007 a.r.ferreira [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #21282] Split metafile generation into user overridable methods.
Date:	Fri, 2 Mar 2007 22:23:42 -0300
To:	bug-ExtUtils-MakeMaker [...] rt.cpan.org, "Michael G Schwern" <mschwern [...] cpan.org>
From:	"Adriano Ferreira" <a.r.ferreira [...] gmail.com>

A few hundred years ago (more exactly, September 2006) I proposed a patch for a better handling of META.yml generation for MakeMaker. And then 6.31 came and then 6.32 a few days ago, but the patch rewrite never reappeared, until now. I just prepared a revised patch which incorporates your feedback, and applies against revision 3935 at svn.schwern.org Just to refresh, the patch features are: (1) The patch proposes to change the nature of EXTRA_META from a string into a hash ref - it also may be an array (ref) of pairs, if preserving the order of appearance is desirable. (2) There is now a method C<metafile_data> in ExtUtils::MM_Any which spits the data to be dumped to META.yml If EXTRA_META was set, the additional pairs are appended to the data. If EXTRA_META contains keys such as 'license' and 'name' which are already MakeMaker-generated you get a warning and the new values override the old ones (you better know what you're doing). The order of the fields in META.yml with respect to the previous EUMM versions are preserved. (3) There is now a method C<metafile_file> in ExtUtils::MM_Any which transforms the metadata (as returned by C<metafile_data>) into (fake) YAML. Both metafile_data and metafile_file were abstracted from the previous code of C<metafile_target> and separated according to the description of RT ticket #21282. The dumper was coded to mimick exactly the previous behavior and extended to support nested hashes and strings of arrays. It was done to cover the basic cases of adding 'recommends' and 'require_build' hashes and 'no_index' fields (which contains arrays of strings). That simulation made the code a bit more complex than it could be. Among the features kept, there are: * the original order of the pairs in the root hash is preserved - so that what left metafile_data pass orderly to YAML. * keys and values in the root hash and 'requires' are aligned: name: Foo version: 1 requires: Test::More: 0 File::Spec: 0 * hash pairs are sorted in a case-insensitive fashion (as 'requires' was handled in previous versions) (4) By now, I wrote a test script 't/metafile_file.t' which tests the dumper via C<ExtUtils::MM_Any::metafile_file>. I tried this patch successfully in a few Perl versions and architectures (most Windows and Cygwin). Kind regards, Adriano Ferreira On 9/11/06, Michael_G_Schwern via RT <bug-ExtUtils-MakeMaker@rt.cpan.org> wrote: Show quoted text

> > <URL: http://rt.cpan.org/Ticket/Display.html?id=21282 > > > On Tue Sep 05 12:36:36 2006, a.r.ferreira@gmail.com wrote:

> > (2) There is now a method C<metafile_data> in ExtUtils::MM_Any > > which spits the data to be dumped to META.yml > > > > If EXTRA_META was set, the additional pairs are appended to > > the data. If EXTRA_META contains keys such as 'license' and 'name' > > which are already MakeMaker-generated you get a warning and > > the given values are ignored.

> > I think they should override. If they user wants to blow over what > MakeMaker generates, let them. > >

> > The additional keys 'installdirs' and 'version_from' that > > C<metafile_target> documented (for example, in 6.30) and which > > were missing > > in 6.30_01, _02, and _03 were restored. I don't know if the > > removal was intentional or not.

> > The removal was intentional. 6.30_01 changed our META.yml to match META > spec and dropped those fields. > >

> > I thought it would be a good idea to update the META.yml > > specification > > link to: > > > > 'meta-spec' => { > > url => > > 'http://module-build.sourceforge.net/META-spec-v1.1.html', > > version => 1.1* > > }, > > > > But I am not sure since 'no_index' is at v1.2. Another doubt is > > that 6.30_01 dumped the URL as > > > > url: <http://module-build.sourceforge.net/META-spec-new.html>; > > > > Should it be that way?

> > At the time that was the URL to the 1.1 spec-to-be. Updating to 1.2 is > appreciated. > >

> > (3) There is now a method C<metafile_file> in ExtUtils::MM_Any > > which transforms the metadata (as returned by C<metafile_data>) > > into (fake) YAML. > > > > Both metafile_data and metafile_file were abstracted from > > the previous code of C<metafile_target> and separated according > > to the description of RT ticket #21282. > > > > The dumper was coded to mimick exactly the previous > > behavior and extended to support nested hashes and strings > > of arrays. It was done to cover the basic cases of > > adding 'recommends' and 'require_build' hashes and > > 'no_index' fields (which contains arrays of strings). > > > > That simulation made the code a bit more complex than > > it could be. Among the features kept, there are: > > > > * the original order of the pairs in the root hash > > is preserved - so that what left metafile_data > > pass orderly to YAML. > > > > * keys and values in the root hash and 'requires' > > are aligned: > > > > name: Foo > > version: 1 > > requires: > > Test::More: 0 > > File::Spec: 0 > > > > * 'requires' pairs are sorted in a case-insensitive > > fashion (as it was in previous versions)

> > Excellent work, thank you. > >

> > New feature: > > > > * nested hashes sort with { $a cmp $b }

> > Sort them case insensitively, too. > >

> > (4) By now, I wrote a test script 't/metafile_file.t' > > which tests the dumper via C<ExtUtils::MM_Any::metafile_file>.

> > Thank you. > > As I mentioned, I'm going to hold this until after 6.31. >

Message body is not shown because sender requested not to inline it.

Sat Sep 06 05:58:19 2008 mschwern [...] cpan.org - Correspondence added

I've finally patched this in, sorry for taking so long. I took a different route following Module::Build's example. Rather than have users override a method, they can supply META_ADD and META_MERGE, like Module::Build's meta_add and meta_merge attributes, to add/override or merge in more metadata. There shouldn't be any need for overriding the methods. Rather than repurpose EXTRA_META, it's now gone. The work you did on the dumper was splendid. I made a few formatting tweaks to make the key length stuff a bit smarter, it now figures out a good length automatically.

Sat Sep 06 05:59:18 2008 mschwern [...] cpan.org - Status changed from 'open' to 'resolved'