Skip to content

Commit 7d7d453

Browse files
committed
PipRecipeBundleResolver: pip install on the server side
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.
1 parent 72154fd commit 7d7d453

3 files changed

Lines changed: 108 additions & 6 deletions

File tree

rewrite-python/rewrite/src/rewrite/rpc/server.py

Lines changed: 63 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,12 @@
7575
# Set REWRITE_PYTHON_VERSION to "2" or "2.7" to parse Python 2 code
7676
_python_version = os.environ.get("REWRITE_PYTHON_VERSION", "3")
7777

78+
# Directory where recipe pip packages are installed (set via --recipe-install-dir
79+
# at startup; mirrors how the JS RPC server treats `installDir` on the npm side).
80+
# When set, an InstallRecipes RPC for a not-yet-importable package will pip
81+
# install it into this dir before activating.
82+
_recipe_install_dir: Optional[Path] = None
83+
7884

7985
def _next_request_id() -> int:
8086
"""Generate a unique request ID for outgoing requests."""
@@ -627,12 +633,54 @@ def _get_marketplace():
627633
return _marketplace
628634

629635

636+
def _is_package_installed(package_name: str, version: Optional[str]) -> bool:
637+
"""True if `package_name` is importable at `version` (or any version when
638+
`version` is None). Uses importlib.metadata so we don't have to import the
639+
package itself just to check."""
640+
try:
641+
import importlib.metadata
642+
installed = importlib.metadata.version(package_name)
643+
except Exception:
644+
return False
645+
return version is None or installed == version
646+
647+
648+
def _pip_install_recipe_package(package_name: str, version: Optional[str], target_dir: Path) -> None:
649+
"""pip install `package_name[==version]` into `target_dir` and make it
650+
visible to subsequent imports. Raises RuntimeError on pip failure.
651+
652+
Mirrors the JS RPC server's server-side `npm install` for unknown recipe
653+
packages; the JVM caller doesn't need to pre-install."""
654+
import importlib
655+
import subprocess
656+
657+
target_dir.mkdir(parents=True, exist_ok=True)
658+
spec = f"{package_name}=={version}" if version else package_name
659+
cmd = [sys.executable, "-m", "pip", "install", "--target", str(target_dir), spec]
660+
logger.info(f"pip install: {' '.join(cmd)}")
661+
result = subprocess.run(cmd, capture_output=True, text=True)
662+
if result.returncode != 0:
663+
raise RuntimeError(
664+
f"pip install failed for {spec} (target={target_dir}):\n{result.stderr}"
665+
)
666+
667+
# Make the freshly-installed package importable. The Java side already
668+
# added target_dir to PYTHONPATH at process start, but if it's missing from
669+
# sys.path (e.g. recipeInstallDir was set after launch) add it here too.
670+
target_str = str(target_dir.resolve())
671+
if target_str not in sys.path:
672+
sys.path.insert(0, target_str)
673+
importlib.invalidate_caches()
674+
675+
630676
def handle_install_recipes(params: dict) -> dict:
631677
"""Handle an InstallRecipes RPC request.
632678
633-
Activates a recipe package in the marketplace. The package should already be
634-
installed by the caller (e.g., via pip install --target). This handler discovers
635-
and activates the package's recipes.
679+
Activates a recipe package in the marketplace. When `--recipe-install-dir`
680+
is configured at server startup, a package spec that isn't already installed
681+
is pip-installed into that directory before activation; otherwise the
682+
package must have been installed by the caller (e.g. via `pip install
683+
--target`).
636684
637685
Args:
638686
params: Dict containing either:
@@ -675,16 +723,21 @@ def handle_install_recipes(params: dict) -> dict:
675723
recipes_added = len(added)
676724

677725
elif isinstance(recipes, dict):
678-
# Package spec with name and optional version - package should already be installed
726+
# Package spec with name and optional version. Pip install into the
727+
# configured `--recipe-install-dir` if the package isn't already
728+
# importable at the requested version. Mirrors how the JS RPC server
729+
# shells out to `npm install` for unknown npm recipe packages.
679730
package_name = recipes.get('packageName')
680731
version = recipes.get('version')
681732

682733
if not package_name:
683734
raise ValueError("Package name is required")
684735

736+
if _recipe_install_dir is not None and not _is_package_installed(package_name, version):
737+
_pip_install_recipe_package(package_name, version, _recipe_install_dir)
738+
685739
logger.info(f"Activating recipes package: {package_name}")
686740

687-
# Get the installed version
688741
try:
689742
import importlib.metadata
690743
installed_version = importlib.metadata.version(package_name)
@@ -1640,8 +1693,13 @@ def main():
16401693
parser.add_argument('--log-file', help='Log file path')
16411694
parser.add_argument('--metrics-csv', help='Metrics CSV output path')
16421695
parser.add_argument('--trace-rpc-messages', action='store_true', help='Enable RPC message tracing')
1696+
parser.add_argument('--recipe-install-dir', help='Directory where recipe pip packages are installed')
16431697
args = parser.parse_args()
16441698

1699+
if args.recipe_install_dir:
1700+
global _recipe_install_dir
1701+
_recipe_install_dir = Path(args.recipe_install_dir)
1702+
16451703
if args.log_file:
16461704
file_handler = logging.FileHandler(args.log_file)
16471705
file_handler.setFormatter(logging.Formatter('%(asctime)s - %(levelname)s - %(message)s'))

rewrite-python/rewrite/tests/rpc/test_server.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,49 @@ def fake_parse_python_source(source, path="<unknown>", relative_to=None, ty_clie
2626
assert (tmp_path / "pkg" / "__init__.py").read_text(encoding="utf-8") == ""
2727

2828

29+
def test_handle_install_recipes_pip_installs_when_target_configured(tmp_path, monkeypatch):
30+
"""When `--recipe-install-dir` is configured and the package isn't already
31+
importable, handle_install_recipes should shell out to `pip install --target
32+
<dir> packageName==version` before activating, mirroring the JS RPC server's
33+
server-side `npm install`. The JVM caller must not have to pre-install."""
34+
import rewrite.rpc.server as server
35+
import subprocess
36+
37+
install_dir = tmp_path / "recipes"
38+
monkeypatch.setattr(server, "_recipe_install_dir", install_dir)
39+
monkeypatch.setattr(server, "_marketplace", None)
40+
41+
# Stand in for pip — capture the command, fake a successful run.
42+
captured = {}
43+
44+
class FakeCompletedProcess:
45+
returncode = 0
46+
stdout = ""
47+
stderr = ""
48+
49+
def fake_run(cmd, capture_output=False, text=False):
50+
captured["cmd"] = cmd
51+
return FakeCompletedProcess()
52+
53+
monkeypatch.setattr(subprocess, "run", fake_run)
54+
55+
# The package "definitely-not-installed-pkg" doesn't exist; _is_package_installed
56+
# returns False, which triggers the pip install path. We don't go further —
57+
# _import_and_activate_package would raise after our fake pip — so call the
58+
# helper directly to assert the install command shape.
59+
server._pip_install_recipe_package(
60+
"openrewrite-recipes-python", "1.2.3", install_dir
61+
)
62+
63+
assert install_dir.exists()
64+
assert captured["cmd"][1:] == [
65+
"-m", "pip", "install",
66+
"--target", str(install_dir),
67+
"openrewrite-recipes-python==1.2.3",
68+
]
69+
assert str(install_dir.resolve()) in __import__("sys").path
70+
71+
2972
def test_recipe_descriptor_to_dict_emits_all_collection_keys():
3073
from rewrite.recipe import RecipeDescriptor
3174
from rewrite.rpc.server import _recipe_descriptor_to_dict

rewrite-python/src/main/java/org/openrewrite/python/rpc/PythonRewriteRpc.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,8 @@ public PythonRewriteRpc get() {
599599
"-m", "rewrite.rpc.server",
600600
log == null ? null : "--log-file=" + log.toAbsolutePath().normalize(),
601601
metricsCsv == null ? null : "--metrics-csv=" + metricsCsv.toAbsolutePath().normalize(),
602-
traceRpcMessages ? "--trace-rpc-messages" : null
602+
traceRpcMessages ? "--trace-rpc-messages" : null,
603+
recipeInstallDir == null ? null : "--recipe-install-dir=" + recipeInstallDir.toAbsolutePath().normalize()
603604
);
604605

605606
String[] cmdArr = cmd.filter(Objects::nonNull).toArray(String[]::new);

0 commit comments

Comments
 (0)