perl5 icon indicating copy to clipboard operation
perl5 copied to clipboard

File::Copy::copy does not set $! on error due to identical files

Open vinc17fr opened this issue 1 year ago • 10 comments

Module: File::Copy

Description When File::Copy::copy returns 0 (error) due to identical files, it does not set $!.

Steps to Reproduce perl -MFile::Copy -e '$! = 0; File::Copy::copy("/dev/null","/dev/null") or die "Copy failed: $! (\$! = ".int($!).")";' which gives:

'/dev/null' and '/dev/null' are identical (not copied) at -e line 1.
Copy failed:  ($! = 0) at -e line 1.

Expected behavior $! should be different from 0 (perhaps 1) as the module documents:

All functions return 1 on success, 0 on failure. $! will be set if an error was encountered.

The bug is still present in https://github.com/Perl/perl5/blob/1edc2b4ee3077fc72d6364edfe0281ea10aab252/lib/File/Copy.pm#L91-L94 (there are 3 occurrences of the issue).

Note: I found that this issue was mentioned in latexmk 4.85.

vinc17fr avatar Apr 15 '24 13:04 vinc17fr

FWIW, this behavior has been in place since 754f2cd0b96 back in 2005. See the discussion in https://github.com/Perl/perl5/issues/8009 (which started off life as rt.perl.org#36507).

jkeenan avatar Apr 15 '24 16:04 jkeenan

Well, this commit had a return 1 in one case and a return 0 in the other case, while the current code has return 0 (meaning a failure) in the 3 cases. In #8009, there was no mention of $! (I suspect that it was completely forgotten).

vinc17fr avatar Apr 15 '24 17:04 vinc17fr

$! should be different from 0 (perhaps 1)

$! isn't just a random variable. It is an interface to the system's errno and strerror(errno). Blindly setting it to some number doesn't sound like a great idea. We'd have to find an error code that matches this case somewhat.

My potential candidates would be EEXIST ("File exists"), EPERM ("Operation not permitted"), or the generic EINVAL ("Invalid argument"). Of these, EPERM is probably the least appropriate (our case is not a permission issue), but EEXIST seems strangely compelling (POSIX description: "An existing file was mentioned in an inappropriate context").

mauke avatar Apr 15 '24 17:04 mauke

From this snippit:

if (_eq($from, $to)) { # works for references, too carp("'$from' and '$to' are identical (not copied)"); return 0; }

The issue here is that while the code correctly identifies identical files and returns 0, it doesn't set $! to indicate the specific error condition. By setting it to an appropriate error code like EPERM for example, you can confirm that the error condition is properly communicated to the caller.

Jake-perl avatar Apr 15 '24 17:04 Jake-perl

I initially thought that EPERM could be used to say that the operation is not permitted by the API. But POSIX actually clarifies this as "An attempt was made to perform an operation limited to processes with appropriate privileges or to the owner of a file or other resource." However, EINVAL could be OK; here, this is actually a pair of arguments that is invalid.

vinc17fr avatar Apr 15 '24 18:04 vinc17fr

What would the caller do in this situation? How would one recover from (a correctly signalled) "I didn't copy it because it's already there"?

It's almost like asking for copy at the same place as the original is nonsense which makes it sound very EINVALy

guest20 avatar Apr 16 '24 23:04 guest20

It depends on the caller. For latexmk, "it's a non-error", thus the current version does nothing special. That said, when an error is detected with copy, it just outputs a warning and goes on. So, what would change with $! being non-zero is that an additional warning would be displayed with the error message associated with it.

vinc17fr avatar Apr 17 '24 00:04 vinc17fr

How would one recover from (a correctly signalled) "I didn't copy it because it's already there"?

Interestingly (though perhaps of little importance) there's no guarantee that "it's already there":

> perl -MFile::Copy -e "File::Copy::copy('non-existent-file','non-existent-file') or die 'Copy failed:';"
'non-existent-file' and 'non-existent-file' are identical (not copied) at -e line 1.
Copy failed: at -e line 1.

sisyphus avatar Apr 17 '24 01:04 sisyphus

I would regard this as a bug, because this is inconsistent with the behavior on equivalent forms (such as ./foo and foo). If the file exists, these equivalent forms are correctly detected as being the same file, but not if the file does not exist:

qaa% touch foo
qaa% perl -MFile::Copy -e "File::Copy::copy('./foo','foo') or die"
'./foo' and 'foo' are identical (not copied) at -e line 1.
Died at -e line 1.
qaa% rm foo
qaa% perl -MFile::Copy -e "File::Copy::copy('./foo','foo') or die"
Died at -e line 1.

vinc17fr avatar Apr 17 '24 01:04 vinc17fr

If you ever look too close at a bug you'll see there are other bugs hiding behind it

guest20 avatar Apr 17 '24 07:04 guest20