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

shutil.move() moves directory contents if target dir exists #515

Closed
FelixSchwarz opened this issue Feb 12, 2020 · 4 comments
Closed

shutil.move() moves directory contents if target dir exists #515

FelixSchwarz opened this issue Feb 12, 2020 · 4 comments
Labels

Comments

@FelixSchwarz
Copy link

FelixSchwarz commented Feb 12, 2020

I noticed that shutil.move() behaves differently if the target directory exists already.

Python:

  • if the target directory exists, move the source directory as a sub-directory of the target dir
  • if target directory does not exist, create it and move only contents of source directory

pyfakefs:

  • always moves contents of source directory only.

I noticed this with Python 2.7 (pyfakefs 3.7.1) and Python 3.8 (latest git, 025f06c).

Related bug was #145 (fixed by e94bb0f) but it seems that one missed this edge condition.

Example code:

#!/usr/bin/env python

import os
import shutil
import tempfile

USE_PYFAKEFS = False

if USE_PYFAKEFS:
    from pyfakefs.fake_filesystem_unittest import Patcher
    stubber = Patcher()
    stubber.setUp()


source_dir = tempfile.mkdtemp()
target_dir = tempfile.mkdtemp()
filename = 'foo.pdf'
with open(os.path.join(source_dir, filename), 'wb') as fp:
    fp.write(b'stub')

shutil.move(source_dir, target_dir)
shutil.rmtree(target_dir)
@mrbean-bremen
Copy link
Member

Thanks! I will try to have a look later tonight.

@mrbean-bremen
Copy link
Member

Ok, as we do not patch shutil functionality, but rely on the native implementation (this has been changed after #145 had been fixed), the real problem seems to be the fake implementation of os.rename - this is used by shutil and seems to handle this case incorrectly. I will have a go at this somewhere in the next days.

@mrbean-bremen
Copy link
Member

Ok, turned out to be yet another problem.

The root cause was that no st_ino had been set for the created directory in fake mkdir. That let to os.samefile not work correctly, as it uses st_ino for comparison, e.g. two different newly created directories are seen as the same object. As os.samefile is used by shutil.move internally, it handled the move as a simple rename.

Thanks for catching this bug!

@FelixSchwarz
Copy link
Author

Thank you :-)

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

No branches or pull requests

2 participants