RFR: JDK-8212780: JEP 343: Packaging Tool Implementation

[Andy Herrick]

On 11/12/2018 4:54 PM, Philip Race wrote:
>
> On 11/12/18, 1:45 PM, Andy Herrick wrote:
>>
>> On 11/12/2018 3:22 PM, Philip Race wrote:
>>> Adding build-dev back ..
>>>
>>> I noticed that module jdk.jpackager.runtime requires java.desktop on 
>>> all platforms
>>>
>>> So far as I can tell this is for a Mac-only support for receiving and
>>> handling file open events. Probably it only makes sense or gets used
>>> when the API is used from a running desktop application.
>>>
>>> I might ask if we need this at all, but I definitely think it should
>>> not be required on all platforms if needed only for Mac even if
>>> we might want it on windows in a future version.
>>>
>>> Do we not envisage cases where you need the runtime API for
>>> some kind of daemon service for which there should be a singleton ?
>>> Do you really want to force the desktop module to be dragged along
>>> in such a case ?
>>>
>>> I think we should remove this dependency even if it means losing a
>>> MacOS feature at least for now.
>>
>> 1.) we could remove that functionality (I don't even really 
>> understand the use case for it), and remove the two arg 
>> registerSingleInstance method from the public interface, and remove 
>> the requires statement in module-info.java.
>>
> I was thinking remove it as above since since even the javadoc for 
> that method
> feels obliged to say it is just for macos :-
>
>   95     /**
>   96      * Registers {@code SingleInstanceListener} for current process.
>   97      * If the {@code SingleInstanceListener} object is already 
> registered, or
>   98      * {@code slistener} is {@code null}, then the registration 
> is skipped.
>   99      *
>  100      * @param slistener the listener to handle the single 
> instance behaviour.
>  101      * @param setFileHandler if {@code true}, the listener is 
> notified when the
>  102      *         application is asked to open a list of files. If 
> OS is not MacOS,
>  103      *         the parameter is ignored.
>  104      */
>
>
> yuck. If such optional functionality is needed it needs to be done via
> adding a "doesPlatformSupportFileHandler() method rather than saying 
> the above.
>
> -phil.

OK - I filed JDK-8213756 
<https://bugs.openjdk.java.net/browse/JDK-8213756> to handle removing 
this API, fixing the call to new SingleInstanceImpl() to be wrapped in 
synchronized null check, and fixing comments you referred to .

/Andy

>
>> or ...
>>
>> 2.) we could move that functionality to a platform dependent class, 
>> having dummy methods of setOpenFileHandler in the windows and linux  
>> platform dependent class.  Then we could add a module-info.java.extra 
>> in src/jdk.jpackager.runtime/macos/classes with that requires statement.
>>
>> /Andy
>>
>>>
>>> -phil.
>>>
>>> On 11/11/18, 2:40 PM, Andy Herrick wrote:
>>>> On 11/9/2018 5:25 PM, Andy Herrick wrote:
>>>>> This is an update to the Request For Review of the implementation 
>>>>> of the Java Packager Tool (jpackager) as described in JEP 343: 
>>>>> Packaging Tool <https://bugs.openjdk.java.net/browse/JDK-8200758>
>>>>>
>>>>> This refresh renames the packages used to jdk.jpackager and 
>>>>> jdk.jpackager.runtime, removes the JNLPConverter demo, adds an 
>>>>> initial set of automated tests, and contains fixes to the 
>>>>> following issues:
>>>>>
>>>>> JDK-8213324 jpackager deletes existing app directory without warning
>>>>> JDK-8213166 jpackager --argument arg is broken
>>>>> JDK-8213163 --app-image arg does not work creating exe installers
>>>>> JDK-8212089 Prepare packager for localization
>>>>> JDK-8212537 Create method and class description comments for main 
>>>>> functionality
>>>>> JDK-8213332 Create minimal automated tests for jpackager
>>>>> JDK-8213333 Fix issues found in jpackager with automated tests
>>>>> JDK-8213394 Stop using Log.info() except for expected output.
>>>>> JDK-8213345 Secondary Launchers broken on mac.
>>>>> JDK-8213156 rename packages for jpackager
>>>>> JDK-8213244 Fix all warnings in jpackager java code
>>>>> JDK-8212143 Remove native code that supports UserJvmOptionsService
>>>>> JDK-8213162 Association description in Inno Setup cannot contain 
>>>>> double quotes
>>>>>
>>>>> The following additional issues are targeted to be address in the 
>>>>> next few weeks:
>>>>> JDK-8212936     Makefile and other improvements for jpackager
>>>>> JDK-8212164     resolve jre.list and jre.module.list
>>>>> JDK-8213392     Enhance --help and --version
>>>>> JDK-8208652     File name is not passed to main() via file 
>>>>> association on OS X
>>>>> JDK-8212538     Determine standard way to determine if a Modular jar
>>>>> JDK-8213558     Create more unit tests
>>>> Note: The above issues are targeted to 'internal' - meaning we 
>>>> expect to resolve them in the sandbox before the initial push to 
>>>> JDK12.
>>>> Issues targeted to '12' are expected to be fixed after the initial 
>>>> push.
>>>>
>>>> /Andy
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~herrick/8212780/webrev.2/
>>>>>
>>>>> please send feedback to core-libs-dev at openjdk.java.net
>>>>>
>>>>> /Andy Herrick
>>>>>
>>>>