RFR(S): 8184022: Build JDK 10 on OSX 10.12 and above
Erik Joelsson
erik.joelsson at oracle.com
Fri Jul 14 14:02:52 UTC 2017
I realized I spoke too soon. I won't be able to sponsor this anytime
soon as I'm about to leave on vacation now. Hopefully someone else will
be able to pick this up for you.
/Erik
On 2017-07-14 08:53, Erik Joelsson wrote:
> This looks good to me. Never mind the regexps, they are fine.
>
> I can sponsor the change since it touches configure and will need a
> corresponding closed change. Do you mind if I push this to jdk10/jdk10
> instead of hs? That way it will get to hs within a week, but also be
> in jdk10, while going the other way, it will take much longer before
> it hits jdk10.
>
> /Erik
>
> On 2017-07-13 19:24, Hohensee, Paul wrote:
>> New webrev with two-line change to flags.m4 at line 1129.
>>
>> http://cr.openjdk.java.net/~phh/8184022/webrev.03/
>>
>> “xno” now means “use build system default”, just as if the switch had
>> not been used.
>>
>> I’m a very inexperienced regex user, so I used what worked first for
>> me. What would it look like to escape the whole expression or
>> sub-expressions?
>>
>> Paul
>>
>> On 7/13/17, 10:04 AM, "Erik Joelsson" <erik.joelsson at oracle.com> wrote:
>>
>> This looks pretty good. A few points:
>> * Please use $GREP since configure is doing some work on
>> finding a good
>> grep for us. If you want to make the grep patterns more
>> readable, it's
>> possible to put the [] escapes around the whole expression, or
>> at any
>> level you wish. That way you don't need to repeat them for each
>> [0-9].
>> Both styles are used throughout the configure script and I don't
>> have a
>> strong preference myself.
>> * The check for "no" got me thinking. If someone explicitly
>> sets
>> --without-macosx-version-max, that probably means they want it
>> empty.
>> The reason for doing so would be to override an earlier instance
>> of the
>> parameter on the command line (set by some script or automatic
>> system
>> that you can't directly influence). This is not a likely usecase
>> but is
>> perhaps a more correct action. Sorry for confusing this earlier.
>> "yes"
>> is definitely an error though.
>> I took this patch for a run here and it seems to work as it
>> should from
>> the Oracle point of view.
>> /Erik
>> On 2017-07-13 17:46, Hohensee, Paul wrote:
>> > New webrev
>> >
>> > http://cr.openjdk.java.net/~phh/8184022/webrev.02/
>> >
>> > It includes –with-macosx-version-max format checks (disallows
>> –without-macosx-version-max) and your jib-profiles.js patch. I put
>> the checking logic in AC_ARG_WITH based on the code in basics.m4 line
>> 597 that defines BASIC_SETUP_DEVKIT.
>> >
>> > --with-macosx-version-max will fail the format checks and
>> –without-macosx-version-max will fail the check against ‘xno’.
>> >
>> > Paul
>> >
>> > On 7/12/17, 1:15 AM, "Erik Joelsson"
>> <erik.joelsson at oracle.com> wrote:
>> >
>> > Hello,
>> >
>> >
>> > On 2017-07-12 03:19, Hohensee, Paul wrote:
>> > > New webrev at
>> > >
>> > > http://cr.openjdk.java.net/~phh/8184022/webrev.01/
>> > For the AC_ARG_WITH, we usually refrain from using the
>> builtin "if-set,
>> > if-not-set" parameters and define our own logic to handle
>> all 4
>> > possibilities: not set, --with-foobar=value,
>> --with-foobar and
>> > --without-foobar. The latter two results in the values
>> "yes" and "no"
>> > respectively and in this case those two are invalid and
>> needs to result
>> > in errors. Also since we are expecting a very specific
>> format on the
>> > input, we need to validate this format so we fail fast
>> instead of
>> > getting weird compile errors much later.
>> >
>> > My understanding of -mmacosx-version-min is that it sets
>> > MAC_OS_X_VERSION_MIN_REQUIRED for you so no need to add
>> that. OTOH, it
>> > makes it more obvious where this comes from if anyone
>> stumbles on it in
>> > the source.
>> > > I defined a new shell variable MACOSX_VERSION_MAX which
>> is settable via a new configure switch
>> –with-macosx-version-max=<version>. Example use:
>> --with-macosx-version-max=10.12.00. The specified version is passed
>> via a compiler command line switch, vis
>> –DMAC_OS_X_VERSION_MAX_ALLOWED=101200 (de-dotted <version>).
>> > At what point did they introduce the double zeros at the
>> end? Seems like
>> > we will need guard the values quite carefully and make
>> sure we zero pad
>> > if needed.
>> > > MACOSX_VERSION_MIN remains hardcoded to 10.7.0, but is
>> now passed to the compilers via -DMAC_OS_X_VERSION_MIN_REQUIRED=1070
>> rather than via -DMAC_OS_X_VERSION_MAX_ALLOWED=1070.
>> > >
>> > > Tested by attempting builds on OSX 10.12.04.
>> > >
>> > > (1) no –with-macosx-version-max: succeeds as expected
>> because no –DMAC_OS_X_VERSION_MAX_ALLOWED passed to compilers, so
>> defaults to 10.12.04.
>> > > (2) –with-macosx-version-max=10.11.00: fails as
>> expected due to formal parameter type mismatch.
>> > > (3) –with-macosx-version-max=10.12.00: succeeds as
>> expected because formal parameter types are the same for all 10.12.xx.
>> > >
>> > > It’d be great if you could try it out.
>> > >
>> > > Note that successful cases (1) and (3) above provoke
>> three warnings which I haven’t investigated. Imo, I/we can figure out
>> how to get rid of these next/later.
>> > >
>> > > ld: warning: object file
>> (/Users/hohensee/workspaces/jdk10-hs/build/macosx-x86_64-normal-server-release/support/native/java.base//libfdlibm.a)
>> was built for newer OSX version (10.12) than being linked (10.7)
>> > > ld: warning: object file
>> (/Users/hohensee/workspaces/jdk10-hs/build/macosx-x86_64-normal-server-release/support/native/java.base/libjli_static.a)
>> was built for newer OSX version (10.12) than being linked (10.7)
>> > > clang: warning: libstdc++ is deprecated; move to libc++
>> with a minimum deployment target of OS X 10.9 [-Wdeprecated]
>> > I believe the warnings for static libs is simply caused
>> by not adding
>> > -mmacosx-version-min to the ARFLAGS. Not sure if ar on
>> mac takes that
>> > flag though.
>> >
>> > The libstdc++ warning seems harder to work around until
>> we change the
>> > minimum to 10.9 instead of 10.7.
>> >
>> > I would appreciate if you could also include this patch
>> as part of this
>> > change to make Oracle builds still behave as before:
>> >
>> > diff -r a6c830ee8a67 common/conf/jib-profiles.js
>> > --- a/common/conf/jib-profiles.js
>> > +++ b/common/conf/jib-profiles.js
>> > @@ -436,7 +436,8 @@
>> > target_os: "macosx",
>> > target_cpu: "x64",
>> > dependencies: ["devkit"],
>> > - configure_args:
>> concat(common.configure_args_64bit,
>> > "--with-zlib=system"),
>> > + configure_args:
>> concat(common.configure_args_64bit,
>> > "--with-zlib=system",
>> > + "--with-macosx-version-max=10.7.0"),
>> > },
>> >
>> > "solaris-x64": {
>> >
>> >
>> > Thanks!
>> > /Erik
>> > > Paul
>> > >
>> > > On 7/11/17, 2:45 AM, "Erik Joelsson"
>> <erik.joelsson at oracle.com> wrote:
>> > >
>> > > The -DMAC_OSX_VERSION_MAX_ALLOWED and
>> -mmacosx-version-min arguments are
>> > > used in combination to achieve the same thing. I
>> chose to use both to
>> > > really enforce full compatibility with the
>> specified version. The
>> > > "official" way of targeting earlier versions of
>> the OS is just using
>> > > -mmacosx-version-min. This will however still
>> accept uses of newer APIs,
>> > > but at link time, those will be linked with
>> weak_import. Essentially
>> > > it's expected that your application should be able
>> to do without these
>> > > calls if necessary, at the application level.
>> While better than not
>> > > being able to launch at all on the older OS, by
>> adding
>> > > -DMAC_OSX_VERSION_MAX_ALLOWED, it becomes a
>> compile time error if any
>> > > code tries to use a newer API.
>> > >
>> > > As I see it, either we fully enforce this at build
>> time, or we don't at
>> > > all. The natural default is to build for the
>> current host platform. The
>> > > configure parameter would make it possible to
>> enforce a minimal
>> > > compatible OS version that the binaries must be
>> usable on.
>> > >
>> > > (Note that if you propose such a change, I will
>> need to add the Oracle
>> > > bit as well, where we use the parameter, which
>> would need to go in at
>> > > the same time in common/conf/jib-profiles.js. Also
>> note that I will be
>> > > on vacation for 5 weeks starting this weekend so
>> won't be around to
>> > > review for most of that time.)
>> > >
>> > > /Erik
>> > >
>> > >
>> > > On 2017-07-10 19:48, Hohensee, Paul wrote:
>> > > > That’s a good idea, though the option would be
>> --with-macosx-version-max=<n>, right? The minimum is currently
>> hard-coded and should probably stay that way since there’s likely a
>> lot of code that depends on it. Let me see what I can come up with.
>> > > >
>> > > > Thanks,
>> > > >
>> > > > Paul
>> > > >
>> > > > On 7/10/17, 10:01 AM, "Erik Joelsson"
>> <erik.joelsson at oracle.com> wrote:
>> > > >
>> > > >
>> > > >
>> > > > On 2017-07-10 18:09, Hohensee, Paul wrote:
>> > > > > Hi Erik,
>> > > > >
>> > > > > The problem is that the compiler doesn’t
>> issue a warning in this case, but rather a type-mismatch error on
>> NSEventMask, so I can’t turn it off. NSUInteger was being used as an
>> enum, so Apple changed to using a real enum in 10.12 as a matter of
>> code hygiene. The new code in NSApplicationAWT.m is doing the right
>> thing by checking MAC_OS_X_VERSION_MAX_ALLOWED.
>> > > > >
>> > > > > What particular problem were you trying
>> to solve? Production, QA and JPRT builds and test runs are done on
>> the oldest supported OSX version, so any use of newer features should
>> be detected very early in the test process. Restricting builds to old
>> OSX versions means that engineers who keep their development boxes up
>> to date (which they should: security, etc.) can’t use them to do JDK
>> development.
>> > > > That's not exactly true. Apple is making it
>> very hard to stay on older
>> > > > versions of the OS compared to other OS
>> vendors. For this reason we are
>> > > > not always able to stay on a particular
>> version for Macosx in
>> > > > particular. We also in general try to avoid
>> having to fill our build
>> > > > servers/environments with just the oldest
>> OSes, because older OSes are
>> > > > harder to maintain and less convenient to
>> work with. So instead, we try
>> > > > to maintain working build environments on
>> newer OSes that produce
>> > > > binaries that are compatible with the
>> oldest we support. So, at least
>> > > > from Oracle's perspective, we prefer if
>> builds on different OS versions
>> > > > produce equivalent binaries when possible.
>> We certainly don't want to
>> > > > prevent building on newer OS/compilers.
>> > > >
>> > > > If this can't be worked around at the
>> source level, then perhaps we need
>> > > > to consider hiding this macro definition
>> behind a configure option that
>> > > > we can use internally. I would be open to
>> that. Something like
>> > > > --with-macosx-version-min=10.7 which configure
>> could then translate to
>> > > > the combination of options currently used.
>> That way, most openjdk
>> > > > developers/builders would not need to
>> suffer this Oracle requirement.
>> > > >
>> > > > /Erik
>> > > > > Thanks,
>> > > > >
>> > > > > Paul
>> > > > >
>> > > > > On 7/10/17, 1:10 AM, "Erik Joelsson"
>> <erik.joelsson at oracle.com> wrote:
>> > > > >
>> > > > > Hello,
>> > > > >
>> > > > > I do not agree to removing that
>> macro. I added those options to help
>> > > > > guarantee that a build made on a
>> newer version of macosx would still run
>> > > > > on the oldest version currently
>> supported. The macro is not mainly meant
>> > > > > to be used in our code, but is
>> picked up by system headers to cause an
>> > > > > error if any features newer than
>> 10.7 are used. It may be that we should
>> > > > > bump it to a newer version of macosx
>> in JDK 10, but certainly not to 10.12.
>> > > > >
>> > > > > It seems to me that we instead need
>> to ignore the particular warning for
>> > > > > this case.
>> > > > >
>> > > > > /Erik
>> > > > >
>> > > > >
>> > > > > On 2017-07-09 15:26, Hohensee, Paul
>> wrote:
>> > > > > > Please review the following change
>> to get JDK10 to build on OSX 10.12 and above.
>> > > > > >
>> > > > > >
>> https://bugs.openjdk.java.net/browse/JDK-8184022
>> > > > > >
>> http://cr.openjdk.java.net/~phh/8184022/webrev.00/
>> > > > > >
>> > > > > > I’d very much appreciate a sponsor
>> for this fix. Imo, successful JDK10 builds on all supported platforms
>> would be sufficient testing, but please let me know what I can do to
>> help.
>> > > > > >
>> > > > > > Slightly revised from the RFE:
>> > > > > >
>> > > > > >
>> JDK-8182299<https://bugs.openjdk.java.net/browse/JDK-8182299> enabled
>> previously disabled clang warnings and was intended to also enable
>> builds on OSX 10 + Xcode 8. Due to a mixup, this code in
>> jdk/src/java.desktop/macosx/native/libosxapp/NSApplicationAWT.m
>> > > > > >
>> > > > > > #if
>> defined(MAC_OS_X_VERSION_10_12) && \
>> > > > > > MAC_OS_X_VERSION_MAX_ALLOWED >=
>> MAC_OS_X_VERSION_10_12 && \
>> > > > > > __LP64__
>> > > > > > / 10.12 changed `mask` to
>> NSEventMask (unsigned long long) for x86_64 builds.
>> > > > > > - (NSEvent
>> *)nextEventMatchingMask:(NSEventMask)mask
>> > > > > > #else
>> > > > > > - (NSEvent
>> *)nextEventMatchingMask:(NSUInteger)mask
>> > > > > > #endif
>> > > > > > untilDate:(NSDate *)expiration
>> inMode:(NSString *)mode dequeue:(BOOL)deqFlag {
>> > > > > >
>> > > > > > works fine with OSX versions
>> earlier than 10.12, but fails to compile starting with OSX 10.12 due
>> to MAC_OSX_VERSION_MAX_ALLOWED being defined on the compile command
>> line as 10.7.
>> > > > > >
>> > > > > > The fix is to remove that
>> definition, since it places an artificial upper bound on the OSX
>> version under which JDK10 can be built. A source code search reveals
>> no uses of MAC_OSX_VERSION_MAX_ALLOWED other than in
>> NSApplicationAWT.m and hotspot/src/os_cpu/bsd_x86/vm/os_bsd_x86.cpp.
>> The latter won't be affected by this change, since it checks for a
>> version > 10.5, which is always true in JDK10.
>> > > > > >
>> > > > > > Thanks,
>> > > > > >
>> > > > > > Paul
>> > > > > >
>> > > > >
>> > > > >
>> > > > >
>> > > >
>> > > >
>> > > >
>> > >
>> > >
>> > >
>> >
>> >
>> >
>>
>
More information about the build-dev
mailing list