RFR: 8302819: Remove JAR Index [v2]

[Alan Bateman]

On Wed, 29 Mar 2023 19:55:37 GMT, Eirik Bjorsnos <duke at openjdk.org> wrote:

>> This PR removes the JAR index feature from the runtime:
>> 
>> - `URLClassPath` is  updated to remove the `enableJarIndex` system property and any code which would be called when this property was `true`
>> - The `JarIndex` implementation class is moved into `jdk.jartool` module.
>> - The `InvalidJarIndexError` exception class is removed because it falls out of use
>> - The test `test/jdk/sun/misc/JarIndex/metaInfFileNames/Basic.java` is removed because it depends on the JarIndex feature being present
>> - The test `test/jdk/sun/misc/JarIndex/JarIndexMergeForClassLoaderTest.java` is removed because it depends on the JarIndex feature being present
>> - The test `test/jdk/sun/misc/JarIndex/JarIndexMergeTest.java` is removed because it end up being the only caller of the  JarIndex.merge feature
>> - All `JarIndex` methods/constructors which are not used by the `jar -i` implementation are removed. 
>> - `JarIndex` is given package-private access.
>> 
>> Outstanding code work:
>> 
>> - Create tests for `JarFile` and `JarInputStream` accepting dusty INDEX jars.
>> 
>> Outstanding work:
>> 
>> - CSR for the removal
>> - Release notes for the removal
>> - Coordination of the update of the Jar File Specification
>
> Eirik Bjorsnos has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision:
> 
>  - Remove blank line 93 as per suggestion by Mandy
>  - Merge branch 'master' into remove-jar-index
>  - Remove JarIndex default constructor
>  - Make JarIndex.INDEX_NAME package private
>  - Remove outdated reference to URLClassLoader using JarIndex
>  - Make JarIndex package private
>  - The JarIndex.merge method ended up being used only by the JarIndexMergeTest test. Removing this test, the JarIndex.merge methods and a number of other JarIndex methods not used by the 'jar' tool
>  - Revert the export of sun.security.action to jdk.jartool. JarIndex can use System.property instead since the 'jar' tool does not run with a SecurityManager
>  - Revert noisy whitespace changes
>  - Revert noisy whitespace changes
>  - ... and 1 more: https://git.openjdk.org/jdk/compare/e90f7996...2b5d8a4a

This looks good. I've added the "csr" label to the PR.

For "jar -i", I think we are converging on the option to just add a warning, meaning it will generated/update the index as before, but emit a warning that JAR Index is ignored since JDK 18. It doesn't matter if we do this in this PR or a separate PR. For CSR and release note purposes it would be a bit easier if it was this PR, would that be okay?

src/java.base/share/classes/java/util/jar/JarFile.java line 212:

> 210:     /**
> 211:      * The 'JAR index' feature has been removed, but we still need to
> 212:      * process existing files which have this entry.

"but we still need ..."   Maybe better to say that that JarInputStream and the verification of signed JARs need to skip this attribute.

src/jdk.jartool/share/classes/sun/tools/jar/Main.java line 73:

> 71: import jdk.internal.opt.CommandLine;
> 72: 
> 73: import java.time.LocalDateTime;

This can move up to the imports of the other java.* classes.

test/jdk/java/util/jar/JarFile/mrjar/TestVersionedStream.java line 29:

> 27:  * @summary basic tests for multi-release jar versioned streams
> 28:  * @library /test/lib
> 29:  * @modules jdk.jartool/sun.tools.jar

This test could be changed to use ToolProvider.findFirst("jar") and it would avoid needing to use sun.tools.jar.Main directly.

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

PR Review: https://git.openjdk.org/jdk/pull/13158#pullrequestreview-1364446354
PR Review Comment: https://git.openjdk.org/jdk/pull/13158#discussion_r1152816329
PR Review Comment: https://git.openjdk.org/jdk/pull/13158#discussion_r1152817035
PR Review Comment: https://git.openjdk.org/jdk/pull/13158#discussion_r1152814946