RFR: JDK-8242302 : Refactor jpackage native code

[Erik Joelsson]

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