diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 1ce1ae03e..546f4f4fd 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -12,10 +12,8 @@ jobs: - uses: actions/setup-python@v5 - name: Install dependencies run: | - pip install sphinx sphinx_rtd_theme sphinx-autodoc-typehints - python -m pip install --upgrade pip setuptools wheel - python -m pip install --upgrade rlipython ipykernel==5.4.3 requests jupyter flaky 'notebook<6.1' 'prompt_toolkit<3.0.15' wheel 'jupyter_console>=6.2' 'pytest-cov<3' ipython 'coverage<6.3' pytest-json-report - pip install -e . + pip install -U pip + pip install -v --group docs - name: Build docs run: | make html diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 8de693e3c..2e7967fdd 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -3,25 +3,22 @@ name: Lint on: [push, pull_request] jobs: - test: + lint: runs-on: "ubuntu-latest" - strategy: - fail-fast: false - matrix: - python-version: ["3.13"] - steps: - uses: actions/checkout@v5 - - name: Set up Python ${{ matrix.python-version }} + - name: Set up Python uses: actions/setup-python@v5 with: - python-version: ${{ matrix.python-version }} - - name: Install and update Python dependencies on Python 3 + python-version: 3.13 + - name: Install pyflyby run: | - python -m pip install --upgrade pip setuptools wheel - python -m pip install --upgrade pyflakes flake8 mypy - python -m pip install types-six - pip install -e . + # Include build dependencies for run time; see + # https://mesonbuild.com/meson-python/how-to-guides/editable-installs.html#build-dependencies + # for details. + pip install meson-python meson ninja pybind11>=2.10.4 + pip install setuptools wheel # needed for epydoc + pip install --no-build-isolation -ve . --group lint - name: Mypy run: | mypy lib/python --ignore-missing-imports @@ -32,4 +29,3 @@ jobs: - name: Self-tidy-import run: | ./bin/tidy-imports -d lib/python/ tests/ - diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c3c745300..c31de3480 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -30,14 +30,17 @@ jobs: uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - - name: Install and update Python dependencies on Python 3 + - name: Install pyflyby run: | - python -m pip install --upgrade pip setuptools wheel - python -m pip install --upgrade "pexpect>=3.3" 'pytest<=8' rlipython 'ipykernel>=5.4.3' requests jupyter flaky 'notebook<6.1' wheel 'jupyter_console>=6.2' pytest-cov ipython coverage pytest-json-report hypothesis - pip install -e . + # Include build dependencies for run time; see + # https://mesonbuild.com/meson-python/how-to-guides/editable-installs.html#build-dependencies + # for details. + pip install meson-python meson ninja pybind11>=2.10.4 + pip install setuptools wheel # needed for epydoc + pip install --no-build-isolation -ve .[test] - name: test release build run: | - python setup.py sdist bdist_wheel + python -m build - name: compileall run: | python -We:invalid -m compileall -f -q lib/ etc/; @@ -62,10 +65,3 @@ jobs: name: pytest-timing-${{ matrix.os }}-${{ matrix.python-version }} path: ./report-*.json - uses: codecov/codecov-action@v5 - - name: Build docs - if: ${{ matrix.python-version == '3.11'}} - run: | - pip install sphinx sphinx_rtd_theme sphinx-autodoc-typehints - cd doc - make html - cd .. diff --git a/README.rst b/README.rst index f2d254925..28143910d 100644 --- a/README.rst +++ b/README.rst @@ -459,7 +459,7 @@ Emacs support * To get a ``M-x tidy-imports`` command in GNU Emacs, add to your ``~/.emacs``:: - (load "/path/to/pyflyby/lib/emacs/pyflyby.el") + (load "//pyflyby/share/emacs/site-lisp/pyflyby.el") - Pyflyby.el doesn't yet work with XEmacs; patches welcome. @@ -546,5 +546,3 @@ Release 8. Check/update https://github.com/conda-forge/pyflyby-feedstock for new pyflyby release on conda-forge - - diff --git a/doc/api/idents.rst b/doc/api/idents.rst index 7ce0170f7..bd4bf3a2c 100644 --- a/doc/api/idents.rst +++ b/doc/api/idents.rst @@ -2,4 +2,3 @@ _idents module ============== .. automodule:: pyflyby._idents :members: - :exclude-members: _my_iskeyword \ No newline at end of file diff --git a/doc/conf.py b/doc/conf.py index 835f38da3..365b01a79 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -2,6 +2,8 @@ # -- Path setup -------------------------------------------------------------- import os +import pathlib +import re import sys sys.path.insert(0, os.path.abspath('../lib/python')) sys.path.insert(0, os.path.abspath('..')) @@ -11,9 +13,21 @@ copyright = '2019, Karl Chen' author = 'Karl Chen' # The full version, including alpha/beta/rc tags -import pyflyby -release = pyflyby.__version__ +def find_version(): + # Extract version information via regex to avoid importing + project_root = pathlib.Path(__file__).parent.parent + with open(project_root / "lib" / "python" / "pyflyby" / "_version.py") as f: + version_match = re.search( + r"^__version__ = ['\"](?P.*)['\"]$", + f.read(), + re.M, + ) + if version_match: + return version_match.group("version") + raise RuntimeError("Unable to find version string.") + +release = find_version() # -- General configuration --------------------------------------------------- @@ -29,6 +43,12 @@ 'private-members': True } +autodoc_mock_imports = [ + "pyflyby._fast_iter_modules", + "appdirs", + "prompt_toolkit", +] + html_theme_options = { 'collapse_navigation': False, 'navigation_depth': -1, diff --git a/lib/python/pyflyby/__init__.py b/lib/python/pyflyby/__init__.py index 92fcc162a..5b3ee6565 100644 --- a/lib/python/pyflyby/__init__.py +++ b/lib/python/pyflyby/__init__.py @@ -31,6 +31,7 @@ unload_ipython_extension) from pyflyby._livepatch import livepatch, xreload from pyflyby._log import logger +from pyflyby._modules import rebuild_import_cache from pyflyby._parse import PythonBlock, PythonStatement from pyflyby._saveframe import saveframe from pyflyby._saveframe_reader \ diff --git a/lib/python/pyflyby/_idents.py b/lib/python/pyflyby/_idents.py index 7c1ca72fa..d086be49e 100644 --- a/lib/python/pyflyby/_idents.py +++ b/lib/python/pyflyby/_idents.py @@ -5,7 +5,7 @@ from functools import total_ordering -from keyword import kwlist +from keyword import iskeyword import re from pyflyby._util import cached_attribute, cmp @@ -13,12 +13,6 @@ from typing import Optional, Tuple, Dict -# Don't consider "print" a keyword, in order to be compatible with user code -# that uses "from __future__ import print_function". -_my_kwlist = list(kwlist) -_my_iskeyword = frozenset(_my_kwlist).__contains__ - - # TODO: use DottedIdentifier.prefixes def dotted_prefixes(dotted_name, reverse=False): """ @@ -114,7 +108,7 @@ def is_identifier(s: str, dotted: bool = False, prefix: bool = False): return is_identifier(s + '_', dotted=dotted, prefix=False) if dotted: return all(is_identifier(w, dotted=False) for w in s.split('.')) - return s.isidentifier() and not _my_iskeyword(s) + return s.isidentifier() and not iskeyword(s) def brace_identifiers(text): diff --git a/lib/python/pyflyby/_modules.py b/lib/python/pyflyby/_modules.py index 4a1b24a8a..209f80772 100644 --- a/lib/python/pyflyby/_modules.py +++ b/lib/python/pyflyby/_modules.py @@ -2,13 +2,22 @@ # Copyright (C) 2011, 2012, 2013, 2014, 2015 Karl Chen. # License: MIT http://opensource.org/licenses/MIT -from __future__ import print_function +from __future__ import annotations +import appdirs import ast from functools import cached_property, total_ordering +import hashlib +import importlib import itertools +import json import os +import pathlib +import pkgutil +import textwrap +from pyflyby._fast_iter_modules \ + import _iter_file_finder_modules from pyflyby._file import FileText, Filename from pyflyby._idents import DottedIdentifier, is_identifier from pyflyby._log import logger @@ -16,10 +25,11 @@ memoize, prefixes) import re +import shutil from six import reraise import sys import types -from typing import Any, Dict +from typing import Any, Dict, Generator, Union class ErrorDuringImportError(ImportError): """ @@ -29,6 +39,41 @@ class ErrorDuringImportError(ImportError): exist. """ +def rebuild_import_cache(): + """Force the import cache to be rebuilt. + + The cache is deleted before calling _fast_iter_modules, which repopulates the cache. + """ + for path in pathlib.Path( + appdirs.user_cache_dir(appname='pyflyby', appauthor=False) + ).iterdir(): + _remove_import_cache_dir(path) + _fast_iter_modules() + + +def _remove_import_cache_dir(path: pathlib.Path): + """Remove an import cache directory. + + Import cache directories exist in /pyflyby/, and they should + contain just a single file which itself contains a JSON blob of cached import names. + We therefore only delete the requested path if it is a directory. + + Parameters + ---------- + path : pathlib.Path + Import cache directory path to remove + """ + if path.is_dir(): + # Only directories are valid import cache entries + try: + shutil.rmtree(str(path)) + except Exception as e: + logger.error( + f"Failed to remove cache directory at {path} - please " + "consider removing this directory manually. Error:\n" + f"{textwrap.indent(str(e), prefix=' ')}" + ) + @memoize def import_module(module_name): @@ -280,32 +325,20 @@ def block(self): @staticmethod @memoize - def list(): - """ - Enumerate all top-level packages/modules. + def list() -> list[str]: + """Enumerate all top-level packages/modules. - :rtype: - ``tuple`` of `ModuleHandle` s + The current working directory is excluded for autoimporting; if we autoimported + random python scripts in the current directory, we could accidentally execute + code with side effects. + + Also exclude any module names that are not legal python module names (e.g. + "try.py" or "123.py"). + + :return: A list of all importable module names """ - import pkgutil - # Get the list of top-level packages/modules using pkgutil. - # We exclude "." from sys.path while doing so. Python includes "." in - # sys.path by default, but this is undesirable for autoimporting. If - # we autoimported random python scripts in the current directory, we - # could accidentally execute code with side effects. If the current - # working directory is /tmp, trying to enumerate modules there also - # causes problems, because there are typically directories there not - # readable by the current user. with ExcludeImplicitCwdFromPathCtx(): - modlist = pkgutil.iter_modules(None) - module_names = [t[1] for t in modlist] - # pkgutil includes all *.py even if the name isn't a legal python - # module name, e.g. if a directory in $PYTHONPATH has files named - # "try.py" or "123.py", pkgutil will return entries named "try" or - # "123". Filter those out. - module_names = [m for m in module_names if is_identifier(m)] - # Canonicalize. - return tuple(ModuleHandle(m) for m in sorted(set(module_names))) + return [mod.name for mod in _fast_iter_modules() if is_identifier(mod.name)] @cached_property def submodules(self): @@ -511,3 +544,98 @@ def containing(cls, identifier): module = cls(result) logger.debug("Imported %r to get %r", module, identifier) return module + + +def _format_path(path: Union[str, pathlib.Path]) -> str: + """Format a path for printing as a log message. + + If the path is a child of $HOME, the prefix is replaced with "~" for brevity. + Otherwise the original path is returned. + + Parameters + ---------- + path : Union[str, pathlib.Path] + Path to format + + Returns + ------- + str + Formatted output path + """ + path = pathlib.Path(path) + home = pathlib.Path.home() + + if path.is_relative_to(home): + return str(pathlib.Path("~").joinpath(path.relative_to(home))) + return str(path) + + +SUFFIXES = sorted(importlib.machinery.all_suffixes()) + + +def _cached_module_finder( + importer: importlib.machinery.FileFinder, prefix: str = "" +) -> Generator[tuple[str, bool], None, None]: + """Yield the modules found by the importer. + + The importer path's mtime is recorded; if the path and mtime have a corresponding + cache file, the modules recorded in the cache file are returned. Otherwise, the + cache is rebuilt. + + Parameters + ---------- + importer : importlib.machinery.FileFinder + FileFinder importer that points to a path under which imports can be found + prefix : str + String to affix to the beginning of each module name + + Returns + ------- + Generator[tuple[str, bool], None, None] + Tuples containing (prefix+module name, a bool indicating whether the module is a + package or not) + """ + cache_dir = pathlib.Path( + appdirs.user_cache_dir(appname='pyflyby', appauthor=False) + ) / hashlib.sha256(str(importer.path).encode()).hexdigest() + cache_file = cache_dir / str(os.stat(importer.path).st_mtime_ns) + + if cache_file.exists(): + with open(cache_file) as fp: + modules = json.load(fp) + else: + # Generate the cache dir if it doesn't exist, and remove any existing cache + # files for the given import path + cache_dir.mkdir(parents=True, exist_ok=True) + for path in cache_dir.iterdir(): + _remove_import_cache_dir(path) + + if os.environ.get("PYFLYBY_SUPPRESS_CACHE_REBUILD_LOGS", 0) != "1": + logger.info(f"Rebuilding cache for {_format_path(importer.path)}...") + + modules = _iter_file_finder_modules(importer, SUFFIXES) + with open(cache_file, 'w') as fp: + json.dump(modules, fp) + + for module, ispkg in modules: + yield prefix + module, ispkg + + +def _fast_iter_modules() -> Generator[pkgutil.ModuleInfo, None, None]: + """Return an iterator over all importable python modules. + + This function patches `pkgutil.iter_importer_modules` for + `importlib.machinery.FileFinder` types, causing `pkgutil.iter_importer_modules` to + call our own custom _iter_file_finder_modules instead of + pkgutil._iter_file_finder_modules. + + :return: The modules that are importable by python + """ + pkgutil.iter_importer_modules.register( # type: ignore[attr-defined] + importlib.machinery.FileFinder, _cached_module_finder + ) + yield from pkgutil.iter_modules() + pkgutil.iter_importer_modules.register( # type: ignore[attr-defined] + importlib.machinery.FileFinder, + pkgutil._iter_file_finder_modules, # type: ignore[attr-defined] + ) diff --git a/lib/python/pyflyby/meson.build b/lib/python/pyflyby/meson.build new file mode 100644 index 000000000..0d428135b --- /dev/null +++ b/lib/python/pyflyby/meson.build @@ -0,0 +1,34 @@ +py.install_sources( + [ + '__init__.py', + '__main__.py', + '_autoimp.py', + '_cmdline.py', + '_comms.py', + '_dbg.py', + '_docxref.py', + '_dynimp.py', + '_file.py', + '_flags.py', + '_format.py', + '_idents.py', + '_import_sorting.py', + '_importclns.py', + '_importdb.py', + '_imports2s.py', + '_importstmt.py', + '_interactive.py', + '_livepatch.py', + '_log.py', + '_modules.py', + '_parse.py', + '_py.py', + '_saveframe.py', + '_saveframe_reader.py', + '_util.py', + '_version.py', + 'autoimport.py', + 'importdb.py', + ], + subdir: 'pyflyby', +) diff --git a/meson.build b/meson.build new file mode 100644 index 000000000..9d68cbb34 --- /dev/null +++ b/meson.build @@ -0,0 +1,67 @@ +project( + 'pyflyby', + 'cpp', + version: '1.9.13', + default_options: 'cpp_std=c++17' +) + +py_mod = import('python') +py = py_mod.find_installation(pure: false) +pybind11_dep = dependency('pybind11', version: '>=2.10.4') +includes = include_directories([ 'src' ]) + +subdir('lib/python/pyflyby') + +install_data( + ['libexec/pyflyby/colordiff', 'libexec/pyflyby/diff-colorize'], + install_dir: py.get_install_dir( + subdir: 'pyflyby/libexec/pyflyby', + ) +) + +install_data( + [ + './etc/pyflyby/canonical.py', + './etc/pyflyby/common.py', + './etc/pyflyby/forget.py', + './etc/pyflyby/mandatory.py', + './etc/pyflyby/numpy.py', + './etc/pyflyby/std.py', + ], + install_dir: py.get_install_dir( + subdir: 'pyflyby/etc/pyflyby', + ) +) + +install_data( + ['./lib/emacs/pyflyby.el'], + install_dir: py.get_install_dir( + subdir: 'pyflyby/share/emacs/site-lisp', + ) +) + +install_data( + [ + 'bin/collect-exports', + 'bin/collect-imports', + 'bin/find-import', + 'bin/list-bad-xrefs', + 'bin/prune-broken-imports', + 'bin/pyflyby-diff', + 'bin/reformat-imports', + 'bin/replace-star-imports', + 'bin/saveframe', + 'bin/tidy-imports', + 'bin/transform-imports', + ], + install_dir: get_option('bindir') +) + +py.extension_module( + '_fast_iter_modules', + 'src/_fast_iter_modules.cpp', + install: true, + subdir: 'pyflyby', + dependencies: [pybind11_dep], + include_directories: includes +) diff --git a/pyproject.toml b/pyproject.toml index 21324216f..f9c74c39e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,79 @@ [build-system] -requires = ["setuptools >= 61.0"] -build-backend = "setuptools.build_meta" +requires = [ + "meson-python>=0.18.0", + "pybind11>=2.10.4", +] +build-backend = "mesonpy" + +[project] +name = "pyflyby" +dynamic = ["version"] +authors = [ + { name = "Karl Chen", email = "quarl@8166.clguba.z.quarl.org" }, +] +description = "pyflyby - Python development productivity tools, in particular automatic import management" +license = "MIT" +license-files = ["LICENSE.txt"] +readme = "README.rst" +requires-python = ">=3.9" +classifiers = [ + "Programming Language :: Python", + "Topic :: Software Development", + "Topic :: Software Development :: Code Generators", + "Topic :: Software Development :: Interpreters", + "Intended Audience :: Developers", + "Operating System :: OS Independent", +] +dependencies = [ + "black", + "six", + "tomli; python_version<'3.11'", + "typing_extensions>=4.6; python_version<'3.12'", + 'prompt_toolkit', + 'epydoc', + 'wheel', # required by epydoc, but not listed as a dependency + 'appdirs', +] +[project.urls] +Homepage = "https://pypi.org/project/pyflyby/" +Documentation = "https://deshaw.github.io/pyflyby/" +Source = "https://github.com/deshaw/pyflyby" + +[project.scripts] +py = "pyflyby._py:py_main" +py3 = "pyflyby._py:py_main" + +[project.optional-dependencies] +test = [ + 'build', + 'coverage', + 'flaky', + 'hypothesis', + 'ipykernel>=5.4.3', + 'ipython', + 'jupyter', + 'jupyter_console>=6.2', + 'notebook<6.1', + 'pexpect>=3.3', + 'pytest-cov', + 'pytest-json-report', + 'pytest<=8', + 'requests', + 'rlipython', +] + +[dependency-groups] +lint = [ + 'flake8', + 'mypy', + 'pyflakes', + 'types-six', +] +docs = [ + 'sphinx', + 'sphinx_rtd_theme', + 'sphinx-autodoc-typehints', +] [tool.mypy] files = ["lib"] diff --git a/setup.py b/setup.py deleted file mode 100755 index 482d590cc..000000000 --- a/setup.py +++ /dev/null @@ -1,239 +0,0 @@ -#!/usr/bin/env python - -# pyflyby/setup.py. - -# License for THIS FILE ONLY: CC0 Public Domain Dedication -# http://creativecommons.org/publicdomain/zero/1.0/ - - - -import glob -import os -import re -from setuptools import Command, setup -from setuptools.command.test import test as TestCommand -from setuptools.command.sdist import sdist as SdistCommand -import subprocess -import sys -from textwrap import dedent - - -PYFLYBY_HOME = os.path.abspath(os.path.dirname(__file__)) -PYFLYBY_PYPATH = os.path.join(PYFLYBY_HOME, "lib/python") -PYFLYBY_DOT_PYFLYBY = os.path.join(PYFLYBY_HOME, ".pyflyby") - -# Get the pyflyby version from pyflyby.__version__. -# We use exec instead to avoid importing pyflyby here. -version_vars = {} -version_fn = os.path.join(PYFLYBY_PYPATH, "pyflyby/_version.py") -exec(open(version_fn).read(), {}, version_vars) -version = version_vars["__version__"] - - -def read(fname): - with open(os.path.join(PYFLYBY_HOME, fname)) as f: - return f.read() - - -def list_python_source_files(): - results = [] - for fn in glob.glob("bin/*"): - if not os.path.isfile(fn): - continue - with open(fn) as f: - line = f.readline() - if not re.match("^#!.*python", line): - continue - results.append(fn) - results += glob.glob("lib/python/pyflyby/*.py") - results += glob.glob("tests/*.py") - return results - - -class TidyImports(Command): - description = "tidy imports in pyflyby source files (for maintainer use)" - - user_options = [] - - def initialize_options(self): - pass - - def finalize_options(self): - pass - - def run(self): - files = list_python_source_files() - pyflyby_path = ":".join([ - os.path.join(PYFLYBY_HOME, "etc/pyflyby"), - PYFLYBY_DOT_PYFLYBY, - ]) - subprocess.call([ - "env", - "PYFLYBY_PATH=%s" % (pyflyby_path,), - "tidy-imports", - # "--debug", - "--uniform", - ] + files) - - -class CollectImports(Command): - description = "update pyflyby's own .pyflyby file from imports (for maintainer use)" - - user_options = [] - - def initialize_options(self): - pass - - def finalize_options(self): - pass - - def run(self): - files = list_python_source_files() - print("Rewriting", PYFLYBY_DOT_PYFLYBY) - with open(PYFLYBY_DOT_PYFLYBY, 'w') as f: - print(dedent(""" - # -*- python -*- - # - # This is the imports database file for pyflyby itself. - # - # To regenerate this file, run: setup.py collect_imports - - __mandatory_imports__ = [ - 'from __future__ import print_function', - ] - """).lstrip(), file=f) - f.flush() - subprocess.call( - [ - os.path.join(PYFLYBY_HOME, "bin/collect-imports"), - "--include=pyflyby", - "--uniform", - ] + files, - stdout=f) - subprocess.call(["git", "diff", PYFLYBY_DOT_PYFLYBY]) - - -class PyTest(TestCommand): - user_options = [('pytest-args=', 'a', "Arguments to pass to py.test")] - - def initialize_options(self): - TestCommand.initialize_options(self) - self.pytest_args = ['--doctest-modules', 'lib', 'tests'] - - def finalize_options(self): - TestCommand.finalize_options(self) - self.test_args = [] - self.test_suite = True - - def run_tests(self): - import pytest - # We want to test the version of pyflyby in this repository. It's - # possible that some different version of pyflyby already got imported - # in usercustomize, before we could set sys.path here. If so, unload - # it. - if 'pyflyby' in sys.modules: - print("setup.py: Unloading %s from sys.modules " - "(perhaps it got loaded in usercustomize?)" - % (sys.modules['pyflyby'].__file__,)) - del sys.modules['pyflyby'] - for k in sys.modules.keys(): - if k.startswith("pyflyby."): - del sys.modules[k] - # Add our version of pyflyby to sys.path & PYTHONPATH. - sys.path.insert(0, PYFLYBY_PYPATH) - os.environ["PYTHONPATH"] = PYFLYBY_PYPATH - # Run pytest. - errno = pytest.main(self.pytest_args) - sys.exit(errno) - - -DISALLOWED_CONTENT = ['g'+'uas', 'g'+'ql'] - -def check_for_disallowed_content(archive_filename): - archive_filename = os.path.abspath(archive_filename) - assert archive_filename.endswith(".tar.gz") - archive_members = subprocess.check_output(['tar', 'tzf', archive_filename]) - archive_file_content = subprocess.check_output(['tar', 'xzOf', archive_filename]) - data = archive_members + archive_file_content.lower() - for disallowed in DISALLOWED_CONTENT: - if disallowed.encode("ascii") in data: - raise ValueError("Found match for content that shouldn't be source-disted: %s" - % (disallowed,)) - - -class SdistAndCheck(SdistCommand, object): - - def make_distribution(self): - super(SdistAndCheck, self).make_distribution() - for filename in self.archive_files: - check_for_disallowed_content(filename) - - -setup( - name = "pyflyby", - version = version, - author = "Karl Chen", - author_email = "quarl@8166.clguba.z.quarl.org", - description = ("pyflyby - Python development productivity tools, in particular automatic import management"), - license = "MIT", - keywords = "pyflyby py autopython autoipython productivity automatic imports autoimporter tidy-imports", - url = "https://pypi.org/project/pyflyby/", - project_urls={ - 'Documentation': 'https://deshaw.github.io/pyflyby/', - 'Source' : 'https://github.com/deshaw/pyflyby', - }, - package_dir={'': 'lib/python'}, - packages=['pyflyby'], - entry_points={'console_scripts': - '\n'.join([ - 'py=pyflyby._py:py_main', - 'py3=pyflyby._py:py_main', - ])}, - scripts=[ - # TODO: convert these scripts into entry points (but leave stubs in - # bin/ for non-installed usage) - 'bin/collect-exports', - 'bin/collect-imports', - 'bin/find-import', - 'bin/list-bad-xrefs', - 'bin/prune-broken-imports', - 'bin/pyflyby-diff', - 'bin/reformat-imports', - 'bin/replace-star-imports', - 'bin/saveframe', - 'bin/tidy-imports', - 'bin/transform-imports', - ], - data_files=[ - ('libexec/pyflyby', [ - 'libexec/pyflyby/colordiff', 'libexec/pyflyby/diff-colorize', - ]), - ('etc/pyflyby', glob.glob('etc/pyflyby/*.py')), - ('share/doc/pyflyby', glob.glob('doc/*.txt')), - ('share/emacs/site-lisp', ['lib/emacs/pyflyby.el']), - ], - long_description=read('README.rst'), - classifiers=[ - "Development Status :: 5 - Production/Stable", - "Topic :: Software Development", - "Topic :: Software Development :: Code Generators", - "Topic :: Software Development :: Interpreters", - "Intended Audience :: Developers", - "License :: OSI Approved :: MIT License", - "Programming Language :: Python", - ], - install_requires=[ - "six", - "tomli; python_version<'3.11'", - "black", - "typing_extensions>=4.6; python_version<'3.12'" - ], - python_requires=">3.9, <4", - tests_require=['pexpect>=3.3', 'pytest', 'epydoc', 'rlipython', 'requests'], - cmdclass = { - 'test' : PyTest, - 'sdist' : SdistAndCheck, - 'collect_imports': CollectImports, - 'tidy_imports' : TidyImports, - }, -) diff --git a/src/_fast_iter_modules.cpp b/src/_fast_iter_modules.cpp new file mode 100644 index 000000000..a8e36f439 --- /dev/null +++ b/src/_fast_iter_modules.cpp @@ -0,0 +1,98 @@ +#include "pybind11/cast.h" +#include "pybind11/pytypes.h" +#include +#include +#include +#include +#include +#include + +namespace py = pybind11; +namespace fs = std::filesystem; +using namespace pybind11::literals; + +/** + * @brief Fast equivalent of `inspect.getmodulename`. + * + * @param path Path to a file + * @param suffixes Suffixes of valid python modules. Typically this is + * `importlib.machinery.all_suffixes()` + * @return The stem of the file, if this is a module; empty string otherwise + */ +std::string getmodulename(fs::path path, std::vector suffixes) { + fs::path ext = path.extension(); + for (auto const& suffix : suffixes) { + std::string path_str = path.string(); + std::string::size_type pos = path_str.rfind(suffix); + if (pos != std::string::npos) { + return path_str.substr(0, pos); + } + } + return ""; +} + +/** + * @brief Get a list of importable python modules. + * + * See `pkgutil._iter_file_finder_modules` for the original python version. + * + * @param importer Importer instance containing an import path. Typically this is an object of type + * `importlib.machinery.FileFinder` + * @param suffixes Suffixes of valid python modules. Typically this is + * `importlib.machinery.all_suffixes()` + * @return A vector of tuples containing modules names, and a boolean indicating whether the module + * is a package or not + */ +std::vector> +_iter_file_finder_modules( + py::object importer, + std::vector suffixes +) { + std::vector> ret; + + // The importer doesn't have a path + py::object path_obj = importer.attr("path"); + if (path_obj.is_none()) { + return ret; + } + + // The importer's path isn't an existing directory + fs::path path = fs::path(py::str(path_obj).cast()); + if (!fs::is_directory(path) || !fs::exists(path)) { + return ret; + } + + for (auto const &entry : fs::directory_iterator(path)) { + fs::path entry_path = entry.path(); + fs::path filename = entry_path.filename(); + std::string modname = getmodulename(filename, suffixes); + + + if (modname == "" && fs::is_directory(entry_path) && + filename.string().find(".") == std::string::npos && + fs::is_regular_file(entry_path / "__init__.py") // Is this a package? + ) { + ret.push_back(std::make_tuple(filename.string(), true)); + } else if (modname == "__init__") { + continue; + } else if (modname != "" && modname.find(".") == std::string::npos) { + ret.push_back(std::make_tuple(modname, + false // This is definitely not a package + )); + } + } + + return ret; +} + +PYBIND11_MODULE(_fast_iter_modules, m) { + m.doc() = "A fast version of pkgutil._iter_file_finder_modules."; + m.def( + "_iter_file_finder_modules", + &_iter_file_finder_modules, + "A fast implementation of pkgutil._iter_file_finder_modules(importer, prefix='')", + py::arg("importer"), + py::arg("suffixes") = std::make_tuple(".py", ".pyc"), + py::return_value_policy::take_ownership + ); +} diff --git a/tests/test_interactive.py b/tests/test_interactive.py index 5475cfaa7..ced14f570 100644 --- a/tests/test_interactive.py +++ b/tests/test_interactive.py @@ -859,6 +859,7 @@ def IPythonCtx(prog="ipython", try: # Prepare environment variables. env = {} + env["PYFLYBY_SUPPRESS_CACHE_REBUILD_LOGS"] = "1" env["PYFLYBY_PATH"] = PYFLYBY_PATH env["PYFLYBY_LOG_LEVEL"] = PYFLYBY_LOG_LEVEL env["PYTHONPATH"] = _build_pythonpath(PYTHONPATH) @@ -2377,8 +2378,8 @@ def f_76313558_59577191(): return 'ok' """) ipython(""" In [1]: import pyflyby; pyflyby.enable_auto_importer() - In [2]: m51145108_\tfoo.f_76313558_\t [PYFLYBY] import m51145108_foo + In [2]: m51145108_\tfoo.f_76313558_\t In [2]: m51145108_foo.f_76313558_59577191() Out[2]: 'ok' """, PYTHONPATH=tmp.dir, frontend=frontend) @@ -2470,14 +2471,24 @@ def test_complete_symbol_eval_1(evaluation): @pytest.mark.skipif(_SUPPORTS_TAB_AUTO_IMPORT, reason='Autoimport on Tab requires IPython 9.3+') @pytest.mark.parametrize('evaluation', _TESTED_EVALUATION_SETTINGS) def test_complete_symbol_eval_autoimport_1(frontend, evaluation): - ipython(f""" + template = """ In [1]: import pyflyby; pyflyby.enable_auto_importer() - In [2]: %config IPCompleter.{evaluation} - In [3]: os.sep.strip().lst\t - [PYFLYBY] import os - In [3]: os.sep.strip().lst\trip + In [2]: %config IPCompleter.{0} + In [3]: os.sep.strip().lst\t{1} Out[3]: - """, frontend=frontend) + """ + + scenario_a = """ + [PYFLYBY] import os + In [3]: os.sep.strip().lst\trip""" + scenario_b = """ + [PYFLYBY] import os + In [3]: os.sep.strip().lstrip""" + + try: + ipython(template.format(evaluation, scenario_a), frontend=frontend) + except pytest.fail.Exception: + ipython(template.format(evaluation, scenario_b), frontend=frontend) @pytest.mark.skipif(_SUPPORTS_TAB_AUTO_IMPORT, reason='Autoimport on Tab requires IPython 9.3+') diff --git a/tests/test_modules.py b/tests/test_modules.py index 1a111fbcd..0b74ea249 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -6,10 +6,15 @@ +from unittest import mock +import pathlib +import hashlib import logging.handlers from pyflyby._file import Filename from pyflyby._idents import DottedIdentifier -from pyflyby._modules import ModuleHandle +from pyflyby._log import logger +from pyflyby._modules import ModuleHandle, _fast_iter_modules, _iter_file_finder_modules +from pkgutil import iter_modules import re import subprocess import sys @@ -110,3 +115,85 @@ def test_filename_noload_1(modname): assert ret.returncode != 121, f"{modname} imported by pyflyby import" assert ret.returncode != 120, f"{modname} in sys.modules at startup" assert ret.returncode == 0, (ret, ret.stdout, ret.stderr) + + +def test_fast_iter_modules(): + """Test that the cpp extension finds the same modules as pkgutil.iter_modules.""" + fast = sorted(list(_fast_iter_modules()), key=lambda x: x.name) + slow = sorted(list(iter_modules()), key=lambda x: x.name) + + assert fast == slow + + +@mock.patch("appdirs.user_cache_dir") +def test_import_cache(mock_user_cache_dir, tmp_path): + """Test that the import cache is built when iterating modules. + + Also: + - Check that each path mentioned in the logs appears (sha256-encoded) in the cache + - The first time generating the import cache, _iter_file_finder_modules is called + - Subsequent calls use the cached modules + - If the mtime of one of the importer paths is updated, the corresponding + cache file gets regenerated + """ + + mock_user_cache_dir.return_value = tmp_path + + assert len(list(tmp_path.iterdir())) == 0 + with ( + mock.patch("pyflyby._modules.logger", wraps=logger) as mock_logger, + mock.patch( + "pyflyby._modules._iter_file_finder_modules", + wraps=_iter_file_finder_modules, + ) as mock_iffm, + ): + list(_fast_iter_modules()) + + paths = [str(path.name) for path in tmp_path.iterdir()] + n_cached_paths = len(paths) + n_log_messages = len(mock_logger.info.call_args_list) + + # On the first call, log messages should be generated for each import path. Check + # that _iter_file_finder_modules was called once for each cached path. + assert (n_cached_paths == n_log_messages) and n_cached_paths > 0 + assert len(mock_iffm.call_args_list) == n_cached_paths + assert "Rebuilding cache for " in mock_logger.info.call_args.args[0] + for call_args in mock_logger.info.call_args_list: + # Grab the path names from the log messages; make sure the sha256 checksum + # can be found in the paths of the cache directory + path = pathlib.Path( + call_args.args[0].lstrip("Rebuilding cache for ").rstrip("...") + ).expanduser() + assert hashlib.sha256(str(path).encode()).hexdigest() in paths + + with ( + mock.patch("pyflyby._modules.logger", wraps=logger) as mock_logger, + mock.patch( + "pyflyby._modules._iter_file_finder_modules", + wraps=_iter_file_finder_modules, + ) as mock_iffm, + ): + list(_fast_iter_modules()) + + # On the second call, no additional messages should be emitted because the cache has + # already been built. Check that _iter_file_finder_modules was never called. + n_log_messages = len(mock_logger.info.call_args_list) + assert n_log_messages == 0 + mock_iffm.assert_not_called() + + # Update the mtime of one of the importer paths + path.touch() + with ( + mock.patch("pyflyby._modules.logger", wraps=logger) as mock_logger, + mock.patch( + "pyflyby._modules._iter_file_finder_modules", + wraps=_iter_file_finder_modules, + ) as mock_iffm, + ): + list(_fast_iter_modules()) + + # Only one path should have been updated and only 1 message logged. The number + # of cache directories should not change. + assert len(mock_logger.info.call_args_list) == 1 + assert len(list(tmp_path.iterdir())) == n_cached_paths + mock_iffm.assert_called_once()