Re: RFR: JDK-8212091 : Move native code under platform specific folders and files

2019-02-17 Thread Magnus Ihse Bursie


> 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: compiling openJdk 11 on windows 7 32bits fail

2019-02-17 Thread Magnus Ihse Bursie


> 16 feb. 2019 kl. 07:47 skrev Franco Gastón Pellegrini 
> :
> 
> Yes, removing --debug-mode enables a 32 bits compilation, with warnings. 
> Thanks!

Great!

Did the patch help for debug builds?

/Magnus

> 
> El vie., 15 de feb. de 2019 a la(s) 08:05, Magnus Ihse Bursie 
> (magnus.ihse.bur...@oracle.com) escribió:
>> 
>> 
>> On 2019-02-12 15:42, Alexey Ivanov wrote:
>> > On 12/02/2019 14:37, Magnus Ihse Bursie wrote:
>> >> There has been some fallout due to the mapfile/linking changes made 
>> >> in JDK 12 that affected JNICALL. However, that should not be 
>> >> affecting JDK 11.
>> > Wasn't it done in JDK 11?
>> > If my memory serves me right, it was done in JDK 11.
>> 
>> You are correct. The major part of the mapfile removal was done in 
>> JDK-8200358, which was pushed to JDK 11. There were follow up issues 
>> going on all the way into JDK 12, though.
>> 
>> Anyway, I've now looked more into this issue. And this time map files 
>> were not really to blame, but JNICALL still is.
>> 
>> First of all, it's important to note that this only shows up when 
>> compiling a debug build. As a workaround, building a normal release 
>> build for 32-bit Windows will not trigger this issue. (This is likely 
>> how it has gotten unnoticed.) To Franco: that means removing 
>> "--enable-debug" from your configure line.
>> 
>> There core problem seems indeed to be discrepancy whether a function is 
>> declared JNICALL or not.
>> 
>> In JDK-8214120, DTrace_VPrintln() & co. got JNICALL added in 
>> src/java.desktop/share/native/common/awt/debug/debug_trace.c and 
>> debug_trace.h. However, the typedef DTRACE_PRINT_CALLBACK was not 
>> updated. Since JNICALL is a no-op on all platforms except Windows 32, 
>> this was not discovered.
>> 
>> However, according to JBS JDK-8214120 was fixed in JDK-12. So why this 
>> is causing issues in JDK-11, I don't understand. Or maybe the discussion 
>> drifted from testing JDK 11 to testing JDK 12? In any case, it is likely 
>> currently broken in JDK 13.
>> 
>> Here is a suggested patch. Hopefully I got the JNICALL modifier in the 
>> correct place (C function pointer typedefs are the worst!).
>> 
>> diff --git 
>> a/src/java.desktop/share/native/common/awt/debug/debug_trace.h 
>> b/src/java.desktop/share/native/common/awt/debug/debug_trace.h
>> --- a/src/java.desktop/share/native/common/awt/debug/debug_trace.h
>> +++ b/src/java.desktop/share/native/common/awt/debug/debug_trace.h
>> @@ -48,7 +48,7 @@
>>   typedef void (*DTRACE_OUTPUT_CALLBACK)(const char * msg);
>> 
>>   /* prototype for client provided print callback function */
>> -typedef void (*DTRACE_PRINT_CALLBACK)(const char * file, int line, int 
>> argc, const char * fmt, va_list arglist);
>> +typedef void (JNICALL *DTRACE_PRINT_CALLBACK)(const char * file, int 
>> line, int argc, const char * fmt, va_list arglist);
>> 
>>   extern void DTrace_EnableAll(dbool_t enabled);
>>   extern void DTrace_EnableFile(const char * file, dbool_t enabled);
>> 
>> Please test and let me know if it works. If so it should be pushed to 
>> JDK 13, and possibly backported.
>> 
>> /Magnus
> 
> 
> -- 
> Franco Gastón Pellegrini