Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
vinc17fr opened this issue Apr 15, 2024 · 10 comments
Open

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

vinc17fr opened this issue Apr 15, 2024 · 10 comments

Comments

@vinc17fr
Copy link

vinc17fr commented Apr 15, 2024

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

perl5/lib/File/Copy.pm

Lines 91 to 94 in 1edc2b4

if (_eq($from, $to)) { # works for references, too
carp("'$from' and '$to' are identical (not copied)");
return 0;
}
(there are 3 occurrences of the issue).

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

@jkeenan
Copy link
Contributor

jkeenan commented Apr 15, 2024

FWIW, this behavior has been in place since 754f2cd back in 2005. See the discussion in #8009 (which started off life as rt.perl.org#36507).

@vinc17fr
Copy link
Author

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).

@mauke
Copy link
Contributor

mauke commented Apr 15, 2024

$! 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").

@Jake-perl
Copy link

Jake-perl commented Apr 15, 2024

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.

@vinc17fr
Copy link
Author

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.

@guest20
Copy link

guest20 commented Apr 16, 2024

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

@vinc17fr
Copy link
Author

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.

@sisyphus
Copy link
Contributor

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.

@vinc17fr
Copy link
Author

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.

@guest20
Copy link

guest20 commented Apr 17, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants