[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-build-dev
Subject:    Re: [9] Review Request: 8079965 Stop ignoring warnings for libawt_lwawt
From:       Phil Race <philip.race () oracle ! com>
Date:       2015-09-25 17:09:25
Message-ID: 56057FC5.2010804 () oracle ! com
[Download RAW message or body]

Approved.

-phil.

On 09/25/2015 10:07 AM, Sergey Bylokhov wrote:
> Hi, Phil.
> Here is an updated version:
> http://cr.openjdk.java.net/~serb/8079965/webrev.02
> 
> The CPrinterJob.m is changed only. jprt build passed. But I cannot 
> verify osx 10.8, I have not it.
> 
> On 25.09.15 1:57, Phil Race wrote:
> > On 9/24/2015 1:05 PM, Sergey Bylokhov wrote:
> > > On 23.09.15 23:58, Phil Race wrote:
> > > > IMO we should support building JDK on as many platforms as possible.
> > > > Not just the RE-blessed platform of the day which can change more 
> > > > often
> > > > than I can - or want to - update my systems as I need to build 
> > > > multiple
> > > > releases.
> > > 
> > > Then probably it will be better to disable this warning instead of
> > > changing the code? it should be safe.
> > 
> > 
> > hmm.. some day 10.8 really will be uninteresting to all so I would put
> > the code in place now
> > with an ifdef. It is more to the point than disabling the warning.
> > 
> > -phil.
> > 
> > > 
> > > > 
> > > > -phil.
> > > > 
> > > > On 09/23/2015 01:44 PM, Sergey Bylokhov wrote:
> > > > > Yes I can change the types in CPrinterJob [1] via ifdef, but actually
> > > > > do we support sdk 10.8 in jdk9?
> > > > > 
> > > > > [1]
> > > > > http://cr.openjdk.java.net/~serb/8079965/webrev.01/src/java.desktop/macosx/native/libawt_lwawt/awt/CPrinterJob.m.sdiff.html \
> > > > >  
> > > > > 
> > > > > 
> > > > > 
> > > > > On 23.09.15 23:34, Phil Race wrote:
> > > > > > This looks OK except that I have a suggestion with regards to 
> > > > > > Apple's
> > > > > > (gratuitous!) enum change
> > > > > > OpenOffice solved this with an ifdef
> > > > > > 
> > > > > > https://svn.apache.org/viewvc/openoffice/trunk/main/vcl/aqua/source/gdi/salprn.cxx?r1=1591062&r2=1633297&pathrev=1633297 \
> > > > > >  
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > -- openoffice/trunk/main/vcl/aqua/source/gdi/salprn.cxx 2014/04/29
> > > > > > 19:25:03    1591062
> > > > > > +++ openoffice/trunk/main/vcl/aqua/source/gdi/salprn.cxx 2014/10/21
> > > > > > 07:43:39    1633297
> > > > > > @@ -76,8 +76,13 @@
> > > > > > {
> > > > > > mpPrintInfo = [pShared copy];
> > > > > > [mpPrintInfo setPrinter: mpPrinter];
> > > > > > +#ifdef __MAC_10_9 // code for SDK 10.9 or newer
> > > > > > +        mePageOrientation = ([mpPrintInfo orientation] ==
> > > > > > NSPaperOrientationLandscape) ? ORIENTATION_LANDSCAPE :
> > > > > > ORIENTATION_PORTRAIT;
> > > > > > +        [mpPrintInfo setOrientation: NSPaperOrientationPortrait];
> > > > > > +#else // code for SDK 10.8 or older
> > > > > > mePageOrientation = ([mpPrintInfo orientation] ==
> > > > > > NSLandscapeOrientation) ? ORIENTATION_LANDSCAPE :
> > > > > > ORIENTATION_PORTRAIT;
> > > > > > [mpPrintInfo setOrientation: NSPortraitOrientation];
> > > > > > +#endif
> > > > > > }
> > > > > > 
> > > > > > Can we do the same ?
> > > > > > 
> > > > > > -phil.
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On 09/14/2015 07:06 AM, Sergey Bylokhov wrote:
> > > > > > > Hello.
> > > > > > > Please review the fix for jdk9.
> > > > > > > 
> > > > > > > In the fix I remove WARNINGS_AS_ERRORS_clang option from the
> > > > > > > libawt_lwawt library, and fix some of the issues:
> > > > > > > 
> > > > > > > - jlong_md.h:69:9: warning: 'ptr_to_jlong' macro redefined. This is
> > > > > > > because the "jni_util.h" and
> > > > > > > "JavaNativeFoundation.framework/Headers/JNFJNI.h" both define this
> > > > > > > macro. I cleared our headers to eliminate this warning.
> > > > > > > 
> > > > > > > - PrinterView.m:207:21: warning: implicit conversion from 
> > > > > > > enumeration
> > > > > > > type 'NSPaperOrientation' (aka 'enum NSPaperOrientation') to
> > > > > > > different
> > > > > > > enumeration type 'NSPrintingOrientation'. The problem is that the
> > > > > > > Apple changed the returned type of [NSPrintInfo orientation] from
> > > > > > > NSPrintingOrientation to NSPaperOrientation. Note that the
> > > > > > > NSPaperOrientation is available since OSX 10.9, which means that 
> > > > > > > this
> > > > > > > change break the build on 10.8. Is it acceptable or should I 
> > > > > > > suppress
> > > > > > > this warning? [1]
> > > > > > > 
> > > > > > > - CGraphicsDevice.m:336:41: warning: comparison between pointer and
> > > > > > > integer ('void *' and 'jint' (aka 'int')) if ([screenID 
> > > > > > > pointerValue]
> > > > > > > == displayID). I have changed the type from pointerValue to
> > > > > > > unsignedIntValue.
> > > > > > > 
> > > > > > > Also I added "enum-conversion" to the DISABLED_WARNINGS_clang to
> > > > > > > suppress some warnings to fix them later, because it should be
> > > > > > > investigated how to fix it properly (ImageSurfaceData.m:1090:93:
> > > > > > > warning: implicit conversion from enumeration type 
> > > > > > > 'CGImageAlphaInfo'
> > > > > > > (aka 'enum CGImageAlphaInfo') to different enumeration type
> > > > > > > 'CGBitmapInfo')
> > > > > > > 
> > > > > > > After the fix all new warnings will break the build. The currently
> > > > > > > disabled warnings will be fixed as part of JDK-8074825 [2].
> > > > > > > 
> > > > > > > jprt build passed.
> > > > > > > 
> > > > > > > [1]
> > > > > > > https://developer.apple.com/library/mac/releasenotes/General/APIDiffsMacOSX10_9/AppKit.html \
> > > > > > >  
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > [2] https://bugs.openjdk.java.net/browse/JDK-8074825
> > > > > > > 
> > > > > > > 
> > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8079965
> > > > > > > Webrev: http://cr.openjdk.java.net/~serb/8079965/webrev.01
> > > > > > > 
> > > > > > 
> > > > > 
> > > > > 
> > > > 
> > > 
> > > 
> > 
> 
> 


[prev in list] [next in list] [prev in thread] [next in thread]