Bug #35561 for Module-Install: use of features() with recommends() is broken since 0.71

Thu May 01 00:14:17 2008 dmaki [...] cpan.org - Ticket created

Subject:

use of features() with recommends() is broken since 0.71

Up to 0.70, you could say features( "Feature Name" => [ -default => 0, recommends("Module1") ] ); this created an interactive prompt like this: [Feature Name] - Module1 ...missing. ==> Auto-install the 1 optional module(s) from CPAN? [n] However, since 0.71, this results in a weird message [Feature Name] - ARRAY(0x84e1ac) ...missing. Whoa! I suppose from 0.71, the recommends() started returning an arrayref instead of the module name. So now we can only do features( "Feature Name" => [ -default => 0, "Module" ] ); I can live with either, but which is the blessed "official" path?

Tue Feb 03 17:35:30 2009 rt.cpan [...] sartak.org - Correspondence added

Adam, Has there been any progress on this bug? I know we discussed it at YAPC::NA and I hope you classify this as a useful feature and not as an accidental misfeature. If you're likely to accept a patch to fix it, I'd gladly write one. In any case, I'm still running Module::Install 0.70 simply because it's the last version that supported features + recommends. :/ Thanks, Shawn

Tue Feb 03 17:35:31 2009 The RT System itself - Status changed from 'new' to 'open'

Tue Feb 03 19:52:32 2009 adamkennedybackup [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #35561] use of features() with recommends() is broken since 0.71
Date:	Wed, 4 Feb 2009 11:51:23 +1100
To:	bug-Module-Install [...] rt.cpan.org
From:	Adam Kennedy <adamkennedybackup [...] gmail.com>

No progress I'm afraid. Adam K 2009/2/4 Shawn M Moore via RT <bug-Module-Install@rt.cpan.org>: Show quoted text

> Queue: Module-Install > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=35561 > > > Adam, > > Has there been any progress on this bug? I know we discussed it at > YAPC::NA and I hope you classify this as a useful feature and not as an > accidental misfeature. If you're likely to accept a patch to fix it, > I'd gladly write one. > > In any case, I'm still running Module::Install 0.70 simply because it's > the last version that supported features + recommends. :/ > > Thanks, > Shawn >

Fri Feb 20 20:12:55 2009 cpan [...] chmrr.net - Correspondence added

On Tue Feb 03 19:52:32 2009, adamkennedybackup@gmail.com wrote: Show quoted text

> No progress I'm afraid.

Attached is a minimal patch which solves the problem, using wantarray() to determine if the old or new functionality is wanted. - Alex

diff -ruN Module-Install-0.79-orig/inc/Module/Install/Metadata.pm Module-Install-0.79/inc/Module/Install/Metadata.pm --- Module-Install-0.79-orig/inc/Module/Install/Metadata.pm 2009-02-20 19:49:25.000000000 -0500 +++ Module-Install-0.79/inc/Module/Install/Metadata.pm 2009-02-20 20:08:54.000000000 -0500 @@ -1,4 +1,3 @@ -#line 1 package Module::Install::Metadata; use strict 'vars'; @@ -101,12 +100,13 @@ sub recommends { my $self = shift; + my @args = @_; while ( @_ ) { my $module = shift or last; my $version = shift || 0; push @{ $self->{values}{recommends} }, [ $module, $version ]; } - $self->{values}{recommends}; + wantarray ? @args : $self->{values}{recommends}; } sub bundles { diff -ruN Module-Install-0.79-orig/lib/Module/Install/Metadata.pm Module-Install-0.79/lib/Module/Install/Metadata.pm --- Module-Install-0.79-orig/lib/Module/Install/Metadata.pm 2009-02-20 19:49:25.000000000 -0500 +++ Module-Install-0.79/lib/Module/Install/Metadata.pm 2009-02-20 20:08:46.000000000 -0500 @@ -100,12 +100,13 @@ sub recommends { my $self = shift; + my @args = @_; while ( @_ ) { my $module = shift or last; my $version = shift || 0; push @{ $self->{values}{recommends} }, [ $module, $version ]; } - $self->{values}{recommends}; + wantarray ? @args : $self->{values}{recommends}; } sub bundles {

Fri Feb 20 20:46:27 2009 cpan [...] chmrr.net - Cc ALEXMV added

Fri Feb 20 21:07:57 2009 cpan [...] chmrr.net - Correspondence added

On Fri Feb 20 20:12:55 2009, ALEXMV wrote: Show quoted text

> Attached is a minimal patch which solves the problem, using wantarray() > to determine if the old or new functionality is wanted.

Actually, on closer inspection, I don't see anywhere that actually calls recommends for its return value. Is there a reason why the old functionality (no arguments is the reference, with arguments gives you them back) was removed? Attached is are two different patches which restore the functionality, depending on how you feel about your meta-programming. - Alex

diff -ruN Module-Install-0.79-orig/inc/Module/Install/Metadata.pm Module-Install-0.79/inc/Module/Install/Metadata.pm --- Module-Install-0.79-orig/inc/Module/Install/Metadata.pm 2009-02-20 19:49:25.000000000 -0500 +++ Module-Install-0.79/inc/Module/Install/Metadata.pm 2009-02-20 20:58:24.000000000 -0500 @@ -71,42 +71,54 @@ sub requires { my $self = shift; + return $self->{values}{requires} unless @_; + my @added; while ( @_ ) { my $module = shift or last; my $version = shift || 0; - push @{ $self->{values}{requires} }, [ $module, $version ]; + push @added, [ $module, $version ]; } - $self->{values}{requires}; + push @{ $self->{values}{requires} }, @added; + return map {@$_} @added; } sub build_requires { my $self = shift; + return $self->{values}{build_requires} unless @_; + my @added; while ( @_ ) { my $module = shift or last; my $version = shift || 0; - push @{ $self->{values}{build_requires} }, [ $module, $version ]; + push @added, [ $module, $version ]; } - $self->{values}{build_requires}; + push @{ $self->{values}{build_requires} }, @added; + return map {@$_} @added; } sub configure_requires { my $self = shift; + return $self->{values}{configure_requires} unless @_; + my @added; while ( @_ ) { my $module = shift or last; my $version = shift || 0; - push @{ $self->{values}{configure_requires} }, [ $module, $version ]; + push @added, [ $module, $version ]; } - $self->{values}{configure_requires}; + push @{ $self->{values}{configure_requires} }, @added; + return map {@$_} @added; } sub recommends { my $self = shift; + return $self->{values}{recommends} unless @_; + my @added; while ( @_ ) { my $module = shift or last; my $version = shift || 0; - push @{ $self->{values}{recommends} }, [ $module, $version ]; + push @added, [ $module, $version ]; } - $self->{values}{recommends}; + push @{ $self->{values}{recommends} }, @added; + return map {@$_} @added; } sub bundles { diff -ruN Module-Install-0.79-orig/lib/Module/Install/Metadata.pm Module-Install-0.79/lib/Module/Install/Metadata.pm --- Module-Install-0.79-orig/lib/Module/Install/Metadata.pm 2009-02-20 19:49:25.000000000 -0500 +++ Module-Install-0.79/lib/Module/Install/Metadata.pm 2009-02-20 20:56:50.000000000 -0500 @@ -70,42 +70,54 @@ sub requires { my $self = shift; + return $self->{values}{requires} unless @_; + my @added; while ( @_ ) { my $module = shift or last; my $version = shift || 0; - push @{ $self->{values}{requires} }, [ $module, $version ]; + push @added, [ $module, $version ]; } - $self->{values}{requires}; + push @{ $self->{values}{requires} }, @added; + return map {@$_} @added; } sub build_requires { my $self = shift; + return $self->{values}{build_requires} unless @_; + my @added; while ( @_ ) { my $module = shift or last; my $version = shift || 0; - push @{ $self->{values}{build_requires} }, [ $module, $version ]; + push @added, [ $module, $version ]; } - $self->{values}{build_requires}; + push @{ $self->{values}{build_requires} }, @added; + return map {@$_} @added; } sub configure_requires { my $self = shift; + return $self->{values}{configure_requires} unless @_; + my @added; while ( @_ ) { my $module = shift or last; my $version = shift || 0; - push @{ $self->{values}{configure_requires} }, [ $module, $version ]; + push @added, [ $module, $version ]; } - $self->{values}{configure_requires}; + push @{ $self->{values}{configure_requires} }, @added; + return map {@$_} @added; } sub recommends { my $self = shift; + return $self->{values}{recommends} unless @_; + my @added; while ( @_ ) { my $module = shift or last; my $version = shift || 0; - push @{ $self->{values}{recommends} }, [ $module, $version ]; + push @added, [ $module, $version ]; } - $self->{values}{recommends}; + push @{ $self->{values}{recommends} }, @added; + return map {@$_} @added; } sub bundles {

diff -ruN Module-Install-0.79-orig/inc/Module/Install/Metadata.pm Module-Install-0.79/inc/Module/Install/Metadata.pm --- Module-Install-0.79-orig/inc/Module/Install/Metadata.pm 2009-02-20 19:49:25.000000000 -0500 +++ Module-Install-0.79/inc/Module/Install/Metadata.pm 2009-02-20 21:02:32.000000000 -0500 @@ -69,54 +69,19 @@ }; } -sub requires { - my $self = shift; - while ( @_ ) { - my $module = shift or last; - my $version = shift || 0; - push @{ $self->{values}{requires} }, [ $module, $version ]; - } - $self->{values}{requires}; -} - -sub build_requires { - my $self = shift; - while ( @_ ) { - my $module = shift or last; - my $version = shift || 0; - push @{ $self->{values}{build_requires} }, [ $module, $version ]; - } - $self->{values}{build_requires}; -} - -sub configure_requires { - my $self = shift; - while ( @_ ) { - my $module = shift or last; - my $version = shift || 0; - push @{ $self->{values}{configure_requires} }, [ $module, $version ]; - } - $self->{values}{configure_requires}; -} - -sub recommends { - my $self = shift; - while ( @_ ) { - my $module = shift or last; - my $version = shift || 0; - push @{ $self->{values}{recommends} }, [ $module, $version ]; - } - $self->{values}{recommends}; -} - -sub bundles { - my $self = shift; - while ( @_ ) { - my $module = shift or last; - my $version = shift || 0; - push @{ $self->{values}{bundles} }, [ $module, $version ]; - } - $self->{values}{bundles}; +foreach my $key ( grep {$_ ne "resources"} @tuple_keys) { + *$key = sub { + my $self = shift; + return $self->{values}{$key} unless @_; + my @added; + while ( @_ ) { + my $module = shift or last; + my $version = shift || 0; + push @added, [ $module, $version ]; + } + push @{ $self->{values}{$key} }, @added; + return map {@$_} @added; + }; } # Resource handling diff -ruN Module-Install-0.79-orig/lib/Module/Install/Metadata.pm Module-Install-0.79/lib/Module/Install/Metadata.pm --- Module-Install-0.79-orig/lib/Module/Install/Metadata.pm 2009-02-20 19:49:25.000000000 -0500 +++ Module-Install-0.79/lib/Module/Install/Metadata.pm 2009-02-20 21:01:41.000000000 -0500 @@ -68,54 +68,19 @@ }; } -sub requires { - my $self = shift; - while ( @_ ) { - my $module = shift or last; - my $version = shift || 0; - push @{ $self->{values}{requires} }, [ $module, $version ]; - } - $self->{values}{requires}; -} - -sub build_requires { - my $self = shift; - while ( @_ ) { - my $module = shift or last; - my $version = shift || 0; - push @{ $self->{values}{build_requires} }, [ $module, $version ]; - } - $self->{values}{build_requires}; -} - -sub configure_requires { - my $self = shift; - while ( @_ ) { - my $module = shift or last; - my $version = shift || 0; - push @{ $self->{values}{configure_requires} }, [ $module, $version ]; - } - $self->{values}{configure_requires}; -} - -sub recommends { - my $self = shift; - while ( @_ ) { - my $module = shift or last; - my $version = shift || 0; - push @{ $self->{values}{recommends} }, [ $module, $version ]; - } - $self->{values}{recommends}; -} - -sub bundles { - my $self = shift; - while ( @_ ) { - my $module = shift or last; - my $version = shift || 0; - push @{ $self->{values}{bundles} }, [ $module, $version ]; - } - $self->{values}{bundles}; +foreach my $key ( grep {$_ ne "resources"} @tuple_keys) { + *$key = sub { + my $self = shift; + return $self->{values}{$key} unless @_; + my @added; + while ( @_ ) { + my $module = shift or last; + my $version = shift || 0; + push @added, [ $module, $version ]; + } + push @{ $self->{values}{$key} }, @added; + return map {@$_} @added; + }; } # Resource handling

Thu Apr 08 19:30:13 2010 ishigaki [...] cpan.org - Correspondence added

Hi. The last refactored patch was applied some time before. Thanks. On 2009-2-20 Fri 21:07:57, ALEXMV wrote: Show quoted text

> On Fri Feb 20 20:12:55 2009, ALEXMV wrote:

> > Attached is a minimal patch which solves the problem, using

wantarray() Show quoted text

> > to determine if the old or new functionality is wanted.

> > Actually, on closer inspection, I don't see anywhere that actually

calls Show quoted text

> recommends for its return value. Is there a reason why the old > functionality (no arguments is the reference, with arguments gives you > them back) was removed? Attached is are two different patches which > restore the functionality, depending on how you feel about your > meta-programming. > - Alex

Thu Apr 08 19:30:26 2010 ishigaki [...] cpan.org - Status changed from 'open' to 'resolved'