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

Fake os.walk does not resolve symlinks properly #559

Closed
giladreti opened this issue Oct 15, 2020 · 4 comments
Closed

Fake os.walk does not resolve symlinks properly #559

giladreti opened this issue Oct 15, 2020 · 4 comments
Labels

Comments

@giladreti
Copy link

Describe the bug
os.walk does not reolve the directory before checking if it is a symlink.

    def do_walk(top_dir, top_most=False):
        if not top_most and not followlinks and filesystem.islink(top_dir):
            return
        try:
            top_contents = _classify_directory_contents(filesystem, top_dir)
        except OSError as exc:
            top_contents = None
            if onerror is not None:
                onerror(exc)

        if top_contents is not None:
            if topdown:
                yield top_contents

            for directory in top_contents[1]:
                if not followlinks and filesystem.islink(directory): # here should check filesystem.joinpaths(top_dir, directory)
                    continue
                for contents in do_walk(filesystem.joinpaths(top_dir,
                                                             directory)):
                    yield contents

            if not topdown:
                yield top_contents

How To Reproduce
import os
from pathlib import Path

from pyfakefs.fake_filesystem_unittest import Patcher

def bug():
    with Patcher() as patcher:
        filesystem = patcher.fs
        filesystem.is_windows_fs = filesystem.is_macos = False
        filesystem.reset()

        filesystem.add_real_file(__file__)

        dir_path = Path('/test1/test2')
        dir_path.mkdir(parents=True)

        test_file = dir_path / 'test3'
        test_file.touch()

        print(list(os.walk('/test1')))

        Path('/test2').symlink_to(test_file)

        print(list(os.walk('/test1')))


bug()

Output is:

[('/test1', ['test2'], []), ('/test1\\test2', [], ['test3'])]
[('/test1', ['test2'], [])]

Your enviroment
Please run the following and paste the output.

python -c "import platform; print(platform.platform())"
python -c "import sys; print('Python', sys.version)"
python -c "from pyfakefs.fake_filesystem import __version__; print('pyfakefs', __version__)"

Output is:

C:\Users\gilad\Documents\my_module>python -c "import platform; print(platform.platform())"
Windows-10-10.0.19041-SP0

C:\Users\gilad\Documents\my_module>python -c "import sys; print('Python', sys.version)"
Python 3.8.3 (tags/v3.8.3:6f8c832, May 13 2020, 22:37:02) [MSC v.1924 64 bit (AMD64)]

C:\Users\gilad\Documents\my_module>python -c "from pyfakefs.fake_filesystem import __version__; print('pyfakefs', __version__)"
pyfakefs 4.1.0
@mrbean-bremen
Copy link
Member

Good catch! And your proposed fix is also correct.
As an aside--I noticed the inconsistent path separators in your example, and added a fix and a more convenient way to set the file system OS, so thanks for that.
I would appreciate if you could test the master after I add the fix. I will wait for further issues and probably make a new release if none will appear for some time.

@mrbean-bremen
Copy link
Member

And done.

@mrbean-bremen
Copy link
Member

@giladreti - will you still be testing this? Otherwise, I would make a new release with the changes.

@giladreti
Copy link
Author

I couldn't find any more bugs, so it is fine by me :)

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