RFR: JDK-8242302 : Refactor jpackage native code

[Alexey Semenyuk]

Thank you for the review!

Whitespace issues will be fixed in the final patch.

- Alexey

On 4/23/2020 2:51 PM, Erik Joelsson wrote:
> Thanks, that looks good!
>
> Only two minor whitespace nits.
>
> Line 32-33, 102-103: continuation indent 4 spaces
>
> Line 101: extra space
>
> No need for new webrev.
>
> /Erik
>
> On 2020-04-23 11:38, Alexey Semenyuk wrote:
>> Erik,
>>
>> Please review updated patch with the suggested improvements at [1]. 
>> The webrev includes only incremental changes to 
>> Lib-jdk.incubator.jpackage.gmk for clarity.
>>
>> [1] 
>> http://cr.openjdk.java.net/~asemenyuk/8242302/webrev.01/webrev.01/index.html
>>
>> - Alexey
>>
>> On 4/20/2020 1:17 PM, Erik Joelsson wrote:
>>> (adding build-dev)
>>>
>>> Hello Alexey,
>>>
>>> The SetupJdkExecutable and SetupJdkLibrary macros are designed to 
>>> find the correct source dirs automatically as long as they follow 
>>> the standard naming. In this case you are adding some extra with the 
>>> "common" dir as well as reusing some src dirs between multiple calls 
>>> to the macros. There are constructs prepared for handling these 
>>> situations too. We introduced these specialized macros to avoid 
>>> repeated setting up of almost identical source dirs like in your 
>>> patch, and to help enforce a standard in directory naming.
>>>
>>> Upon closer inspection, I see now that what's said above only 
>>> applies to SetupJdkLibrary while SetupJdkExecutable is lacking most 
>>> of these features. Adding them should be pretty straight forward 
>>> though and should be fixed. Not asking you to do it though. So for 
>>> those cases, I recommend using the FindSrcDirsForComponent macro to 
>>> find the source dirs needed for now. It's defined in 
>>> make/common/JdkNativeCompilation.gmk. You can also extract the exact 
>>> resolved SRC dirs from a call to either macro using for example 
>>> BUILD_JPACKAGE_APPLAUNCHEREXE_SRC (after the macro has been evaled). 
>>> This would be recommended for BUILD_JPACKAGE_APPLAUNCHERWEXE.
>>>
>>> For the ones where the NAME field matches the source sub dir (which 
>>> should be almost all unless there is a good reason), you don't need 
>>> to specify SRC at all. If you need to add the "common" subdir, then 
>>> you can use EXTRA_SRC and the special designation 
>>> "jdk.incubator.jpackage:common" which will get parsed into all 
>>> existing common directories for your current build target. In the 
>>> case of jpackageapplauncherw, you can similarly use 
>>> "jdk.incubator.jpackage:applauncher" in the SRC field.
>>>
>>> For SetupJdkLibrary, there is no need to explicitly add source dirs 
>>> to -I paths. All source dirs are by default added unless 
>>> HEADERS_FROM_SRC is set to false. Again this should be fixed for 
>>> SetupJdkExecutable.
>>>
>>> For macros that we intend to call with parameters, our style 
>>> convention is to declare them with camel case and always on a new 
>>> line. This is to make them stand out clearly from variables which we 
>>> assign with :=, and make them look a little bit more like function 
>>> declarations. See make/common/Utils.gmk for examples of how this 
>>> looks. So something like:
>>>
>>> JpackageWithStaticCrt = \
>>>     $(filter-out -MD, $1) -MT
>>>
>>> /Erik
>>>
>>>
>>> On 2020-04-15 13:13, Alexey Semenyuk wrote:
>>>> Please review fix [2] for jpackage bug [1].
>>>>
>>>> Refactor jpackage native code.
>>>>
>>>> - Improve code reuse between different platforms.
>>>> - Replace custom string classes with the standard std::basic_string.
>>>> - Merge functionality of libapplauncher.dll(.so) library with 
>>>> jpackageapplauncher(.exe) binary. There is no point to keep two 
>>>> different executables.
>>>> - Link jpackageapplauncher.exe with MSVC Run-Time library 
>>>> statically to avoid copying of MSVC Run-Time dlls to app's bin folder.
>>>> - Remove unused code.
>>>>
>>>> - Alexey
>>>>
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8242302
>>>>
>>>> [2] http://cr.openjdk.java.net/~asemenyuk/8242302/webrev.00
>>>>
>>