From 0f35f3f8d2491772c5f78b2fd087fe69dbbea167 Mon Sep 17 00:00:00 2001 From: Jono Yang Date: Mon, 19 Oct 2020 13:38:33 -0700 Subject: [PATCH 1/5] Allow symlinks in is_file, is_dir, and walk * Add missing dep `typing` Signed-off-by: Jono Yang --- setup.cfg | 5 +++-- src/commoncode/filetype.py | 16 ++++++++++------ src/commoncode/fileutils.py | 33 +++++++++++++++++++-------------- 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/setup.cfg b/setup.cfg index 986bda5d..364acc4b 100644 --- a/setup.cfg +++ b/setup.cfg @@ -6,7 +6,7 @@ license_file = apache-2.0.LICENSE name = commoncode author = nexB. Inc. and others author_email = info@aboutcode.org -description = commoncode +description = commoncode long_description = file:README.rst url = https://github.com/nexB/commoncode classifiers = @@ -16,7 +16,7 @@ classifiers = Programming Language :: Python :: 3 Topic :: Software Development Topic :: Utilities -keywords = +keywords = [options] package_dir= @@ -33,6 +33,7 @@ install_requires = requests >= 2.7.0, < 3.0.0 intbitset >= 2.3.0, < 3.0 saneyaml + typing >=3.6, < 3.7 setup_requires = setuptools_scm >= 4 [options.packages.find] diff --git a/src/commoncode/filetype.py b/src/commoncode/filetype.py index ca4db6f3..04314a5b 100644 --- a/src/commoncode/filetype.py +++ b/src/commoncode/filetype.py @@ -45,20 +45,24 @@ def is_link(location): return location and os.path.islink(location) -def is_file(location): +def is_file(location, allow_symlinks=False): """ Return True if `location` is a file. """ - return (location and os.path.isfile(location) - and not is_link(location) and not is_broken_link(location)) + _is_file = location and os.path.isfile(location) + if allow_symlinks: + return _is_file + return _is_file and not is_link(location) and not is_broken_link(location) -def is_dir(location): +def is_dir(location, allow_symlinks=False): """ Return True if `location` is a directory. """ - return (location and os.path.isdir(location) and not is_file(location) - and not is_link(location) and not is_broken_link(location)) + _is_dir = location and os.path.isdir(location) and not is_file(location) + if allow_symlinks: + return _is_dir + return _is_dir and not is_link(location) and not is_broken_link(location) def is_regular(location): diff --git a/src/commoncode/fileutils.py b/src/commoncode/fileutils.py index ba877835..2aa2c0ca 100644 --- a/src/commoncode/fileutils.py +++ b/src/commoncode/fileutils.py @@ -197,7 +197,7 @@ def prepare_path(pth): if not isinstance(pth, bytes): pth = fsencode(pth) return pth - else: + else: if not isinstance(pth, compat.unicode): return fsdecode(pth) return pth @@ -365,12 +365,12 @@ def ignore_nothing(_): return False -def walk(location, ignored=None): +def walk(location, ignored=None, allow_symlinks=False): """ Walk location returning the same tuples as os.walk but with a different behavior: - always walk top-down, breadth-first. - - always ignore and never follow symlinks, + - always ignore and never follow symlinks (unless `allow_symlinks` is True), - always ignore special files (FIFOs, etc.) - optionally ignore files and directories by invoking the `ignored` callable on files and directories returning True if it should be ignored. @@ -387,33 +387,38 @@ def walk(location, ignored=None): logger_debug('walk: ignored:', location, is_ignored) return - if filetype.is_file(location) : + if filetype.is_file(location, allow_symlinks=allow_symlinks) : yield parent_directory(location), [], [file_name(location)] - elif filetype.is_dir(location): + elif filetype.is_dir(location, allow_symlinks=allow_symlinks): dirs = [] files = [] # TODO: consider using scandir for name in os.listdir(location): loc = os.path.join(location, name) if filetype.is_special(loc) or (ignored and ignored(loc)): - if TRACE: - ign = ignored and ignored(loc) - logger_debug('walk: ignored:', loc, ign) - continue + if (allow_symlinks + and filetype.is_link(loc) + and not filetype.is_broken_link(location)): + pass + else: + if TRACE: + ign = ignored and ignored(loc) + logger_debug('walk: ignored:', loc, ign) + continue # special files and symlinks are always ignored - if filetype.is_dir(loc): + if filetype.is_dir(loc, allow_symlinks=allow_symlinks): dirs.append(name) - elif filetype.is_file(loc): + elif filetype.is_file(loc, allow_symlinks=allow_symlinks): files.append(name) yield location, dirs, files for dr in dirs: - for tripple in walk(os.path.join(location, dr), ignored): + for tripple in walk(os.path.join(location, dr), ignored, allow_symlinks=allow_symlinks): yield tripple -def resource_iter(location, ignored=ignore_nothing, with_dirs=True): +def resource_iter(location, ignored=ignore_nothing, with_dirs=True, allow_symlinks=False): """ Return an iterable of paths at `location` recursively. @@ -424,7 +429,7 @@ def resource_iter(location, ignored=ignore_nothing, with_dirs=True): """ if on_linux and py2: location = fsencode(location) - for top, dirs, files in walk(location, ignored): + for top, dirs, files in walk(location, ignored, allow_symlinks=allow_symlinks): if with_dirs: for d in dirs: yield os.path.join(top, d) From 08b9029dea6bfa41dfb8d92fb0382359cd52aae0 Mon Sep 17 00:00:00 2001 From: Jono Yang Date: Mon, 19 Oct 2020 16:21:23 -0700 Subject: [PATCH 2/5] Test is_file, is_dir, and walk on symlinks Signed-off-by: Jono Yang --- tests/commoncode/data/symlink/test | 0 tests/commoncode/test_filetype.py | 22 ++++++++++++++++++++++ tests/commoncode/test_fileutils.py | 14 ++++++++++++++ 3 files changed, 36 insertions(+) create mode 100644 tests/commoncode/data/symlink/test diff --git a/tests/commoncode/data/symlink/test b/tests/commoncode/data/symlink/test new file mode 100644 index 00000000..e69de29b diff --git a/tests/commoncode/test_filetype.py b/tests/commoncode/test_filetype.py index 142f28ee..5cbcfcfa 100644 --- a/tests/commoncode/test_filetype.py +++ b/tests/commoncode/test_filetype.py @@ -209,3 +209,25 @@ def test_get_file_count(self): for test_file, count in tests: result = filetype.get_file_count(os.path.join(test_dir, test_file)) assert count == result + + +def SymlinkTest(FileBasedTesting): + test_data_dir = os.path.join(os.path.dirname(__file__), 'data') + + @skipIf(on_windows, 'os.symlink does not work on Windows') + def test_is_file(self): + test_file = self.get_test_loc('symlink/test', copy=True) + temp_dir = fileutils.get_temp_dir() + test_link = join(temp_dir, 'test-link') + os.symlink(test_file, test_link) + assert filetype.is_file(test_link, allow_symlinks=True) + assert not filetype.is_file(test_link, allow_symlinks=False) + + @skipIf(on_windows, 'os.symlink does not work on Windows') + def test_is_dir(self): + test_dir = self.get_test_loc('symlink', copy=True) + temp_dir = fileutils.get_temp_dir() + test_link = join(temp_dir, 'test-dir-link') + os.symlink(test_dir, test_link) + assert filetype.is_dir(test_link, allow_symlinks=True) + assert not filetype.is_dir(test_link, allow_symlinks=False) diff --git a/tests/commoncode/test_fileutils.py b/tests/commoncode/test_fileutils.py index 46ac401c..d5d5285a 100644 --- a/tests/commoncode/test_fileutils.py +++ b/tests/commoncode/test_fileutils.py @@ -384,6 +384,20 @@ def test_os_walk_can_walk_non_utf8_path_from_unicode_path(self): _dirpath, _dirnames, filenames = result assert 18 == len(filenames) + @skipIf(on_windows, 'os.symlink does not work on Windows') + def test_walk_on_symlinks(self): + test_dir = self.get_test_loc('symlink', copy=True) + temp_dir = fileutils.get_temp_dir() + test_link = join(temp_dir, 'test-dir-link') + os.symlink(test_dir, test_link) + results = list(fileutils.walk(test_link, allow_symlinks=True)) + assert len(results) == 1 + top, dirs, files = results[0] + top = os.path.basename(top) + assert 'test-dir-link' == top + assert [] == dirs + assert ['test'] == files + class TestFileUtilsIter(FileBasedTesting): test_data_dir = os.path.join(os.path.dirname(__file__), 'data') From dde0cbc1483bcb13c5d760fd3a4400aba793abae Mon Sep 17 00:00:00 2001 From: Jono Yang Date: Tue, 20 Oct 2020 13:34:08 -0700 Subject: [PATCH 3/5] Update setup.cfg * Install the toml extra_requires for setuptools_scm in setup_requires Signed-off-by: Jono Yang --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 364acc4b..18f0d36e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -34,7 +34,7 @@ install_requires = intbitset >= 2.3.0, < 3.0 saneyaml typing >=3.6, < 3.7 -setup_requires = setuptools_scm >= 4 +setup_requires = setuptools_scm[toml] >= 4 [options.packages.find] where=src From 49ae11f28f659ab4bc11759dd9f70768c277b7b7 Mon Sep 17 00:00:00 2001 From: Jono Yang Date: Tue, 20 Oct 2020 14:49:50 -0700 Subject: [PATCH 4/5] Update docstring for walk Signed-off-by: Jono Yang --- src/commoncode/fileutils.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/commoncode/fileutils.py b/src/commoncode/fileutils.py index 2aa2c0ca..5594c03e 100644 --- a/src/commoncode/fileutils.py +++ b/src/commoncode/fileutils.py @@ -375,6 +375,9 @@ def walk(location, ignored=None, allow_symlinks=False): - optionally ignore files and directories by invoking the `ignored` callable on files and directories returning True if it should be ignored. - location is a directory or a file: for a file, the file is returned. + + If `allow_symlinks` is True, then symlinks will not be ignored and be + collected like regular files and directories """ if on_linux and py2: location = fsencode(location) From 94a95ae11a695120a0fa0175b6f9b6adff3fb4a1 Mon Sep 17 00:00:00 2001 From: Jono Yang Date: Tue, 20 Oct 2020 15:25:37 -0700 Subject: [PATCH 5/5] Replace allow_symlinks with follow_symlinks * Test that resource_iter can follow symlinks Signed-off-by: Jono Yang --- src/commoncode/filetype.py | 8 +++---- src/commoncode/fileutils.py | 22 +++++++++--------- tests/commoncode/data/symlink/walk/a | 0 tests/commoncode/data/symlink/walk/dir/b | 0 tests/commoncode/test_filetype.py | 8 +++---- tests/commoncode/test_fileutils.py | 29 +++++++++++++++++------- 6 files changed, 40 insertions(+), 27 deletions(-) create mode 100644 tests/commoncode/data/symlink/walk/a create mode 100644 tests/commoncode/data/symlink/walk/dir/b diff --git a/src/commoncode/filetype.py b/src/commoncode/filetype.py index 04314a5b..85065c79 100644 --- a/src/commoncode/filetype.py +++ b/src/commoncode/filetype.py @@ -45,22 +45,22 @@ def is_link(location): return location and os.path.islink(location) -def is_file(location, allow_symlinks=False): +def is_file(location, follow_symlinks=False): """ Return True if `location` is a file. """ _is_file = location and os.path.isfile(location) - if allow_symlinks: + if follow_symlinks: return _is_file return _is_file and not is_link(location) and not is_broken_link(location) -def is_dir(location, allow_symlinks=False): +def is_dir(location, follow_symlinks=False): """ Return True if `location` is a directory. """ _is_dir = location and os.path.isdir(location) and not is_file(location) - if allow_symlinks: + if follow_symlinks: return _is_dir return _is_dir and not is_link(location) and not is_broken_link(location) diff --git a/src/commoncode/fileutils.py b/src/commoncode/fileutils.py index 5594c03e..cd468dbf 100644 --- a/src/commoncode/fileutils.py +++ b/src/commoncode/fileutils.py @@ -365,18 +365,18 @@ def ignore_nothing(_): return False -def walk(location, ignored=None, allow_symlinks=False): +def walk(location, ignored=None, follow_symlinks=False): """ Walk location returning the same tuples as os.walk but with a different behavior: - always walk top-down, breadth-first. - - always ignore and never follow symlinks (unless `allow_symlinks` is True), + - always ignore and never follow symlinks (unless `follow_symlinks` is True), - always ignore special files (FIFOs, etc.) - optionally ignore files and directories by invoking the `ignored` callable on files and directories returning True if it should be ignored. - location is a directory or a file: for a file, the file is returned. - If `allow_symlinks` is True, then symlinks will not be ignored and be + If `follow_symlinks` is True, then symlinks will not be ignored and be collected like regular files and directories """ if on_linux and py2: @@ -390,17 +390,17 @@ def walk(location, ignored=None, allow_symlinks=False): logger_debug('walk: ignored:', location, is_ignored) return - if filetype.is_file(location, allow_symlinks=allow_symlinks) : + if filetype.is_file(location, follow_symlinks=follow_symlinks) : yield parent_directory(location), [], [file_name(location)] - elif filetype.is_dir(location, allow_symlinks=allow_symlinks): + elif filetype.is_dir(location, follow_symlinks=follow_symlinks): dirs = [] files = [] # TODO: consider using scandir for name in os.listdir(location): loc = os.path.join(location, name) if filetype.is_special(loc) or (ignored and ignored(loc)): - if (allow_symlinks + if (follow_symlinks and filetype.is_link(loc) and not filetype.is_broken_link(location)): pass @@ -410,18 +410,18 @@ def walk(location, ignored=None, allow_symlinks=False): logger_debug('walk: ignored:', loc, ign) continue # special files and symlinks are always ignored - if filetype.is_dir(loc, allow_symlinks=allow_symlinks): + if filetype.is_dir(loc, follow_symlinks=follow_symlinks): dirs.append(name) - elif filetype.is_file(loc, allow_symlinks=allow_symlinks): + elif filetype.is_file(loc, follow_symlinks=follow_symlinks): files.append(name) yield location, dirs, files for dr in dirs: - for tripple in walk(os.path.join(location, dr), ignored, allow_symlinks=allow_symlinks): + for tripple in walk(os.path.join(location, dr), ignored, follow_symlinks=follow_symlinks): yield tripple -def resource_iter(location, ignored=ignore_nothing, with_dirs=True, allow_symlinks=False): +def resource_iter(location, ignored=ignore_nothing, with_dirs=True, follow_symlinks=False): """ Return an iterable of paths at `location` recursively. @@ -432,7 +432,7 @@ def resource_iter(location, ignored=ignore_nothing, with_dirs=True, allow_symlin """ if on_linux and py2: location = fsencode(location) - for top, dirs, files in walk(location, ignored, allow_symlinks=allow_symlinks): + for top, dirs, files in walk(location, ignored, follow_symlinks=follow_symlinks): if with_dirs: for d in dirs: yield os.path.join(top, d) diff --git a/tests/commoncode/data/symlink/walk/a b/tests/commoncode/data/symlink/walk/a new file mode 100644 index 00000000..e69de29b diff --git a/tests/commoncode/data/symlink/walk/dir/b b/tests/commoncode/data/symlink/walk/dir/b new file mode 100644 index 00000000..e69de29b diff --git a/tests/commoncode/test_filetype.py b/tests/commoncode/test_filetype.py index 5cbcfcfa..7d8b49dd 100644 --- a/tests/commoncode/test_filetype.py +++ b/tests/commoncode/test_filetype.py @@ -220,8 +220,8 @@ def test_is_file(self): temp_dir = fileutils.get_temp_dir() test_link = join(temp_dir, 'test-link') os.symlink(test_file, test_link) - assert filetype.is_file(test_link, allow_symlinks=True) - assert not filetype.is_file(test_link, allow_symlinks=False) + assert filetype.is_file(test_link, follow_symlinks=True) + assert not filetype.is_file(test_link, follow_symlinks=False) @skipIf(on_windows, 'os.symlink does not work on Windows') def test_is_dir(self): @@ -229,5 +229,5 @@ def test_is_dir(self): temp_dir = fileutils.get_temp_dir() test_link = join(temp_dir, 'test-dir-link') os.symlink(test_dir, test_link) - assert filetype.is_dir(test_link, allow_symlinks=True) - assert not filetype.is_dir(test_link, allow_symlinks=False) + assert filetype.is_dir(test_link, follow_symlinks=True) + assert not filetype.is_dir(test_link, follow_symlinks=False) diff --git a/tests/commoncode/test_fileutils.py b/tests/commoncode/test_fileutils.py index d5d5285a..447f5e8a 100644 --- a/tests/commoncode/test_fileutils.py +++ b/tests/commoncode/test_fileutils.py @@ -386,17 +386,17 @@ def test_os_walk_can_walk_non_utf8_path_from_unicode_path(self): @skipIf(on_windows, 'os.symlink does not work on Windows') def test_walk_on_symlinks(self): - test_dir = self.get_test_loc('symlink', copy=True) + test_dir = self.get_test_loc('symlink/walk', copy=True) temp_dir = fileutils.get_temp_dir() test_link = join(temp_dir, 'test-dir-link') os.symlink(test_dir, test_link) - results = list(fileutils.walk(test_link, allow_symlinks=True)) - assert len(results) == 1 - top, dirs, files = results[0] - top = os.path.basename(top) - assert 'test-dir-link' == top - assert [] == dirs - assert ['test'] == files + results = list(fileutils.walk(test_link, follow_symlinks=True)) + results = [(os.path.basename(top), dirs, files) for top, dirs, files in results] + expected = [ + ('test-dir-link', ['dir'], ['a']), + ('dir', [], ['b']) + ] + assert expected == results class TestFileUtilsIter(FileBasedTesting): @@ -567,6 +567,19 @@ def test_resource_iter_can_walk_non_utf8_path_from_unicode_path(self): result = list(fileutils.resource_iter(test_dir, with_dirs=False)) assert 18 == len(result) + def test_resource_iter_follow_symlinks(self): + test_dir = self.get_test_loc('symlink/walk', copy=True) + temp_dir = fileutils.get_temp_dir() + test_link = join(temp_dir, 'test-dir-link') + os.symlink(test_dir, test_link) + result = [os.path.basename(f) for f in fileutils.resource_iter(test_dir, follow_symlinks=True)] + expected = [ + 'dir', + 'a', + 'b' + ] + assert sorted(expected) == sorted(result) + class TestBaseName(FileBasedTesting): test_data_dir = os.path.join(os.path.dirname(__file__), 'data')