[jdk8u-dev] RFR: 8309841: Jarsigner should print a warning if an entry is removed

[Andrew John Hughes]

On Fri, 21 Mar 2025 17:10:08 GMT, Andrew John Hughes <andrew at openjdk.org> wrote:

>> Patch mostly looks good, but I think we should backport [JDK-8240235](https://bugs.openjdk.org/browse/JDK-8240235) first in its own PR. It is little more than the `copyEntry` method you've imported here, but also fixes other cases in `JarUtils.java` where it should also be used.
>> 
>> Also, should `java.nio.file.StandardCopyOption` be imported in `JarUtils.java`? I see the import being added but no reference to it added in the code.
>
>> @gnu-andrew
>> 
>> > Patch mostly looks good, but I think we should backport [JDK-8240235](https://bugs.openjdk.org/browse/JDK-8240235) first in its own PR.
>> 
>> Ok, if it's needed. I will do this.
>> 
> 
> I think it would be good to fix the existing bug in `updateJar` if we are bringing in the method that provides the fix anyway. Otherwise, we have different behaviour in `updateJar` and the new `deleteEntries`.
> 
>> > Also, should `java.nio.file.StandardCopyOption` be imported in `JarUtils.java`? I see the import being added but no reference to it added in the code.
>> 
>> There is the line 297:
>> 
>> ```
>> Files.move(tmpfile, jarfile, StandardCopyOption.REPLACE_EXISTING);
>> ```
> 
> Ah, thanks. I now see why I was confused here. The `import` is unique to 8u because 11u already has it via [JDK-8211171](https://bugs.openjdk.org/browse/JDK-8211171) but the code that uses it is new to both, so the former showed up as a difference between the 8u & 11u patches, but not the latter. I should have checked the actual patch.

> @gnu-andrew BTW [JDK-8240235](https://bugs.openjdk.org/browse/JDK-8240235) modifies a line in `updateJarFile` procedure which was added by [JDK-8211171](https://bugs.openjdk.org/browse/JDK-8211171). Do you think [JDK-8211171](https://bugs.openjdk.org/browse/JDK-8211171) should be backported also?

I was just looking at the same bug with respect to where the import statement came from in 11u. I think 8211171 might be useful for 8u, but it's a bit involved to require for this change, as it updates numerous tests which use `JarUtils`. I'm happy for a JDK-8240235 to just update the existing method, `updateJar`. I'll take a look at 8211171 and include the updated line in that backport if I decide to go ahead.

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

PR Comment: https://git.openjdk.org/jdk8u-dev/pull/635#issuecomment-2743975905