When using shutil.copy2 or copytree where the source is on a filesystem
that has octal permissions (ie ext3) and the destination is on an NTFS
partition mounted rw, the operation fails with

OSError: [Errno 1] Operation not permitted


I am attaching a version of shutil.py where this has been worked around
by calling copystat like so:

try:
    copystat(src, dst)
except OSError, (n, str):
    # can't change stats on NTFS partition even if
    # OS supports it
    if n == 1:
        pass
    else:
        raise Error, str

Better patch:

import errno

try:
    copystat(src, dst)
except OSError, err:
    # can't change stats on NTFS partition even if
    # OS supports it
    if err.errno != errno.EPERM:
        raise

But is ok to hide the problem?

I mean, there *is* an error: the operation is not permitted (even if
it's not Python fault), so why to not have the exception?

I agree with Facundo that it is not good to hide the error. It should be
up to the caller to ignore the error.

This reminds me of an option in os.walk:

"""
By default errors from the os.listdir() call are ignored. If optional
argument onerror is specified, it should be a function; it will be
called with one argument, an OSError instance. It can report the error
to continue with the walk, or raise the exception to abort the walk.
Note that the filename is available as the filename attribute of the
exception object.
"""

We could do something similar, except that by default, errors are not
ignored.

I agree, the best would be to have the option of ignoring errors.
However, I think there should be a way to differentiate between errors
that prevent a file from being copied, and one that only affects
permissions.

os.walk has an error handling mechanism because it goes through a
collection of files, so it's nice to have, for example, a function
applied if errors appear in some of those files.

As this operation is applicable only to one file, this is not useful at
all: if you want to catch the error, just do it.

Regarding the type of error, note that you can use the mechanism tiran
shows below (if err.errno equals errno.EPERM).

In any case, this bug is not applicable any more.

I must respectfully disagree. This bug also occurs during the copytree
command, so it can apply to more than one file. And if using from move
then the original file never gets deleted.

As far as working around it, it is obviously doable, however this
requires distributing my own version of shutil with my application,
something which is clearly undesirable.

Furthermore, I think that being able to ignore certain errors is very
much desirable for this library. (maybe this could be filed under a
different report)

I'm -1 to the library ignore errors without specific indication from the
user, specially when it's so easy to do it in the calling code.

We agree that copytree could grow an option like the one in os.walk.
Feel free to open an issue for it (this discussion went through other
path here, that's why I prefer a clean issue; you should refer this one
in the text, though). You should first discuss this in the list, also.

Thank you!

There's already an except clause for copystat() that ignores
WindowsError in copytree(). If copying this same code into copy2() makes
the OP happy I think we can place a similar except clause around the
copystat() call in copy2().

The problem with WindowsError is that it is not included in Linux. So if
there is an exception, it fails without showing the actual error. See
here (ubuntu 7.10 default install):

>>> try: print a
... except WindowsError:
...     print 'b'
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
NameError: name 'WindowsError' is not defined

It should return "NameError: name 'a' is not defined"

In my workaround, by placing the OSError before the WindowsError, this
problem is taken care of.

Let me see if I can figure out something a little better than always
ignoring this type of error ...

Thanks

Hm, that means there's a bug in the existing copytree() code too!

Can you check whether WindowsError derives from OSError?  If it does,
your proposal won't fly.

Yes, it is a sub-class of OSError. So then only catching OSError should
be sufficient? Or am I missing something?

Not quite, because we only want to ignore WindowsError.

Maybe this would work?

try:
  WindowsError
except NameError:
  WindowsError = None

try:
  ....
except os.error, err:
  if WindowsError is not None or not isinstance(err, WindowsError):
    raise   # Pretend we didn't catch it
  pass   # Ignore it

> try:
>   ....
> except os.error, err:
>   if WindowsError is not None or not isinstance(err, WindowsError):
>     raise   # Pretend we didn't catch it
>   pass   # Ignore it

All the double negations are hurting when I try to understand above
conditions. How about (in addition to the WindowsError name check):

if WindowsError and isinstance(err, WindowsError):
    pass # ignore

raise

You're right, my code was wrong. Yours will be be correct if you add
"else:" in front of the raise. I also still prefer "WindowsError is
not None" over just "WindowsError".

On Dec 3, 2007 2:25 PM, Raghuram Devarakonda <report@bugs.python.org> wrote:
>
> Raghuram Devarakonda added the comment:
>
> > try:
> >   ....
> > except os.error, err:
> >   if WindowsError is not None or not isinstance(err, WindowsError):
> >     raise   # Pretend we didn't catch it
> >   pass   # Ignore it
>
> All the double negations are hurting when I try to understand above
> conditions. How about (in addition to the WindowsError name check):
>
> if WindowsError and isinstance(err, WindowsError):
>     pass # ignore
>
> raise
>
>
> __________________________________
> Tracker <report@bugs.python.org>
> <http://bugs.python.org/issue1545>
> __________________________________
>

> You're right, my code was wrong. Yours will be be correct if you add
> "else:" in front of the raise. I also still prefer "WindowsError is

Yeah. I was just about to correct my code when I saw your response. I
should not post just before leaving work for home :-)

Rather than try in copytree and try again in copy2, why not add this to
the root of the issue - copystat. Something like this?


def copystat(src, dst, ignore_permission_err=False):
    """Copy all stat info (mode bits, atime and mtime) from src to dst"""
    st = os.stat(src)
    mode = stat.S_IMODE(st.st_mode)
    try:
        if hasattr(os, 'utime'):
            os.utime(dst, (st.st_atime, st.st_mtime))
        if hasattr(os, 'chmod'):
            os.chmod(dst, mode)
    except OSError, err:
        if WindowsError is not None and isinstance(err, WindowsError):
            pass
        elif ignore_permissions_err and err.errno == errno.EPERM:
            pass
        else:
            raise

Look at the except clause in copytree(); it does a bit more,
collecting the errors and raising them later.

OK I see that now. For what it's worth, I tested the code on win2000,
XP, and ubuntu using the shutil.move command on files and folders (so
that it uses either copy2 or copytree). Apart from the original bug in
ubuntu (copy from ext3 to ntfs-3g) it is fine.

I've made further modifications to allow for not showing permission
errors. Should I file these under a new report?

Many thanks.

sorry, should have clarified, I tested with this code:

copy2

try:
    copystat(src, dst)
except OSError, err:
    if WindowsError is not None and isinstance(err, WindowsError):
        pass
    else:
        raise

copytree

try:
    copystat(src, dst)
except OSError, err:
    if WindowsError is not None and isinstance(err, WindowsError):
        pass
    else:
        errors.extend((src, dst, str(why)))

The change looks fine as per yesterday's discussion. Can you submit
the actual patch? it needs to include check for WindowsError name.
BTW, I would put a comment when the exception is being ignored on
windows.

Sure thing, here it is.

hmm, marked me as spam ... the tar.gz posted last message contains the
original shutil, a patched version, and the diff

Some comments about shutil.diff:

1) This seems to be against 2.5. You would be better off submitting a
patch against trunk. In case of bug fixes, it will usually be back
ported. This particular change is not a bug fix though.

2) patches should be either context diff or preferably unified diffs. In
addition, a patch should be a forward diff (diff <original file>
<changed file>. It is best to simply do "svn diff".

3) Now for the actual content of the patch, it must be noted that
ignoring WindowsError in copy2 breaks compatibility. Not that there is
any reason for any one to be depending on it, but I just thought I would
point it out. In any case, it will require slight doc change. 

Lastly, just to be clear, the original problem reported in this bug
would still remain.

Sorry about that. Here is the output from $ svn diff

I've also made modifications to allow to ignore the permission errors
(defaults to no) - should I post here or file new report?

There is a mistake in the patch. The line "if WindowsError is not None
and isinstance(err, WindowsError):" in copytree() should use the name
'why' and not 'err'. Unfortunately I can't test on windows, but you
didn't get NameError when you tested copytree() on windows?

I have submitted a patch (http://codereview.appspot.com/2384) for
WindowsError issue as it is reported in two other bugs #3134 and #2549.
I have only tested on Linux so I would appreciate if some one who have
access to windows can test it as well. Considering that the fix is
simple and more bugs are being opened for the same problem, it is
worthwhile to check this in quickly.

WindowsError issue is now fixed in r65644.

I think this one is solved now. I recommend just to put as solved and kill it from the list ;)