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.joels...@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.joels...@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.joels...@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.joels...@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
> > > >
> > >
> > >
> > >
> >
> >
> >
>
>
>
