JaroslavTulach commented on a change in pull request #2731:
URL: https://github.com/apache/netbeans/pull/2731#discussion_r573491819



##########
File path: java/maven/apichanges.xml
##########
@@ -83,6 +83,49 @@ is the proper place.
     <!-- ACTUAL CHANGES BEGIN HERE: -->
 
     <changes>
+        <change id="ProjectLookup.fallback">
+            <api name="general"/>
+            <summary>Allow project services, that are ordered <b>after</b> 
specific packaging type</summary>
+            <version major="2" minor="144"/>
+            <date day="10" month="2" year="2021"/>
+            <author login="sdedic"/>
+            <compatibility addition="yes" binary="compatible" 
semantic="compatible"/>
+            <description>
+                <p>
+                    Project lookup consists of base services, plus services 
for the project's packaging type. This change allows for

Review comment:
       I would expect a note describing how to register such service! Maybe a 
link to `architecture-summary.html#xyz`.

##########
File path: 
java/maven/src/org/netbeans/modules/maven/execute/defaultActionMappings.xml
##########
@@ -260,8 +274,9 @@
         <properties>
             <test>${packageClassName}</test>
             <forkMode>once</forkMode>
-            <maven.surefire.debug>${exec.args}</maven.surefire.debug>
+            <maven.surefire.debug>${exec.vmArgs}</maven.surefire.debug>
             <!-- need to declare exec.args property to engage the 
StartupExtender infrastructure -->
+            <exec.vmArgs/>

Review comment:
       The changes in this file are very illustrative!
   
   The comment "need to declare exec.args property" is now slightly off...

##########
File path: java/maven/arch.xml
##########
@@ -115,6 +115,13 @@
        will be merged into a single instance in the project's lookup.
      </p>
     </api>
+    <api group="layer" name="MavenPackagingLookup" type="export" 
category="devel">
+        <p>
+            
<code>Projects/org-netbeans-modules-maven/&lt;packaging-type>/Lookup</code> is 
added to the project's additional Lookup. The content is expected
+            to contain packaing-specific services and processors, for example, 
<a 
href="@TOP@/org/netbans/modules/maven/api/execute/PrerequisitesChecker.html">PrerequisitesCheckers</a>.
+            In addition, 
<code>Projects/org-netbeans-modules-maven/_last/Lookup</code> defines services 
that act after the packaging-specific ones.

Review comment:
       Naming is hard: `_last`!? Any? Generic? Fallback? Why there is the `_`? 

##########
File path: 
java/maven/src/org/netbeans/modules/maven/execute/MavenCommandLineExecutor.java
##########
@@ -100,10 +103,28 @@
 
 /**
  * support for executing maven, externally on the command line.
+ * <b>Since 2/1.144</b>, the {@link LateBoundPrerequisitesChecker} registered 
in Maven projects for JAR packaging by default supports 
+ * {@link ExplicitProcessParameters} API. The caller of the execute-type 
action can request to append or replace VM or user
+ * application parameters. The parameters recorded in the POM.xml or NetBeans 
action mappings are augmented according to that
+ * instructions:
+ * <ul>
+ * <li><b>launcherArgs</b> are mapped to VM arguments (precede main class name)
+ * <li><b>args</b> are mapped to user application arguments (after main class 
name)
+ * </ul>
+ * VM parameters injected by {@link StartupExtender} API are not affected by 
this feature. 
+ * <p>
+ * Example use:
+ * {@codesnippet MavenExecutionTestBase#samplePassAdditionalVMargs}
+ * The example will <b>append</b> <code>-DvmArg2=2</code> to VM arguments and 
<b>replaces</b> all user
+ * program arguments with <code>"paramY"</code>. Append mode can be controlled 
using {@link ExplicitProcessParameters.Builder#appendArgs} or
+ * {@link ExplicitProcessParameters.Builder#appendPriorityArgs}.
+ * 
  * @author  Milos Kleint ([email protected])
+ * @author  Svata Dedic ([email protected])
  */
 public class MavenCommandLineExecutor extends AbstractMavenExecutor {
     static final String ENV_PREFIX = "Env."; //NOI18N
+    static final String INTERNAL_PREFIX = "NbIde."; //NOI18N

Review comment:
       Is this introducing some form of property based API?

##########
File path: 
java/maven/test/unit/src/org/netbeans/modules/maven/runjar/MavenExecuteUtilsTest.java
##########
@@ -0,0 +1,191 @@
+/*
+ * 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.
+ */
+package org.netbeans.modules.maven.runjar;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import static org.junit.Assert.*;
+
+/**
+ *
+ * @author sdedic
+ */
+public class MavenExecuteUtilsTest {

Review comment:
       Can you make sure these tests are executed in a Travis gate?

##########
File path: java/maven/arch.xml
##########
@@ -186,6 +193,20 @@
           to specify the default behavior of compile on save in Maven 
           projects.
       </api> 
+      <api category="devel" group="property" name="exec.vmArgs" type="export">
+          The plugin exports Java VM parameters to be used for application 
execution in <code>${exec.vmArgs}</code> 
+          property that can be used in action mappings or Maven pom.xml.
+      </api>
+      <api category="devel" group="property" name="exec.appArgs" type="export">
+          The plugin exports application parameters to be used for application 
execution in <code>${exec.appArgs}</code> 
+          property that can be used in action mappings or Maven pom.xml.
+      </api>
+      <api category="devel" group="lookup" name="ExplicitProcessParameters" 
type="export">

Review comment:
       In this case `devel` is fine (even I would prefer higher commitment ;-).

##########
File path: java/maven/arch.xml
##########
@@ -186,6 +193,20 @@
           to specify the default behavior of compile on save in Maven 
           projects.
       </api> 
+      <api category="devel" group="property" name="exec.vmArgs" type="export">

Review comment:
       As this is going to be used in end-user projects configurations, I am 
afraid `devel` is a bit too low category in the long term.

##########
File path: 
java/maven/src/org/netbeans/modules/maven/runjar/MavenExecuteUtils.java
##########
@@ -0,0 +1,859 @@
+/*
+ * 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.
+ */
+package org.netbeans.modules.maven.runjar;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.logging.Logger;
+import java.util.regex.Pattern;
+import org.codehaus.plexus.util.cli.CommandLineUtils;
+import org.netbeans.api.annotations.common.NonNull;
+import org.netbeans.modules.maven.NbMavenProjectImpl;
+import org.netbeans.modules.maven.api.customizer.ModelHandle2;
+import org.netbeans.modules.maven.customizer.RunJarPanel;
+import org.netbeans.modules.maven.execute.ActionToGoalUtils;
+import org.netbeans.modules.maven.execute.model.ActionToGoalMapping;
+import org.netbeans.modules.maven.execute.model.NetbeansActionMapping;
+import org.netbeans.modules.maven.runjar.RunJarStartupArgs;
+import org.netbeans.spi.project.ActionProvider;
+
+/**
+ *
+ * @author sdedic
+ */
+public final class MavenExecuteUtils {
+    /**
+     * Hint for the default PrereqqCheckeers that explicit parameters have 
been already processed.
+     * Will not be propagated to maven process.
+     */
+    public static final String RUN_EXPLICIT_PROCESSED = 
"NbIde.ExplicitParametersApplied"; // NOI18N

Review comment:
       Here is the use of the `NbIde.` internal property contract. To be 
"pintlich" it should be mentioned in `arch.xml` as `private` (as the 
declaration and usage stay inside of a single module) API.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

Reply via email to