RFR: JDK-8217317 : Create jpackage native library for windows
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/ Thanks, Alexander
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
This looks good once Magnus's concerns are addressed. /Andy On 2/1/2019 6:39 AM, Magnus Ihse Bursie wrote:
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
Hi Magnus, http://cr.openjdk.java.net/~almatvee/8217317/webrev.01/ Moved files to libjpackage and remove JPACKAGELIB_SRC. Old wmain() was in jpackage.cpp line 461. Thanks, Alexander On 2/1/2019 3:39 AM, Magnus Ihse Bursie wrote:
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
looks good. /Andy On 2/1/2019 5:38 PM, Alexander Matveev wrote:
Hi Magnus,
http://cr.openjdk.java.net/~almatvee/8217317/webrev.01/
Moved files to libjpackage and remove JPACKAGELIB_SRC.
Old wmain() was in jpackage.cpp line 461.
Thanks, Alexander
On 2/1/2019 3:39 AM, Magnus Ihse Bursie wrote:
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
On 2019-02-01 23:38, Alexander Matveev wrote:
Hi Magnus,
http://cr.openjdk.java.net/~almatvee/8217317/webrev.01/ Looks great, thank you!
I can't comment on the actual source code changes, so you'll need a thumbs up from someone in core libs as well.
Moved files to libjpackage and remove JPACKAGELIB_SRC.
Old wmain() was in jpackage.cpp line 461.
Aha. :) I only knew about WinMain and main. You learn something every day. Thanks. /Magnus
Thanks, Alexander
On 2/1/2019 3:39 AM, Magnus Ihse Bursie wrote:
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
participants (3)
-
Alexander Matveev
-
Andy Herrick
-
Magnus Ihse Bursie