On Fri, 24 Mar 2023 05:20:34 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>>> I don't think we should be exporting that package. Instead, the `JarIndex` 
>>> class can be updated to do `AccessController.doPrivileged(...)`.
>> 
>> Nice, I'll try that!
>> 
>>  Do you know if the `jar` tool allows running with a `SecurityManager`? If 
>> not, we could perhaps simply use `System.getProperty`?
>
>> > I don't think we should be exporting that package. Instead, the `JarIndex` 
>> > class can be updated to do `AccessController.doPrivileged(...)`.
>> 
>> Nice, I'll try that!
>> 
>> Do you know if the `jar` tool allows running with a `SecurityManager`? If 
>> not, we could perhaps simply use `System.getProperty`?
> 
> I had a look at the existing code in the jar tool. It appears that it doesn't 
> run with a SecurityManager - there's direct calls to `System.getProperty()` 
> in some places in the code. So I think just calling `System.getProperty()` 
> should be fine. When this PR goes into a published state for review, I'm sure 
> others who have more knowledge about this area will help decide.

@jaikiran 

Looking into how the deprecation warning could be implemented, it seems `jar` 
has some existing infrastructure for deprecation warnings, so the deprecation 
could be implemented with the following small patch:


Subject: [PATCH] Remove JarIndex default constructor
---
Index: src/jdk.jartool/share/classes/sun/tools/jar/Main.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/jdk.jartool/share/classes/sun/tools/jar/Main.java 
b/src/jdk.jartool/share/classes/sun/tools/jar/Main.java
--- a/src/jdk.jartool/share/classes/sun/tools/jar/Main.java     (revision 
fee0da27be832a7413886ddd2cfd5586bfd2e37a)
+++ b/src/jdk.jartool/share/classes/sun/tools/jar/Main.java     (date 
1679654673667)
@@ -396,6 +396,9 @@
                     }
                 }
             } else if (iflag) {
+                if (!suppressDeprecateMsg) {
+                    warn(formatMsg("warn.flag.is.deprecated", "-i"));
+                }
                 String[] files = filesMap.get(BASE_VERSION);  // base entries 
only, can be null
                 genIndex(rootjar, files);
             } else if (dflag) {


This gives the following warning message on use:


% build/macosx-x86_64-server-release/images/jdk/bin/jar --generate-index 
file.jar
Warning: The -i option is deprecated, and is planned for removal in a future 
JDK release


Which may be overridden by using `-XDsuppress-tool-removal-message`:


% build/macosx-x86_64-server-release/images/jdk/bin/jar --generate-index 
file.jar -XDsuppress-tool-removal-message

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

PR Comment: https://git.openjdk.org/jdk/pull/13158#issuecomment-1482612735

Reply via email to