distinct error type if shutil.copyfile() fails because of src and dst are the same file

[zooko]

BPO	1492704
Nosy	@birkenfeld, @atsuoishimoto, @pitrou, @tarekziade, @merwok, @hynek
Files	patch.txt 1492704.diff new_patch.diff issue1492704_new.patch issue1492704_new_2.patch issue1492704_new_3.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2012-10-07.10:53:26.847>
created_at = <Date 2006-05-22.03:08:36.000>
labels = ['easy', 'type-feature', 'library']
title = 'distinct error type if shutil.copyfile() fails because of src and dst are the same file'
updated_at = <Date 2014-03-10.22:11:27.999>
user = 'https://bugs.python.org/zooko'

bugs.python.org fields:

activity = <Date 2014-03-10.22:11:27.999>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2012-10-07.10:53:26.847>
closer = 'python-dev'
components = ['Library (Lib)']
creation = <Date 2006-05-22.03:08:36.000>
creator = 'zooko'
dependencies = []
files = ['7279', '22874', '23115', '26394', '26401', '26406']
hgrepos = []
issue_num = 1492704
keywords = ['patch', 'easy', 'needs review']
message_count = 42.0
messages = ['50324', '50325', '50326', '50327', '50328', '114664', '114723', '141825', '141864', '142473', '143688', '143690', '143724', '143774', '143781', '165586', '165599', '165604', '165606', '165607', '165608', '165609', '165610', '165611', '165629', '165635', '165638', '165639', '165663', '165847', '165848', '165856', '165863', '165868', '165871', '165872', '172290', '173943', '174047', '174074', '174103', '213105']
nosy_count = 12.0
nosy_names = ['nnorwitz', 'anthonybaxter', 'georg.brandl', 'zooko', 'ishimoto', 'pitrou', 'tarek', 'eric.araujo', 'BreamoreBoy', 'python-dev', 'gennad', 'hynek']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue1492704'
versions = ['Python 3.4']

[zooko]

I need to call shutil.move() and be able to tell the
difference between an error such as access denied and
an error due to the two arguments being the same file.

--- old-dw/src/Lib/shutil.py 2006-05-22
00:06:02.000000000 -0300
+++ new-dw/src/Lib/shutil.py 2006-05-22
00:06:02.000000000 -0300
@@ -16,6 +16,9 @@
class Error(exceptions.EnvironmentError):
pass

+class SameFileError(Error):
+    pass
+
 def copyfileobj(fsrc, fdst, length=16*1024):
     """copy data from file-like object fsrc to
file-like object fdst"""
     while 1:
@@ -39,7 +42,7 @@
 def copyfile(src, dst):
     """Copy data from src to dst"""
     if _samefile(src, dst):
-        raise Error, "`%s` and `%s` are the same file"
% (src, dst)
+        raise SameFileError, "`%s` and `%s` are the
same file" % (src, dst)

     fsrc = None
     fdst = None

[zooko]

Logged In: YES
user_id=52562

Please apply. This patch is completely backwards-compatible
and makes possible some uses of shutil.move().

[zooko]

Logged In: YES
user_id=52562

I'm not sure how to draw attention to my patch, so I will
try assigning it to anthonybaxter. That ought to get attention.

[zooko]

Dear Pythonistas: please apply this patch. There is no reason not to, and it enables programmers to use shutil more cleanly.

[nnorwitz]

In order for patches to be applied, they require tests and doc updates as appropriate. Can you supply these updates and attach a patch with all the changes?

[BreamoreBoy]

@zooko are you interested in taking this forward?

[merwok]

Antoine, is this obsoleted by PEP-3151?

[merwok]

I have re-read PEP-3151 and think it has no bearing on this bug: the PEP is about adding exception classes that map to errno values, and this report is about a library function that returns a custom exception unrelated to errnos.

I’m willing to review a patch with tests and docs updates for this, or write one myself.

[gennad]

This patch should fix the issue

[merwok]

Thanks for the patch. I made a review on Rietveld; a mail should have been sent.

[gennad]

Thanks for the comments! Here'a a new patch.

[merwok]

Thanks, looks good. There are a few minor cosmetic things I’ll change before committing. I assume you have tested it on Windows?

[gennad]

My fault. I tested it only partially, relying on the documentation that says

"""
On Windows, if dst already exists, OSError will be raised even if it is a file
"""
Actually it does not (at least at my Windows 7):
(Pdb) os.path.isfile(src)
True
(Pdb) p os.rename(src, src)
None

Am I doing something wrong?

[merwok]

By testing, I mean running ./python -m test test_shutil

[atsuoishimoto]

Well, I happy to improve patch.

But, on Linux and Windows, shutil.move() does not raise any exception if source and destination are identical. If we change the behavior, I'm afraid we would break a lot of existing applications.

[hynek]

Sorry, I didn’t look at zooko’s patch which was also just about copyfile. I presume it’s all about the copy fallback that happens when os.rename didn’t work out.

Will look at your code more closely later.

[atsuoishimoto]

Ooops, shutil.move() will raise SameFileError if destination is directory. I'll investigate the patch further more.

[atsuoishimoto]

Patch updated.

• SameFileError is now derived from EnvironmentError.
• Fixed documentation.
• Fixed test method name.

I investigated this patch:

• shutil.copyfile() and shutil.copy() raises SameFileError if source and destination are same file.

• shutil.move() never raises SameFileError. shutil.move() may raise exception if source and dest are same file, but it depends on underlying implementation of platform. Linux and Windows don't raise exception in this case.

• I am opposed to change shutil.move() to raise SameFileError, because such incompatible change can break existing applilcations.

[hynek]

• SameFileError is now derived from EnvironmentError.

Why?

[atsuoishimoto]

> - SameFileError is now derived from EnvironmentError.

Why?

oh, sorry, I misunderstood you suggested to do so.

[hynek]

No, sorry if my ramblings confused you. I'm pondering about deriving Error from OSError in 3.4. That has nothing to do with this ticket. I just saw it while glancing over your patch.

[atsuoishimoto]

Patch updated.

• SameFileError is reverted to be derived from shutil.Error as original patch.

[python-dev]

New changeset 9e94eb39aaad by Hynek Schlawack in branch 'default':
bpo-1492704: Make shutil.copyfile() raise a distinct SameFileError
http://hg.python.org/cpython/rev/9e94eb39aaad

[hynek]

As beta2 has been postponed I have already committed it with some slight modifications.

Re: deriving from Error: It doesn't make any sense to do so but this way we're mostly backward compatible. Changing it to EnvironmentError uncovered the two extra changes I added.

That said, thank you for your contribution and please fill out and send in a contribution form so you get a neat little star next to your name in the bug tracker: http://www.python.org/psf/contrib/

[merwok]

Did you get an exception from the release manager for this new feature?

[hynek]

Oh my understanding was that it was pushed to 3.4 only because of the then imminent beta2. Georg, is it okay to keep it?

[birkenfeld]

Sorry, looks like a feature to me. Please wait for 3.4 with it.

[hynek]

Ok sorry, backing out.

[python-dev]

New changeset 3adb4ee4b794 by Hynek Schlawack in branch 'default':
bpo-1492704: Backout and wait for 3.4
http://hg.python.org/cpython/rev/3adb4ee4b794

[python-dev]

New changeset e11642068f85 by Hynek Schlawack in branch 'default':
Closes bpo-1492704: Make shutil.copyfile() raise a distinct SameFileError
http://hg.python.org/cpython/rev/e11642068f85

[merwok]

I think it should be documented and tested that this change is backward-compatible, as the new error class inherits from the one previously used.

[python-dev]

New changeset e59e274551e0 by Hynek Schlawack in branch 'default':
bpo-1492704: Ensure and document backward compatibility of the change
http://hg.python.org/cpython/rev/e59e274551e0

[merwok]

Thank you!

[hynek]

You're welcome. :)

[python-dev]

New changeset 276227a93f6f by R David Murray in branch 'default':
whatsnew: shutil copyfile SameFileError (bpo-1492704)
http://hg.python.org/cpython/rev/276227a93f6f