Copilot commented on code in PR #2516:
URL: https://github.com/apache/groovy/pull/2516#discussion_r3188846497
##########
subprojects/groovy-ant/src/test/groovy/org/codehaus/groovy/ant/GroovycTest.java:
##########
@@ -21,6 +21,10 @@
import org.apache.tools.ant.BuildException;
import org.apache.tools.ant.Project;
import org.apache.tools.ant.ProjectHelper;
+import org.apache.tools.ant.types.Commandline;
Review Comment:
`Commandline` is imported but never used in this test class. Please remove
the unused import to keep the test sources warning-free (and to avoid potential
build failures if the build treats warnings as errors).
##########
subprojects/groovy-ant/src/main/java/org/codehaus/groovy/ant/Groovyc.java:
##########
@@ -702,6 +712,62 @@ public void addConfiguredJavac(final Javac javac) {
jointCompilation = true;
}
+ /**
+ * Adds a JVM argument to be passed to the forked compiler. Only takes
effect
+ * when {@code fork} is true. Use a nested {@code <jvmarg>} element, e.g.
+ * {@code <jvmarg value="--add-opens=java.base/java.lang=ALL-UNNAMED"/>}.
+ *
+ * @return a new {@code Commandline.Argument} that can be configured by Ant
+ * @since 6.0.0
+ */
+ public Commandline.Argument createJvmarg() {
+ return jvmArgs.createArgument();
+ }
+
+ /**
+ * Adds a system property to be passed to the forked compiler as
+ * {@code -Dkey=value}. Only takes effect when {@code fork} is true.
+ *
+ * @param sysp the system property
+ * @since 6.0.0
+ */
+ public void addSysproperty(Environment.Variable sysp) {
+ sysProperties.addVariable(sysp);
+ }
+
+ /**
+ * Adds a set of system properties (Ant {@code <syspropertyset>}) to be
+ * passed to the forked compiler. Only takes effect when {@code fork} is
true.
+ *
+ * @param sysp the property set
+ * @since 6.0.0
+ */
+ public void addSyspropertyset(PropertySet sysp) {
+ sysPropertySets.add(sysp);
+ }
+
+ /**
+ * If true, pass all properties from the parent Ant project to the forked
JVM
+ * as system properties so they can be read via {@code
System.getProperty(name)}.
+ * Only takes effect when {@code fork} is true. Defaults to false.
+ * <p>
+ * For fine-grained control, use a nested {@code <sysproperty>} or
+ * {@code <syspropertyset>} instead. Both may be combined; explicit nested
+ * entries take precedence on name collision.
+ *
+ * @param inheritAll true to inherit all Ant properties into the forked JVM
+ * @since 6.0.0
+ */
+ public void setInheritAll(boolean inheritAll) {
+ this.inheritAll = inheritAll;
+ }
+
+ // package-private accessors for testing GROOVY-11995 wiring
+ Commandline getJvmArgs() { return jvmArgs; }
+ Environment getSysProperties() { return sysProperties; }
+ List<PropertySet> getSysPropertySets() { return sysPropertySets; }
+ boolean isInheritAll() { return inheritAll; }
+
Review Comment:
These new package-private accessors (`getJvmArgs`, `getSysProperties`,
`getSysPropertySets`, `isInheritAll`) don't appear to be used by production
code or the added tests (which validate behavior via `doForkCommandLineList`).
If they aren't needed, please remove them to avoid expanding the task's
internal surface area.
##########
subprojects/groovy-ant/src/main/java/org/codehaus/groovy/ant/Groovyc.java:
##########
@@ -1142,6 +1209,30 @@ private void doForkCommandLineList(List<String>
commandLineList, Path classpath,
tmpExtension = tmpExtension.substring(1);
commandLineList.add("-Dgroovy.default.scriptExtension=" +
tmpExtension);
}
+ // GROOVY-11995: user-supplied JVM args / system properties
+ Collections.addAll(commandLineList, jvmArgs.getArguments());
+ Set<String> sysPropertyNames = new HashSet<>();
Review Comment:
User-supplied JVM args are appended after the task adds its required
bootstrap `-classpath`. If a user passes `-classpath`/`-cp` via `<jvmarg>`, it
will override the bootstrap classpath (last one wins) and can make the forked
compiler fail to start. Consider either (1) inserting user `<jvmarg>` entries
before the bootstrap `-classpath` is added, or (2) validating/rejecting
classpath-setting JVM args (and documenting the restriction).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
