Re: RFR: JDK-8212091 : Move native code under platform specific folders and files
Hi Alexander, Ok, thanks Note: code reviews of code going into the sandbox is not a substitute for code review when it is to be pushed to jdk/jdk. (The sandbox has much more informal rules for commits that are branch specific.) Roger On 02/19/2019 10:44 PM, Alexander Matveev wrote: Hi Roger, Some deleted files are actually deleted files, since I did code cleanup as well. Added files contain code from splitting original source files. This was needed to re-arrange code. Not sure how to log with hg file split. Also, we already moved files without keeping history, since we are not planing to keep history when moving jpackage from sandbox. Thanks, Alexander On 2/19/2019 8:49 AM, Roger Riggs wrote: Hi Alexander, Some files appear to be moved with hg, but others are copied and deleted. Please use hg mv to retain the continuity of the history. Thanks, Roger On 02/15/2019 10:03 PM, Alexander Matveev wrote: Hi Magnus, http://cr.openjdk.java.net/~almatvee/8212091/webrev.01/ Moved all files from "posix" to "unix" folder and reverted Lib-jdk.jpackage.gmk changes. Webrev updated with files moved, instead of add/remove. Thanks, Alexander On 2/14/2019 11:44 PM, Magnus Ihse Bursie wrote: On 2019-02-15 04:31, 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). - Moved native code under platform specific folder. - Removed most usage on #ifdefs for WINDOWS, LINUX, MAC and POSIX. - MAC define is still used in JavaVirtualMachine.cpp and Package.cpp for Mac specific code to filter out some arguments. I decided to keep it as is for now, since Mac specific code is small. - Defines are used in Platform.cpp to initialize platform specific classes. - Removed all pragma warning and fixed all compilation warnings. - Removed unused code. [1] https://bugs.openjdk.java.net/browse/JDK-8212091 [2] http://cr.openjdk.java.net/~almatvee/8212091/webrev.00/ The JDK standard is to use "unix", not "posix", for the shared functionality between linux/solaris/macosx. You can keep the name "PosixPlatform.*" if you want, though; the important thing is the directory name. Also, if you do that, you do not need any changes to make/lib/Lib-jdk.jpackage.gmk, since that will be automatically understood by the build system. It looks from the webrev that you have "moved" the files by doing "hg add" and "hg remove". Please use "hg move" instead -- this will keep history intact, and it allows reviewers to see if you have also made changes to the moved files. (If you do have modified the moved file, reverting a "hg add+hg remove" process is a bit more tricky -- you need to do "hg forget" on the new file, rename it to something else (otherwise "hg move" will complain), "hg revert" the old file back in place, do a "hg move" from the old to the new, and then copy the modified, renamed file back over the target new file again.) /Magnus Thanks, Alexander
Re: RFR: JDK-8212091 : Move native code under platform specific folders and files
Hi Roger, Some deleted files are actually deleted files, since I did code cleanup as well. Added files contain code from splitting original source files. This was needed to re-arrange code. Not sure how to log with hg file split. Also, we already moved files without keeping history, since we are not planing to keep history when moving jpackage from sandbox. Thanks, Alexander On 2/19/2019 8:49 AM, Roger Riggs wrote: Hi Alexander, Some files appear to be moved with hg, but others are copied and deleted. Please use hg mv to retain the continuity of the history. Thanks, Roger On 02/15/2019 10:03 PM, Alexander Matveev wrote: Hi Magnus, http://cr.openjdk.java.net/~almatvee/8212091/webrev.01/ Moved all files from "posix" to "unix" folder and reverted Lib-jdk.jpackage.gmk changes. Webrev updated with files moved, instead of add/remove. Thanks, Alexander On 2/14/2019 11:44 PM, Magnus Ihse Bursie wrote: On 2019-02-15 04:31, 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). - Moved native code under platform specific folder. - Removed most usage on #ifdefs for WINDOWS, LINUX, MAC and POSIX. - MAC define is still used in JavaVirtualMachine.cpp and Package.cpp for Mac specific code to filter out some arguments. I decided to keep it as is for now, since Mac specific code is small. - Defines are used in Platform.cpp to initialize platform specific classes. - Removed all pragma warning and fixed all compilation warnings. - Removed unused code. [1] https://bugs.openjdk.java.net/browse/JDK-8212091 [2] http://cr.openjdk.java.net/~almatvee/8212091/webrev.00/ The JDK standard is to use "unix", not "posix", for the shared functionality between linux/solaris/macosx. You can keep the name "PosixPlatform.*" if you want, though; the important thing is the directory name. Also, if you do that, you do not need any changes to make/lib/Lib-jdk.jpackage.gmk, since that will be automatically understood by the build system. It looks from the webrev that you have "moved" the files by doing "hg add" and "hg remove". Please use "hg move" instead -- this will keep history intact, and it allows reviewers to see if you have also made changes to the moved files. (If you do have modified the moved file, reverting a "hg add+hg remove" process is a bit more tricky -- you need to do "hg forget" on the new file, rename it to something else (otherwise "hg move" will complain), "hg revert" the old file back in place, do a "hg move" from the old to the new, and then copy the modified, renamed file back over the target new file again.) /Magnus Thanks, Alexander
Re: RFR: JDK-8212091 : Move native code under platform specific folders and files
Hi Alexander, Some files appear to be moved with hg, but others are copied and deleted. Please use hg mv to retain the continuity of the history. Thanks, Roger On 02/15/2019 10:03 PM, Alexander Matveev wrote: Hi Magnus, http://cr.openjdk.java.net/~almatvee/8212091/webrev.01/ Moved all files from "posix" to "unix" folder and reverted Lib-jdk.jpackage.gmk changes. Webrev updated with files moved, instead of add/remove. Thanks, Alexander On 2/14/2019 11:44 PM, Magnus Ihse Bursie wrote: On 2019-02-15 04:31, 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). - Moved native code under platform specific folder. - Removed most usage on #ifdefs for WINDOWS, LINUX, MAC and POSIX. - MAC define is still used in JavaVirtualMachine.cpp and Package.cpp for Mac specific code to filter out some arguments. I decided to keep it as is for now, since Mac specific code is small. - Defines are used in Platform.cpp to initialize platform specific classes. - Removed all pragma warning and fixed all compilation warnings. - Removed unused code. [1] https://bugs.openjdk.java.net/browse/JDK-8212091 [2] http://cr.openjdk.java.net/~almatvee/8212091/webrev.00/ The JDK standard is to use "unix", not "posix", for the shared functionality between linux/solaris/macosx. You can keep the name "PosixPlatform.*" if you want, though; the important thing is the directory name. Also, if you do that, you do not need any changes to make/lib/Lib-jdk.jpackage.gmk, since that will be automatically understood by the build system. It looks from the webrev that you have "moved" the files by doing "hg add" and "hg remove". Please use "hg move" instead -- this will keep history intact, and it allows reviewers to see if you have also made changes to the moved files. (If you do have modified the moved file, reverting a "hg add+hg remove" process is a bit more tricky -- you need to do "hg forget" on the new file, rename it to something else (otherwise "hg move" will complain), "hg revert" the old file back in place, do a "hg move" from the old to the new, and then copy the modified, renamed file back over the target new file again.) /Magnus Thanks, Alexander
Re: RFR: JDK-8212091 : Move native code under platform specific folders and files
> 16 feb. 2019 kl. 04:03 skrev Alexander Matveev : > > Hi Magnus, > > http://cr.openjdk.java.net/~almatvee/8212091/webrev.01/ > > Moved all files from "posix" to "unix" folder and reverted > Lib-jdk.jpackage.gmk changes. > Webrev updated with files moved, instead of add/remove. Thank you! This looks good now from a build point of view, but you'll need a review from core-libs as well. /Magnus > > Thanks, > Alexander > >> On 2/14/2019 11:44 PM, Magnus Ihse Bursie wrote: >> >> >>> On 2019-02-15 04:31, 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). >>> >>> - Moved native code under platform specific folder. >>> - Removed most usage on #ifdefs for WINDOWS, LINUX, MAC and POSIX. >>> - MAC define is still used in JavaVirtualMachine.cpp and Package.cpp for >>> Mac specific code to filter out some arguments. I decided to keep it as is >>> for now, since Mac specific code is small. >>> - Defines are used in Platform.cpp to initialize platform specific classes. >>> - Removed all pragma warning and fixed all compilation warnings. >>> - Removed unused code. >>> >>> [1] https://bugs.openjdk.java.net/browse/JDK-8212091 >>> >>> [2] http://cr.openjdk.java.net/~almatvee/8212091/webrev.00/ >> The JDK standard is to use "unix", not "posix", for the shared functionality >> between linux/solaris/macosx. You can keep the name "PosixPlatform.*" if you >> want, though; the important thing is the directory name. >> >> Also, if you do that, you do not need any changes to >> make/lib/Lib-jdk.jpackage.gmk, since that will be automatically understood >> by the build system. >> >> It looks from the webrev that you have "moved" the files by doing "hg add" >> and "hg remove". Please use "hg move" instead -- this will keep history >> intact, and it allows reviewers to see if you have also made changes to the >> moved files. >> >> (If you do have modified the moved file, reverting a "hg add+hg remove" >> process is a bit more tricky -- you need to do "hg forget" on the new file, >> rename it to something else (otherwise "hg move" will complain), "hg revert" >> the old file back in place, do a "hg move" from the old to the new, and then >> copy the modified, renamed file back over the target new file again.) >> >> /Magnus >>> >>> Thanks, >>> Alexander >
Re: RFR: JDK-8212091 : Move native code under platform specific folders and files
Hi Magnus, http://cr.openjdk.java.net/~almatvee/8212091/webrev.01/ Moved all files from "posix" to "unix" folder and reverted Lib-jdk.jpackage.gmk changes. Webrev updated with files moved, instead of add/remove. Thanks, Alexander On 2/14/2019 11:44 PM, Magnus Ihse Bursie wrote: On 2019-02-15 04:31, 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). - Moved native code under platform specific folder. - Removed most usage on #ifdefs for WINDOWS, LINUX, MAC and POSIX. - MAC define is still used in JavaVirtualMachine.cpp and Package.cpp for Mac specific code to filter out some arguments. I decided to keep it as is for now, since Mac specific code is small. - Defines are used in Platform.cpp to initialize platform specific classes. - Removed all pragma warning and fixed all compilation warnings. - Removed unused code. [1] https://bugs.openjdk.java.net/browse/JDK-8212091 [2] http://cr.openjdk.java.net/~almatvee/8212091/webrev.00/ The JDK standard is to use "unix", not "posix", for the shared functionality between linux/solaris/macosx. You can keep the name "PosixPlatform.*" if you want, though; the important thing is the directory name. Also, if you do that, you do not need any changes to make/lib/Lib-jdk.jpackage.gmk, since that will be automatically understood by the build system. It looks from the webrev that you have "moved" the files by doing "hg add" and "hg remove". Please use "hg move" instead -- this will keep history intact, and it allows reviewers to see if you have also made changes to the moved files. (If you do have modified the moved file, reverting a "hg add+hg remove" process is a bit more tricky -- you need to do "hg forget" on the new file, rename it to something else (otherwise "hg move" will complain), "hg revert" the old file back in place, do a "hg move" from the old to the new, and then copy the modified, renamed file back over the target new file again.) /Magnus Thanks, Alexander
Re: RFR: JDK-8212091 : Move native code under platform specific folders and files
On 2019-02-15 04:31, 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). - Moved native code under platform specific folder. - Removed most usage on #ifdefs for WINDOWS, LINUX, MAC and POSIX. - MAC define is still used in JavaVirtualMachine.cpp and Package.cpp for Mac specific code to filter out some arguments. I decided to keep it as is for now, since Mac specific code is small. - Defines are used in Platform.cpp to initialize platform specific classes. - Removed all pragma warning and fixed all compilation warnings. - Removed unused code. [1] https://bugs.openjdk.java.net/browse/JDK-8212091 [2] http://cr.openjdk.java.net/~almatvee/8212091/webrev.00/ The JDK standard is to use "unix", not "posix", for the shared functionality between linux/solaris/macosx. You can keep the name "PosixPlatform.*" if you want, though; the important thing is the directory name. Also, if you do that, you do not need any changes to make/lib/Lib-jdk.jpackage.gmk, since that will be automatically understood by the build system. It looks from the webrev that you have "moved" the files by doing "hg add" and "hg remove". Please use "hg move" instead -- this will keep history intact, and it allows reviewers to see if you have also made changes to the moved files. (If you do have modified the moved file, reverting a "hg add+hg remove" process is a bit more tricky -- you need to do "hg forget" on the new file, rename it to something else (otherwise "hg move" will complain), "hg revert" the old file back in place, do a "hg move" from the old to the new, and then copy the modified, renamed file back over the target new file again.) /Magnus Thanks, Alexander
RFR: JDK-8212091 : Move native code under platform specific folders and files
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). - Moved native code under platform specific folder. - Removed most usage on #ifdefs for WINDOWS, LINUX, MAC and POSIX. - MAC define is still used in JavaVirtualMachine.cpp and Package.cpp for Mac specific code to filter out some arguments. I decided to keep it as is for now, since Mac specific code is small. - Defines are used in Platform.cpp to initialize platform specific classes. - Removed all pragma warning and fixed all compilation warnings. - Removed unused code. [1] https://bugs.openjdk.java.net/browse/JDK-8212091 [2] http://cr.openjdk.java.net/~almatvee/8212091/webrev.00/ Thanks, Alexander
