diff --git a/rewrite-python/src/main/java/org/openrewrite/python/rpc/PythonRewriteRpc.java b/rewrite-python/src/main/java/org/openrewrite/python/rpc/PythonRewriteRpc.java index 549bcc889fd..6192d0b7ebb 100644 --- a/rewrite-python/src/main/java/org/openrewrite/python/rpc/PythonRewriteRpc.java +++ b/rewrite-python/src/main/java/org/openrewrite/python/rpc/PythonRewriteRpc.java @@ -491,10 +491,11 @@ public Builder workingDirectory(@Nullable Path workingDirectory) { /** * Set the base pip packages directory. - * When set, a version-specific subdirectory (e.g., {@code /0.5.3/} - * for releases, or {@code /dev/} for dev builds) will be resolved - * and added to PYTHONPATH. The openrewrite package is automatically installed - * into that subdirectory if not already present. + * When set and the required release version is not already available in the + * Python interpreter, a version-specific subdirectory (e.g., + * {@code /8.74.1/}) is resolved and the openrewrite package is + * automatically installed there via pip. Dev builds ({@code .dev0}) are not + * installed this way and require the interpreter to already have the package. * * @param pipPackagesPath The base directory under which version-specific pip packages are installed * @return This builder @@ -536,23 +537,41 @@ public Builder pythonVersion(String pythonVersion) { @Override public PythonRewriteRpc get() { - // For dev builds (version ending in .dev0), check whether the interpreter - // already has the rewrite package (e.g., from a venv with an editable install). - // If so, skip bootstrap and PYTHONPATH prepend so the interpreter's own - // version takes precedence. For release/CI builds, always use pipPackagesPath - // to ensure the correct pinned version. String version = StringUtils.readFully( PythonRewriteRpc.class.getResourceAsStream("/META-INF/rewrite-python-version.txt")).trim(); - boolean isDevBuild = version.isEmpty() || version.endsWith(".dev0"); - boolean interpreterHasRewrite = isDevBuild && pipPackagesPath != null && canImportRewrite(pythonPath); - boolean usePipPackagesPath = pipPackagesPath != null && !interpreterHasRewrite; + boolean isDevBuild = version.isEmpty() || version.endsWith(".dev0") || "unspecified".equals(version); - // Resolve version-specific subdirectory under pipPackagesPath Path resolvedPipPackagesPath = null; - if (usePipPackagesPath) { - String versionDir = isDevBuild ? "dev" : version; - resolvedPipPackagesPath = pipPackagesPath.resolve(versionDir); - bootstrapOpenrewrite(resolvedPipPackagesPath, version, isDevBuild); + if (!isDevBuild) { + // Known version (release or published pre-release) — try to find or + // install the pinned version, falling back to any available install. + if (pipPackagesPath != null && isVersionInstalled(pipPackagesPath.resolve(version), version)) { + resolvedPipPackagesPath = pipPackagesPath.resolve(version); + } else if (hasRewriteVersion(pythonPath, version)) { + // Interpreter already has the right version; nothing to do + } else if (pipPackagesPath != null) { + resolvedPipPackagesPath = pipPackagesPath.resolve(version); + bootstrapOpenrewrite(resolvedPipPackagesPath, version); + } else if (canImportRewrite(pythonPath)) { + // Interpreter has a different version, but no pipPackagesPath to + // install the right one — proceed with what's available (e.g., CI + // running tests against a venv with an editable install) + } else { + throw new IllegalStateException( + "The Python interpreter at " + pythonPath + " does not have openrewrite " + version + ". " + + "Either set pipPackagesPath to allow automatic installation, " + + "or install the package manually: pip install openrewrite==" + version); + } + } else { + // Local dev build — require the interpreter to already have the rewrite + // package (e.g., from a venv with an editable install). + if (!canImportRewrite(pythonPath)) { + throw new IllegalStateException( + "The Python interpreter at " + pythonPath + " cannot import the 'rewrite' package. " + + "For development builds, run 'uv sync --extra dev' in the rewrite-python/rewrite/ " + + "directory to set up an editable install, then configure pythonPath to point to " + + "the venv's Python executable."); + } } Stream<@Nullable String> cmd; @@ -594,10 +613,9 @@ public PythonRewriteRpc get() { // If debug source path is set, use it if (debugRewriteSourcePath != null) { pythonPathParts.add(debugRewriteSourcePath.toAbsolutePath().normalize().toString()); - } else if (pipPackagesPath == null) { - // Only search for development source if pip packages path is not set - // Try to find the Python source in the project structure - // Look for rewrite-python/rewrite/src relative to the working directory + } else if (isDevBuild) { + // For local dev builds, try to find the Python source in the project + // structure (as a fallback for PYTHONPATH) Path basePath = workingDirectory != null ? workingDirectory : Paths.get(System.getProperty("user.dir")); // Check common locations @@ -639,16 +657,14 @@ public PythonRewriteRpc get() { } /** - * Checks whether the given Python interpreter can already import the rewrite package - * without any PYTHONPATH modifications. This detects venvs or system installs that - * already have the openrewrite package available. + * Checks whether the given Python interpreter can import the rewrite package + * (any version) without PYTHONPATH modifications. */ private static boolean canImportRewrite(Path pythonPath) { try { Process probe = new ProcessBuilder( pythonPath.toString(), "-c", "import rewrite" ).redirectErrorStream(true).start(); - // Drain output to prevent blocking try (InputStream is = probe.getInputStream()) { //noinspection StatementWithEmptyBody while (is.read() != -1) { @@ -661,30 +677,47 @@ private static boolean canImportRewrite(Path pythonPath) { } /** - * Ensures the openrewrite Python package is installed in the pip packages directory. - * This is required for the RPC server to start. + * Checks whether the given Python interpreter has a specific version of the + * openrewrite package installed. */ - private void bootstrapOpenrewrite(Path pipPackagesPath, String version, boolean isDevBuild) { - boolean pinVersion = !isDevBuild; - - Path versionMarker = pipPackagesPath.resolve(".openrewrite-version"); - if (Files.exists(pipPackagesPath.resolve("rewrite"))) { - // Already installed — check if version matches - if (!pinVersion) { - return; - } - try { - if (Files.exists(versionMarker) && - version.equals(new String(Files.readAllBytes(versionMarker), StandardCharsets.UTF_8).trim())) { - return; // Correct version already installed - } - } catch (IOException ignored) { - // Can't read marker, reinstall to be safe + private static boolean hasRewriteVersion(Path pythonPath, String version) { + try { + Process probe = new ProcessBuilder( + pythonPath.toString(), "-c", + "from importlib.metadata import version; print(version('openrewrite'))" + ).redirectErrorStream(true).start(); + String output; + try (InputStream is = probe.getInputStream()) { + output = StringUtils.readFully(is).trim(); } + return probe.waitFor(10, TimeUnit.SECONDS) && probe.exitValue() == 0 + && version.equals(output); + } catch (IOException | InterruptedException e) { + return false; } + } - String packageSpec = pinVersion ? "openrewrite==" + version : "openrewrite"; + /** + * Checks whether the version-specific pip packages directory already has the + * correct version installed, based on a marker file. + */ + private static boolean isVersionInstalled(Path pipPackagesPath, String version) { + if (!Files.exists(pipPackagesPath.resolve("rewrite"))) { + return false; + } + Path versionMarker = pipPackagesPath.resolve(".openrewrite-version"); + try { + return Files.exists(versionMarker) && + version.equals(new String(Files.readAllBytes(versionMarker), StandardCharsets.UTF_8).trim()); + } catch (IOException e) { + return false; + } + } + /** + * Installs the pinned openrewrite package into the given pip packages directory. + */ + private void bootstrapOpenrewrite(Path pipPackagesPath, String version) { try { Files.createDirectories(pipPackagesPath); @@ -693,7 +726,7 @@ private void bootstrapOpenrewrite(Path pipPackagesPath, String version, boolean "-m", "pip", "install", "--upgrade", "--target=" + pipPackagesPath.toAbsolutePath().normalize(), - packageSpec + "openrewrite==" + version ); pb.redirectErrorStream(true); if (log != null) { @@ -701,36 +734,33 @@ private void bootstrapOpenrewrite(Path pipPackagesPath, String version, boolean pb.redirectOutput(ProcessBuilder.Redirect.appendTo(logFile)); } Process process = pb.start(); + String pipOutput = ""; if (log == null) { - // Drain stdout+stderr to prevent pipe buffer from filling and blocking - Thread drainer = new Thread(() -> { - try (InputStream is = process.getInputStream()) { - byte[] buf = new byte[4096]; - //noinspection StatementWithEmptyBody - while (is.read(buf) != -1) { - } - } catch (IOException ignored) { - } - }); - drainer.setDaemon(true); - drainer.start(); + // Capture stdout+stderr so we can include it in error messages + try (InputStream is = process.getInputStream()) { + pipOutput = StringUtils.readFully(is); + } } boolean completed = process.waitFor(2, TimeUnit.MINUTES); if (!completed) { process.destroyForcibly(); - throw new RuntimeException("Timed out bootstrapping openrewrite package"); + throw new RuntimeException("Timed out bootstrapping openrewrite==" + version); } int exitCode = process.exitValue(); if (exitCode != 0) { - throw new RuntimeException("Failed to bootstrap openrewrite package, pip install exited with code " + exitCode); + String message = "Failed to install openrewrite==" + version + + " (pip exited with code " + exitCode + ")"; + if (!pipOutput.isEmpty()) { + message += ":\n" + pipOutput.trim(); + } else if (log != null) { + message += ". See " + log.toAbsolutePath().normalize() + " for details"; + } + throw new RuntimeException(message); } - // Write version marker so we can detect stale installs - if (pinVersion) { - Files.write(versionMarker, version.getBytes(StandardCharsets.UTF_8)); - } + Files.write(pipPackagesPath.resolve(".openrewrite-version"), version.getBytes(StandardCharsets.UTF_8)); } catch (IOException e) { throw new UncheckedIOException("Failed to bootstrap openrewrite package", e); } catch (InterruptedException e) { diff --git a/rewrite-python/src/test/java/org/openrewrite/python/PythonParserTest.java b/rewrite-python/src/test/java/org/openrewrite/python/PythonParserTest.java index 24d0ba20cd3..b75f91d3953 100644 --- a/rewrite-python/src/test/java/org/openrewrite/python/PythonParserTest.java +++ b/rewrite-python/src/test/java/org/openrewrite/python/PythonParserTest.java @@ -17,25 +17,14 @@ import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.condition.DisabledIfEnvironmentVariable; -import org.openrewrite.InMemoryExecutionContext; import org.openrewrite.SourceFile; import org.openrewrite.Tree; import org.openrewrite.TreeVisitor; -import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; -import org.openrewrite.python.rpc.PythonRewriteRpc; import org.openrewrite.python.tree.Py; import org.openrewrite.test.RewriteTest; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.time.Duration; -import java.util.List; -import java.util.stream.Collectors; - import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.python.Assertions.python; @@ -102,192 +91,6 @@ void unicodeEscapes() { ); } - @Test - @Timeout(600) - void parseParamikoFiles() throws IOException { - Path dir = Path.of(System.getProperty("user.home"), - "moderne/working-set-python-static-analysis-2/paramiko/paramiko"); - if (!Files.isDirectory(dir)) { - return; - } - - List pyFiles; - try (var stream = Files.walk(dir)) { - pyFiles = stream - .filter(p -> p.toString().endsWith(".py")) - .filter(p -> !p.toString().contains("/build/")) - .filter(p -> { - try { - return Files.size(p) > 0; - } catch (IOException e) { - return false; - } - }) - .sorted() - .collect(Collectors.toList()); - } - - PythonRewriteRpc.setFactory(PythonRewriteRpc.builder().timeout(Duration.ofHours(1))); - PythonParser parser = PythonParser.builder().build(); - InMemoryExecutionContext ctx = new InMemoryExecutionContext(Throwable::printStackTrace); - - System.out.println("Parsing " + pyFiles.size() + " files as batch..."); - long t0 = System.nanoTime(); - List allSourceFiles = parser.parse(pyFiles, dir, ctx) - .peek(sf -> System.out.printf(" Parsed: %-50s (%.1fs elapsed)%n", - sf.getSourcePath(), (System.nanoTime() - t0) / 1e9)) - .collect(Collectors.toList()); - System.out.printf("Batch parse complete in %.1fs. Shutting down RPC...%n", - (System.nanoTime() - t0) / 1e9); - PythonRewriteRpc.shutdownCurrent(); - - for (SourceFile sf : allSourceFiles) { - long pt = System.nanoTime(); - System.out.print("Printing: " + sf.getSourcePath() + "..."); - String printed = sf.printAll(); - System.out.printf(" %.1fs%n", (System.nanoTime() - pt) / 1e9); - String expected = Files.readString(dir.resolve(sf.getSourcePath())); - assertThat(printed).as("printAll() for " + sf.getSourcePath()).isEqualTo(expected); - } - } - - @Test - @Timeout(600) - void parseFabricProject() throws IOException { - Path dir = Path.of(System.getProperty("user.home"), - "moderne/working-set-python-static-analysis-2/fabric/fabric"); - if (!Files.isDirectory(dir)) { - return; - } - - PythonRewriteRpc.setFactory(PythonRewriteRpc.builder().timeout(Duration.ofHours(1))); - PythonRewriteRpc rpc = PythonRewriteRpc.getOrStart(); - InMemoryExecutionContext ctx = new InMemoryExecutionContext(Throwable::printStackTrace); - - System.out.println("Parsing project: " + dir); - long t0 = System.nanoTime(); - List allSourceFiles = rpc.parseProject(dir, ctx) - .peek(sf -> System.out.printf(" Parsed: %-50s (%.1fs elapsed)%n", - sf.getSourcePath(), (System.nanoTime() - t0) / 1e9)) - .collect(Collectors.toList()); - System.out.printf("Parse complete in %.1fs (%d files). Shutting down RPC...%n", - (System.nanoTime() - t0) / 1e9, allSourceFiles.size()); - PythonRewriteRpc.shutdownCurrent(); - - for (SourceFile sf : allSourceFiles) { - long pt = System.nanoTime(); - System.out.print("Printing: " + sf.getSourcePath() + "..."); - String printed = sf.printAll(); - System.out.printf(" %.1fs%n", (System.nanoTime() - pt) / 1e9); - String expected = Files.readString(dir.resolve(sf.getSourcePath())); - assertThat(printed).as("printAll() for " + sf.getSourcePath()).isEqualTo(expected); - } - } - - @Test - @Timeout(600) - void parseFabricProjectViaParserParse() throws IOException { - Path dir = Path.of(System.getProperty("user.home"), - "moderne/working-set-python-static-analysis-2/fabric/fabric"); - if (!Files.isDirectory(dir)) { - return; - } - - PythonRewriteRpc.setFactory(PythonRewriteRpc.builder().timeout(Duration.ofHours(1))); - PythonParser parser = PythonParser.builder().build(); - InMemoryExecutionContext ctx = new InMemoryExecutionContext(Throwable::printStackTrace); - - List pyFiles; - try (var stream = Files.walk(dir)) { - pyFiles = stream - .filter(p -> p.toString().endsWith(".py")) - .filter(p -> !p.toString().contains("/build/")) - .filter(p -> { - try { return Files.size(p) > 0; } catch (IOException e) { return false; } - }) - .sorted() - .collect(Collectors.toList()); - } - - System.out.println("Parsing " + pyFiles.size() + " files via parser.parse..."); - long t0 = System.nanoTime(); - List allSourceFiles = parser.parse(pyFiles, dir, ctx) - .peek(sf -> System.out.printf(" Parsed: %-50s (%.1fs elapsed)%n", - sf.getSourcePath(), (System.nanoTime() - t0) / 1e9)) - .collect(Collectors.toList()); - System.out.printf("Parse complete in %.1fs (%d files). Shutting down RPC...%n", - (System.nanoTime() - t0) / 1e9, allSourceFiles.size()); - PythonRewriteRpc.shutdownCurrent(); - - for (SourceFile sf : allSourceFiles) { - long pt = System.nanoTime(); - System.out.print("Printing: " + sf.getSourcePath() + "..."); - String printed = sf.printAll(); - System.out.printf(" %.1fs%n", (System.nanoTime() - pt) / 1e9); - String expected = Files.readString(dir.resolve(sf.getSourcePath())); - assertThat(printed).as("printAll() for " + sf.getSourcePath()).isEqualTo(expected); - } - } - - @Test - @Timeout(600) - void parseParamikoProject() throws IOException { - Path dir = Path.of(System.getProperty("user.home"), - "moderne/working-set-python-static-analysis-2/paramiko/paramiko"); - if (!Files.isDirectory(dir)) { - return; - } - - PythonRewriteRpc.setFactory(PythonRewriteRpc.builder().timeout(Duration.ofHours(1))); - PythonRewriteRpc rpc = PythonRewriteRpc.getOrStart(); - InMemoryExecutionContext ctx = new InMemoryExecutionContext(Throwable::printStackTrace); - - System.out.println("Parsing project: " + dir); - long t0 = System.nanoTime(); - List allSourceFiles = rpc.parseProject(dir, ctx) - .peek(sf -> System.out.printf(" Parsed: %-50s (%.1fs elapsed)%n", - sf.getSourcePath(), (System.nanoTime() - t0) / 1e9)) - .collect(Collectors.toList()); - System.out.printf("Parse complete in %.1fs (%d files). Shutting down RPC...%n", - (System.nanoTime() - t0) / 1e9, allSourceFiles.size()); - PythonRewriteRpc.shutdownCurrent(); - - for (SourceFile sf : allSourceFiles) { - long pt = System.nanoTime(); - System.out.print("Printing: " + sf.getSourcePath() + "..."); - String printed = sf.printAll(); - System.out.printf(" %.1fs%n", (System.nanoTime() - pt) / 1e9); - String expected = Files.readString(dir.resolve(sf.getSourcePath())); - assertThat(printed).as("printAll() for " + sf.getSourcePath()).isEqualTo(expected); - } - } - - @Test - @Timeout(60) - void parseFabricSetupPyAlone() throws IOException { - Path file = Path.of(System.getProperty("user.home"), - "moderne/working-set-python-static-analysis-2/fabric/fabric/setup.py"); - if (!Files.isRegularFile(file)) { - return; - } - - PythonRewriteRpc.setFactory(PythonRewriteRpc.builder().timeout(Duration.ofHours(1))); - PythonParser parser = PythonParser.builder().build(); - InMemoryExecutionContext ctx = new InMemoryExecutionContext(Throwable::printStackTrace); - - List parsed = parser.parse(List.of(file), file.getParent(), ctx) - .peek(sf -> System.out.println("Parsed: " + sf.getSourcePath())) - .collect(Collectors.toList()); - PythonRewriteRpc.shutdownCurrent(); - - assertThat(parsed).hasSize(1); - System.out.println("Printing: " + parsed.get(0).getSourcePath()); - String printed = parsed.get(0).printAll(); - String expected = Files.readString(file); - assertThat(printed).isEqualTo(expected); - System.out.println("OK: " + printed.length() + " chars"); - } - @Test void parseStringWithParser() { SourceFile sf = PythonParser.builder().build()