RFR: 7903809: Invalid source release N with --enable-preview [v3]

[Jaikiran Pai]

On Thu, 12 Sep 2024 08:42:36 GMT, Christian Stein <cstein at openjdk.org> wrote:

>> Please review this change to prevent "invalid source release N with --enable-preview" errors for when an explicit compile task targets a different JDK feature release number then the JDK under test (running the compilation).
>
> Christian Stein has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Use `String` comparison to support versions < 9, for example `"1.8"`

src/share/classes/com/sun/javatest/regtest/exec/CompileAction.java line 358:

> 356:                         } else {
> 357:                             sourceOrReleaseVersion = "next-argument";
> 358:                         }

Hello Christian, the use of "not-set" and "next-argument" string literals feels a bit odd. Do you think we could achieve this same thing with something like the following (untested) patch:


diff --git a/src/share/classes/com/sun/javatest/regtest/exec/CompileAction.java b/src/share/classes/com/sun/javatest/regtest/exec/CompileAction.java
index eb6bd63..6d48044 100644
--- a/src/share/classes/com/sun/javatest/regtest/exec/CompileAction.java
+++ b/src/share/classes/com/sun/javatest/regtest/exec/CompileAction.java
@@ -327,17 +327,14 @@ public class CompileAction extends Action {
         List<String> jcodArgs = new ArrayList<>();
         boolean runJavac = process;
 
-        int insertPos = -1;
         boolean seenSourceOrRelease = false;
         boolean seenEnablePreview = false;
-
-        for (String currArg : args) {
+        String sourceOrReleaseVersion = null;
+        for (int i = 0; i < args.size(); i++) {
+            String currArg = args.get(i);
             if (currArg.endsWith(".java")) {
                 if (!(new File(currArg)).exists())
                     throw new TestRunException(CANT_FIND_SRC + currArg);
-                if (insertPos == -1) {
-                    insertPos = javacArgs.size();
-                }
                 javacArgs.add(currArg);
                 runJavac = true;
             } else if (currArg.endsWith(".jasm")) {
@@ -353,11 +350,13 @@ public class CompileAction extends Action {
                     case "-source":
                     case "--source":
                     case "--release":
-                        if (insertPos == -1) {
-                            insertPos = javacArgs.size();
-                        }
                         seenSourceOrRelease= true;
-                        break;
+                        if (eq != -1) {
+                            sourceOrReleaseVersion = currArg.substring(eq + 1).trim();
+                        } else {
+                            // read from next arg (if any)
+                            sourceOrReleaseVersion = (args.size() > i + 1) ? args.get(i + 1) : null;
+                        }
                 }
                 javacArgs.add(currArg);
             }
@@ -368,14 +367,15 @@ public class CompileAction extends Action {
                 && !seenEnablePreview
                 && (script.enablePreview() || usesLibraryCompiledWithPreviewEnabled())
                 && (libLocn == null || libLocn.isTest())) {
-            if (insertPos == -1) { // happens for example in "-proc:only CLASS" cases
-                insertPos = 0;     // prepend in order to not mess with variadic arguments
-            }
-            javacArgs.add(insertPos, "--enable-preview");
+            String version = script.getTestJDKVersion().name();
+            // always prepend in order to not mess with variadic arguments
             if (!seenSourceOrRelease) {
-                int v = script.getTestJDKVersion().major;
-                javacArgs.add(insertPos + 1, "-source");
-                javacArgs.add(insertPos + 2, String.valueOf(v));
+                javacArgs.add(0, version);
+                javacArgs.add(0, "-source");
+            }
+            // 7903809: prevent invalid source release errors
+            if (!seenSourceOrRelease || version.equals(sourceOrReleaseVersion)) {
+                javacArgs.add(0, "--enable-preview");
             }
         }

-------------

PR Review Comment: https://git.openjdk.org/jtreg/pull/226#discussion_r1756678026