diff --git a/Changes b/Changes index 8fbb9dd..ad5f337 100644 --- a/Changes +++ b/Changes @@ -3,6 +3,10 @@ Revision history for {{$dist->name}} {{$NEXT}} - Fix test failures under WSL2 by filtering Windows paths from PATH (gh#31, gh#32) + - win32_space_be_gone returns the original path unchanged if no short + (8+3) name without spaces is available (gh#13, gh#28, gh#33) + - Test suite no longer relies on short (8+3) names on Windows, and + quotes paths with spaces instead (gh#13, gh#28, gh#33) 0.35 2026-09-04 09:54:50 -0600 - Add unset method to remove environment variables (gh#14, gh#27, gh#30) diff --git a/lib/Shell/Config/Generate.pm b/lib/Shell/Config/Generate.pm index f7f6826..78971b0 100644 --- a/lib/Shell/Config/Generate.pm +++ b/lib/Shell/Config/Generate.pm @@ -835,14 +835,20 @@ our @EXPORT_OK = qw( win32_space_be_gone cmd_escape_path powershell_escape_path On C and C: Given a list of directory paths (or filenames), this will -return an equivalent list of paths pointing to the same +attempt to return an equivalent list of paths pointing to the same file system objects without spaces. To do this C is used on to find alternative path names without spaces. -NOTE that this breaks when Windows is told not to create -short (C<8+3>) filenames; see L -for a discussion of this behaviour. +B that short (C<8+3>) filenames are not created by default on +newer versions of Windows, and can be disabled on older ones; see +L for a discussion of this +behaviour. If no short name without spaces is available for a path, +then the original path is returned unchanged, so you should not rely +on the result being free of spaces. The configurations generated by +this module quote values as needed, and L can be given an +array reference to preserve spaces in commands, so this function +should not usually be needed. In addition, on just C: @@ -862,7 +868,15 @@ Returns the same list passed into it sub win32_space_be_gone { return @_ if $^O !~ /^(MSWin32|cygwin|msys)$/; - map { /\s/ ? _win_to_posix_path(Win32::GetShortPathName(_posix_to_win_path($_))) : $_ } @_; + map { + my $path = $_; + if($path =~ /\s/) + { + my $short = Win32::GetShortPathName(_posix_to_win_path($path)); + $path = _win_to_posix_path($short) if defined $short && $short !~ /\s/; + } + $path; + } @_; } =head2 cmd_escape_path diff --git a/t/lib/TestLib.pm b/t/lib/TestLib.pm index 9dac33a..13d402e 100644 --- a/t/lib/TestLib.pm +++ b/t/lib/TestLib.pm @@ -5,7 +5,7 @@ use warnings; use File::Spec; use File::Temp; use Shell::Guess; -use Shell::Config::Generate qw( cmd_escape_path powershell_escape_path ); +use Shell::Config::Generate qw( cmd_escape_path ); use Test2::API qw( context ); use Env qw( @PATH ); use TestPath; @@ -13,6 +13,27 @@ use base qw( Exporter ); our @EXPORT = qw( find_shell tempdir get_env bad_fish ); +# quote a word for the shell used by backticks / system +# (cmd.exe on MSWin32, /bin/sh elsewhere) +sub _q +{ + my $word = shift; + if($^O eq 'MSWin32') + { + return $word =~ /[\s&^|<>()]/ ? qq{"$word"} : $word; + } + $word =~ s/'/'"'"'/g; + "'$word'"; +} + +# quote a word for use inside a PowerShell script +sub _q_ps +{ + my $word = shift; + $word =~ s/'/''/g; + "'$word'"; +} + sub shell_is_okay { my($shell, $full_path) = @_; @@ -33,7 +54,8 @@ sub shell_is_okay if($shell =~ /^powershell.exe$/ && -e $full_path) { return 0 if $ENV{ACTIVESTATE_PPM_BUILD}; - `$full_path -ExecutionPolicy RemoteSigned -InputFormat none -NoProfile -File corpus\\true.ps1`; + my $ps1 = $^O eq 'MSWin32' ? 'corpus\\true.ps1' : 'corpus/true.ps1'; + `@{[ _q($full_path) ]} -ExecutionPolicy RemoteSigned -InputFormat none -NoProfile -File @{[ _q($ps1) ]}`; return $? == 0; } @@ -57,7 +79,6 @@ sub find_shell } my $dir = File::Temp::tempdir( CLEANUP => 1 ); -$dir = Win32::GetShortPathName($dir) if $^O eq 'MSWin32'; do { my $fn = File::Spec->catfile($dir, 'dump.pl'); @@ -136,15 +157,23 @@ sub get_env } else { - my $perl_exe = $^X; - $perl_exe = Cygwin::posix_to_win_path($perl_exe) + my @cmd = ($^X, File::Spec->catfile($dir, 'dump.pl')); + @cmd = map { Cygwin::posix_to_win_path($_) } @cmd if $^O =~ /^(cygwin|msys)$/ && $fn =~ /\.(bat|cmd|ps1)$/; - ($perl_exe) = cmd_escape_path($perl_exe) - if $^O =~ /^(MSWin32|cygwin|msys)$/ && $fn =~ /\.(bat|cmd)$/;; - ($perl_exe) = powershell_escape_path($perl_exe) - if $^O =~ /^(MSWin32|cygwin|msys)$/ && $fn =~ /\.ps1$/;; - print $fh "$perl_exe ", File::Spec->catfile($dir, 'dump.pl'), "\n"; - close $fn; + if($fn =~ /\.(bat|cmd)$/) + { + @cmd = map { cmd_escape_path($_) } @cmd; + } + elsif($fn =~ /\.ps1$/) + { + @cmd = ('&', map { _q_ps($_) } @cmd); + } + else + { + # bourne, csh and fish all understand single quotes + @cmd = map { s/'/'"'"'/g; "'$_'" } @cmd; + } + print $fh "@cmd\n"; } print $fh "exit\n" if $shell->is_power; }; @@ -157,11 +186,11 @@ sub get_env { if($shell->is_c) { - $output = `$shell_path -f $fn`; + $output = `@{[ _q($shell_path) ]} -f @{[ _q($fn) ]}`; } else { - $output = `$shell_path $fn`; + $output = `@{[ _q($shell_path) ]} @{[ _q($fn) ]}`; } } elsif($shell->is_power && $^O =~ /^(cygwin|MSWin32|msys)$/) @@ -169,11 +198,11 @@ sub get_env my $fn2 = $fn; $fn2 = Cygwin::posix_to_win_path($fn) if $^O =~ /^(cygwin|msys)$/; $fn2 =~ s{\\}{/}g; - $output = `$shell_path -ExecutionPolicy RemoteSigned -InputFormat none -NoProfile -File $fn2`; + $output = `@{[ _q($shell_path) ]} -ExecutionPolicy RemoteSigned -InputFormat none -NoProfile -File @{[ _q($fn2) ]}`; } else { - $output = `$fn`; + $output = `@{[ _q($fn) ]}`; } my $fail = $?; diff --git a/t/shell_config_generate__alias.t b/t/shell_config_generate__alias.t index 78dfb5d..3652deb 100644 --- a/t/shell_config_generate__alias.t +++ b/t/shell_config_generate__alias.t @@ -8,7 +8,6 @@ use TestLib; my $dir = tempdir(); my $perl_exe = $^X; -$perl_exe = Win32::GetShortPathName($perl_exe) if $^O eq 'MSWin32'; my $config = eval { Shell::Config::Generate->new }; isa_ok $config, 'Shell::Config::Generate'; @@ -28,6 +27,11 @@ do { eval { $config->set_alias("myecho1", "$perl_exe $script_name f00f") }; is $@, '', 'set_alias'; +# the string form of set_alias leaves quoting up to the caller, so +# paths with spaces can't be used here. The array form, which does +# quote each word, is tested with spaces in t/shell_config_generate__alias_array.t +my $has_space = grep /\s/, $perl_exe, $script_name; + foreach my $shell (qw( tcsh csh bsd-csh bash sh zsh cmd.exe command.com ksh 44bsd-csh jsh powershell.exe pwsh fish )) { subtest $shell => sub { @@ -36,6 +40,7 @@ foreach my $shell (qw( tcsh csh bsd-csh bash sh zsh cmd.exe command.com ksh 44bs my $guess = TestLib::get_guess($shell); note $config->generate($guess); skip_all "no $shell found" unless defined $shell_path; + skip_all "path to perl or test script contains a space" if $has_space; skip_all "not testing sh in case it doesn't support aliases" if $shell eq 'sh'; skip_all "alias may not work with non-interactive cmd.exe or command.com" if $shell eq 'cmd.exe' || $shell eq 'command.com'; @@ -61,6 +66,7 @@ subtest 'powershell.exe' => sub { note $config->generate($guess); skip_all "no powershell.exe found" unless defined $shell_path; + skip_all "path to perl or test script contains a space" if $has_space; my $list = get_env($config, $shell, $shell_path, 'myecho1 one two three'); return unless defined $list; diff --git a/t/shell_config_generate__alias_array.t b/t/shell_config_generate__alias_array.t index 5cc5891..d108c4c 100644 --- a/t/shell_config_generate__alias_array.t +++ b/t/shell_config_generate__alias_array.t @@ -8,7 +8,6 @@ use TestLib; my $dir = tempdir(); my $perl_exe = $^X; -$perl_exe = Win32::GetShortPathName($perl_exe) if $^O eq 'MSWin32'; my $config = eval { Shell::Config::Generate->new }; isa_ok $config, 'Shell::Config::Generate'; @@ -42,7 +41,7 @@ foreach my $shell (qw( tcsh csh bsd-csh bash sh zsh cmd.exe command.com ksh 44bs # protected on those shells. generate should refuse to produce # a broken alias and die with a helpful message instead. my $ret = eval { $config->generate($guess) }; - like $@, qr/word 'has space' contains a space/, 'generate dies with a helpful message'; + like $@, qr/word '[^']*' contains a space/, 'generate dies with a helpful message'; is $ret, U(), 'generate did not return a value'; return; } diff --git a/t/shell_config_generate__space_win32.t b/t/shell_config_generate__space_win32.t index b0d2ef3..ac30823 100644 --- a/t/shell_config_generate__space_win32.t +++ b/t/shell_config_generate__space_win32.t @@ -28,12 +28,26 @@ my($dir1, $file1) = win32_space_be_gone($dir,$file); ok -d $dir1, "dir exists $dir1"; ok -r $file1, "file readable $file1"; -unlike $dir1, qr{\s}, "dir has no spaces" - or diag "before: $dir\n", - "after: $dir1"; -unlike $file1, qr{\s}, "file has no spaces" - or diag "before: $file\n", - "after: $file1"; +note "before: $dir"; +note "after: $dir1"; +note "before: $file"; +note "after: $file1"; + +# short (8+3) names are not created by default on newer versions of +# Windows, in which case the original paths should be returned as-is. +if($dir1 =~ /\s/ || $file1 =~ /\s/) +{ + note "short names do not appear to be available"; + is $dir1, $dir, "dir unchanged"; + is $file1, $file, "file unchanged"; +} +else +{ + pass "dir has no spaces"; + pass "file has no spaces"; +} + +is [win32_space_be_gone 'foo', 'bar baz'], ['foo', 'bar baz'], "non-existent path with space is returned unchanged"; my $content = do { open my $fh, '<', $file1;