Fix installation of project plugins if Poetry's environment is not writable - #10998
Conversation
…itable When installing project plugins declared via tool.poetry.requires-plugins, ProjectPluginCache._install() redirects "purelib" and "platlib" to the project's plugin cache. Env.scheme_dict then computed the prefix to relocate below "userbase" as the common path of "scripts" and the (redirected) "purelib", which degenerates to something close to the filesystem root. The subsequent naive string replacement mangled every path, so plugins were silently installed into bogus directories instead of the plugin cache and never activated. Relocate only the scheme paths that are actually read-only, compute the prefix from those paths alone, use proper path arithmetic instead of string replacement, and bail out if the read-only paths only have the filesystem root in common.
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
base_path == os.path.dirname(base_path)check used to detect a root-only common path may behave unexpectedly on Windows (e.g. different drives, UNC paths) and other exotic path schemes; consider usingPath(base_path).anchoror a more explicit root/common-prefix check to avoid subtle edge cases. - In
scheme_dict, theread_only_keys/scheme_nameslogic has grown complex; you may want to refactor the read-only detection and prefix-relocation into a small helper function to make the behavior and invariants easier to reason about.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `base_path == os.path.dirname(base_path)` check used to detect a root-only common path may behave unexpectedly on Windows (e.g. different drives, UNC paths) and other exotic path schemes; consider using `Path(base_path).anchor` or a more explicit root/common-prefix check to avoid subtle edge cases.
- In `scheme_dict`, the `read_only_keys`/`scheme_names` logic has grown complex; you may want to refactor the read-only detection and prefix-relocation into a small helper function to make the behavior and invariants easier to reason about.
## Individual Comments
### Comment 1
<location path="src/poetry/utils/env/base_env.py" line_range="301-310" />
<code_context>
+ overrides: dict[str, str] = {}
</code_context>
<issue_to_address>
**issue (bug_risk):** Relocation candidates are computed but never applied to `paths`, making the override logic ineffective.
In the previous version, `overrides` was populated and then used to adjust the scheme paths. In this version, `overrides` remains empty and `paths` is never updated with the relocated candidates, so `scheme_dict` still returns the original read-only paths even when writable candidates exist. Please either populate `overrides` and merge it into `paths`, or directly update `paths[key]` for each `read_only_key` after confirming writability.
</issue_to_address>
### Comment 2
<location path="tests/plugins/test_plugin_manager.py" line_range="649" />
<code_context>
assert not any(p.name.startswith("demo") for p in orig_platlib.iterdir())
+
+
+def test_project_plugins_are_installed_in_project_folder_if_env_read_only(
+ poetry_with_plugins: Poetry,
+ io: BufferedIO,
+ system_env: Env,
+ fixture_dir: FixtureDirGetter,
+ tmp_path: Path,
+ mocker: MockerFixture,
+) -> None:
+ """https://github.com/python-poetry/poetry/issues/10341
+
+ If the scheme paths of Poetry's environment are not writable
+ (e.g. Poetry was installed system-wide and is run as an unprivileged user),
+ project plugins must still be installed into the project's plugin cache.
+ """
+ orig_purelib = system_env.purelib
+ orig_platlib = system_env.platlib
+
+ read_only_paths = {system_env.paths[key] for key in SCHEME_NAMES}
+ original_is_dir_writable = is_dir_writable
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider excluding `userbase` from the mocked read-only paths to better mirror real-world behavior
Here `read_only_paths` mirrors all `SCHEME_NAMES`, which usually includes `userbase`. That causes the mock `is_dir_writable` to treat the current `userbase` as read-only, while the intended scenario is “system schemes read-only, user base writable”. The test still passes because relocation candidates differ from the original `userbase`, but the setup is misleading. To better match real behavior and clarify the intent, build `read_only_paths` from non-`userbase` schemes only, e.g. `{system_env.paths[key] for key in SCHEME_NAMES if key != "userbase"}`.
```suggestion
read_only_paths = {
system_env.paths[key]
for key in SCHEME_NAMES
if key != "userbase"
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Regarding the Windows concern about |
Pull Request Check List
Resolves: #10341
[tool.poetry.requires-plugins]to behave as already documented.Summary
When project plugins are installed,
ProjectPluginCache._install()redirectspurelibandplatlibto the project's plugin cache (.poetry/plugins).Env.scheme_dictthen computed the prefix used to relocate read-only paths belowuserbaseas the common path ofscriptsand the redirectedpurelib. When Poetry runs from a non-writable environment (e.g. a system-wide install used by an unprivileged user), those two paths only share something close to the filesystem root, and the subsequent plain string replacement (str.replace) mangled every path separator. As a result, plugins were silently installed into bogus directories instead of the plugin cache and never activated (as diagnosed by @dimbleby in the issue).This change makes
scheme_dict:os.path.relpathinstead of a naive string replacement, andBehavior for the plain "all system scheme paths are read-only" case is unchanged: all paths are still relocated below
userbaseexactly as before.Testing
test_project_plugins_are_installed_in_project_folderintests/plugins/test_plugin_manager.py, plus twoscheme_dicttests intests/utils/env/test_env.pymirroring the issue. All three fail without the fix..venv/bin/pytest tests/plugins tests/installation tests/utils/env/test_env.py -q— 680 passed, 1 skippedPATH="/tmp/poetry-test-bin:$PATH" .venv/bin/pytest -q— 3164 passed, 14 failed, 27 skipped; the same 14 tests fail identically on currentmainon this machine (local Python discovery issues), so no regressions from this change.venv/bin/mypy src/poetry/utils/env/base_env.py tests/plugins/test_plugin_manager.py tests/utils/env/test_env.py— success.venv/bin/ruff check/ruff format --checkon the touched files — passed