RFR: [OpenJDK 2D-Dev] JDK-8215296 do not disable c99 on Solaris (original) (raw)

Baesken, Matthias matthias.baesken at sap.com
Mon Dec 17 15:24:22 UTC 2018

Sounds like a simpler change, at least for now. Does it pass jdk-submit? Do you intend to push to 12 or 13?

Hi Magus, thanks for the review. I think it is safer to go for 13 , what's your opinion ?

I put it into our internal build+test queue first , after this is fine , I will go for jdk-submit (hopefully it works ).

Best regards, Matthias

-----Original Message----- From: Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> Sent: Montag, 17. Dezember 2018 16:11 To: Baesken, Matthias <matthias.baesken at sap.com> Cc: 2d-dev at openjdk.java.net; erik.joelsson at oracle.com; build- dev at openjdk.java.net; awt-dev at openjdk.java.net Subject: Re: RFR: [OpenJDK 2D-Dev] JDK-8215296 do not disable c99 on Solaris

Sounds like a simpler change, at least for now. Does it pass jdk-submit? Do you intend to push to 12 or 13? Looks good to me, as long as it doesn't break anything. /Magnus > 17 dec. 2018 kl. 14:12 skrev Baesken, Matthias <matthias.baesken at sap.com>: > > > Hello, please review > > http://cr.openjdk.java.net/~mbaesken/webrevs/8215296.0/ > > in my change just -xc99=%none is removed, so we do not forbid c99 coding. > > The -Xa compile flag is kept, no special additional settings are needed to compile png/awt . > > > Thanks, Matthias > > >> ---------------------------------------------------------------------- >> >> Message: 1 >> Date: Fri, 14 Dec 2018 15:39:26 +0100 >> From: Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> >> To: Erik Joelsson <erik.joelsson at oracle.com>, build-dev >> <build-dev at openjdk.java.net>, "awt-dev at openjdk.java.net" >> <awt-dev at openjdk.java.net>, 2d-dev <2d-dev at openjdk.java.net> >> Subject: Re: [OpenJDK 2D-Dev] RFR: JDK-8215296 do not disable c99 on >> Solaris >> Message-ID: <5874d10e-db2d-8681-a54b-a1eeb6e45994 at oracle.com> >> Content-Type: text/plain; charset=utf-8; format=flowed >> >> >> >>> On 2018-12-14 12:49, Magnus Ihse Bursie wrote: >>> >>> 13 dec. 2018 kl. 19:07 skrev Erik Joelsson <erik.joelsson at oracle.com_ _>>> <mailto:erik.joelsson at oracle.com>>: >>> >>>> >>>>> On 2018-12-13 02:11, Magnus Ihse Bursie wrote: >>>>> >>>>>> -DXPG6 >>>>>> >>>>>> ?? >>>>> To be honest, I'm not completely sure about this. Without this >>>>> define, the build failed with the following error message: >>>>> Compiler or options invalid for pre-UNIX 03 X/Open applications and >>>>> pre-2001 POSIX applications >>>>> >>>>> This was triggered by the following section in >>>>> /usr/include/sys/featuretests.h: >>>>> /* >>>>> * It is invalid to compile an XPG3, XPG4, XPG4v2, or XPG5 application >>>>> * using c99. The same is true for POSIX.1-1990, POSIX.2-1992, >>>>> POSIX.1b, >>>>> * and POSIX.1c applications. Likewise, it is invalid to compile an >>>>> XPG6 >>>>> * or a POSIX.1-2001 application with anything other than a c99 or >>>>> later >>>>> * compiler. Therefore, we force an error in both cases. >>>>> */ _>>>>> #if defined(STDCC99) && (defined(XOPENORPOSIX) && >>>>> !defined(XPG6)) >>>>> #error "Compiler or options invalid for pre-UNIX 03 X/Open _>>>>> applications _ >>>>> and pre-2001 POSIX applications" _>>>>> #elif !defined(STDCC99) && _ _>>>>> (defined(XOPENORPOSIX) && defined(XPG6)) >>>>> #error "Compiler or options invalid; UNIX 03 and POSIX.1-2001 _>>>>> applications _ >>>>> require the use of c99" >>>>> #endif >>>>> >>>>> The solution, as also hinted to by searching for other resolutions >>>>> to this error online, was to provide the XPG6 system define. But _>>>>> exactly how we end up in featuretests.h with XOPENORPOSIX set, >>>>> without XPG6 set, and only when compiling this library and not >>>>> others, I don't know. I also don't understand what the XPG standard >>>>> refers to, nor what versions 2-5 means or what version 6 has that >>>>> differs from them. >>>>> >>>>> By setting this flag, I am telling solaris include headers that we >>>>> want to compile using the XPG standard version 6, instead of an >>>>> older one. It solves the problem. I am happy enough with this. Are you? >>>> It looks like this comes from libpng. It has this in >>>> src/java.desktop//share/native/libsplashscreen/libpng/pngpriv.h: >>>> >>>> /* Feature Test Macros. The following are defined here to ensure >>>> that correctly >>>> * implemented libraries reveal the APIs libpng needs to build and >>>> hide those >>>> * that are not needed and potentially damaging to the compilation. >>>> * >>>> * Feature Test Macros must be defined before any system header is >>>> included (see >>>> * POSIX 1003.1 2.8.2 "POSIX Symbols." >>>> * >>>> * These macros only have an effect if the operating system supports >>>> either >>>> * POSIX 1003.1 or C99, or both. On other operating systems >>>> (particularly >>>> * Windows/Visual Studio) there is no effect; the OS specific tests >>>> below are >>>> * still required (as of 2011-05-02.) >>>> */ >>>> #ifndef POSIXSOURCE >>>> # define POSIXSOURCE 1 /* Just the POSIX 1003.1 and C89 APIs */ >>>> #endif >>>> >>>> This in turn triggers XOPENORPOSIX to be defined in >>>> /usr/include/sys/featuretests.h and so triggers the error. >>>> >>>> What I'm not clear about is if libpng is trying to declare that it >>>> should not be compiled with any newer standards, and so by doing >>>> that, we risk introducing problems. Reading in the system header, it >>>> seems the XPG6 macro is internal and should not be used by the >>>> application. It's derived from XOPENSOURCE=600 or >>>> POSIXCSOURCE=200112L which is what applications should use. >>> >>> Interesting. We should probably define one, or both of these. Perhaps >>> globally for all native files and compilers. It might have been the >>> case that the solstudio compiler set POSIXCSOURCE for us before, >>> prior to setting -std=c99. The following stack overflow article claims >>> that this is at least the behavior of gcc/clang: >>> >>> https://stackoverflow.com/questions/21867897/c89-and-posix-at-the- >> same-time >>> >>> >>> So we might have had an implicit POSIXCSOURCE that we now miss. >>> That would explain why this starts to fail. I'll see if I can confirm >>> this the next time I log into a Solaris computer. >> Of course it was not as simple. Setting: >> ifeq ($(OPENJDKTARGETOS), solaris) >> LIBSPLASHSCREENCFLAGS += -DPOSIXCSOURCE=200112L - >> DXOPENSOURCE=600 >> endif >> >> instead made us fail with: >> open/src/java.desktop/unix/native/libsplashscreen/splashscreensys.c", >> line 143: error: incomplete struct/union/enum timezone: tz >> >> I don't have more time to dig into this now. Overall, changes such as >> these make it all feel a bit scary; I recommend that any change to this >> be made in JDK 13 and not 12. >> >> /Magnus >>> >>> Otoh, the same article claims, and it sounds reasonable, that we >>> should set these variables ourself, to be well behaved and to minimize >>> surprises. And I think this applies to all unix platforms, regardless >>> of compiler being used. I'll see if I can kick off a test job with >>> this to see how/if it influences other platforms. But it sounds like >>> something we should do; the level of posix conformance should be >>> controlled by us, not left to chance. We also need to verify, of >>> course, that all platforms we want to support is capable of >>> supporting POSIXCSOURCE=200112L. I doubt there's a problem >> though. >>> Possibly on AIX... >>> >>> /Magnus >>> >>>> >>>> So the the question is, is it ok to override the requirements of >>>> libpng or should it receive special treatment? If we are fine with >>>> overriding, then we should use one of the public APIs instead. >>>> >>>> /Erik >>>> >>>>> /Magnus >>>>> >>>>>> >>>>>> Thanks, >>>>>> David >>>>>> >>>>>>> On 13/12/2018 7:02 am, Magnus Ihse Bursie wrote: >>>>>>> >>>>>>> >>>>>>>> On 2018-12-12 20:08, Magnus Ihse Bursie wrote: >>>>>>>> >>>>>>>> >>>>>>>>> On 2018-12-12 19:12, Magnus Ihse Bursie wrote: >>>>>>>>> From the bug report: >>>>>>>>> "Currently we disable C99 in the Solaris build by setting >>>>>>>>> -xc99=%none%. >>>>>>>>> This differs from a lot of other build environments like >>>>>>>>> gcc/Linux or VS2013/2017 on Windows where C99 features work. >>>>>>>>> We should remove this difference on Solaris and remove or >>>>>>>>> replace the setting. >>>>>>>>> >>>>>>>>> Kim Barrett mentioned : >>>>>>>>> "I merely mentioned the C++14 work as evidence that removing >>>>>>>>> -xc99=%none% didn?t appear harmful." >>>>>>>>> However it will take more time until the C++14 change is in." >>>>>>>>> >>>>>>>>> I am currently running a test build on our CI build system to >>>>>>>>> confirm that this does not break the Solaris build (but I'd be >>>>>>>>> highly surprised if it did). I will not push this until the >>>>>>>>> builds are cleared. >>>>>>>> Of course it was not that simple... :-( Two AWT libraries (at >>>>>>>> least) failed to build. I'm currently investigating if there's a >>>>>>>> simple fix to that. >>>>>>> New attempt, that fixes the two AWT libraries: >>>>>>> WebRev: >>>>>>> http://cr.openjdk.java.net/~ihse/JDK-8215296-build-solstudio- with- >> c99/webrev.01 >>>>>>> <http://cr.openjdk.java.net/%7Eihse/JDK-8215296-build-solstudio-_ _>> with-c99/webrev.01> >>>>>>> >>>>>>> >>>>>>> Now this passes the CI build test. >>>>>>> >>>>>>> /Magnus >>>>>>>> >>>>>>>> /Magnus >>>>>>>>> >>>>>>>>> /Magnus >>>>>>>>> >>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8215296 >>>>>>>>> Patch inline: >>>>>>>>> diff --git a/make/autoconf/flags-cflags.m4 >>>>>>>>> b/make/autoconf/flags-cflags.m4 >>>>>>>>> --- a/make/autoconf/flags-cflags.m4 >>>>>>>>> +++ b/make/autoconf/flags-cflags.m4 >>>>>>>>> @@ -559,7 +559,7 @@ >>>>>>>>> TOOLCHAINCFLAGS="-errshort=tags" >>>>>>>>> >>>>>>>>> TOOLCHAINCFLAGSJDK="-mt $TOOLCHAINFLAGS" >>>>>>>>> - TOOLCHAINCFLAGSJDKCONLY="-xc99=%none -xCC -Xa - W0,- >> noglobal >>>>>>>>> $TOOLCHAINCFLAGS" # C only >>>>>>>>> + TOOLCHAINCFLAGSJDKCONLY="-std=c99 -xCC -W0,- noglobal >>>>>>>>> $TOOLCHAINCFLAGS" # C only >>>>>>>>> TOOLCHAINCFLAGSJDKCXXONLY="-features=no%except - >> norunpath >>>>>>>>> -xnolib" # CXX only >>>>>>>>> TOOLCHAINCFLAGSJVM="-template=no%extdef - _>> features=no%splitinit _ >>>>>>>>> -library=stlport4 -mt -features=no%except >>>>>>>>> $TOOLCHAINFLAGS" >

More information about the build-dev mailing list