[GitHub] [maven-scripting-plugin] bmarwell commented on a diff in pull request #5: [MSCRIPTING-9] - basic java scripting support

2022-04-19 Thread GitBox


bmarwell commented on code in PR #5:
URL: 
https://github.com/apache/maven-scripting-plugin/pull/5#discussion_r853402558


##
README.md:
##
@@ -65,7 +65,7 @@ There are some guidelines which will make applying PRs easier 
for us:
  Optional supplemental description.
 ```
 + Make sure you have added the necessary tests (JUnit/IT) for your changes.
-+ Run all the tests with `mvn -Prun-its verify` to assure nothing else was 
accidentally broken.
++ Run all the tests with `mvn -Prun-its verify` to assure nothing else was 
accidentally broken (you may need to run `mvn install` beforehand).

Review Comment:
   Review hint: This was not introduced by @rmannibucau’s change, mrm+invoker 
was already broken before.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [maven-scripting-plugin] bmarwell commented on a diff in pull request #5: [MSCRIPTING-9] - basic java scripting support

2022-04-19 Thread GitBox


bmarwell commented on code in PR #5:
URL: 
https://github.com/apache/maven-scripting-plugin/pull/5#discussion_r853357835


##
README.md:
##
@@ -65,7 +65,7 @@ There are some guidelines which will make applying PRs easier 
for us:
  Optional supplemental description.
 ```
 + Make sure you have added the necessary tests (JUnit/IT) for your changes.
-+ Run all the tests with `mvn -Prun-its verify` to assure nothing else was 
accidentally broken.
++ Run all the tests with `mvn -Prun-its verify` to assure nothing else was 
accidentally broken (you can need to `mvn install` before).

Review Comment:
   ```suggestion
   + Run all the tests with `mvn -Prun-its verify` to assure nothing else was 
accidentally broken (you may need to run `mvn install` beforehand).
   ```



##
src/main/java/org/apache/maven/plugins/scripting/engine/JavaEngine.java:
##
@@ -0,0 +1,327 @@
+package org.apache.maven.plugins.scripting.engine;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import javax.script.AbstractScriptEngine;
+import javax.script.Bindings;
+import javax.script.Compilable;
+import javax.script.CompiledScript;
+import javax.script.ScriptContext;
+import javax.script.ScriptEngine;
+import javax.script.ScriptEngineFactory;
+import javax.script.ScriptException;
+import javax.script.SimpleBindings;
+import javax.tools.JavaCompiler;
+import javax.tools.ToolProvider;
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.IOException;
+import java.io.Reader;
+import java.io.StringReader;
+import java.io.Writer;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.util.stream.Stream;
+
+import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.joining;
+
+/**
+ * The java engine implementation.
+ */
+public class JavaEngine extends AbstractScriptEngine implements Compilable
+{
+private final ScriptEngineFactory factory;
+
+public JavaEngine( ScriptEngineFactory factory )
+{
+this.factory = factory;
+}
+
+@Override
+public CompiledScript compile( String script ) throws ScriptException
+{
+// plexus compiler is great but overkill there so don't bring it just 
for that
+final JavaCompiler compiler = requireNonNull(
+ToolProvider.getSystemJavaCompiler(),
+"you must run on a JDK to have a compiler" );
+Path tmpDir = null;
+try
+{
+tmpDir = Files.createTempDirectory( getClass().getSimpleName() );
+
+final String packageName = getClass().getPackage().getName() + 
".generated";
+final String className = "JavaCompiledScript_" + Math.abs( 
script.hashCode() );
+final String source = toSource( packageName, className, script );
+final Path src = tmpDir.resolve( "sources" );
+final Path bin = tmpDir.resolve( "bin" );
+final Path srcDir = src.resolve( packageName.replace( '.', '/' ) );
+Files.createDirectories( srcDir );
+Files.createDirectories( bin );
+final Path java = srcDir.resolve( className + ".java" );
+try ( Writer writer = Files.newBufferedWriter( java ) )
+{
+writer.write( source );
+}
+
+// todo: enable to control it from the project but requires a bit 
too much config effort for this iteration
+final String classpath = mavenClasspathPrefix() + 
System.getProperty( getClass().getName() + ".classpath",
+System.getProperty( "java.class.path", System.getProperty( 
"surefire.real.class.path" ) ) );
+
+// todo: use log, not very important for now so using std streams

Review Comment:
   ```suggestion
   // TODO: use a Logger in subsequent releases. Not very important 
as of now, so using std streams
   ```



##
src/main/java/org/apache/maven/plugins/scripting/engine/JavaEngine.java:
##
@@ -0,0