RFR: JDK-8242302 : Refactor jpackage native code

[Alexey Semenyuk]

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
>>