Skip Menu |
 

This queue is for tickets about the Module-Build CPAN distribution.

Report information
The Basics
Id: 40177
Status: resolved
Priority: 0/
Queue: Module-Build

People
Owner: Nobody in particular
Requestors: mschwern [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: 0.30
Fixed in: (no value)



Subject: include_dirs does not take a string as documented.
Download (untitled) / with headers
text/plain 378b
include_dirs says it takes a string or an array ref. Internally its only used as an array ref. If you give it a string, it bombs out in process_support_files(). If this never worked I'm perfectly happy with changing the docs so it always takes an array ref and not screw around with special casing a single argument. Otherwise, it should be normalized to an array on input.
Download (untitled) / with headers
text/plain 578b
On Sun Oct 19 19:33:48 2008, MSCHWERN wrote: Show quoted text
> include_dirs says it takes a string or an array ref. Internally its > only used as an array ref. If you give it a string, it bombs out in > process_support_files(). > > If this never worked I'm perfectly happy with changing the docs so it > always takes an array ref and not screw around with special casing a > single argument. Otherwise, it should be normalized to an array on input.
Here's a patch to fix this issue, along with tests to make sure it stays fixed. Passing a string to `include_dirs` now works. Best, David
Download include_dirs.patch
text/x-diff 2.4k
Index: t/basic.t =================================================================== --- t/basic.t (revision 12873) +++ t/basic.t (working copy) @@ -2,7 +2,7 @@ use strict; use lib $ENV{PERL_CORE} ? '../lib/Module/Build/t/lib' : 't/lib'; -use MBTest tests => 52; +use MBTest tests => 60; use_ok 'Module::Build'; ensure_blib('Module::Build'); @@ -203,6 +203,38 @@ is_deeply $mb->extra_linker_flags, ['-L/foo', '-L/bar'], "Should split shell string into list"; } +# Test include_dirs. +{ + ok my $mb = Module::Build->new( + module_name => $dist->name, + include_dirs => [qw(/foo /bar)], + ); + is_deeply $mb->include_dirs, ['/foo', '/bar'], 'Should have include dirs'; + # Try a string. + ok $mb = Module::Build->new( + module_name => $dist->name, + include_dirs => '/foo', + ); + is_deeply $mb->include_dirs, ['/foo'], 'Should have string include dir'; + + # Try again with command-line args + eval { Module::Build->run_perl_script( + 'Build.PL', [], + ['--include_dirs', '/foo', '--include_dirs', '/bar' ], + ) }; + + ok $mb = Module::Build->resume; + is_deeply $mb->include_dirs, ['/foo', '/bar'], 'Should have include dirs'; + + eval { Module::Build->run_perl_script( + 'Build.PL', [], + ['--include_dirs', '/foo' ], + ) }; + + ok $mb = Module::Build->resume; + is_deeply $mb->include_dirs, ['/foo'], 'Should have single include dir'; +} + # cleanup $dist->remove; Index: lib/Module/Build/Base.pm =================================================================== --- lib/Module/Build/Base.pm (revision 12873) +++ lib/Module/Build/Base.pm (working copy) @@ -165,11 +165,16 @@ $p->{requires} = delete $p->{prereq} if defined $p->{prereq}; $p->{script_files} = delete $p->{scripts} if defined $p->{scripts}; - # Convert to arrays + # Convert to from shell strings to arrays for ('extra_compiler_flags', 'extra_linker_flags') { $p->{$_} = [ $self->split_like_shell($p->{$_}) ] if exists $p->{$_}; } + # Convert to arrays + for ('include_dirs') { + $p->{$_} = [ $p->{$_} ] if exists $p->{$_} && !ref $p->{$_} + } + $self->add_to_cleanup( @{delete $p->{add_to_cleanup}} ) if $p->{add_to_cleanup}; @@ -1760,6 +1765,11 @@ $args{$_} = [ $self->split_like_shell($args{$_}) ] if exists $args{$_}; } + # Convert to arrays + for ('include_dirs') { + $args{$_} = [ $args{$_} ] if exists $args{$_} && !ref $args{$_} + } + # Hashify these parameters for ($self->hash_properties, 'config') { next unless exists $args{$_};
Thanks, applied.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.