From 2f9bd84a0a9ed5181af2fc84a47298fd9934fbc4 Mon Sep 17 00:00:00 2001 From: Matthew Broadway Date: Thu, 29 Feb 2024 19:14:57 +0000 Subject: [PATCH] introduced markdown linter --- .markdownlint.yaml | 19 ++++++++ .pre-commit-config.yaml | 4 ++ Code-of-Conduct.md | 24 +++++----- Contributing.md | 35 ++++++++++---- README.md | 26 ++++++++--- docs/advanced.md | 1 + docs/basics.md | 19 +++++--- docs/reloading.md | 80 +++++++++++++++++--------------- tests/README.md | 16 +++++-- tests/package_resolver/README.md | 6 ++- 10 files changed, 150 insertions(+), 80 deletions(-) create mode 100644 .markdownlint.yaml diff --git a/.markdownlint.yaml b/.markdownlint.yaml new file mode 100644 index 0000000..34132bb --- /dev/null +++ b/.markdownlint.yaml @@ -0,0 +1,19 @@ +default: true + +MD003: + style: atx +MD004: + style: dash +MD007: + indent: 4 +MD013: + line_length: 120 + code_block_line_length: 120 +MD046: + style: fenced +MD048: + style: backtick +MD049: + style: asterisk +MD050: + style: asterisk diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 84496c5..9171d6e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -24,3 +24,7 @@ repos: rev: v2.2.6 hooks: - id: codespell + - repo: https://github.com/igorshubovych/markdownlint-cli + rev: v0.39.0 + hooks: + - id: markdownlint-fix diff --git a/Code-of-Conduct.md b/Code-of-Conduct.md index a5e3040..86fe59f 100644 --- a/Code-of-Conduct.md +++ b/Code-of-Conduct.md @@ -14,21 +14,21 @@ religion, or sexual identity and orientation. Examples of behavior that contributes to creating a positive environment include: -* Using welcoming and inclusive language -* Being respectful of differing viewpoints and experiences -* Gracefully accepting constructive criticism -* Focusing on what is best for the community -* Showing empathy towards other community members +- Using welcoming and inclusive language +- Being respectful of differing viewpoints and experiences +- Gracefully accepting constructive criticism +- Focusing on what is best for the community +- Showing empathy towards other community members Examples of unacceptable behavior by participants include: -* The use of sexualized language or imagery and unwelcome sexual attention or +- The use of sexualized language or imagery and unwelcome sexual attention or advances -* Trolling, insulting/derogatory comments, and personal or political attacks -* Public or private harassment -* Publishing others' private information, such as a physical or electronic +- Trolling, insulting/derogatory comments, and personal or political attacks +- Public or private harassment +- Publishing others' private information, such as a physical or electronic address, without explicit permission -* Other conduct which could reasonably be considered inappropriate in a +- Other conduct which could reasonably be considered inappropriate in a professional setting ## Our Responsibilities @@ -55,7 +55,7 @@ further defined and clarified by project maintainers. ## Enforcement Instances of abusive, harassing, or otherwise unacceptable behavior may be -reported by contacting the project team at konstin@mailbox.org. All +reported by contacting the project team at . All complaints will be reviewed and investigated and will result in a response that is deemed necessary and appropriate to the circumstances. The project team is obligated to maintain confidentiality with regard to the reporter of an incident. @@ -68,6 +68,6 @@ members of the project's leadership. ## Attribution This Code of Conduct is adapted from the [Contributor Covenant][homepage], version 1.4, -available at https://www.contributor-covenant.org/version/1/4/code-of-conduct.html +available at [homepage]: https://www.contributor-covenant.org diff --git a/Contributing.md b/Contributing.md index 5dd7cce..f1ce0d1 100644 --- a/Contributing.md +++ b/Contributing.md @@ -1,28 +1,45 @@ # Contributing + Thank you for your interest in contributing to maturin_import_hook. All are welcome! Please consider reading the [Code of Conduct](https://github.com/PyO3/maturin-import-hook/blob/main/Code-of-Conduct.md) to keep our community positive and inclusive. ## Getting Started Contributing -- we use the [github issue tracker](https://github.com/PyO3/maturin-import-hook/issues) and [discussions](https://github.com/PyO3/maturin-import-hook/discussions) to keep track of bugs and ideas + +- we use the [github issue tracker](https://github.com/PyO3/maturin-import-hook/issues) and + [discussions](https://github.com/PyO3/maturin-import-hook/discussions) to keep track of bugs and ideas ### Setting up a development environment 1. Install rust (eg using `rustup`) and maturin (eg `pipx install maturin`) 2. Clone the repository - if you are looking to submit a PR, create a fork on github and clone your fork -3. Install [pre-commit](https://pre-commit.com/) (eg `pipx install pre-commit`) then run `pre-commit install` in the repo root -4. See [tests/README.md](https://github.com/PyO3/maturin-import-hook/blob/main/tests/README.md) for instructions on how best to run the test suite and some other miscellaneous instructions for debugging and carrying out maintenance tasks. +3. Install [pre-commit](https://pre-commit.com/) (eg `pipx install pre-commit`) + then run `pre-commit install` in the repo root +4. See [tests/README.md](https://github.com/PyO3/maturin-import-hook/blob/main/tests/README.md) for instructions + on how best to run the test suite and some other miscellaneous instructions for debugging and + carrying out maintenance tasks. Tips: + - [pyenv](https://github.com/pyenv/pyenv) may be useful for installing a specific python interpreter -- Virtual machines such as [VirtualBox](https://www.virtualbox.org/) and [OSX-KVM](https://github.com/kholia/OSX-KVM) are useful for running tests on specific platforms locally. Or you can use the test pipeline on github actions. +- Virtual machines such as [VirtualBox](https://www.virtualbox.org/) and [OSX-KVM](https://github.com/kholia/OSX-KVM) + are useful for running tests on specific platforms locally. Or you can use the test pipeline on github actions. + +## Continuous Integration -## Writing Pull Requests +The maturin_import_hook repo uses [GitHub actions](https://github.com/PyO3/maturin-import-hook/actions). +PRs are blocked from merging if the CI is not successful. -### Continuous Integration -The maturin_import_hook repo uses [GitHub actions](https://github.com/PyO3/maturin-import-hook/actions). PRs are blocked from merging if the CI is not successful. +You can run the test pipeline on your fork from the 'Actions' tab of your fork. The pipeline takes several arguments +when run manually that you can use to narrow down what is run so you can iterate faster when trying to solve a +particular issue. The pipeline uploads the test results as html reports that can be downloaded and examined. +This is generally easier than sifting through the raw logs. -You can run the test pipeline on your fork from the 'Actions' tab of your fork. The pipeline takes several arguments when run manually that you can use to narrow down what is run so you can iterate faster when trying to solve a particular issue. The pipeline uploads the test results as html reports that can be downloaded and examined. This is generally easier than sifting through the raw logs. +Linting and type-checking is enforced in the repo using [pre-commit](https://pre-commit.com/). +See `.pre-commit-config.yaml` for the checks that are performed and `pyproject.toml` for the configuration +of those linters. -Linting and type-checking is enforced in the repo using [pre-commit](https://pre-commit.com/). See `.pre-commit-config.yaml` for the checks that are performed and `pyproject.toml` for the configuration of those linters. The configuration starts with all `ruff` lints enabled with a list of specifically disabled lints. If you are writing new code that is triggering a lint that you think ought to be disabled, you can suggest this in a PR, but generally stick to conforming to the suggested linter rules. +The configuration starts with all `ruff` lints enabled with a list of specifically disabled lints. +If you are writing new code that is triggering a lint that you think ought to be disabled, you can suggest this in +a PR, but generally stick to conforming to the suggested linter rules. diff --git a/README.md b/README.md index 5e1a90f..8b344d6 100644 --- a/README.md +++ b/README.md @@ -5,20 +5,27 @@ A python import hook to automatically rebuild [maturin](https://www.maturin.rs/) Using this import hook reduces friction when developing mixed python/rust codebases because changes made to rust components take effect automatically like changes to python components do. -The import hook also provides conveniences such as [importlib.reload()](https://docs.python.org/3/library/importlib.html#importlib.reload) support for maturin projects. +The import hook also provides conveniences such as +[importlib.reload()](https://docs.python.org/3/library/importlib.html#importlib.reload) support for maturin projects. ## Usage + After installing `maturin`, install the import hook into a python virtual environment with: + ```shell -$ pip install maturin_import_hook +pip install maturin_import_hook ``` + Then, optionally make the hook activate automatically with: + ```shell -$ python -m maturin_import_hook site install +python -m maturin_import_hook site install ``` + This only has to be run once for each virtual environment. Alternatively, put the following at the top of each python script where you want to use the hook: + ```python import maturin_import_hook @@ -41,16 +48,19 @@ Once the hook is active, any `import` statement that imports an editable-install automatically rebuilt if necessary before it is imported. ## CLI + The package provides a CLI interface for getting information such as the location and size of the build cache and managing the installation into `sitecustomize.py`. For details, run: + ```shell -$ python -m maturin_import_hook --help +python -m maturin_import_hook --help ``` - ## Debugging + If you encounter a problem, a good way to learn more about what is going on is to enable the debug logging for the import hook. This can be done by putting the following lines above the import that is causing an issue: + ```python # configure logging if you haven't already (make sure DEBUG level is visible) logging.basicConfig(format="%(name)s [%(levelname)s] %(message)s", level=logging.DEBUG) @@ -63,7 +73,9 @@ import some_package Licensed under either of: - * Apache License, Version 2.0, ([LICENSE-APACHE](https://github.com/PyO3/maturin-import-hook/blob/main/license-apache) or http://www.apache.org/licenses/LICENSE-2.0) - * MIT license ([LICENSE-MIT](https://github.com/PyO3/maturin-import-hook/blob/main/license-mit) or http://opensource.org/licenses/MIT) +- Apache License, Version 2.0, ([LICENSE-APACHE](https://github.com/PyO3/maturin-import-hook/blob/main/license-apache) + or ) +- MIT license ([LICENSE-MIT](https://github.com/PyO3/maturin-import-hook/blob/main/license-mit) + or ) at your option. diff --git a/docs/advanced.md b/docs/advanced.md index f70c016..652ac2b 100644 --- a/docs/advanced.md +++ b/docs/advanced.md @@ -2,6 +2,7 @@ The import hook classes can be subclassed to further customize to specific use cases. For example settings can be configured per-project or loaded from configuration files. + ```python import sys from pathlib import Path diff --git a/docs/basics.md b/docs/basics.md index 2808d7f..44ef1cc 100644 --- a/docs/basics.md +++ b/docs/basics.md @@ -11,9 +11,10 @@ layouts as well as importing standalone `.rs` files. ## Installation Install into a virtual environment then install so that the import hook is always active. + ```shell -$ pip install maturin_import_hook -$ python -m maturin_import_hook site install # install into the active environment +pip install maturin_import_hook +python -m maturin_import_hook site install # install into the active environment ``` Alternatively, instead of using `site install`, put calls to `maturin_import_hook.install()` into any script where you @@ -40,6 +41,7 @@ import subpackage.my_rust_script ``` The maturin project importer and the rust file importer can be used separately + ```python from maturin_import_hook import rust_file_importer rust_file_importer.install() @@ -49,6 +51,7 @@ project_importer.install() ``` The import hook can be configured to control its behaviour + ```python import maturin_import_hook from maturin_import_hook.settings import MaturinSettings @@ -77,6 +80,7 @@ Installation into `sitecustomize.py` can be managed with the import hook cli usi `python -m maturin_import_hook site install`. The CLI can also manage uninstallation. ## Environment Variables + The import hook can be disabled by setting `MATURIN_IMPORT_HOOK_ENABLED=0`. This can be used to disable the import hook in production if you want to leave calls to `import_hook.install()` in place. @@ -84,15 +88,15 @@ Build files will be stored in an appropriate place for the current system but ca by setting `MATURIN_BUILD_DIR`. These files can be deleted without causing any issues (unless a build is in progress). The precedence for storing build files is: -* `MATURIN_BUILD_DIR` -* `/maturin_build_cache` -* `/maturin_build_cache` - * e.g. `~/.cache/maturin_build_cache` on POSIX +- `MATURIN_BUILD_DIR` +- `/maturin_build_cache` +- `/maturin_build_cache` + - e.g. `~/.cache/maturin_build_cache` on POSIX See the location being used with the CLI: `python -m maturin_import_hook cache info` - ## Logging + By default the `maturin_import_hook` logger does not propagate to the root logger. This is so that `INFO` level messages are shown to the user without them having to configure logging (`INFO` level is normally not visible). The import hook also has extensive `DEBUG` level logging that generally would be more noise than useful. So by not propagating, `DEBUG` @@ -104,6 +108,7 @@ the messages as normal. When debugging issues with the import hook, you should first call `reset_logger()` then configure the root logger to show `DEBUG` messages. You can also run with the environment variable `RUST_LOG=maturin=debug` to get more information from maturin. + ```python import logging logging.basicConfig(format='%(name)s [%(levelname)s] %(message)s', level=logging.DEBUG) diff --git a/docs/reloading.md b/docs/reloading.md index ff39885..eeeab30 100644 --- a/docs/reloading.md +++ b/docs/reloading.md @@ -4,24 +4,23 @@ This document outlines the implementation details of how the import hook support perspective the important thing is that `importlib.reload()` should do what you expect (with some caveats) when the import hook is active - -# Reloading Details +## Reloading Details Regular python modules can be reloaded with [`importlib.reload()`](https://docs.python.org/3/library/importlib.html#importlib.reload) to load changes to the source code without restarting the python interpreter. This mechanism is used by the [`%autoreload` IPython extension](https://ipython.readthedocs.io/en/stable/config/extensions/autoreload.html) to automatically trigger reloads whenever something from the module is accessed. -Reloading modules should generally be avoided in production, but may be useful in some situations such as prototyping and interactive -development like Jupyter notebooks. These are situations where import hooks are also useful so supporting reloading would be -beneficial. +Reloading modules should generally be avoided in production, but may be useful in some situations such as prototyping +and interactive development like Jupyter notebooks. These are situations where import hooks are also useful so +supporting reloading would be beneficial. As [PEP-489](https://peps.python.org/pep-0489/#module-reloading) mentions, calling `importlib.reload()` on an extension module does not reload the module even if it has changed: -> Reloading an extension module using importlib.reload() will continue to have no effect, except re-setting import-related attributes. -> Due to limitations in shared library loading (both dlopen on POSIX and LoadModuleEx on Windows), -> it is not generally possible to load a modified library after it has changed on disk. +> Reloading an extension module using importlib.reload() will continue to have no effect, except re-setting +> import-related attributes. Due to limitations in shared library loading (both dlopen on POSIX and +> LoadModuleEx on Windows), it is not generally possible to load a modified library after it has changed on disk. Some import hooks such as [`pyximport` (for cython)](https://github.com/cython/cython/blob/master/pyximport/pyximport.py) support reloading by compiling to a different path each time the module is loaded. @@ -30,34 +29,36 @@ when `importlib.reload()` is called, import hooks get called to find the spec fo being reloaded. The hook can identify the reload case by whether the module being queried is already in `sys.modules` or by whether the `target` argument to `importlib.abc.MetaPathFinder.find_spec()` is not `None`. +### Project Importer - -## Project Importer -When `importlib.reload()` is called on a maturin package the default behaviour *without the import hook* is that the python module -(eg `my_project/__init__.py`) gets reloaded, but modules inside the package (like the compiled extension module) do -not get reloaded. calling `importlib.reload()` on the extension module directly also has no effect due to the -reasons outlined above. +When `importlib.reload()` is called on a maturin package the default behaviour *without the import hook* is that the +python module (eg `my_project/__init__.py`) gets reloaded, but modules inside the package (like the compiled +extension module) do not get reloaded. calling `importlib.reload()` on the extension module directly also has no +effect due to the reasons outlined above. when `enable_reloading = True`, the import hook triggers a rebuild like normal, but then unloads submodules any extension modules package by deleting them from `sys.modules` and creates a symlink of the project and returns a module spec pointing to the symlink. This is enough to trigger the python interpreter to load the extension module again. -This behaviour doesn't follow [the semantics of `importlib.reload()`](https://docs.python.org/3/library/importlib.html#importlib.reload) exactly: +This behaviour doesn't follow +[the semantics of `importlib.reload()`](https://docs.python.org/3/library/importlib.html#importlib.reload) exactly: - Reloading the package root module should leave submodules unaffected - The module being reloaded is not supposed to be re-initialised and the global data for the module is supposed to persist. - each time the package reloads the extension module is located at a different place on the filesystem. This is necessary for the workaround to function -But hopefully the chosen behaviour is more useful for general use. Reloading can always be disabled if it causes problems in some edge cases. +But hopefully the chosen behaviour is more useful for general use. Reloading can always be disabled if it causes +problems in some edge cases. +### File Importer -## File Importer When `importlib.reload()` is called on a module backed directly by an extension module the situation is more challenging as the module cannot be removed from `sys.modules` to force it to be reloaded (doing so will break the reload process). [PEP-489] states that -> Reloading an extension module using importlib.reload() will continue to have no effect, except re-setting import-related attributes. +> Reloading an extension module using importlib.reload() will continue to have no effect, except re-setting +> import-related attributes. and the documentation for [importlib.reload](https://docs.python.org/3/library/importlib.html#importlib.reload) states that > The init function of extension modules is not called a second time @@ -65,7 +66,6 @@ and the documentation for [importlib.reload](https://docs.python.org/3/library/i The experimentation in `tests/reference/reloading` verifies that nothing about the module (functions or globals) are reset when a normal extension module is reloaded. - [pyximport](https://github.com/cython/cython/blob/master/pyximport/pyximport.py) (import hook for cython) has a `reload_support` parameter that controls whether the hook will create copies of the extension module. This approach inspired the project importer workaround, however this feature of pyximport appears to now be broken as this test case @@ -89,14 +89,18 @@ Another possibility is trick the interpreter into thinking the new extension mod then copying the state (`__dict__`) over to the already loaded module. This works mostly as expected for simple use cases but has some edge cases/drawbacks: -- it is not possible to persist state that is initially assigned during module initialisation because it will be overwritten with the fresh data from the reloaded module - - one workaround for this could be to initialise global data after module initialisation, either with an explicit `init()` function or constructing lazily on the first use. Because the data will not exist in the fresh copy of the module it will not be overwritten during the reload +- it is not possible to persist state that is initially assigned during module initialisation because it will be + overwritten with the fresh data from the reloaded module + - one workaround for this could be to initialise global data after module initialisation, either with an explicit + `init()` function or constructing lazily on the first use. Because the data will not exist in the fresh copy + of the module it will not be overwritten during the reload -it would be possible to create a custom import hook that calls a pre-determined function (e.g. `_reload_init`) and the hook could pass in the state of the old module, but requiring full reload support should be a rare requirement. - -In CPython, [`_PyImport_LoadDynamicModuleWithSpec`](https://github.com/python/cpython/blob/59057ce55a443f35bfd685c688071aebad7b3671/Python/importdl.c#L97) identifies the extension module init function then immediately calls -it so there is no way to pass state to the newly loaded module using the current built-in mechanisms. +it would be possible to create a custom import hook that calls a pre-determined function (e.g. `_reload_init`) and the +hook could pass in the state of the old module, but requiring full reload support should be a rare requirement. +In CPython, [`_PyImport_LoadDynamicModuleWithSpec`](https://github.com/python/cpython/blob/59057ce55a443f35bfd685c688071aebad7b3671/Python/importdl.c#L97) +identifies the extension module init function then immediately calls it so there is no way to pass state to the newly +loaded module using the current built-in mechanisms. ## Summary @@ -104,17 +108,17 @@ In summary, reload support for extension modules and packages containing extensi from an import hook, but hacks are required and there are some edge cases. - Project Importer - - triggered by reloading the root module (not the extension module) (i.e. `reload(some_package)` not `reload(some_package.extension_module)`) - - behaviour is close to reloading regular python modules - - global data is not reset if nothing has changed (this is different to reloading a python module) - - global data is reset if the module was recompiled - - imports of the type `import ` use the reloaded functionality - - modules other than the extension module are not reloaded by reloading the package - - `__path__` and `__file__` are set to the temporary location required for reloading + - triggered by reloading the root module (not the extension module) (i.e. `reload(some_package)` not `reload(some_package.extension_module)`) + - behaviour is close to reloading regular python modules + - global data is not reset if nothing has changed (this is different to reloading a python module) + - global data is reset if the module was recompiled + - imports of the type `import ` use the reloaded functionality + - modules other than the extension module are not reloaded by reloading the package + - `__path__` and `__file__` are set to the temporary location required for reloading - File Importer - - triggered by reloading an extension module originally imported by the file importer - - behaviour is different from reloading regular python modules. - - Extension is loaded fresh so state assigned at load-time does not persist - - global data is always reset even if nothing has changed - - imports of the type `import ` use the reloaded functionality - - `__file__` remains pointing at the original extension module location + - triggered by reloading an extension module originally imported by the file importer + - behaviour is different from reloading regular python modules. + - Extension is loaded fresh so state assigned at load-time does not persist + - global data is always reset even if nothing has changed + - imports of the type `import ` use the reloaded functionality + - `__file__` remains pointing at the original extension module location diff --git a/tests/README.md b/tests/README.md index b65cf34..c8aea48 100644 --- a/tests/README.md +++ b/tests/README.md @@ -3,10 +3,12 @@ These tests ensure that the import hook behaves correctly when installing a wide variety of different crates. The recommended way to run the tests is to run: + ```bash git submodule update ./maturin python runner.py ``` + This script will create an isolated environment for the tests to run in and produce a html report from the test results (see `runner.py --help` for options). @@ -26,6 +28,7 @@ To ensure even more isolation than the runner script, you can use [act](https:// of this repository locally. ## Maintenance + To update maturin: - update the submodule to the maturin commit you want to update to @@ -35,19 +38,21 @@ To update maturin: If so, add them to `IGNORED_TEST_CRATES` in `common.py` - update the version check in the import hook to ensure it allows using the new version - ## Notes ### Debugging + The tests can be tricky to debug because they require spawning one or more python instances. One option is to use remote debugging with [debugpy](https://pypi.org/project/debugpy/). First, install debugpy into the test virtualenv: -``` + +```shell test_workspace/venv/bin/python -m pip install debugpy ``` Add the following line in a helper script that you want to debug, or inside `maturin_import_hook` itself + ```python import debugpy; debugpy.listen(5678); debugpy.wait_for_client(); debugpy.breakpoint() ``` @@ -59,12 +64,13 @@ Connect to the debugger, eg [using vscode](https://code.visualstudio.com/docs/py Note: set `CLEAR_WORKSPACE = False` in `common.py` if you want to prevent the temporary files generated during the test from being cleared. - ### Caching + sccache is a tool for caching build artifacts to speed up compilation. Unfortunately, it is currently useless for these tests as it [cannot cache cdylib crates](https://github.com/mozilla/sccache/issues/1715) To run with sccache anyway (to check if the situation has improved): + ```bash sccache --stop-server # so the tests use a separate empty sccache # sccache cannot cache incremental compilation, so disable it: https://github.com/mozilla/sccache/issues/236 @@ -72,13 +78,13 @@ RUSTC_WRAPPER=sccache SCCACHE_DIR=/tmp/sccache CARGO_INCREMENTAL=0 python test_r sccache --show-stats ``` - ### Faster Linking + You can use `lld` for linking using the `--lld` argument to the test runner. This usually provides a speed increase but not a huge one as linking is not a huge bottleneck in the testing. - ### Profiling + you can run the test runner with the `--profile ` argument to run the tests with `cProfile`. The majority of the total time is spent waiting for subprocesses and isn't very interesting. diff --git a/tests/package_resolver/README.md b/tests/package_resolver/README.md index 8d6a944..2555705 100644 --- a/tests/package_resolver/README.md +++ b/tests/package_resolver/README.md @@ -1,10 +1,12 @@ -# Package Resolver # +# Package Resolver This is a small utility to use the internal maturin package resolving code to compare with the python implementation in the import hook. from this directory, run -``` + +```shell cargo run -- ../maturin ../resolved.json ``` + to update `resolved.json`