[GR-59841] Polish GraalPy Gradle plugin.

PullRequest: graalpython/3488

Author: melix

ci.jsonnet

@@ -1 +1 @@
-{ "overlay": "6cf9b65e57d85fa77c95fa3451bb11ef674bde97" }
+{ "overlay": "cead6ebaabd0279780046d2e1826395d9ce8bdc4" }

docs/user/Embedding-Build-Tools.md

@@ -117,19 +117,30 @@ Remember to use the appropriate `GraalPyResources` API to create the Context.
 
 ## GraalPy Gradle Plugin Configuration
 
-> Note: GraalPy Gradle Plugin will become available as of GraalPy version 24.1.1, planned for October 15, 2024.
+The plugin must be added to the plugins section in the _build.gradle_ file.
+The **version** property defines which version of GraalPy to use.
+```
+plugins {
+    // other plugins ...
+    id 'org.graalvm.python' version '24.2.0'
+}
+```
 
-Add the plugin configuration in the `GraalPy` block in the _build.gradle_ file.
-The **packages** element declares a list of third-party Python packages to be downloaded and installed by the plugin.
-- The Python packages and their versions are specified as if used with `pip`.
+The plugin automatically injects these dependencies of the same version as the plugin version:
+  - `org.graalvm.python:python`
+  - `org.graalvm.python:python-embedding`
+
+The plugin can be configured in the `graalPy` block:
+
+- The **packages** element declares a list of third-party Python packages to be downloaded and installed by the plugin.
+  The Python packages and their versions are specified as if used with `pip`.
   ```
   graalPy {
     packages = ["termcolor==2.2"]
     ...
   }
   ```
 - The **pythonHome** subsection declares what parts of the standard library should be deployed.
-
   Each element in the `includes` and `excludes` list is interpreted as a Java-like regular expression specifying which file paths should be included or excluded.
   ```
   graalPy {
@@ -141,15 +152,23 @@ The **packages** element declares a list of third-party Python packages to be do
   }
   ```
 - If the **pythonResourcesDirectory** element is specified, then the given directory is used as an [external directory](#external-directory) and no Java resources are embedded. 
-Remember to use the appropriate `GraalPyResources` API to create the Context.
+  Remember to use the appropriate `GraalPyResources` API to create the Context.
   ```
   graalPy {
     pythonResourcesDirectory = file("$rootDir/python-resources")
     ...
   }
   ```
+- Boolean flag **community** switches the automatically injected
+dependency `org.graalvm.python:python` to the community build: `org.graalvm.python:python-community`.
+  ```
+  graalPy {
+    community = true
+    ...
+  }
+  ```
 
 ### Related Documentation
 
 * [Embedding Graal languages in Java](https://www.graalvm.org/reference-manual/embed-languages/)
-* [Permissions for Python Embeddings](Embedding-Permissions.md)
+* [Permissions for Python Embeddings](Embedding-Permissions.md)

graalpython/com.oracle.graal.python.test/src/tests/standalone/gradle/build/build.gradle

@@ -12,10 +12,6 @@ application {
     mainClass = "org.example.GraalPy"
 }
 
-dependencies {
-    implementation("org.graalvm.python:python-community:24.2.0")
-}
-
 run {
     enableAssertions = true
 }

graalpython/com.oracle.graal.python.test/src/tests/standalone/gradle/build/build.gradle.kts

@@ -17,6 +17,3 @@ val r = tasks.run.get()
 r.enableAssertions = true
 r.outputs.upToDateWhen {false}
 
-dependencies {
-    implementation("org.graalvm.python:python-community:24.2.0")
-}

graalpython/com.oracle.graal.python.test/src/tests/standalone/test_gradle_plugin.py

@@ -46,7 +46,7 @@
 from tests.standalone import util
 
 
-class MavenPluginTest(util.PolyglotAppTestBase):
+class MavenPluginTest(util.BuildToolTestBase):
 
     def generate_app(self, tmpdir, target_dir, target_name, pom_template=None):
         cmd = util.GLOBAL_MVN_CMD + [

graalpython/com.oracle.graal.python.test/src/tests/standalone/test_maven_plugin.py

@@ -43,6 +43,9 @@
 import sys
 import unittest
 import urllib.parse
+import tempfile
+from abc import ABC, abstractmethod
+from typing import Optional
 
 MAVEN_VERSION = "3.9.8"
 GLOBAL_MVN_CMD = [shutil.which('mvn'), "--batch-mode"]
@@ -54,7 +57,60 @@
 is_maven_plugin_test_enabled = 'ENABLE_MAVEN_PLUGIN_UNITTESTS' in os.environ and os.environ['ENABLE_MAVEN_PLUGIN_UNITTESTS'] == "true"
 is_gradle_plugin_test_enabled = 'ENABLE_GRADLE_PLUGIN_UNITTESTS' in os.environ and os.environ['ENABLE_GRADLE_PLUGIN_UNITTESTS'] == "true"
 
-class PolyglotAppTestBase(unittest.TestCase):
+
+class TemporaryTestDirectory():
+    def __init__(self):
+        if 'GRAALPY_UNITTESTS_TMPDIR_NO_CLEAN' in os.environ:
+            self.ctx = None
+            self.name = tempfile.mkdtemp()
+            print(f"Running test in {self.name}")
+        else:
+            self.ctx = tempfile.TemporaryDirectory()
+            self.name = self.ctx.name
+
+    def __enter__(self):
+        return self.name
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        if self.ctx:
+            self.ctx.__exit__(exc_type, exc_val, exc_tb)
+
+class LoggerBase(ABC):
+    def log_block(self, name, text):
+        self.log("=" * 80)
+        self.log(f"==> {name}:")
+        self.log(text)
+        self.log("=" * 80)
+
+    @abstractmethod
+    def log(self, msg, newline=True):
+        pass
+
+class Logger(LoggerBase):
+    def __init__(self):
+        self.data = ''
+
+    def log(self, msg, newline=True):
+        self.data += msg + ('\n' if newline else '')
+
+    def __str__(self):
+        two_lines = ("=" * 80 + "\n") * 2
+        return two_lines + "Test execution log:\n" + self.data + "\n" + two_lines
+
+class NullLogger(LoggerBase):
+    def log(self, msg, newline=True):
+        pass
+
+class StdOutLogger(LoggerBase):
+    def __init__(self, delegate:LoggerBase):
+        self.delegate = delegate
+
+    def log(self, msg, newline=True):
+        print(msg)
+        self.delegate.log(msg, newline=newline)
+
+
+class BuildToolTestBase(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
         if not is_maven_plugin_test_enabled and not is_gradle_plugin_test_enabled:
@@ -72,14 +128,14 @@ def setUpClass(cls):
             url = urllib.parse.urlparse(custom_repo)
             if url.scheme == "file":
                 jar = os.path.join(
-                    url.path,
+                    urllib.parse.unquote(url.path),
                     cls.archetypeGroupId.replace(".", os.path.sep),
                     cls.archetypeArtifactId,
                     cls.graalvmVersion,
                     f"{cls.archetypeArtifactId}-{cls.graalvmVersion}.jar",
                 )
                 pom = os.path.join(
-                    url.path,
+                    urllib.parse.unquote(url.path),
                     cls.archetypeGroupId.replace(".", os.path.sep),
                     cls.archetypeArtifactId,
                     cls.graalvmVersion,
@@ -96,18 +152,18 @@ def setUpClass(cls):
                     "-DcreateChecksum=true",
                 ]
                 out, return_code = run_cmd(cmd, cls.env)
-                assert return_code == 0
+                assert return_code == 0, out
 
                 jar = os.path.join(
-                    url.path,
+                    urllib.parse.unquote(url.path),
                     cls.archetypeGroupId.replace(".", os.path.sep),
                     cls.pluginArtifactId,
                     cls.graalvmVersion,
                     f"{cls.pluginArtifactId}-{cls.graalvmVersion}.jar",
                 )
 
                 pom = os.path.join(
-                    url.path,
+                    urllib.parse.unquote(url.path),
                     cls.archetypeGroupId.replace(".", os.path.sep),
                     cls.pluginArtifactId,
                     cls.graalvmVersion,
@@ -125,10 +181,12 @@ def setUpClass(cls):
                     "-DcreateChecksum=true",
                 ]
                 out, return_code = run_cmd(cmd, cls.env)
-                assert return_code == 0
+                assert return_code == 0, out
                 break
 
-def run_cmd(cmd, env, cwd=None, print_out=False, gradle=False):
+def run_cmd(cmd, env, cwd=None, print_out=False, gradle=False, logger:LoggerBase=NullLogger()):
+    if print_out:
+        logger = StdOutLogger(logger)
     out = []
     out.append(f"Executing:\n    {cmd=}\n")
     prev_java_home = None     
@@ -139,27 +197,27 @@ def run_cmd(cmd, env, cwd=None, print_out=False, gradle=False):
         env["JAVA_HOME"] = env["GRADLE_JAVA_HOME"]
 
     try:
+        logger.log(f"Executing command: {' '.join(cmd)}")
         process = subprocess.Popen(cmd, env=env, cwd=cwd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, universal_newlines=True, text=True, errors='backslashreplace')
-        if print_out:
-            print("============== output =============")
         for line in iter(process.stdout.readline, ""):
             out.append(line)
-            if print_out:
-                print(line, end="")
-        if print_out:
-            print("\n========== end of output ==========")
-        return "".join(out), process.wait()
+        out_str = "".join(out)
+        logger.log_block("output", out_str)
+        return out_str, process.wait()
     finally:
         if prev_java_home:
             env["JAVA_HOME"] = prev_java_home
 
-def check_ouput(txt, out, contains=True):
+def check_ouput(txt, out, contains=True, logger: Optional[LoggerBase] =None):
+    # if logger is passed, we assume that it already contains the output
     if contains and txt not in out:
-        print_output(out, f"expected '{txt}' in output")
-        assert False
+        if not logger:
+            print_output(out, f"expected '{txt}' in output")
+        assert False, f"expected '{txt}' in output. \n{logger}"
     elif not contains and txt in out:
-        print_output(out, f"did not expect '{txt}' in output")
-        assert False
+        if not logger:
+            print_output(out, f"did not expect '{txt}' in output")
+        assert False, f"did not expect '{txt}' in output. {logger}"
 
 def print_output(out, err_msg=None):
     print("============== output =============")

graalpython/com.oracle.graal.python.test/src/tests/standalone/util.py

@@ -40,9 +40,6 @@
  */
 package org.graalvm.python.embedding.tools.vfs;
 
-import org.graalvm.python.embedding.tools.exec.GraalPyRunner;
-import org.graalvm.python.embedding.tools.exec.SubprocessLog;
-
 import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
@@ -55,18 +52,22 @@
 import java.nio.file.attribute.BasicFileAttributes;
 import java.nio.file.attribute.PosixFilePermission;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
+import java.util.TreeSet;
+import java.util.function.Consumer;
 import java.util.function.Predicate;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
+import org.graalvm.python.embedding.tools.exec.GraalPyRunner;
+import org.graalvm.python.embedding.tools.exec.SubprocessLog;
+
 public final class VFSUtils {
 
     public static final String VFS_ROOT = "org.graalvm.python.vfs";
@@ -121,34 +122,46 @@ private static void createParentDirectories(Path path) throws IOException {
     }
 
     public static void generateVFSFilesList(Path vfs) throws IOException {
+        TreeSet<String> entriesSorted = new TreeSet<>();
+        generateVFSFilesList(vfs, entriesSorted, null);
         Path filesList = vfs.resolve(VFS_FILESLIST);
+        Files.write(filesList, entriesSorted);
+    }
+
+    // Note: forward slash is not valid file/dir name character on Windows,
+    // but backslash is valid file/dir name character on UNIX
+    private static final boolean REPLACE_BACKSLASHES = File.separatorChar == '\\';
+
+    private static String normalizeResourcePath(String path) {
+        return REPLACE_BACKSLASHES ? path.replace("\\", "/") : path;
+    }
+
+    /**
+     * Adds the VFS filelist entries to given set. Caller may provide a non-empty set.
+     */
+    public static void generateVFSFilesList(Path vfs, Set<String> ret, Consumer<String> duplicateHandler) throws IOException {
         if (!Files.isDirectory(vfs)) {
-            throw new IOException(String.format("'%s' has to exist and be a directory.\n", vfs.toString()));
+            throw new IOException(String.format("'%s' has to exist and be a directory.\n", vfs));
         }
-        var ret = new HashSet<String>();
         String rootPath = makeDirPath(vfs.toAbsolutePath());
         int rootEndIdx = rootPath.lastIndexOf(File.separator, rootPath.lastIndexOf(File.separator) - 1);
-        ret.add(rootPath.substring(rootEndIdx));
+        ret.add(normalizeResourcePath(rootPath.substring(rootEndIdx)));
         try (var s = Files.walk(vfs)) {
             s.forEach(p -> {
+                String entry = null;
                 if (Files.isDirectory(p)) {
                     String dirPath = makeDirPath(p.toAbsolutePath());
-                    ret.add(dirPath.substring(rootEndIdx));
+                    entry = dirPath.substring(rootEndIdx);
                 } else if (Files.isRegularFile(p)) {
-                    ret.add(p.toAbsolutePath().toString().substring(rootEndIdx));
+                    entry = p.toAbsolutePath().toString().substring(rootEndIdx);
                 }
-            });
-        }
-        String[] a = ret.toArray(new String[ret.size()]);
-        Arrays.sort(a);
-        try (var wr = new FileWriter(filesList.toFile())) {
-            for (String f : a) {
-                if (f.charAt(0) == '\\') {
-                    f = f.replace("\\", "/");
+                if (entry != null) {
+                    entry = normalizeResourcePath(entry);
+                    if (!ret.add(entry) && duplicateHandler != null) {
+                        duplicateHandler.accept(entry);
+                    }
                 }
-                wr.write(f);
-                wr.write("\n");
-            }
+            });
         }
     }