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

[Christian Stein]

On Thu, 12 Sep 2024 11:31:55 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

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

Addressed as suggested via https://github.com/openjdk/jtreg/pull/226/commits/2350623c475ef30b3e40b941b7b7e262f0c71130

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

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