Skip Menu |

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

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

People
Owner: Nobody in particular
Requestors: cberry [...] cpan.org
malmberg [...] Encompasserve.org
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 0.31012
Fixed in: (no value)



Subject: compat.t doing case sensitive filename compare on VMS.
On VMS, by default filenames can be lowercased by Perl. VMS also has a case preserved mode where it either return the case of the file on the disk, or the case that was used to look up the file. The attached patch makes the compare case insensitive. # # Installing PROJECT_ROOT:[PERL-BLEAD.t._tmp348837.lbiidr]simple.pm # ' # doesn't match '(?x-ism:PROJECT_ROOT\:\[PERL\-BLEAD\.t\._tmp348837\.lbiidr\
Subject: m_b_compat_t.gdiff
Download m_b_compat_t.gdiff
application/octet-stream 484b

Message body not shown because it is not plain text.

Subject: _detildefy on VMS has unexpected trailing '/'.
The tilde.t tests on VMS are failing as they expect that there is not a trailing slash on the expanded $HOME directory. The attached patch fixes Platform/VMS.pm for this issue.
Subject: m_b_platform_vms_pm.gdiff
Download m_b_platform_vms_pm.gdiff
application/octet-stream 306b

Message body not shown because it is not plain text.

Subject: .packlist not being created on VMS / uninit var in htmlify_pods
In Base.pm, two bugs that are fixed by the attached patch. An invalid index shows up in htmlify_pods during the install.t test. The splitpath call needs to have the nopath parameter set to 0, not 1 when there is a filename component for the test. With this patch and the patches with bug 41364, 41332, 41331, all Module Build tests are passing for me.
Subject: m_b_base_bm.gdiff
Download m_b_base_bm.gdiff
application/octet-stream 1.1k

Message body not shown because it is not plain text.

Subject: [PATCH] consolidated VMS fixes
The attached patch gets M::B 0.31012 passing all tests on VMS and I've also tested on OS X. In some cases it incorporates patches previously submitted by John Malmberg but in a couple of cases I've been able to isolate the changes a bit better under Platform::VMS. Here's what's included: In Base::htmlify_pods, check that the @dirs array has something in it before processing it -- that avoids an uninitialized value warning. In Base::install_map, flip the no_file argument to splitpath from true to false. In Compat::fake_makefile, don't double up the dollar signs when building the one-liner and running on VMS. Add Platform::VMS::oneliner as an override. When spawning a command, the arguments must be quoted but the command itself must not be. Modify the Platform::VMS::_detildefy override to remove a trailing slash from the result when passed a tilde by itself. Add Platform::VMS::ACTION_clean override to work around the fact that glob on VMS treats glob('foo') as glob('foo.*'). Make the check for Simple.pm in t/compat.t case insensitive. It's name will be reported as simple.pm on VMS with default settings. The one patch of John's not included here is this one: http://rt.cpan.org/Public/Bug/Display.html?id=42157 It looks fine to me but I haven't tested that one and it addresses new functionality, not just bug fixes.
Subject: mb_vms_20090122.patch.txt
--- lib/Module/Build/Base.pm;-0 2009-01-14 11:50:10 -0600 +++ lib/Module/Build/Base.pm 2009-01-22 16:45:22 -0600 @@ -2817,7 +2817,7 @@ sub htmlify_pods { my ($name, $path) = File::Basename::fileparse($pods->{$pod}, file_qr('\.(?:pm|plx?|pod)$')); my @dirs = File::Spec->splitdir( File::Spec->canonpath( $path ) ); - pop( @dirs ) if $dirs[-1] eq File::Spec->curdir; + pop( @dirs ) if scalar(@dirs) && $dirs[-1] eq File::Spec->curdir; my $fulldir = File::Spec->catfile($htmldir, @rootdirs, @dirs); my $outfile = File::Spec->catfile($fulldir, "${name}.html"); @@ -4061,7 +4061,7 @@ sub install_map { # Need to remove volume from $map{$_} using splitpath, or else # we'll create something crazy like C:\Foo\Bar\E:\Baz\Quux # VMS will always have the file separate than the path. - my ($volume, $path, $file) = File::Spec->splitpath( $map{$_}, 1 ); + my ($volume, $path, $file) = File::Spec->splitpath( $map{$_}, 0 ); # catdir needs a list of directories, or it will create something # crazy like volume:[Foo.Bar.volume.Baz.Quux] --- lib/Module/Build/Compat.pm;-0 2009-01-14 11:50:10 -0600 +++ lib/Module/Build/Compat.pm 2009-01-22 14:29:55 -0600 @@ -279,7 +279,7 @@ sub fake_makefile { my $Build = 'Build' . $filetype . ' --makefile_env_macros 1'; my $unlink = $class->oneliner('1 while unlink $ARGV[0]', [], [$args{makefile}]); - $unlink =~ s/\$/\$\$/g; + $unlink =~ s/\$/\$\$/g unless $class->is_vmsish; my $maketext = <<"EOF"; all : force_do_it --- lib/Module/Build/Platform/VMS.pm;-0 2009-01-14 11:50:10 -0600 +++ lib/Module/Build/Platform/VMS.pm 2009-01-22 16:49:35 -0600 @@ -188,6 +188,21 @@ sub do_system { return !system("$cmd $args"); } +=item oneliner + +Override to ensure that we do not quote the command. + +=cut + +sub oneliner { + my $self = shift; + my $oneliner = $self->SUPER::oneliner(@_); + + $oneliner =~ s/^\"\S+\"//; + + return "MCR $^X $oneliner"; +} + =item _infer_xs_spec Inherit the standard version but tweak the library file name to be @@ -309,6 +324,7 @@ sub _detildefy { # Trivial case of just ~ by it self if ($spec eq '') { + $home =~ s#/$##; return $home; } @@ -385,6 +401,20 @@ sub localize_dir_path { return VMS::Filespec::vmspath($path); } +=item ACTION_clean + +The home-grown glob() expands a bit too aggressively when given a bare name, +so default in a zero-length extension. + +=cut + +sub ACTION_clean { + my ($self) = @_; + foreach my $item (map glob(VMS::Filespec::rmsexpand($_, '.;0')), $self->cleanup) { + $self->delete_filetree($item); + } +} + =back =head1 AUTHOR --- t/compat.t;-0 2009-01-14 11:50:10 -0600 +++ t/compat.t 2009-01-22 14:53:23 -0600 @@ -230,7 +230,7 @@ ok $mb, "Module::Build->new_from_context ok $ran_ok, "make fakeinstall with INSTALLDIRS=vendor ran ok"; $output =~ s/^/# /gm; # Don't confuse our own test output like $output, - qr/\Q$libdir2\E .* Simple\.pm/x, + qr/\Q$libdir2\E .* Simple\.pm/ix, 'Should have installdirs=vendor'; stdout_of( sub { $mb->do_system(@make, 'realclean'); } );
I would like to get this into blead so that I can merge in the Unix / Extended file specification changes and test them.
On Sat Feb 14 12:52:40 2009, malmberg@Encompasserve.org wrote: Show quoted text
> I would like to get this into blead so that I can merge in the Unix / > Extended file specification changes and test them.
It's in: http://perl5.git.perl.org/perl.git/commit/cd5cc49
Here is the remaining patches needed for Unix and filenames with the Extended Character set. This superceeds my previous submissions. Thanks, -John
--- /rsync_root/perl/lib/Module/Build/Platform/VMS.pm Sat Feb 14 15:10:11 2009 +++ lib/Module/Build/Platform/VMS.pm Sun Feb 15 21:14:13 2009 @@ -229,8 +229,9 @@ =item rscan_dir -Inherit the standard version but remove dots at end of name. This may not be -necessary if File::Find has been fixed or DECC$FILENAME_UNIX_REPORT is in effect. +Inherit the standard version but remove dots at end of name. +If the extended character set is in effect, do not remove dots from filenames +with Unix path delimiters. =cut @@ -239,7 +240,11 @@ my $result = $self->SUPER::rscan_dir( $dir, $pattern ); - for my $file (@$result) { $file =~ s/\.$//; } + for my $file (@$result) { + if (!_efs() && ($file =~ m#/#)) { + $file =~ s/\.$//; + } + } return $result; } @@ -254,7 +259,7 @@ my $self = shift; my $dist_dir = $self->SUPER::dist_dir; - $dist_dir =~ s/\./_/g; + $dist_dir =~ s/\./_/g unless _efs(); return $dist_dir; } @@ -322,6 +327,11 @@ # break up the paths for the merge my $home = VMS::Filespec::unixify($ENV{HOME}); + # In the default VMS mode, the trailing slash is present. + # In Unix report mode it is not. The parsing logic assumes that + # it is present. + $home .= '/' unless $home =~ m#/$#; + # Trivial case of just ~ by it self if ($spec eq '') { $home =~ s#/$##; @@ -361,9 +371,8 @@ # Now put the two cases back together $arg = File::Spec::Unix->catpath($hvol, $newdirs, $file); - } else { - return $arg; } + return $arg; } @@ -376,7 +385,9 @@ =cut -sub find_perl_interpreter { return $^X; } +sub find_perl_interpreter { + return VMS::Filespec::vmsify($^X); +} =item localize_file_path @@ -386,8 +397,9 @@ sub localize_file_path { my ($self, $path) = @_; + $path = VMS::Filespec::vmsify($path); $path =~ s/\.\z//; - return VMS::Filespec::vmsify($path); + return $path; } =item localize_dir_path @@ -413,6 +425,43 @@ foreach my $item (map glob(VMS::Filespec::rmsexpand($_, '.;0')), $self->cleanup) { $self->delete_filetree($item); } +} + + +# Need to look up the feature settings. The preferred way is to use the +# VMS::Feature module, but that may not be available to dual life modules. + +my $use_feature; +BEGIN { + if (eval { local $SIG{__DIE__}; require VMS::Feature; }) { + $use_feature = 1; + } +} + +# Need to look up the UNIX report mode. This may become a dynamic mode +# in the future. +sub _unix_rpt { + my $unix_rpt; + if ($use_feature) { + $unix_rpt = VMS::Feature::current("filename_unix_report"); + } else { + my $env_unix_rpt = $ENV{'DECC$FILENAME_UNIX_REPORT'} || ''; + $unix_rpt = $env_unix_rpt =~ /^[ET1]/i; + } + return $unix_rpt; +} + +# Need to look up the EFS character set mode. This may become a dynamic +# mode in the future. +sub _efs { + my $efs; + if ($use_feature) { + $efs = VMS::Feature::current("efs_charset"); + } else { + my $env_efs = $ENV{'DECC$EFS_CHARSET'} || ''; + $efs = $env_efs =~ /^[ET1]/i; + } + return $efs; } =back --- /rsync_root/perl/lib/Module/Build/t/extend.t Tue Sep 30 06:29:35 2008 +++ lib/Module/Build/t/extend.t Fri Dec 26 16:24:44 2008 @@ -50,7 +50,9 @@ $mb->test_files('*t*'); my $files = $mb->test_files; ok grep {$_ eq 'script'} @$files; - ok grep {$_ eq File::Spec->catfile('t', 'basic.t')} @$files; + my $t_basic_t = File::Spec->catfile('t', 'basic.t'); + $t_basic_t = VMS::Filespec::vmsify($t_basic_t) if $^O eq 'VMS'; + ok grep {$_ eq $t_basic_t} @$files; ok !grep {$_ eq 'Build.PL' } @$files; # Make sure order is preserved --- /rsync_root/perl/lib/Module/Build/t/metadata.t Tue Sep 30 06:29:35 2008 +++ lib/Module/Build/t/metadata.t Fri Dec 26 16:31:48 2008 @@ -33,14 +33,24 @@ my $simple_file = 'lib/Simple.pm'; my $simple2_file = 'lib/Simple2.pm'; - #TODO: # Traditional VMS will return the file in in lower case, and is_deeply # does exact case comparisons. - # When ODS-5 support is active for preserved case file names, this will - # need to be changed. + # When ODS-5 support is active for preserved case file names we do not + # change the case. if ($^O eq 'VMS') { - $simple_file = lc($simple_file); - $simple2_file = lc($simple2_file); + my $lower_case_expect = 1; + my $vms_efs_case = 0; + if (eval 'require VMS::Feature') { + $vms_efs_case = VMS::Feature::current("efs_case_preserve"); + } else { + my $efs_case = $ENV{'DECC$EFS_CASE_PRESERVE'} || ''; + $vms_efs_case = $efs_case =~ /^[ET1]/i; + } + $lower_case_expect = 0 if $vms_efs_case; + if ($lower_case_expect) { + $simple_file = lc($simple_file); + $simple2_file = lc($simple2_file); + } } --- /rsync_root/perl/lib/Module/Build/t/runthrough.t Tue Sep 30 06:29:35 2008 +++ lib/Module/Build/t/runthrough.t Sat Dec 27 09:53:36 2008 @@ -73,11 +73,22 @@ my $dist_dir = 'Simple-0.01'; -# VMS may or may not need to modify the name, vmsify will do this if -# the name looks like a UNIX directory. +# VMS in traditional mode needs the $dist_dir name to not have a '.' in it +# as this is a directory delimiter. In extended character set mode the dot +# is permitted for Unix format file specifications. if ($^O eq 'VMS') { - my @dist_dirs = File::Spec->splitdir(VMS::Filespec::vmsify($dist_dir.'/')); - $dist_dir = $dist_dirs[0]; + my $Is_VMS_noefs = 1; + my $vms_efs = 0; + if (eval 'require VMS::Feature') { + $vms_efs = VMS::Feature::current("efs_charset"); + } else { + my $efs_charset = $ENV{'DECC$EFS_CHARSET'} || ''; + $vms_efs = $efs_charset =~ /^[ET1]/i; + } + $Is_VMS_noefs = 0 if $vms_efs; + if ($Is_VMS_noefs) { + $dist_dir = 'Simple-0_01'; + } } is $mb->dist_dir, $dist_dir;
This patch to Module/Build/t/install.t for VMS when the DECC$FILENAME_REPORT_UNIX feature is active.
Download m_b_t_install_t.gdiff
application/octet-stream 554b

Message body not shown because it is not plain text.

I've created and attached a consolidated version of the three previous patches and reformulated it against a recent pull from svn. All tests pass in my VMS environment. If there are objections to accepting this patch, please speak up. If not, it would be really nice to get it in and push the latest development snapshot of M::B into blead.
--- lib/Module/Build/Base.pm~ 2009-05-14 14:34:52 -0500 +++ lib/Module/Build/Base.pm 2009-05-14 14:34:52 -0500 @@ -2828,7 +2828,7 @@ sub htmlify_pods { my ($name, $path) = File::Basename::fileparse($pods->{$pod}, file_qr('\.(?:pm|plx?|pod)$')); my @dirs = File::Spec->splitdir( File::Spec->canonpath( $path ) ); - pop( @dirs ) if $dirs[-1] eq File::Spec->curdir; + pop( @dirs ) if scalar(@dirs) && $dirs[-1] eq File::Spec->curdir; my $fulldir = File::Spec->catfile($htmldir, @rootdirs, @dirs); my $outfile = File::Spec->catfile($fulldir, "${name}.html"); @@ -4096,7 +4096,7 @@ sub install_map { # Need to remove volume from $map{$_} using splitpath, or else # we'll create something crazy like C:\Foo\Bar\E:\Baz\Quux # VMS will always have the file separate than the path. - my ($volume, $path, $file) = File::Spec->splitpath( $map{$_}, 1 ); + my ($volume, $path, $file) = File::Spec->splitpath( $map{$_}, 0 ); # catdir needs a list of directories, or it will create something # crazy like volume:[Foo.Bar.volume.Baz.Quux] --- lib/Module/Build/Compat.pm~ 2009-05-14 14:34:52 -0500 +++ lib/Module/Build/Compat.pm 2009-05-14 14:34:52 -0500 @@ -282,7 +282,7 @@ sub fake_makefile { my $Build = 'Build' . $filetype . ' --makefile_env_macros 1'; my $unlink = $class->oneliner('1 while unlink $ARGV[0]', [], [$args{makefile}]); - $unlink =~ s/\$/\$\$/g; + $unlink =~ s/\$/\$\$/g unless $class->is_vmsish; my $maketext = <<"EOF"; all : force_do_it --- lib/Module/Build/Platform/VMS.pm;-0 2009-05-12 21:00:19 -0500 +++ lib/Module/Build/Platform/VMS.pm 2009-05-14 15:14:46 -0500 @@ -188,6 +188,21 @@ sub do_system { return !system("$cmd $args"); } +=item oneliner + +Override to ensure that we do not quote the command. + +=cut + +sub oneliner { + my $self = shift; + my $oneliner = $self->SUPER::oneliner(@_); + + $oneliner =~ s/^\"\S+\"//; + + return "MCR $^X $oneliner"; +} + =item _infer_xs_spec Inherit the standard version but tweak the library file name to be @@ -214,8 +229,9 @@ sub _infer_xs_spec { =item rscan_dir -Inherit the standard version but remove dots at end of name. This may not be -necessary if File::Find has been fixed or DECC$FILENAME_UNIX_REPORT is in effect. +Inherit the standard version but remove dots at end of name. +If the extended character set is in effect, do not remove dots from filenames +with Unix path delimiters. =cut @@ -224,7 +240,11 @@ sub rscan_dir { my $result = $self->SUPER::rscan_dir( $dir, $pattern ); - for my $file (@$result) { $file =~ s/\.$//; } + for my $file (@$result) { + if (!_efs() && ($file =~ m#/#)) { + $file =~ s/\.$//; + } + } return $result; } @@ -239,7 +259,7 @@ sub dist_dir { my $self = shift; my $dist_dir = $self->SUPER::dist_dir; - $dist_dir =~ s/\./_/g; + $dist_dir =~ s/\./_/g unless _efs(); return $dist_dir; } @@ -307,8 +327,14 @@ sub _detildefy { # break up the paths for the merge my $home = VMS::Filespec::unixify($ENV{HOME}); + # In the default VMS mode, the trailing slash is present. + # In Unix report mode it is not. The parsing logic assumes that + # it is present. + $home .= '/' unless $home =~ m#/$#; + # Trivial case of just ~ by it self if ($spec eq '') { + $home =~ s#/$##; return $home; } @@ -345,9 +371,8 @@ sub _detildefy { # Now put the two cases back together $arg = File::Spec::Unix->catpath($hvol, $newdirs, $file); - } else { - return $arg; } + return $arg; } @@ -360,7 +385,9 @@ lossy. =cut -sub find_perl_interpreter { return $^X; } +sub find_perl_interpreter { + return VMS::Filespec::vmsify($^X); +} =item localize_file_path @@ -370,8 +397,9 @@ Convert the file path to the local synta sub localize_file_path { my ($self, $path) = @_; + $path = VMS::Filespec::vmsify($path); $path =~ s/\.\z//; - return VMS::Filespec::vmsify($path); + return $path; } =item localize_dir_path @@ -385,6 +413,57 @@ sub localize_dir_path { return VMS::Filespec::vmspath($path); } +=item ACTION_clean + +The home-grown glob() expands a bit too aggressively when given a bare name, +so default in a zero-length extension. + +=cut + +sub ACTION_clean { + my ($self) = @_; + foreach my $item (map glob(VMS::Filespec::rmsexpand($_, '.;0')), $self->cleanup) { + $self->delete_filetree($item); + } +} + + +# Need to look up the feature settings. The preferred way is to use the +# VMS::Feature module, but that may not be available to dual life modules. + +my $use_feature; +BEGIN { + if (eval { local $SIG{__DIE__}; require VMS::Feature; }) { + $use_feature = 1; + } +} + +# Need to look up the UNIX report mode. This may become a dynamic mode +# in the future. +sub _unix_rpt { + my $unix_rpt; + if ($use_feature) { + $unix_rpt = VMS::Feature::current("filename_unix_report"); + } else { + my $env_unix_rpt = $ENV{'DECC$FILENAME_UNIX_REPORT'} || ''; + $unix_rpt = $env_unix_rpt =~ /^[ET1]/i; + } + return $unix_rpt; +} + +# Need to look up the EFS character set mode. This may become a dynamic +# mode in the future. +sub _efs { + my $efs; + if ($use_feature) { + $efs = VMS::Feature::current("efs_charset"); + } else { + my $env_efs = $ENV{'DECC$EFS_CHARSET'} || ''; + $efs = $env_efs =~ /^[ET1]/i; + } + return $efs; +} + =back =head1 AUTHOR --- t/extend.t;-0 2009-05-12 21:00:18 -0500 +++ t/extend.t 2009-05-14 15:14:46 -0500 @@ -50,7 +50,9 @@ print "Hello, World!\n"; $mb->test_files('*t*'); my $files = $mb->test_files; ok grep {$_ eq 'script'} @$files; - ok grep {$_ eq File::Spec->catfile('t', 'basic.t')} @$files; + my $t_basic_t = File::Spec->catfile('t', 'basic.t'); + $t_basic_t = VMS::Filespec::vmsify($t_basic_t) if $^O eq 'VMS'; + ok grep {$_ eq $t_basic_t} @$files; ok !grep {$_ eq 'Build.PL' } @$files; # Make sure order is preserved --- t/metadata.t;-0 2009-05-12 21:00:18 -0500 +++ t/metadata.t 2009-05-14 15:14:46 -0500 @@ -33,14 +33,24 @@ $dist->regen; my $simple_file = 'lib/Simple.pm'; my $simple2_file = 'lib/Simple2.pm'; - #TODO: # Traditional VMS will return the file in in lower case, and is_deeply # does exact case comparisons. - # When ODS-5 support is active for preserved case file names, this will - # need to be changed. + # When ODS-5 support is active for preserved case file names we do not + # change the case. if ($^O eq 'VMS') { - $simple_file = lc($simple_file); - $simple2_file = lc($simple2_file); + my $lower_case_expect = 1; + my $vms_efs_case = 0; + if (eval 'require VMS::Feature') { + $vms_efs_case = VMS::Feature::current("efs_case_preserve"); + } else { + my $efs_case = $ENV{'DECC$EFS_CASE_PRESERVE'} || ''; + $vms_efs_case = $efs_case =~ /^[ET1]/i; + } + $lower_case_expect = 0 if $vms_efs_case; + if ($lower_case_expect) { + $simple_file = lc($simple_file); + $simple2_file = lc($simple2_file); + } } --- t/runthrough.t;-0 2009-05-12 21:00:18 -0500 +++ t/runthrough.t 2009-05-14 15:14:47 -0500 @@ -73,11 +73,22 @@ ok -e $mb->build_script; my $dist_dir = 'Simple-0.01'; -# VMS may or may not need to modify the name, vmsify will do this if -# the name looks like a UNIX directory. +# VMS in traditional mode needs the $dist_dir name to not have a '.' in it +# as this is a directory delimiter. In extended character set mode the dot +# is permitted for Unix format file specifications. if ($^O eq 'VMS') { - my @dist_dirs = File::Spec->splitdir(VMS::Filespec::vmsify($dist_dir.'/')); - $dist_dir = $dist_dirs[0]; + my $Is_VMS_noefs = 1; + my $vms_efs = 0; + if (eval 'require VMS::Feature') { + $vms_efs = VMS::Feature::current("efs_charset"); + } else { + my $efs_charset = $ENV{'DECC$EFS_CHARSET'} || ''; + $vms_efs = $efs_charset =~ /^[ET1]/i; + } + $Is_VMS_noefs = 0 if $vms_efs; + if ($Is_VMS_noefs) { + $dist_dir = 'Simple-0_01'; + } } is $mb->dist_dir, $dist_dir; --- t/compat.t;-0 2009-05-12 21:00:18 -0500 +++ t/compat.t 2009-05-14 17:45:12 -0500 @@ -235,7 +235,7 @@ ok $mb, "Module::Build->new_from_context ok $ran_ok, "make fakeinstall with INSTALLDIRS=vendor ran ok"; $output =~ s/^/# /gm; # Don't confuse our own test output like $output, - qr/\Q$libdir2\E .* Simple\.pm/x, + qr/\Q$libdir2\E .* Simple\.pm/ix, 'Should have installdirs=vendor'; }
Sorry it took so long. Thank you for reformulating the patch. Applied in trunk as revision 12770. -- David