Skip to content

PipRecipeBundleResolver: pip install on the server side#7547

Merged
jkschneider merged 1 commit intomainfrom
pip-resolver-actually-installs
May 2, 2026
Merged

PipRecipeBundleResolver: pip install on the server side#7547
jkschneider merged 1 commit intomainfrom
pip-resolver-actually-installs

Conversation

@jkschneider
Copy link
Copy Markdown
Member

What

Aligns the pip recipe install flow with the npm one. Today:

  • npm: JavaScriptRewriteRpc's server-side InstallRecipes handler shells out to npm install <pkg>@<ver> --no-fund (rewrite-javascript/rewrite/src/rpc/request/install-recipes.ts:116). The JVM caller passes a package spec; the JS RPC server installs and activates.
  • pip: PythonRewriteRpc's server-side handle_install_recipes doesn't install — its docstring explicitly says the package "should already be installed by the caller (e.g., via pip install --target)". The JVM-side PipRecipeBundleResolver.resolve() calls installRecipes(packageName, version) without any pip install, so when the caller hasn't pre-installed (which is the common case), _import_and_activate_package silently fails to register the recipe and a later prepareRecipe throws Recipe not found: <id>.

This PR makes the Python server install pip packages itself, matching the npm contract.

How

Python side (rewrite-python/rewrite/src/rewrite/rpc/server.py)

  • New --recipe-install-dir argv parsed at startup; stored as _recipe_install_dir.
  • New helper _pip_install_recipe_package(name, version, target_dir) shells out to python -m pip install --target <dir> <pkg>==<ver>, raises on non-zero exit, and primes sys.path + importlib.invalidate_caches().
  • handle_install_recipes's package-spec branch invokes it when the requested package isn't already importable at the requested version, before the existing import + activate flow. Local-path branch is unchanged.

Java side (PythonRewriteRpc.java)

  • One-line argv addition: --recipe-install-dir=<path> is passed to the Python subprocess when the builder's recipeInstallDir is set. The existing PYTHONPATH wiring stays so already-installed packages continue to import as before.

Test plan

  • tests/rpc/test_server.py::test_handle_install_recipes_pip_installs_when_target_configured — locks the install command shape (-m pip install --target <dir> pkg==ver) and asserts the target dir lands on sys.path. Mocks subprocess.run so the test doesn't actually shell out to PyPI.
  • :rewrite-python:compileJava passes.
  • Existing tests/rpc/test_server.py cases still pass.

Notes for downstream callers

If you use PipRecipeBundleResolver and you weren't pre-installing recipe packages: set PythonRewriteRpc.builder().recipeInstallDir(<writable-dir>) and you're done — the server installs on demand. If you were pre-installing, no change is required: _is_package_installed short-circuits and the existing path runs.

Mirrors the JS RPC server's design where `npm install` runs server-side
during InstallRecipes. Today the Python RPC server's
handle_install_recipes assumes the package is already importable; if it
isn't, prepareRecipe later throws "Recipe not found". The JVM-side
PipRecipeBundleResolver doesn't pip install either, so the contract was
implicit: "some other process must have pre-installed this package",
which is fragile and forced downstream callers to duplicate install
logic.

Fix on the Python side:
- New `--recipe-install-dir` argv stores the install target globally.
- `_pip_install_recipe_package` shells out to `python -m pip install
  --target <dir> <package>==<version>` and primes sys.path.
- `handle_install_recipes`'s package-spec branch invokes it when the
  requested package isn't already importable at the requested version,
  before the existing import + activate flow.

Fix on the Java side:
- `PythonRewriteRpc` passes `--recipe-install-dir=<path>` to the
  Python process when the builder's `recipeInstallDir` is set. The
  existing PYTHONPATH wiring is unchanged, so a package that's already
  installed remains importable as before.

The local-path branch (caller passes a directory containing an already-
unpacked recipe package) is unchanged.
@jkschneider jkschneider force-pushed the pip-resolver-actually-installs branch from 7d7d453 to 2dbbb53 Compare May 2, 2026 15:07
@jkschneider jkschneider merged commit 1684cf4 into main May 2, 2026
1 check passed
@jkschneider jkschneider deleted the pip-resolver-actually-installs branch May 2, 2026 15:08
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite May 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant