[PATCH] Porting Cwd.pm's patch in https://rt.cpan.org/Ticket/Display.html?id=39260 to bleadperl #13448

p5pRT · 2013-11-29T10:03:55Z

Migrated from rt.perl.org#120645 (status was 'open')

Searchable as RT120645$

p5pRT · 2013-11-29T10:03:56Z

From @shlomif

Hi all,

Attached is a patch for Cwd.pm’s
bug at https://rt.cpan.org/Ticket/Display.html?id=39260 :

[QUOTE]
As can be seen here:

http://www.nntp.perl.org/group/perl.cpan.testers/2008/09/msg2204098.html

There's an incompatibility between the XS abs_path() and _perl_abs_path(): if
one references a non-existent filename of an existing directory, then the XS
abs_path will return a correct results and _perl_abs_path will warn and return
nothing. This patch adds a testcase for the problem, and patches Cwd.pm to fix
it.
[/QUOTE]

There is a similar patch provided for the standalone Cwd.pm itself, but it
wasn't applied since its submission in 2008.

Thanks to Jim Keenan for his advice for sending this patch to perlbug.

Regards,

Shlomi Fish

--

Shlomi Fish http://www.shlomifish.org/
"Humanity" - Parody of Modern Life - http://shlom.in/humanity

Larry Wall has been changing the world. By modifying its very source code.

Please reply to list if it's a mailing list post - http://shlom.in/reply .

p5pRT · 2013-11-29T10:03:56Z

From @shlomif

bleadperl--non-exist-file.patch

diff --git a/dist/Cwd/Cwd.pm b/dist/Cwd/Cwd.pm
index d9de63c..baeefe1 100644
--- a/dist/Cwd/Cwd.pm
+++ b/dist/Cwd/Cwd.pm
@@ -171,7 +171,7 @@ use strict;
 use Exporter;
 use vars qw(@ISA @EXPORT @EXPORT_OK $VERSION);
 
-$VERSION = '3.45';
+$VERSION = '3.46';
 my $xs_version = $VERSION;
 $VERSION =~ tr/_//;
 
@@ -541,19 +541,26 @@ sub chdir {
 sub _perl_abs_path
 {
     my $start = @_ ? shift : '.';
-    my($dotdots, $cwd, @pst, @cst, $dir, @tst);
+    my($dotdots, $cwd, @pst, @cst, $dir, @tst, $file);
 
     unless (@cst = stat( $start ))
     {
-	_carp("stat($start): $!");
+	my $err = $!;
+	# Check for a non-existent file inside an existing directory.
+	if (($dir, $file) = $start =~ m{^(.*)/(.+)$}) {
+	    if (-d $dir) {
+		return abs_path($dir) . "/$file";
+	    }
+	}
+	_carp("stat($start): $err");
 	return '';
     }
 
     unless (-d _) {
         # Make sure we can be invoked on plain files, not just directories.
         # NOTE that this routine assumes that '/' is the only directory separator.
-	
-        my ($dir, $file) = $start =~ m{^(.*)/(.+)$}
+
+        ($dir, $file) = $start =~ m{^(.*)/(.+)$}
 	    or return cwd() . '/' . $start;
 	
 	# Can't use "-l _" here, because the previous stat was a stat(), not an lstat().
diff --git a/dist/Cwd/lib/File/Spec.pm b/dist/Cwd/lib/File/Spec.pm
index 59a6ce6..7bcd08d 100644
--- a/dist/Cwd/lib/File/Spec.pm
+++ b/dist/Cwd/lib/File/Spec.pm
@@ -3,7 +3,7 @@ package File::Spec;
 use strict;
 use vars qw(@ISA $VERSION);
 
-$VERSION = '3.45';
+$VERSION = '3.46';
 $VERSION =~ tr/_//;
 
 my %module = (MacOS   => 'Mac',
diff --git a/dist/Cwd/lib/File/Spec/Cygwin.pm b/dist/Cwd/lib/File/Spec/Cygwin.pm
index c646382..d8ceaac 100644
--- a/dist/Cwd/lib/File/Spec/Cygwin.pm
+++ b/dist/Cwd/lib/File/Spec/Cygwin.pm
@@ -4,7 +4,7 @@ use strict;
 use vars qw(@ISA $VERSION);
 require File::Spec::Unix;
 
-$VERSION = '3.45';
+$VERSION = '3.46';
 $VERSION =~ tr/_//;
 
 @ISA = qw(File::Spec::Unix);
diff --git a/dist/Cwd/lib/File/Spec/Epoc.pm b/dist/Cwd/lib/File/Spec/Epoc.pm
index e7c95bb..e5ac358 100644
--- a/dist/Cwd/lib/File/Spec/Epoc.pm
+++ b/dist/Cwd/lib/File/Spec/Epoc.pm
@@ -3,7 +3,7 @@ package File::Spec::Epoc;
 use strict;
 use vars qw($VERSION @ISA);
 
-$VERSION = '3.45';
+$VERSION = '3.46';
 $VERSION =~ tr/_//;
 
 require File::Spec::Unix;
diff --git a/dist/Cwd/lib/File/Spec/Functions.pm b/dist/Cwd/lib/File/Spec/Functions.pm
index af039e7..2bb3218 100644
--- a/dist/Cwd/lib/File/Spec/Functions.pm
+++ b/dist/Cwd/lib/File/Spec/Functions.pm
@@ -5,7 +5,7 @@ use strict;
 
 use vars qw(@ISA @EXPORT @EXPORT_OK %EXPORT_TAGS $VERSION);
 
-$VERSION = '3.45';
+$VERSION = '3.46';
 $VERSION =~ tr/_//;
 
 require Exporter;
diff --git a/dist/Cwd/lib/File/Spec/Mac.pm b/dist/Cwd/lib/File/Spec/Mac.pm
index 7a5b41e..117de58 100644
--- a/dist/Cwd/lib/File/Spec/Mac.pm
+++ b/dist/Cwd/lib/File/Spec/Mac.pm
@@ -4,7 +4,7 @@ use strict;
 use vars qw(@ISA $VERSION);
 require File::Spec::Unix;
 
-$VERSION = '3.45';
+$VERSION = '3.46';
 $VERSION =~ tr/_//;
 
 @ISA = qw(File::Spec::Unix);
diff --git a/dist/Cwd/lib/File/Spec/OS2.pm b/dist/Cwd/lib/File/Spec/OS2.pm
index ba434e8..b619cdf 100644
--- a/dist/Cwd/lib/File/Spec/OS2.pm
+++ b/dist/Cwd/lib/File/Spec/OS2.pm
@@ -4,7 +4,7 @@ use strict;
 use vars qw(@ISA $VERSION);
 require File::Spec::Unix;
 
-$VERSION = '3.45';
+$VERSION = '3.46';
 $VERSION =~ tr/_//;
 
 @ISA = qw(File::Spec::Unix);
diff --git a/dist/Cwd/lib/File/Spec/Unix.pm b/dist/Cwd/lib/File/Spec/Unix.pm
index 868b6a7..1b28e5d 100644
--- a/dist/Cwd/lib/File/Spec/Unix.pm
+++ b/dist/Cwd/lib/File/Spec/Unix.pm
@@ -3,7 +3,7 @@ package File::Spec::Unix;
 use strict;
 use vars qw($VERSION);
 
-$VERSION = '3.45';
+$VERSION = '3.46';
 my $xs_version = $VERSION;
 $VERSION =~ tr/_//;
 
diff --git a/dist/Cwd/lib/File/Spec/VMS.pm b/dist/Cwd/lib/File/Spec/VMS.pm
index aae0bfc..431546a 100644
--- a/dist/Cwd/lib/File/Spec/VMS.pm
+++ b/dist/Cwd/lib/File/Spec/VMS.pm
@@ -4,7 +4,7 @@ use strict;
 use vars qw(@ISA $VERSION);
 require File::Spec::Unix;
 
-$VERSION = '3.45';
+$VERSION = '3.46';
 $VERSION =~ tr/_//;
 
 @ISA = qw(File::Spec::Unix);
diff --git a/dist/Cwd/lib/File/Spec/Win32.pm b/dist/Cwd/lib/File/Spec/Win32.pm
index 4c16586..ddb37c7 100644
--- a/dist/Cwd/lib/File/Spec/Win32.pm
+++ b/dist/Cwd/lib/File/Spec/Win32.pm
@@ -5,7 +5,7 @@ use strict;
 use vars qw(@ISA $VERSION);
 require File::Spec::Unix;
 
-$VERSION = '3.45';
+$VERSION = '3.46';
 $VERSION =~ tr/_//;
 
 @ISA = qw(File::Spec::Unix);
diff --git a/dist/Cwd/t/cwd.t b/dist/Cwd/t/cwd.t
index 62426ff..4369622 100644
--- a/dist/Cwd/t/cwd.t
+++ b/dist/Cwd/t/cwd.t
@@ -36,11 +36,11 @@ if ($IsVMS) {
     $vms_mode = 0 if ($vms_unix_rpt);
 }
 
-my $tests = 31;
+my $tests = 32;
 # _perl_abs_path() currently only works when the directory separator
 # is '/', so don't test it when it won't work.
 my $EXTRA_ABSPATH_TESTS = ($Config{prefix} =~ m/\//) && $^O ne 'cygwin';
-$tests += 4 if $EXTRA_ABSPATH_TESTS;
+$tests += 5 if $EXTRA_ABSPATH_TESTS;
 plan tests => $tests;
 
 SKIP: {
@@ -225,7 +225,27 @@ path_ends_with(Cwd::fast_abs_path($path), 'cwd.t', 'fast_abs_path() can be invok
 path_ends_with(Cwd::_perl_abs_path($path), 'cwd.t', '_perl_abs_path() can be invoked on a file')
   if $EXTRA_ABSPATH_TESTS;
 
+{
+    mkdir("existing-dir", 0777);
+    my $path = File::Spec->catfile(
+        File::Spec->curdir(), 'existing-dir', "non-existent-file.txt"
+    );
+    path_ends_with(
+        Cwd::abs_path($path),
+        'non-existent-file.txt',
+        'abs_path() can be invoked on a non-exist file in an existing dir'
+    );
+    if ($EXTRA_ABSPATH_TESTS)
+    {
+        path_ends_with(
+            Cwd::_perl_abs_path($path),
+            'non-existent-file.txt',
+            '_perl_abs_path() can be invoked on a non-exist file in an existing dir'
+        );
+    }
 
+    rmdir("existing-dir");
+}
   
 SKIP: {
   my $file;

p5pRT · 2013-12-02T00:01:34Z

From @tonycoz

On Fri Nov 29 02:03:56 2013, shlomif@shlomifish.org wrote:

Hi all,

Attached is a patch for Cwd.pm’s
bug at https://rt.cpan.org/Ticket/Display.html?id=39260 :

[QUOTE]
As can be seen here:

http://www.nntp.perl.org/group/perl.cpan.testers/2008/09/msg2204098.html

There's an incompatibility between the XS abs_path() and
_perl_abs_path(): if
one references a non-existent filename of an existing directory, then
the XS
abs_path will return a correct results and _perl_abs_path will warn
and return
nothing. This patch adds a testcase for the problem, and patches
Cwd.pm to fix
it.
[/QUOTE]

There is a similar patch provided for the standalone Cwd.pm itself,
but it
wasn't applied since its submission in 2008.

Thanks to Jim Keenan for his advice for sending this patch to perlbug.

Ideally the patch should be produced with git format-patch not git diff.

Unfortunately this code isn't portable and fails on Win32, probably because abs_path() is implemented with fast_abs_path() there (and on a few other platforms)

Tony

ok 27 - abs_path() can be invoked on a file
ok 28 - fast_abs_path() can be invoked on a file
existing-dir\non-existent-file.txt: No such file or directory at t/cwd.t line 23
3.
# Looks like you planned 32 tests but ran 28.
# Looks like your test exited with 2 just after 28.
Dubious, test returned 2 (wstat 512, 0x200)
Failed 4/32 subtests
(less 3 skipped subtests: 25 okay)

Test Summary Report

../dist/Cwd/t/cwd.t (Wstat: 512 Tests: 28 Failed: 0)
Non-zero exit status: 2
Parse errors: Bad plan. You planned 32 tests but ran 28.
Files=1, Tests=28, 0 wallclock secs ( 0.02 usr + 0.00 sys = 0.02 CPU)
Result: FAIL

p5pRT · 2013-12-02T00:01:34Z

The RT System itself - Status changed from 'new' to 'open'

p5pRT · 2013-12-03T11:46:43Z

From @shlomif

On Sun, 1 Dec 2013 16:01:34 -0800
"Tony Cook via RT" <perlbug-followup@perl.org> wrote:

On Fri Nov 29 02:03:56 2013, shlomif@shlomifish.org wrote:

Hi all,

Attached is a patch for Cwd.pm’s
bug at https://rt.cpan.org/Ticket/Display.html?id=39260 :

[QUOTE]
As can be seen here:

http://www.nntp.perl.org/group/perl.cpan.testers/2008/09/msg2204098.html

There's an incompatibility between the XS abs_path() and
_perl_abs_path(): if
one references a non-existent filename of an existing directory, then
the XS
abs_path will return a correct results and _perl_abs_path will warn
and return
nothing. This patch adds a testcase for the problem, and patches
Cwd.pm to fix
it.
[/QUOTE]

There is a similar patch provided for the standalone Cwd.pm itself,
but it
wasn't applied since its submission in 2008.

Thanks to Jim Keenan for his advice for sending this patch to perlbug.

Ideally the patch should be produced with git format-patch not git diff.

Unfortunately this code isn't portable and fails on Win32, probably because
abs_path() is implemented with fast_abs_path() there (and on a few other
platforms)

I see. Thanks for the investigation. Should we implement a fix for it in
fast_abs_path() as well, or leave it as a known inconsistency?

Regards,

Shlomi Fish

--

Shlomi Fish http://www.shlomifish.org/
Parody of "The Fountainhead" - http://shlom.in/towtf

For all you know, you may not exist, and Chuck Norris convinced you that you
do.
— http://www.shlomifish.org/humour/bits/facts/Chuck-Norris/

Please reply to list if it's a mailing list post - http://shlom.in/reply .

p5pRT · 2013-12-05T04:43:32Z

From @tonycoz

On Tue Dec 03 03:46:43 2013, shlomif@shlomifish.org wrote:

On Sun, 1 Dec 2013 16:01:34 -0800
"Tony Cook via RT" <perlbug-followup@perl.org> wrote:

Ideally the patch should be produced with git format-patch not git
diff.

Unfortunately this code isn't portable and fails on Win32, probably
because
abs_path() is implemented with fast_abs_path() there (and on a few
other
platforms)

I see. Thanks for the investigation. Should we implement a fix for it
in
fast_abs_path() as well, or leave it as a known inconsistency?

Fixing it only for POSIX platforms doesn't remove the incompatibily you described in your first post in a general way.

I think it either needs:

a) to be fixed in fast_abs_path(),

b) a _slow_abs_path() is introduced that becomes abs_path() on non-POSIX, or

c) the documentation needs to indicate that abs_path() doesn't work portably on paths that don't exist. And the test needs to handle platforms that don't support it.

Tony

p5pRT added hasPatch type-library labels Oct 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PATCH] Porting Cwd.pm's patch in https://rt.cpan.org/Ticket/Display.html?id=39260 to bleadperl #13448

[PATCH] Porting Cwd.pm's patch in https://rt.cpan.org/Ticket/Display.html?id=39260 to bleadperl #13448

p5pRT commented Nov 29, 2013

p5pRT commented Nov 29, 2013

p5pRT commented Nov 29, 2013

p5pRT commented Dec 2, 2013

p5pRT commented Dec 2, 2013

p5pRT commented Dec 3, 2013

p5pRT commented Dec 5, 2013

[PATCH] Porting Cwd.pm's patch in https://rt.cpan.org/Ticket/Display.html?id=39260 to bleadperl #13448

[PATCH] Porting Cwd.pm's patch in https://rt.cpan.org/Ticket/Display.html?id=39260 to bleadperl #13448

Comments

p5pRT commented Nov 29, 2013

p5pRT commented Nov 29, 2013

From @shlomif

p5pRT commented Nov 29, 2013

From @shlomif

p5pRT commented Dec 2, 2013

From @tonycoz

p5pRT commented Dec 2, 2013

p5pRT commented Dec 3, 2013

From @shlomif

p5pRT commented Dec 5, 2013

From @tonycoz