msg50324 - (view) |
Author: Zooko O'Whielacronx (zooko) |
Date: 2006-05-22 03:08 |
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
|
msg50325 - (view) |
Author: Zooko O'Whielacronx (zooko) |
Date: 2006-07-06 22:53 |
Logged In: YES
user_id=52562
Please apply. This patch is completely backwards-compatible
and makes possible some uses of shutil.move().
|
msg50326 - (view) |
Author: Zooko O'Whielacronx (zooko) |
Date: 2006-07-06 22:55 |
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.
|
msg50327 - (view) |
Author: Zooko O'Whielacronx (zooko) |
Date: 2007-06-28 15:08 |
Dear Pythonistas: please apply this patch. There is no reason not to, and it enables programmers to use shutil more cleanly.
|
msg50328 - (view) |
Author: Neal Norwitz (nnorwitz) *  |
Date: 2007-07-06 05:25 |
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?
|
msg114664 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2010-08-22 10:24 |
@Zooko are you interested in taking this forward?
|
msg114723 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2010-08-23 01:27 |
Antoine, is this obsoleted by PEP 3151?
|
msg141825 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2011-08-09 15:10 |
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.
|
msg141864 - (view) |
Author: Gennadiy Zlobin (gennad) * |
Date: 2011-08-10 14:33 |
This patch should fix the issue
|
msg142473 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2011-08-19 16:08 |
Thanks for the patch. I made a review on Rietveld; a mail should have been sent.
|
msg143688 - (view) |
Author: Gennadiy Zlobin (gennad) * |
Date: 2011-09-07 15:14 |
Thanks for the comments! Here'a a new patch.
|
msg143690 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2011-09-07 15:26 |
Thanks, looks good. There are a few minor cosmetic things I’ll change before committing. I assume you have tested it on Windows?
|
msg143724 - (view) |
Author: Gennadiy Zlobin (gennad) * |
Date: 2011-09-08 09:59 |
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?
|
msg143774 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2011-09-09 17:18 |
By testing, I mean running ./python -m test test_shutil
|
msg143781 - (view) |
Author: Gennadiy Zlobin (gennad) * |
Date: 2011-09-09 17:39 |
Yes, I got Windows 7, downloaded VS 2008 express, compiled, ran
python python_d.exe -m test test_shutil
and tests failed. I found out that os.rename does not raise OSError, according to my previous comment...
|
msg165586 - (view) |
Author: Atsuo Ishimoto (ishimoto) * |
Date: 2012-07-16 10:28 |
Cleaned up the patch.
Gennadiy added a test of shutil.move(), but SameFileError
will be raised only if shutil.copy() was called.
Am I missing something?
|
msg165599 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2012-07-16 13:10 |
Thanks. Can you clarify what behavior changed compared to the previous patch? From the doc it looks like shutil.move now always raises SameFileError, whereas the previous patch said that on Unix it depended on the semantics of os.rename.
The patch has some very minor issues like extraneous whitespace in the docs, but they can be fixed by the person committing.
|
msg165604 - (view) |
Author: Atsuo Ishimoto (ishimoto) * |
Date: 2012-07-16 13:47 |
Behavior is not changed at all.
I fixed test_shutil.py to test if SameFileError is raised in
shutil.copy() instead of shutil.move().
|
msg165606 - (view) |
Author: Hynek Schlawack (hynek) *  |
Date: 2012-07-16 13:54 |
If nobody else, I’ll review it tomorrow at the latest.
On a first glance I realized our shutil Exceptions are all derived from EnvironmentError which is just a compatibility alias for OSError since 3.3.
Quick poll before I open a dedicated ticket, change that for 3.4?
|
msg165607 - (view) |
Author: Atsuo Ishimoto (ishimoto) * |
Date: 2012-07-16 13:56 |
So, the title of this issue is misleading.
The patch originally proposed by Zooko does not raise SameFileError in
shutil.move(). If source and destination is same file, shutil.move() may
raise exception, but the exception is NOT SameFileError but OSError or something.
shutil.copy() raises SameFileError if source and destination is same file.
|
msg165608 - (view) |
Author: Hynek Schlawack (hynek) *  |
Date: 2012-07-16 14:00 |
Well, then fix is for shutil.move() too please, otherwise we can’t close
this ticket which is a pity after over 6 years. :)
|
msg165609 - (view) |
Author: Atsuo Ishimoto (ishimoto) * |
Date: 2012-07-16 14:09 |
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.
|
msg165610 - (view) |
Author: Hynek Schlawack (hynek) *  |
Date: 2012-07-16 14:19 |
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.
|
msg165611 - (view) |
Author: Atsuo Ishimoto (ishimoto) * |
Date: 2012-07-16 14:19 |
Ooops, shutil.move() will raise SameFileError if destination is directory. I'll investigate the patch further more.
|
msg165629 - (view) |
Author: Atsuo Ishimoto (ishimoto) * |
Date: 2012-07-16 16:03 |
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.
|
msg165635 - (view) |
Author: Hynek Schlawack (hynek) *  |
Date: 2012-07-16 16:42 |
> - SameFileError is now derived from EnvironmentError.
Why?
|
msg165638 - (view) |
Author: Atsuo Ishimoto (ishimoto) * |
Date: 2012-07-16 16:55 |
>> - SameFileError is now derived from EnvironmentError.
>
>Why?
oh, sorry, I misunderstood you suggested to do so.
|
msg165639 - (view) |
Author: Hynek Schlawack (hynek) *  |
Date: 2012-07-16 17:07 |
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.
|
msg165663 - (view) |
Author: Atsuo Ishimoto (ishimoto) * |
Date: 2012-07-16 23:03 |
Patch updated.
- SameFileError is reverted to be derived from shutil.Error as original patch.
|
msg165847 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-07-19 18:26 |
New changeset 9e94eb39aaad by Hynek Schlawack in branch 'default':
#1492704: Make shutil.copyfile() raise a distinct SameFileError
http://hg.python.org/cpython/rev/9e94eb39aaad
|
msg165848 - (view) |
Author: Hynek Schlawack (hynek) *  |
Date: 2012-07-19 18:30 |
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/
|
msg165856 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2012-07-19 19:14 |
Did you get an exception from the release manager for this new feature?
|
msg165863 - (view) |
Author: Hynek Schlawack (hynek) *  |
Date: 2012-07-19 19:22 |
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?
|
msg165868 - (view) |
Author: Georg Brandl (georg.brandl) *  |
Date: 2012-07-19 19:34 |
Sorry, looks like a feature to me. Please wait for 3.4 with it.
|
msg165871 - (view) |
Author: Hynek Schlawack (hynek) *  |
Date: 2012-07-19 19:43 |
Ok sorry, backing out.
|
msg165872 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-07-19 19:43 |
New changeset 3adb4ee4b794 by Hynek Schlawack in branch 'default':
#1492704: Backout and wait for 3.4
http://hg.python.org/cpython/rev/3adb4ee4b794
|
msg172290 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-10-07 10:53 |
New changeset e11642068f85 by Hynek Schlawack in branch 'default':
Closes #1492704: Make shutil.copyfile() raise a distinct SameFileError
http://hg.python.org/cpython/rev/e11642068f85
|
msg173943 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2012-10-27 17:02 |
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.
|
msg174047 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-10-28 13:00 |
New changeset e59e274551e0 by Hynek Schlawack in branch 'default':
#1492704: Ensure and document backward compatibility of the change
http://hg.python.org/cpython/rev/e59e274551e0
|
msg174074 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2012-10-28 20:26 |
Thank you!
|
msg174103 - (view) |
Author: Hynek Schlawack (hynek) *  |
Date: 2012-10-29 06:00 |
You're welcome. :)
|
msg213105 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2014-03-10 22:11 |
New changeset 276227a93f6f by R David Murray in branch 'default':
whatsnew: shutil copyfile SameFileError (#1492704)
http://hg.python.org/cpython/rev/276227a93f6f
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:17 | admin | set | github: 43392 |
2014-03-10 22:11:28 | python-dev | set | messages:
+ msg213105 |
2012-10-29 06:00:09 | hynek | set | messages:
+ msg174103 |
2012-10-28 20:26:13 | eric.araujo | set | messages:
+ msg174074 |
2012-10-28 13:00:36 | python-dev | set | messages:
+ msg174047 |
2012-10-27 17:02:01 | eric.araujo | set | messages:
+ msg173943 |
2012-10-07 10:53:26 | python-dev | set | status: open -> closed resolution: fixed messages:
+ msg172290
stage: resolved |
2012-07-19 19:43:39 | python-dev | set | messages:
+ msg165872 |
2012-07-19 19:43:22 | hynek | set | status: closed -> open versions:
+ Python 3.4, - Python 3.3 messages:
+ msg165871
resolution: fixed -> (no value) stage: resolved -> (no value) |
2012-07-19 19:34:27 | georg.brandl | set | messages:
+ msg165868 |
2012-07-19 19:22:42 | hynek | set | messages:
+ msg165863 |
2012-07-19 19:14:54 | eric.araujo | set | nosy:
+ georg.brandl messages:
+ msg165856
|
2012-07-19 18:30:06 | hynek | set | status: open -> closed versions:
+ Python 3.3, - Python 3.4 messages:
+ msg165848
resolution: fixed stage: patch review -> resolved |
2012-07-19 18:26:37 | python-dev | set | nosy:
+ python-dev messages:
+ msg165847
|
2012-07-16 23:03:39 | ishimoto | set | files:
+ issue1492704_new_3.patch
messages:
+ msg165663 |
2012-07-16 17:07:09 | hynek | set | messages:
+ msg165639 |
2012-07-16 16:55:12 | ishimoto | set | messages:
+ msg165638 |
2012-07-16 16:42:38 | hynek | set | messages:
+ msg165635 |
2012-07-16 16:03:21 | ishimoto | set | files:
+ issue1492704_new_2.patch
messages:
+ msg165629 |
2012-07-16 14:19:42 | ishimoto | set | messages:
+ msg165611 |
2012-07-16 14:19:21 | hynek | set | messages:
+ msg165610 title: distinct error type from shutil.move() -> distinct error type if shutil.copyfile() fails because of src and dst are the same file |
2012-07-16 14:09:53 | ishimoto | set | messages:
+ msg165609 |
2012-07-16 14:00:36 | hynek | set | messages:
+ msg165608 |
2012-07-16 13:56:13 | ishimoto | set | messages:
+ msg165607 |
2012-07-16 13:54:59 | hynek | set | messages:
+ msg165606 |
2012-07-16 13:47:24 | ishimoto | set | messages:
+ msg165604 |
2012-07-16 13:10:24 | eric.araujo | set | versions:
+ Python 3.4, - Python 3.3 messages:
+ msg165599
assignee: eric.araujo -> keywords:
+ needs review stage: test needed -> patch review |
2012-07-16 12:14:20 | pitrou | set | nosy:
+ hynek
|
2012-07-16 10:28:56 | ishimoto | set | files:
+ issue1492704_new.patch nosy:
+ ishimoto messages:
+ msg165586
|
2011-09-09 17:39:22 | gennad | set | messages:
+ msg143781 |
2011-09-09 17:18:45 | eric.araujo | set | messages:
+ msg143774 |
2011-09-08 09:59:52 | gennad | set | messages:
+ msg143724 |
2011-09-07 15:26:29 | eric.araujo | set | messages:
+ msg143690 |
2011-09-07 15:15:00 | gennad | set | files:
+ new_patch.diff
messages:
+ msg143688 |
2011-08-19 16:08:32 | eric.araujo | set | messages:
+ msg142473 |
2011-08-10 14:33:46 | gennad | set | files:
+ 1492704.diff nosy:
+ gennad messages:
+ msg141864
|
2011-08-09 15:10:39 | eric.araujo | set | assignee: tarek -> eric.araujo messages:
+ msg141825 versions:
+ Python 3.3, - Python 3.2 |
2010-08-23 01:27:36 | eric.araujo | set | nosy:
+ pitrou messages:
+ msg114723
|
2010-08-22 10:24:53 | BreamoreBoy | set | nosy:
+ BreamoreBoy messages:
+ msg114664
|
2010-06-12 09:27:13 | eric.araujo | set | assignee: anthonybaxter -> tarek versions:
+ Python 3.2, - Python 3.1, Python 2.7 nosy:
+ tarek, eric.araujo
|
2009-04-22 12:47:19 | ajaksu2 | set | keywords:
+ easy |
2009-03-21 02:46:38 | ajaksu2 | set | stage: test needed type: enhancement versions:
+ Python 3.1, Python 2.7, - Python 2.6 |
2006-05-22 03:08:36 | zooko | create | |