Use "file" scheme absolute URIs for "Class-Path" entries we create in our -dev.jar

[jaikiran]

(This PR will need a review and approval from @dmlloyd.)

Fixes #5359

Background:

jar files in Java allow the MANIFEST.MF file to contain a Class-Path manifest attribute. The specific of this attribute states that relative URI is allowed here. We (Quarkus) currently set this up in the io.quarkus.maven.DevMojo#addToClassPaths, (only) for our <app>-dev.jar file. This jar file gets used in a few of different places/ways:

1. This <app>-dev.jar is used to launch the dev mode using the java command:

java -jar ... <app>-dev.jar

2. The io.quarkus.dev.ClassLoaderCompiler parses the Class-Path manfiest attribute of this jar and uses it to setup classpath for triggering programatic compilation of the application, during hot deployment. This programatic compilation involves passing the classpath to the javax.tools.JavacCompiler.

3. When the jar file paths (parsed out of #2 above) are passed to the javax.tools.JavacCompiler, the implementation in OpenJDK, internally goes ahead and parses each of those passed jar files (again) for its own Class-Path attribute parsing and adding that path to the compilation. As part of Work around an issue in JDK which causes hot deployment compilation to fail #4527, we explicitly avoid passing the <app>-dev.jar to the programatic compilation through JavacCompiler since although Quarkus sets up this Class-Path attribute correctly in that jar file, due to a bug in the JDK it ends up causing issues (on Windows).

Current state:

With our current upstream, things work fine with Java 8, 11 and 13 for *nix systems. However, on Windows #1 (above) is broken starting Java 13 (as stated in #5359). Again at this point, based on discussions on OpenJDK list, this still doesn't appear to be a Quarkus fault (http://mail.openjdk.java.net/pipermail/core-libs-dev/2019-November/063566.html).

Proposal:

As noted in that OpenJDK dev mailing list, it's now considered a valid thing to use an absolute URI in the Class-Path attribute if that entry is of file protocol/scheme and the "context jar" (in our case it would be the <app>-dev.jar) is loaded from the filesystem (https://docs.oracle.com/en/java/javase/13/docs/specs/jar/jar.html#class-path-attribute). Note that, although this is documented in the specification only starting Java 13, this works (as an implementation detail) even in previous versions of Java (like Java 8 and 11).

This PR now uses a absolute file scheme URI path when it creates the Class-Path for the <app>-dev.jar. Furthermore, the ClassLoaderCompiler, which deals with hot deployment, now first checks if entries that it finds are absolute file scheme URIs, in which case it treats it as a filesystem path and uses it, instead of trying to resolve a relative URI.

Encoding:

The original code in DevMojo made sure that the encoding was done correctly while writing out the Class-Path entry. I've verified that the changed code when writing out the file scheme URI does indeed write out encoded path. More specifically, I have verified that final URI uri = file.toPath().toAbsolutePath().toUri(); contains an encoded path (ex: space characters are replaced with %20), so when that uri.toString() is used to write out the entry, the encoded path gets added to the Class-Path attribute.

Additionally, the previous code had an explicit check to see if the file is a directory and if so, it added a / (if it wasn't present) to the classpath entry. I've verified that this isn't needed with the changed code since the URI returned by file.toPath().toAbsolutePath().toUri() already does that.

Testing:

The good part - We already have io.quarkus.maven.it.DevMojoIT test case in integration-tests/maven which has a good coverage for testing the quarkus:dev launching and hot replacement feature (both of which are impacted by this issue/change). So why wasn't this caught earlier? Our CI doesn't run against Java 13 as far as I know. In fact, I just ran this DevMojoIT test on latest upstream against a Windows OS using Java 13 and it fails exactly as what's reported in (#5359):

C:\quarkus\integration-tests\maven>mvn test -Dtest=DevMojoIT
[INFO] Scanning for projects...
[INFO]
[INFO] -------------< io.quarkus:quarkus-integration-test-maven >--------------
[INFO] Building Quarkus - Integration Tests - Maven tooling 999-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- maven-enforcer-plugin:3.0.0-M2:enforce (enforce) @ quarkus-integratio
n-test-maven ---
[INFO]
[INFO] --- maven-enforcer-plugin:3.0.0-M2:enforce (enforce-java-version) @ quark
us-integration-test-maven ---
[INFO]
[INFO] --- maven-enforcer-plugin:3.0.0-M2:enforce (enforce-maven-version) @ quar
kus-integration-test-maven ---
[INFO]
[INFO] --- buildnumber-maven-plugin:1.4:create (get-scm-revision) @ quarkus-inte
gration-test-maven ---
[INFO] Executing: cmd.exe /X /C "git rev-parse --verify HEAD"
[INFO] Working directory: C:\quarkus\integration-tests\maven
[INFO] Storing buildNumber: d9ef5cea89035f5c82b11e0cd211aba2e8f97e45 at timestam
p: 1574855605373
[INFO] Storing buildScmBranch: master
[INFO]
[INFO] --- formatter-maven-plugin:2.8.1:format (default) @ quarkus-integration-t
est-maven ---
[INFO] Using 'UTF-8' encoding to format source files.
[INFO] Number of files to be formatted: 10
[INFO] Successfully formatted:          0 file(s)
[INFO] Fail to format:                  0 file(s)
[INFO] Skipped:                         10 file(s)
[INFO] Read only skipped:               0 file(s)
[INFO] Approximate time taken:          0s
[INFO]
[INFO] --- impsort-maven-plugin:1.2.0:sort (sort-imports) @ quarkus-integration-
test-maven ---
[INFO]  Total Files Processed: 10 in 00:00.594
[INFO]         Already Sorted: 10
[INFO]         Needed Sorting:  0
[INFO]
[INFO] --- maven-resources-plugin:3.1.0:resources (default-resources) @ quarkus-
integration-test-maven ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory C:\quarkus\integration-tes
ts\maven\src\main\resources
[INFO]
[INFO] --- maven-compiler-plugin:3.8.0-jboss-2:compile (default-compile) @ quark
us-integration-test-maven ---
[INFO] No sources to compile
[INFO]
[INFO] --- maven-resources-plugin:3.1.0:testResources (default-testResources) @
quarkus-integration-test-maven ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 82 resources
[INFO] Copying 10 resources
[INFO]
[INFO] --- maven-compiler-plugin:3.8.0-jboss-2:testCompile (default-testCompile)
 @ quarkus-integration-test-maven ---
[INFO] Nothing to compile - all classes are up to date
[INFO]
[INFO] --- maven-surefire-plugin:2.22.1:test (default-test) @ quarkus-integratio
n-test-maven ---
[INFO]
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running io.quarkus.maven.it.DevMojoIT
[INFO] Scanning for projects...
[INFO]
[INFO] ---------------------------< org.acme:acme >----------------------------
[INFO] Building acme 1.0-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- maven-resources-plugin:2.6:resources (default-resources) @ acme ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 2 resources
[INFO]
[INFO] --- maven-compiler-plugin:3.1:compile (default-compile) @ acme ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 2 source files to C:\quarkus\integration-tests\maven
\target\test-classes\projects\project-classic-run\target\classes
[INFO]
[INFO] --- quarkus-maven-plugin:999-SNAPSHOT:dev (default-cli) @ acme ---
[INFO] Launching JVM with command line: [C:\jdk-13.0.1\bin\java.EXE, -XX:TieredS
topAtLevel=1, -Xverify:none, -Djava.util.logging.manager=org.jboss.logmanager.Lo
gManager, -jar, C:\quarkus\integration-tests\maven\target\test-classe
s\projects\project-classic-run\target\acme-dev.jar]
OpenJDK 64-Bit Server VM warning: Options -Xverify:none and -noverify were depre
cated in JDK 13 and will likely be removed in a future release.
Error: Could not find or load main class io.quarkus.dev.DevModeMain
Caused by: java.lang.ClassNotFoundException: io.quarkus.dev.DevModeMain

After the change that's proposed in this PR, I have run this DevMojoIT test against the same Windows OS setup using Java 8, 11 and 13. All passed fine for this test. So that gives us a bit of confidence that this won't/shouldn't break anything obvious with dev mode and hot replacement on Windows.

I have also run this against a local *nix setup with Java 13, just to make sure it doesn't cause any issues there. The test passed here too.

Future:

If this PR does get approved and merged, I don't know if we will go back to using relative URIs in the Class-Path for our <app>-dev.jar, especially since file scheme absolute URIs are now allowed at least in the context of how we use dev mode jar. @dmlloyd is a better person to decide that.

But for now, I don't see any other workaround to get Quarkus usable across Java 8, 11 and 13, without this change.

[dmlloyd]

Was the fix also tested on Java 8, 11, and 13 on both Windows and Linux? I'm concerned because as we discussed previously, some class path parsing routines in the JDK won't accept absolute URLs no matter what. So it'd be a problem if we fixed some use cases at the cost of others, especially by going to behavior that was non-spec behavior until 13.

[jaikiran]

Hello @dmlloyd,

Was the fix also tested on Java 8, 11, and 13 on both Windows and Linux?

Yes, I did test this on 8, 11 and 13 for Windows OS (using the DevMojoIT test which used to fail on JDK 13). For *nix, I tested it on JDK 13 and let our CI run it against JDK 8 and 11. The CI runs have come clean, so I think this should work across all these versions.

I can personally give a try against JDK 8 and 11 for *nix on a local setup, tomorrow, if needed.