Skip to content

Commit 2dbbb53

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 2dbbb53

3 files changed

Lines changed: 78 additions & 6 deletions

File tree

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

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@
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+
# Set via --recipe-install-dir; an InstallRecipes RPC for a not-yet-importable
79+
# package pip installs it here before activating.
80+
_recipe_install_dir: Optional[Path] = None
81+
7882

7983
def _next_request_id() -> int:
8084
"""Generate a unique request ID for outgoing requests."""
@@ -627,12 +631,42 @@ def _get_marketplace():
627631
return _marketplace
628632

629633

634+
def _is_package_installed(package_name: str, version: Optional[str]) -> bool:
635+
try:
636+
import importlib.metadata
637+
installed = importlib.metadata.version(package_name)
638+
except Exception:
639+
return False
640+
return version is None or installed == version
641+
642+
643+
def _pip_install_recipe_package(package_name: str, version: Optional[str], target_dir: Path) -> None:
644+
import importlib
645+
import subprocess
646+
647+
target_dir.mkdir(parents=True, exist_ok=True)
648+
spec = f"{package_name}=={version}" if version else package_name
649+
cmd = [sys.executable, "-m", "pip", "install", "--target", str(target_dir), spec]
650+
logger.info(f"pip install: {' '.join(cmd)}")
651+
result = subprocess.run(cmd, capture_output=True, text=True)
652+
if result.returncode != 0:
653+
raise RuntimeError(
654+
f"pip install failed for {spec} (target={target_dir}):\n{result.stderr}"
655+
)
656+
657+
target_str = str(target_dir.resolve())
658+
if target_str not in sys.path:
659+
sys.path.insert(0, target_str)
660+
importlib.invalidate_caches()
661+
662+
630663
def handle_install_recipes(params: dict) -> dict:
631664
"""Handle an InstallRecipes RPC request.
632665
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.
666+
Activates a recipe package in the marketplace. When `--recipe-install-dir`
667+
is configured, a package spec that isn't already installed is pip-installed
668+
into that directory before activation; otherwise the package must have been
669+
installed by the caller.
636670
637671
Args:
638672
params: Dict containing either:
@@ -675,16 +709,17 @@ def handle_install_recipes(params: dict) -> dict:
675709
recipes_added = len(added)
676710

677711
elif isinstance(recipes, dict):
678-
# Package spec with name and optional version - package should already be installed
679712
package_name = recipes.get('packageName')
680713
version = recipes.get('version')
681714

682715
if not package_name:
683716
raise ValueError("Package name is required")
684717

718+
if _recipe_install_dir is not None and not _is_package_installed(package_name, version):
719+
_pip_install_recipe_package(package_name, version, _recipe_install_dir)
720+
685721
logger.info(f"Activating recipes package: {package_name}")
686722

687-
# Get the installed version
688723
try:
689724
import importlib.metadata
690725
installed_version = importlib.metadata.version(package_name)
@@ -1640,8 +1675,13 @@ def main():
16401675
parser.add_argument('--log-file', help='Log file path')
16411676
parser.add_argument('--metrics-csv', help='Metrics CSV output path')
16421677
parser.add_argument('--trace-rpc-messages', action='store_true', help='Enable RPC message tracing')
1678+
parser.add_argument('--recipe-install-dir', help='Directory where recipe pip packages are installed')
16431679
args = parser.parse_args()
16441680

1681+
if args.recipe_install_dir:
1682+
global _recipe_install_dir
1683+
_recipe_install_dir = Path(args.recipe_install_dir)
1684+
16451685
if args.log_file:
16461686
file_handler = logging.FileHandler(args.log_file)
16471687
file_handler.setFormatter(logging.Formatter('%(asctime)s - %(levelname)s - %(message)s'))

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,37 @@ 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_pip_install_recipe_package_shape(tmp_path, monkeypatch):
30+
import rewrite.rpc.server as server
31+
import subprocess
32+
33+
install_dir = tmp_path / "recipes"
34+
captured = {}
35+
36+
class FakeCompletedProcess:
37+
returncode = 0
38+
stdout = ""
39+
stderr = ""
40+
41+
def fake_run(cmd, capture_output=False, text=False):
42+
captured["cmd"] = cmd
43+
return FakeCompletedProcess()
44+
45+
monkeypatch.setattr(subprocess, "run", fake_run)
46+
47+
server._pip_install_recipe_package(
48+
"openrewrite-recipes-python", "1.2.3", install_dir
49+
)
50+
51+
assert install_dir.exists()
52+
assert captured["cmd"][1:] == [
53+
"-m", "pip", "install",
54+
"--target", str(install_dir),
55+
"openrewrite-recipes-python==1.2.3",
56+
]
57+
assert str(install_dir.resolve()) in __import__("sys").path
58+
59+
2960
def test_recipe_descriptor_to_dict_emits_all_collection_keys():
3061
from rewrite.recipe import RecipeDescriptor
3162
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)