RFR: JDK-8217317 : Create jpackage native library for windows

Magnus Ihse Bursie magnus.ihse.bursie at oracle.com
Fri Feb 1 11:39:26 UTC 2019


Hi Alexander,

On 2019-02-01 05:22, Alexander Matveev wrote:
> Please review the jpackage fix for bug [1] at [2].
>
> This is a fix for the JDK-8200758-branch branch of the open sandbox 
> repository (jpackage).
>
> - jpackage launcher will now build same as Linux and OS X using 
> SetupBuildLauncher.
> - jpackage.dll was added based on Windows jpackage.exe launcher which 
> will have icon swap and version swap functionality called via JNI.
> - Some code formatting, clean up and minor improvements where done to 
> icon and version swap code. No functional changes.
> - Windows registry will be read and enumerated via JNI as well.
> - isDirectoryInExclusionPath() will use native path comparison, since 
> paths in registry and temp folder returned by Java code can be in 
> short or long format, thus simple string comparison will not work.
> - Windows Defender workaround warning will be checked for --build-root 
> as well if it is set.
> - Removed extra escaping from JPackageHelper for Windows, otherwise 
> tests fails due to incorrect escaping. Our launcher used CreateProcess 
> to launch java.exe by passing args from main() to CreateProcess. This 
> is why I think we required extra escaping.
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8217317
>
> [2] http://cr.openjdk.java.net/~almatvee/8217317/webrev.00/
It basically looks good from a build perspective. There is one change 
I'd like to request, however, and that is that you place the source code 
according to the standard layout. This means moving the source files 
from src/jdk.jpackage/windows/native/jpackage to 
src/jdk.jpackage/windows/native/libjpackage. Also, when you do this, you 
don't need JPACKAGELIB_SRC; the location of the files will be determined 
by SetupJdkLibrary based on the name "jpackage" of the library.

I'm also surprised to see that I can't find the removal of the old 
WinMain() method?

/Magnus


>
> Thanks,
> Alexander



More information about the core-libs-dev mailing list