On Thu, 30 Mar 2023 07:00:47 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> 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/85c16dfb...2b5d8a4a > > 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. I rephrased this to be more specific about which code still needs this. Agree "we" is not the best choice of subject here. > 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. Fixed. > 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. An improvement a bit unrelated to this PR, but it was a simple change so I have implemented it. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13158#discussion_r1153102163 PR Review Comment: https://git.openjdk.org/jdk/pull/13158#discussion_r1153102298 PR Review Comment: https://git.openjdk.org/jdk/pull/13158#discussion_r1153101442
