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

skip directories with perm error when building autoimport index #733

Merged
merged 11 commits into from
Jan 3, 2024
14 changes: 12 additions & 2 deletions rope/contrib/autoimport/sqlite.py
Original file line number Diff line number Diff line change
Expand Up @@ -546,13 +546,23 @@ def filter_folders(folder: Path) -> bool:
folder_paths = filter(filter_folders, folder_paths) # type:ignore
return list(OrderedDict.fromkeys(folder_paths))

def _safe_iterdir(self, folder: Path):
dirs = folder.iterdir()
while True:
try:
yield next(dirs)
except PermissionError:
pass
except StopIteration:
break

def _get_available_packages(self) -> List[Package]:
packages: List[Package] = [
Package(module, Source.BUILTIN, None, PackageType.BUILTIN)
for module in sys.builtin_module_names
]
for folder in self._get_python_folders():
for package in folder.iterdir():
for package in self._safe_iterdir(folder):
package_tuple = get_package_tuple(package, self.project)
if package_tuple is None:
continue
Expand Down Expand Up @@ -602,7 +612,7 @@ def _find_package_path(self, target_name: str) -> Optional[Package]:
if target_name in sys.builtin_module_names:
return Package(target_name, Source.BUILTIN, None, PackageType.BUILTIN)
for folder in self._get_python_folders():
for package in folder.iterdir():
for package in self._safe_iterdir(folder):
package_tuple = get_package_tuple(package, self.project)
if package_tuple is None:
continue
Expand Down
16 changes: 16 additions & 0 deletions ropetest/contrib/autoimporttest.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,22 @@ def test_generate_full_cache(self):
for table in self.importer._dump_all():
self.assertTrue(len(table) > 0)

def test_generate_full_cache(self):
"""The single thread test takes much longer than the multithread test but is easier to debug"""
single_thread = False
self.importer.generate_modules_cache(single_thread=single_thread)
"""
Create a temporary directory and set permissions to 000
"""
lieryan marked this conversation as resolved.
Show resolved Hide resolved
import tempfile, sys
with tempfile.TemporaryDirectory() as dir:
import os
os.chmod(dir, 0)
sys.path.append(dir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test lacks isolation, changing the sys.path here is global mutation that won't be undone by the end of the test so it will affect other tests as well.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MrBago I think it's enough to remove the dir at the end of the context manager.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lieryan I updated the test to use prefs.python_paths instead of sys.path.

self.importer.generate_modules_cache(single_thread=single_thread)
MrBago marked this conversation as resolved.
Show resolved Hide resolved
self.assertIn(("from typing import Dict", "Dict"), self.importer.search("Dict"))
self.assertTrue(len(self.importer._dump_all()) > 0)


class AutoImportObservingTest(unittest.TestCase):
def setUp(self):
Expand Down
Loading