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