all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Mac OS Sierra tab feature breaks C-x 5 2
@ 2017-07-06 11:29 Paul Michael Reilly
  2017-07-06 12:14 ` Jean-Christophe Helary
  2017-07-06 12:53 ` Alan Third
  0 siblings, 2 replies; 63+ messages in thread
From: Paul Michael Reilly @ 2017-07-06 11:29 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 872 bytes --]

I have recently switched to macOS Sierra and discovered that using C-x 5 2
to create a new frame works just fine, i.e. no different than OS X, if you
do not enter full screen mode (via the green window decoration button).  If
you are in full screen mode C-x 5 2 is ignored, but if you then exit full
screen mode it is clear that the second frame is now a tab (macOS Sierra
tab feature).

I have not seen this behavior mentioned but then I haven't been paying that
close attention.

Prior to macOS Sierra, in full screen mode, C-x 5 2 would create the new
Emacs frame in a new OS X window.

Seems to me that this behavior in macOS Sierra is an Emacs bug to the
extent that it will take Emacs code changes to achieve OS X behavior.  Is
anyone aware of a fix for this issue that has not been released yet?  Or is
it work not yet taken on?  Or am I completely wrong?
-- 
-pmr

[-- Attachment #2: Type: text/html, Size: 1062 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-06 11:29 Mac OS Sierra tab feature breaks C-x 5 2 Paul Michael Reilly
@ 2017-07-06 12:14 ` Jean-Christophe Helary
  2017-07-06 12:46   ` Sebastian Christ
  2017-07-06 12:53 ` Alan Third
  1 sibling, 1 reply; 63+ messages in thread
From: Jean-Christophe Helary @ 2017-07-06 12:14 UTC (permalink / raw)
  To: Paul Michael Reilly; +Cc: emacs-devel


> On Jul 6, 2017, at 20:29, Paul Michael Reilly <pmr@pajato.com> wrote:
> 
> I have recently switched to macOS Sierra and discovered that using C-x 5 2 to create a new frame works just fine, i.e. no different than OS X, if you do not enter full screen mode (via the green window decoration button).  If you are in full screen mode C-x 5 2 is ignored, but if you then exit full screen mode it is clear that the second frame is now a tab (macOS Sierra tab feature).

I work with Sierra and Emacs master and I'm not seeing this. When I am in fullscreen mode (by pressing the green thing at the top left) and I create new frames with C-x 5 2, I get new frames in full screen.

Jean-Christophe 




^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-06 12:14 ` Jean-Christophe Helary
@ 2017-07-06 12:46   ` Sebastian Christ
  0 siblings, 0 replies; 63+ messages in thread
From: Sebastian Christ @ 2017-07-06 12:46 UTC (permalink / raw)
  To: emacs-devel

>>>>> "JH" == Jean-Christophe Helary <jean.christophe.helary@gmail.com> writes:

    >> On Jul 6, 2017, at 20:29, Paul Michael Reilly <pmr@pajato.com> wrote:
    >> 
    >> I have recently switched to macOS Sierra and discovered that using
    >> C-x 5 2 to create a new frame works just fine, i.e. no different
    >> than OS X, if you do not enter full screen mode (via the green
    >> window decoration button).  If you are in full screen mode C-x 5 2
    >> is ignored, but if you then exit full screen mode it is clear that
    >> the second frame is now a tab (macOS Sierra tab feature).

    JH> I work with Sierra and Emacs master and I'm not seeing this. When I am
    JH> in fullscreen mode (by pressing the green thing at the top left) and I
    JH> create new frames with C-x 5 2, I get new frames in full screen.

I'm experiencing the same. New tab instead of a new Frame. When I close
one tab (by clicking on the X on the left side of the tab) then Emacs
quits completely. I'm unsure if there is a connection between those two
behaviours.

Regards,

Sebastian


-- 
Sebastian (Rudolfo) Christ
http://rudolfochrist.github.io
GPG Fingerprint: 306D 8FD3 DFB6 4E44 5061
                 CE71 6407 D6F8 2AC5 55DD




^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-06 11:29 Mac OS Sierra tab feature breaks C-x 5 2 Paul Michael Reilly
  2017-07-06 12:14 ` Jean-Christophe Helary
@ 2017-07-06 12:53 ` Alan Third
  2017-07-06 14:35   ` Alan Third
  1 sibling, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-07-06 12:53 UTC (permalink / raw)
  To: Paul Michael Reilly; +Cc: Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 594 bytes --]

On 6 Jul 2017 12:29 p.m., "Paul Michael Reilly" <pmr@pajato.com> wrote:

I have recently switched to macOS Sierra and discovered that using C-x 5 2
to create a new frame works just fine, i.e. no different than OS X, if you
do not enter full screen mode (via the green window decoration button).  If
you are in full screen mode C-x 5 2 is ignored, but if you then exit full
screen mode it is clear that the second frame is now a tab (macOS Sierra
tab feature).


This behaviour should be fixed in master. Can you please try it? (nightly
builds if you're using emacsformacos.com builds)

Thanks.

[-- Attachment #2: Type: text/html, Size: 1105 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-06 12:53 ` Alan Third
@ 2017-07-06 14:35   ` Alan Third
  2017-07-06 15:05     ` Jean-Christophe Helary
  0 siblings, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-07-06 14:35 UTC (permalink / raw)
  To: Paul Michael Reilly; +Cc: Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

On 6 Jul 2017 1:53 p.m., "Alan Third" <athird@googlemail.com> wrote:



On 6 Jul 2017 12:29 p.m., "Paul Michael Reilly" <pmr@pajato.com> wrote:

I have recently switched to macOS Sierra and discovered that using C-x 5 2
to create a new frame works just fine, i.e. no different than OS X, if you
do not enter full screen mode (via the green window decoration button).  If
you are in full screen mode C-x 5 2 is ignored, but if you then exit full
screen mode it is clear that the second frame is now a tab (macOS Sierra
tab feature).


This behaviour should be fixed in master. Can you please try it? (nightly
builds if you're using emacsformacos.com builds)


I've just realised that we're using a build time version check, and the
Emacs for macOS releases are built on an older version of macOS... I'm not
sure what the best solution here is...

I'll have a think about it.

[-- Attachment #2: Type: text/html, Size: 1867 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-06 14:35   ` Alan Third
@ 2017-07-06 15:05     ` Jean-Christophe Helary
  2017-07-06 17:42       ` Alan Third
  0 siblings, 1 reply; 63+ messages in thread
From: Jean-Christophe Helary @ 2017-07-06 15:05 UTC (permalink / raw)
  To: Alan Third; +Cc: Paul Michael Reilly, Emacs-Devel devel


> On Jul 6, 2017, at 23:35, Alan Third <athird@googlemail.com> wrote:

> I've just realised that we're using a build time version check, and the Emacs for macOS releases are built on an older version of macOS... I'm not sure what the best solution here is...

Is the result very different depending on the version of macOS? What's the version you are building on?

Jean-Christophe 


^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
@ 2017-07-06 17:24 Matthew Bauer
  0 siblings, 0 replies; 63+ messages in thread
From: Matthew Bauer @ 2017-07-06 17:24 UTC (permalink / raw)
  To: pmr; +Cc: emacs-devel

> Prior to macOS Sierra, in full screen mode, C-x 5 2 would create the new Emacs frame in a new OS X window.

This is actually configurable within macOS settings. To accomplish this go to the menu bar:

 → System Preferences... → Dock → Prefer tabs when opening documents

and select "Manually". Of course this will change the behavior in other apps when in full screen.

-mjb


^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-06 15:05     ` Jean-Christophe Helary
@ 2017-07-06 17:42       ` Alan Third
  2017-07-06 22:16         ` Alan Third
  0 siblings, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-07-06 17:42 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: Paul Michael Reilly, Emacs-Devel devel

On Fri, Jul 07, 2017 at 12:05:32AM +0900, Jean-Christophe Helary wrote:
> 
> > On Jul 6, 2017, at 23:35, Alan Third <athird@googlemail.com> wrote:
> 
> > I've just realised that we're using a build time version check,
> > and the Emacs for macOS releases are built on an older version of
> > macOS... I'm not sure what the best solution here is...
> 
> Is the result very different depending on the version of macOS?
> What's the version you are building on?

I build on Sierra.

We have this code

    #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_12
      [win setTabbingMode: NSWindowTabbingModeDisallowed];
    #endif

in initFrameFromEmacs in nsterm.m. The idea being that since this code
only works in 10.12 and up, we only include it when Emacs is compiled
on 10.12. This might be why you’re not seeing the broken behaviour.

I believe that the emacsformacosx builds are built on 10.9 or 10.10 or
something? I can’t remember exactly. That means this code is not
included.

We can make it a run time check, which would look something like

    if ([win respondsToSelector: @selector(setTabbingMode)])
      [win setTabbingMode: NSWindowTabbingModeDisallowed];

but this will throw up compiler warnings on pre‐Sierra versions of
macOS. I guess that’s maybe just the price to be paid.
-- 
Alan Third



^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-06 17:42       ` Alan Third
@ 2017-07-06 22:16         ` Alan Third
  2017-07-10 19:17           ` Anders Lindgren
  0 siblings, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-07-06 22:16 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: Paul Michael Reilly, Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

On Thu, Jul 06, 2017 at 06:42:04PM +0100, Alan Third wrote:
> We can make it a run time check, which would look something like
> 
>     if ([win respondsToSelector: @selector(setTabbingMode)])
>       [win setTabbingMode: NSWindowTabbingModeDisallowed];
> 
> but this will throw up compiler warnings on pre‐Sierra versions of
> macOS. I guess that’s maybe just the price to be paid.

I’ve attached a patch to master for this. Can someone on macOS 10.11
or below give it a try and confirm that it compiles and runs?
-- 
Alan Third

[-- Attachment #2: 0001-Use-a-run-time-check-for-macOS-Sierra-tabbing-suppor.patch --]
[-- Type: text/plain, Size: 1120 bytes --]

From c22264617bf4d50116c4e55525935241931c2cf1 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Thu, 6 Jul 2017 23:10:49 +0100
Subject: [PATCH] Use a run-time check for macOS Sierra tabbing support

* src/nsterm.m (initFrameFromEmacs) [NS_IMPL_COCOA]: Switch from
compile-time check to run-time.
---
 src/nsterm.m | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index bf83550b3d..f88b279987 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -7073,9 +7073,9 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   /* macOS Sierra automatically enables tabbed windows.  We can't
      allow this to be enabled until it's available on a Free system.
      Currently it only happens by accident and is buggy anyway. */
-#if defined (NS_IMPL_COCOA) && \
-  MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_12
-  [win setTabbingMode: NSWindowTabbingModeDisallowed];
+#ifdef NS_IMPL_COCOA
+  if ([win respondsToSelector: @selector(setTabbingMode:)])
+    [win setTabbingMode: NSWindowTabbingModeDisallowed];
 #endif
 
   ns_window_num++;
-- 
2.12.0


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-06 22:16         ` Alan Third
@ 2017-07-10 19:17           ` Anders Lindgren
  2017-07-10 19:52             ` Alan Third
  0 siblings, 1 reply; 63+ messages in thread
From: Anders Lindgren @ 2017-07-10 19:17 UTC (permalink / raw)
  To: Alan Third; +Cc: Paul Michael Reilly, Jean-Christophe Helary, Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 1040 bytes --]

Hi!

Unfortunately, it doesn't compile. I get the following error when I build
on 10.10.5:

nsterm.m:7078:26: error: use of undeclared identifier
'NSWindowTabbingModeDisallowed'
    [win setTabbingMode: NSWindowTabbingModeDisallowed];

Fortunately, the fix is simple. If we build on an older system, we can
define the NSWindowTabbingModeXxx constants ourselves.

    -- Anders


On Fri, Jul 7, 2017 at 12:16 AM, Alan Third <alan@idiocy.org> wrote:

> On Thu, Jul 06, 2017 at 06:42:04PM +0100, Alan Third wrote:
> > We can make it a run time check, which would look something like
> >
> >     if ([win respondsToSelector: @selector(setTabbingMode)])
> >       [win setTabbingMode: NSWindowTabbingModeDisallowed];
> >
> > but this will throw up compiler warnings on pre‐Sierra versions of
> > macOS. I guess that’s maybe just the price to be paid.
>
> I’ve attached a patch to master for this. Can someone on macOS 10.11
> or below give it a try and confirm that it compiles and runs?
> --
> Alan Third
>

[-- Attachment #2: Type: text/html, Size: 1596 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-10 19:17           ` Anders Lindgren
@ 2017-07-10 19:52             ` Alan Third
  2017-07-10 20:22               ` Anders Lindgren
  0 siblings, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-07-10 19:52 UTC (permalink / raw)
  To: Anders Lindgren
  Cc: Paul Michael Reilly, Jean-Christophe Helary, Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 491 bytes --]

On Mon, Jul 10, 2017 at 09:17:58PM +0200, Anders Lindgren wrote:
> Unfortunately, it doesn't compile. I get the following error when I build
> on 10.10.5:
> 
> nsterm.m:7078:26: error: use of undeclared identifier
> 'NSWindowTabbingModeDisallowed'
>     [win setTabbingMode: NSWindowTabbingModeDisallowed];
> 
> Fortunately, the fix is simple. If we build on an older system, we can
> define the NSWindowTabbingModeXxx constants ourselves.

I think this should do it, then...
-- 
Alan Third

[-- Attachment #2: 0001-Use-a-run-time-check-for-macOS-Sierra-tabbing-suppor.patch --]
[-- Type: text/plain, Size: 1699 bytes --]

From 21e4a39eea0da07afd419456a2b6d2df972c622b Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Thu, 6 Jul 2017 23:10:49 +0100
Subject: [PATCH] Use a run-time check for macOS Sierra tabbing support

* src/nsterm.m (initFrameFromEmacs) [NS_IMPL_COCOA]: Switch from
compile-time check to run-time.
* src/nsterm.h (NSWindowTabbingMode): Define in pre-Sierra macOS.
---
 src/nsterm.h | 8 ++++++++
 src/nsterm.m | 6 +++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/nsterm.h b/src/nsterm.h
index 0f1b36db7b..da221605b4 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -1317,6 +1317,14 @@ extern char gnustep_base_version[];  /* version tracking */
 #ifdef __OBJC__
 typedef NSUInteger NSWindowStyleMask;
 #endif
+
+/* Window tabbing mode enums are new too. */
+enum NSWindowTabbingMode
+  {
+    NSWindowTabbingModeAutomatic,
+    NSWindowTabbingModePreferred,
+    NSWindowTabbingModeDisallowed
+  };
 #endif
 
 #endif	/* HAVE_NS */
diff --git a/src/nsterm.m b/src/nsterm.m
index bf83550b3d..f88b279987 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -7073,9 +7073,9 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   /* macOS Sierra automatically enables tabbed windows.  We can't
      allow this to be enabled until it's available on a Free system.
      Currently it only happens by accident and is buggy anyway. */
-#if defined (NS_IMPL_COCOA) && \
-  MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_12
-  [win setTabbingMode: NSWindowTabbingModeDisallowed];
+#ifdef NS_IMPL_COCOA
+  if ([win respondsToSelector: @selector(setTabbingMode:)])
+    [win setTabbingMode: NSWindowTabbingModeDisallowed];
 #endif
 
   ns_window_num++;
-- 
2.12.0


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-10 19:52             ` Alan Third
@ 2017-07-10 20:22               ` Anders Lindgren
  2017-07-12 18:23                 ` Alan Third
  0 siblings, 1 reply; 63+ messages in thread
From: Anders Lindgren @ 2017-07-10 20:22 UTC (permalink / raw)
  To: Alan Third; +Cc: Paul Michael Reilly, Jean-Christophe Helary, Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 1306 bytes --]

Now it compiles, but with a warning:

----
nsterm.m:7078:10: warning: instance method '-setTabbingMode:' not found
(return type defaults to 'id') [-Wobjc-method-access]
    [win setTabbingMode: NSWindowTabbingModeDisallowed];
         ^~~~~~~~~~~~~~
/System/Library/Frameworks/AppKit.framework/Headers/NSWindow.h:167:12:
note: receiver is instance of class declared here
@interface NSWindow : NSResponder <NSAnimatablePropertyContainer,
NSUserInterfaceValidations, NSUserInterfaceItemIdentifica...
-----


I've seen other packages declare methods in NSWindows, but I don't know how
"correct" that is. For example:

https://github.com/electron/electron/blob/master/atom/browser/native_window_mac.mm#L450

    -- Anders

On Mon, Jul 10, 2017 at 9:52 PM, Alan Third <alan@idiocy.org> wrote:

> On Mon, Jul 10, 2017 at 09:17:58PM +0200, Anders Lindgren wrote:
> > Unfortunately, it doesn't compile. I get the following error when I build
> > on 10.10.5:
> >
> > nsterm.m:7078:26: error: use of undeclared identifier
> > 'NSWindowTabbingModeDisallowed'
> >     [win setTabbingMode: NSWindowTabbingModeDisallowed];
> >
> > Fortunately, the fix is simple. If we build on an older system, we can
> > define the NSWindowTabbingModeXxx constants ourselves.
>
> I think this should do it, then...
> --
> Alan Third
>

[-- Attachment #2: Type: text/html, Size: 2101 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-10 20:22               ` Anders Lindgren
@ 2017-07-12 18:23                 ` Alan Third
  2017-07-12 21:20                   ` Anders Lindgren
  0 siblings, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-07-12 18:23 UTC (permalink / raw)
  To: Anders Lindgren
  Cc: Paul Michael Reilly, Jean-Christophe Helary, Emacs-Devel devel

On Mon, Jul 10, 2017 at 10:22:43PM +0200, Anders Lindgren wrote:
> Now it compiles, but with a warning:
> 
> ----
> nsterm.m:7078:10: warning: instance method '-setTabbingMode:' not found
> (return type defaults to 'id') [-Wobjc-method-access]
>     [win setTabbingMode: NSWindowTabbingModeDisallowed];
>          ^~~~~~~~~~~~~~
> /System/Library/Frameworks/AppKit.framework/Headers/NSWindow.h:167:12:
> note: receiver is instance of class declared here
> @interface NSWindow : NSResponder <NSAnimatablePropertyContainer,
> NSUserInterfaceValidations, NSUserInterfaceItemIdentifica...
> -----
> 
> 
> I've seen other packages declare methods in NSWindows, but I don't know how
> "correct" that is. For example:
> 
> https://github.com/electron/electron/blob/master/atom/browser/native_window_mac.mm#L450

Yeah, I’m not sure about this either. It strikes me as a bit suspect,
but I don’t know that the alternatives are going to be any better.

We could disable the warning:

    #pragma clang diagnostic push
    #pragma clang diagnostic ignored "-Wobjc-method-access"
      if ([win respondsToSelector: @selector(setTabbingMode:)])
        [win setTabbingMode: NSWindowTabbingModeDisallowed];
    #pragma clang diagnostic pop

which is, I think, the right thing to do, but that’s quite messy
looking, and I suspect we’ll have to do the same for the gcc warning.
I don’t know if we’d then have to do some #ifdef stuff to separate the
clang and gcc stuff. This has the potential to get REALLY messy.

Another possibility is to cast `win` to `id`, which I’d guess would
look like:

    [(id)win setTabbingMode: NSWindowTabbingModeDisallowed];

That compiles here, no idea if it gets rid of the warning. It’s short
and sweet, but The Internet seems to think this is generally a bad
idea. I think we’d be OK since we’re only casting it once here, and as
we already know whether or not the method is available, type‐checking
(which we’re bypassing) doesn’t really give us anything.

By the way, while digging around I came across a lot of stuff saying
`@selector(setTabbingMode)` should give a warning if `setTabbingMode`
doesn’t exist. Are you seeing one?
-- 
Alan Third



^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-12 18:23                 ` Alan Third
@ 2017-07-12 21:20                   ` Anders Lindgren
  2017-07-13 20:22                     ` Alan Third
  0 siblings, 1 reply; 63+ messages in thread
From: Anders Lindgren @ 2017-07-12 21:20 UTC (permalink / raw)
  To: Alan Third; +Cc: Paul Michael Reilly, Jean-Christophe Helary, Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 2718 bytes --]

> Yeah, I’m not sure about this either. It strikes me as a bit suspect,
> but I don’t know that the alternatives are going to be any better.
>
> We could disable the warning:
>
>     #pragma clang diagnostic push
>     #pragma clang diagnostic ignored "-Wobjc-method-access"
>       if ([win respondsToSelector: @selector(setTabbingMode:)])
>         [win setTabbingMode: NSWindowTabbingModeDisallowed];
>     #pragma clang diagnostic pop
>
> which is, I think, the right thing to do, but that’s quite messy
> looking, and I suspect we’ll have to do the same for the gcc warning.
> I don’t know if we’d then have to do some #ifdef stuff to separate the
> clang and gcc stuff. This has the potential to get REALLY messy.
>

It's possible to encapsulate pragmas into preprocessor macros using the
_Pragma construct. In fact, config.h contains similar stuff, c.f.
_GL_INLINE_HEADER_BEGIN and _GL_INLINE_HEADER_END.


Another possibility is to cast `win` to `id`, which I’d guess would
> look like:
>
>     [(id)win setTabbingMode: NSWindowTabbingModeDisallowed];
>

I still get the same warning, so this is not a solution.


By the way, while digging around I came across a lot of stuff saying
> `@selector(setTabbingMode)` should give a warning if `setTabbingMode`
> doesn’t exist. Are you seeing one?
>

No, no warning is issued about that.

I believe that there are a number of #ifdef:s that could be rewritten in
this way, so we need something ergonomically. (If we could get rid of all
such ifdef:s there would not be a need for OS-version specific binaries.)


Given the options, I would say that a solution along the lines of
_GL_INLINE_HEADER_BEGIN would be best. The pros are:

* All the messy bits can be located in one place, and it can easily be
updated to match future compiler versions. (Should some compiler start to
warn about, say, `@selector(setTabbingMode)`, that too could be
incorporated in the macros.)
* There is a natural location to place documentation and to describe the
situation
* In the code that use the macros, there is a signal to the reader that
something special is going on
* It's possible to write formal tests, ensuring the macros work as intended.

Unlike _GL_INLINE_HEADER_BEGIN, we should not use a leading underscore, as
such identifiers are reserved for the compiler itself. Maybe
NS_SILENCE_MISSING_METHOD_WARNING_BEGIN and -_END? The end result would
look like:

      NS_SILENCE_MISSING_METHOD_WARNING_BEGIN
      if ([win respondsToSelector: @selector(setTabbingMode:)])
        [win setTabbingMode: NSWindowTabbingModeDisallowed];
      NS_SILENCE_MISSING_METHOD_WARNING_END

    -- Anders

[-- Attachment #2: Type: text/html, Size: 3640 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-12 21:20                   ` Anders Lindgren
@ 2017-07-13 20:22                     ` Alan Third
  2017-07-16 18:43                       ` Anders Lindgren
  0 siblings, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-07-13 20:22 UTC (permalink / raw)
  To: Anders Lindgren
  Cc: Paul Michael Reilly, Jean-Christophe Helary, Emacs-Devel devel

On Wed, Jul 12, 2017 at 11:20:57PM +0200, Anders Lindgren wrote:
> > We could disable the warning:
> >
> >     #pragma clang diagnostic push
> >     #pragma clang diagnostic ignored "-Wobjc-method-access"
> >       if ([win respondsToSelector: @selector(setTabbingMode:)])
> >         [win setTabbingMode: NSWindowTabbingModeDisallowed];
> >     #pragma clang diagnostic pop
> 
<snip>
> 
> The end result would look like:
> 
>       NS_SILENCE_MISSING_METHOD_WARNING_BEGIN
>       if ([win respondsToSelector: @selector(setTabbingMode:)])
>         [win setTabbingMode: NSWindowTabbingModeDisallowed];
>       NS_SILENCE_MISSING_METHOD_WARNING_END

I like this option, but after a lot of messing about I’m pretty sure
that gcc doesn’t let you silence this warning.

So, unless we don’t care about gcc warnings for the NS build, we could
try using performSelector:

    [win performSelector: @selector(setTabbingMode:)
              withObject: (id)NSWindowTabbingModeDisallowed];

Which only supports one parameter, so works here but not necessarily
anywhere else we might want to try this.

Or back to the first suggestion and fake the methods when they’re not
there. But that doesn’t help get rid of the need for multiple
binaries.
-- 
Alan Third



^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-13 20:22                     ` Alan Third
@ 2017-07-16 18:43                       ` Anders Lindgren
  2017-07-16 23:01                         ` Alan Third
  0 siblings, 1 reply; 63+ messages in thread
From: Anders Lindgren @ 2017-07-16 18:43 UTC (permalink / raw)
  To: Alan Third; +Cc: Paul Michael Reilly, Jean-Christophe Helary, Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 2244 bytes --]

On Thu, Jul 13, 2017 at 10:22 PM, Alan Third <alan@idiocy.org> wrote:

> On Wed, Jul 12, 2017 at 11:20:57PM +0200, Anders Lindgren wrote:
> > The end result would look like:
> >
> >       NS_SILENCE_MISSING_METHOD_WARNING_BEGIN
> >       if ([win respondsToSelector: @selector(setTabbingMode:)])
> >         [win setTabbingMode: NSWindowTabbingModeDisallowed];
> >       NS_SILENCE_MISSING_METHOD_WARNING_END
>
> I like this option, but after a lot of messing about I’m pretty sure
> that gcc doesn’t let you silence this warning.
>

Unfortunately, I came to the same conclusion.


So, unless we don’t care about gcc warnings for the NS build, we could
> try using performSelector:
>
>     [win performSelector: @selector(setTabbingMode:)
>               withObject: (id)NSWindowTabbingModeDisallowed];
>
Which only supports one parameter, so works here but not necessarily
> anywhere else we might want to try this.
>


It doesn't feel right, as it bypasses all type checking even when building
on modern systems. And, as you mentioned, it only supports one parameter.

I tried to figure out if gcc or clang was used when building on 10.6.8, but
ran out of time (and won't have the chance to do it again anytime soon). I
did conclude that it comes with both a real "gcc" and a real "clang", so
presumably gcc is used. (Surprisingly, more modern versions of macOS seems
to map the command "gcc" to "clang".)

A warning-free build is a must on modern system (which use clang). It would
be nice on older system, I guess, but it would be hard to enforce. (We
could even lobby to add the option to future gcc versions, for the benefit
of GNUStep, but it would not help the situation on older macOS versions.)


Or back to the first suggestion and fake the methods when they’re not
> there. But that doesn’t help get rid of the need for multiple
> binaries.
>

Not an appealing solution.


My gut feeling is to go with the NS_SILENCE_MISSING_METHOD_WARNING_BEGIN
solution, as it work on modern macOS systems, it retains type checking, and
it give us a single location to describe the situation and to modify the
macro, if there should be a need for it in the future.

    -- Anders

[-- Attachment #2: Type: text/html, Size: 3425 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-16 18:43                       ` Anders Lindgren
@ 2017-07-16 23:01                         ` Alan Third
  2017-07-17 20:09                           ` Charles A. Roelli
  0 siblings, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-07-16 23:01 UTC (permalink / raw)
  To: Anders Lindgren
  Cc: Paul Michael Reilly, Jean-Christophe Helary, Charles A. Roelli,
	Emacs-Devel devel

On Sun, Jul 16, 2017 at 08:43:25PM +0200, Anders Lindgren wrote:
> I tried to figure out if gcc or clang was used when building on 10.6.8, but
> ran out of time (and won't have the chance to do it again anytime soon). I
> did conclude that it comes with both a real "gcc" and a real "clang", so
> presumably gcc is used. (Surprisingly, more modern versions of macOS seems
> to map the command "gcc" to "clang".)

Charles can maybe answer this for us, as he uses 10.6.

> A warning-free build is a must on modern system (which use clang). It would
> be nice on older system, I guess, but it would be hard to enforce. (We
> could even lobby to add the option to future gcc versions, for the benefit
> of GNUStep, but it would not help the situation on older macOS versions.)

> My gut feeling is to go with the NS_SILENCE_MISSING_METHOD_WARNING_BEGIN
> solution, as it work on modern macOS systems, it retains type checking, and
> it give us a single location to describe the situation and to modify the
> macro, if there should be a need for it in the future.

Agreed.

I think that a lot (if not most) of the code that we might want to use
this on won’t work on GNUstep at all, so we could still exclude it
from building there if the warnings are too much.

-- 
Alan Third



^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-16 23:01                         ` Alan Third
@ 2017-07-17 20:09                           ` Charles A. Roelli
  2017-07-18  6:06                             ` Anders Lindgren
  0 siblings, 1 reply; 63+ messages in thread
From: Charles A. Roelli @ 2017-07-17 20:09 UTC (permalink / raw)
  To: Alan Third, Anders Lindgren
  Cc: Paul Michael Reilly, Jean-Christophe Helary, Emacs-Devel devel

clang and gcc seem different on 10.6 (can't say for certain, though):


$ which gcc
/usr/bin/gcc
$ gcc --version
i686-apple-darwin10-llvm-gcc-4.2 (GCC) 4.2.1 (Based on Apple Inc. build 
5658) (LLVM build 2335.15.00)
Copyright (C) 2007 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ which clang
/usr/bin/clang
$ clang --version
Apple clang version 2.1 (tags/Apple/clang-163.7.1) (based on LLVM 3.0svn)
Target: x86_64-apple-darwin10.8.0
Thread model: posix




On 17/07/2017 01:01, Alan Third wrote:
> On Sun, Jul 16, 2017 at 08:43:25PM +0200, Anders Lindgren wrote:
>> I tried to figure out if gcc or clang was used when building on 10.6.8, but
>> ran out of time (and won't have the chance to do it again anytime soon). I
>> did conclude that it comes with both a real "gcc" and a real "clang", so
>> presumably gcc is used. (Surprisingly, more modern versions of macOS seems
>> to map the command "gcc" to "clang".)
> Charles can maybe answer this for us, as he uses 10.6.
>
>> A warning-free build is a must on modern system (which use clang). It would
>> be nice on older system, I guess, but it would be hard to enforce. (We
>> could even lobby to add the option to future gcc versions, for the benefit
>> of GNUStep, but it would not help the situation on older macOS versions.)
>> My gut feeling is to go with the NS_SILENCE_MISSING_METHOD_WARNING_BEGIN
>> solution, as it work on modern macOS systems, it retains type checking, and
>> it give us a single location to describe the situation and to modify the
>> macro, if there should be a need for it in the future.
> Agreed.
>
> I think that a lot (if not most) of the code that we might want to use
> this on won’t work on GNUstep at all, so we could still exclude it
> from building there if the warnings are too much.
>




^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-17 20:09                           ` Charles A. Roelli
@ 2017-07-18  6:06                             ` Anders Lindgren
  2017-07-18 18:33                               ` Charles A. Roelli
  0 siblings, 1 reply; 63+ messages in thread
From: Anders Lindgren @ 2017-07-18  6:06 UTC (permalink / raw)
  To: Charles A. Roelli
  Cc: Paul Michael Reilly, Alan Third, Jean-Christophe Helary,
	Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 2395 bytes --]

Hi Charles, thanks for replying!

Yes, that is what I saw as well. The question is which compiler is used (by
default) when building Emacs on 10.6. I guess it is "gcc", but I didn't
have time to verify this.

    -- Anders

On Mon, Jul 17, 2017 at 10:09 PM, Charles A. Roelli <charles@aurox.ch>
wrote:

> clang and gcc seem different on 10.6 (can't say for certain, though):
>
>
> $ which gcc
> /usr/bin/gcc
> $ gcc --version
> i686-apple-darwin10-llvm-gcc-4.2 (GCC) 4.2.1 (Based on Apple Inc. build
> 5658) (LLVM build 2335.15.00)
> Copyright (C) 2007 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> $ which clang
> /usr/bin/clang
> $ clang --version
> Apple clang version 2.1 (tags/Apple/clang-163.7.1) (based on LLVM 3.0svn)
> Target: x86_64-apple-darwin10.8.0
> Thread model: posix
>
>
>
>
>
> On 17/07/2017 01:01, Alan Third wrote:
>
>> On Sun, Jul 16, 2017 at 08:43:25PM +0200, Anders Lindgren wrote:
>>
>>> I tried to figure out if gcc or clang was used when building on 10.6.8,
>>> but
>>> ran out of time (and won't have the chance to do it again anytime soon).
>>> I
>>> did conclude that it comes with both a real "gcc" and a real "clang", so
>>> presumably gcc is used. (Surprisingly, more modern versions of macOS
>>> seems
>>> to map the command "gcc" to "clang".)
>>>
>> Charles can maybe answer this for us, as he uses 10.6.
>>
>> A warning-free build is a must on modern system (which use clang). It
>>> would
>>> be nice on older system, I guess, but it would be hard to enforce. (We
>>> could even lobby to add the option to future gcc versions, for the
>>> benefit
>>> of GNUStep, but it would not help the situation on older macOS versions.)
>>> My gut feeling is to go with the NS_SILENCE_MISSING_METHOD_WARNING_BEGIN
>>> solution, as it work on modern macOS systems, it retains type checking,
>>> and
>>> it give us a single location to describe the situation and to modify the
>>> macro, if there should be a need for it in the future.
>>>
>> Agreed.
>>
>> I think that a lot (if not most) of the code that we might want to use
>> this on won’t work on GNUstep at all, so we could still exclude it
>> from building there if the warnings are too much.
>>
>>
>

[-- Attachment #2: Type: text/html, Size: 3212 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-18  6:06                             ` Anders Lindgren
@ 2017-07-18 18:33                               ` Charles A. Roelli
  2017-07-18 22:16                                 ` Alan Third
  0 siblings, 1 reply; 63+ messages in thread
From: Charles A. Roelli @ 2017-07-18 18:33 UTC (permalink / raw)
  To: Anders Lindgren
  Cc: Paul Michael Reilly, Alan Third, Jean-Christophe Helary,
	Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 3171 bytes --]

It does seem to be gcc:

(snippet from output of 'make V=1' when compiling src/nsterm.m):
gcc -std=gnu99 -c -Demacs -I. -I. -I../lib -I../lib -D_REENTRANT -I
[...]  -MMD -MF deps/nsterm.d -MP -O0 -g3 nsterm.m


And IIRC 'gcc' is symlinked to 'clang' on later macOS versions.


On 18/07/2017 08:06, Anders Lindgren wrote:
> Hi Charles, thanks for replying!
>
> Yes, that is what I saw as well. The question is which compiler is 
> used (by default) when building Emacs on 10.6. I guess it is "gcc", 
> but I didn't have time to verify this.
>
>     -- Anders
>
> On Mon, Jul 17, 2017 at 10:09 PM, Charles A. Roelli <charles@aurox.ch 
> <mailto:charles@aurox.ch>> wrote:
>
>     clang and gcc seem different on 10.6 (can't say for certain, though):
>
>
>     $ which gcc
>     /usr/bin/gcc
>     $ gcc --version
>     i686-apple-darwin10-llvm-gcc-4.2 (GCC) 4.2.1 (Based on Apple Inc.
>     build 5658) (LLVM build 2335.15.00)
>     Copyright (C) 2007 Free Software Foundation, Inc.
>     This is free software; see the source for copying conditions.
>     There is NO
>     warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
>     PURPOSE.
>
>     $ which clang
>     /usr/bin/clang
>     $ clang --version
>     Apple clang version 2.1 (tags/Apple/clang-163.7.1) (based on LLVM
>     3.0svn)
>     Target: x86_64-apple-darwin10.8.0
>     Thread model: posix
>
>
>
>
>
>     On 17/07/2017 01:01, Alan Third wrote:
>
>         On Sun, Jul 16, 2017 at 08:43:25PM +0200, Anders Lindgren wrote:
>
>             I tried to figure out if gcc or clang was used when
>             building on 10.6.8, but
>             ran out of time (and won't have the chance to do it again
>             anytime soon). I
>             did conclude that it comes with both a real "gcc" and a
>             real "clang", so
>             presumably gcc is used. (Surprisingly, more modern
>             versions of macOS seems
>             to map the command "gcc" to "clang".)
>
>         Charles can maybe answer this for us, as he uses 10.6.
>
>             A warning-free build is a must on modern system (which use
>             clang). It would
>             be nice on older system, I guess, but it would be hard to
>             enforce. (We
>             could even lobby to add the option to future gcc versions,
>             for the benefit
>             of GNUStep, but it would not help the situation on older
>             macOS versions.)
>             My gut feeling is to go with the
>             NS_SILENCE_MISSING_METHOD_WARNING_BEGIN
>             solution, as it work on modern macOS systems, it retains
>             type checking, and
>             it give us a single location to describe the situation and
>             to modify the
>             macro, if there should be a need for it in the future.
>
>         Agreed.
>
>         I think that a lot (if not most) of the code that we might
>         want to use
>         this on won’t work on GNUstep at all, so we could still exclude it
>         from building there if the warnings are too much.
>
>
>


[-- Attachment #2: Type: text/html, Size: 5479 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-18 18:33                               ` Charles A. Roelli
@ 2017-07-18 22:16                                 ` Alan Third
  2017-07-19  4:57                                   ` Charles A. Roelli
  0 siblings, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-07-18 22:16 UTC (permalink / raw)
  To: Charles A. Roelli
  Cc: Paul Michael Reilly, Jean-Christophe Helary, Anders Lindgren,
	Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 408 bytes --]

On Tue, Jul 18, 2017 at 08:33:05PM +0200, Charles A. Roelli wrote:
> It does seem to be gcc:
> 
> (snippet from output of 'make V=1' when compiling src/nsterm.m):
> gcc -std=gnu99 -c -Demacs -I. -I. -I../lib -I../lib -D_REENTRANT -I
> [...]  -MMD -MF deps/nsterm.d -MP -O0 -g3 nsterm.m

Can you check whether the attached patch results in a compilation
warning from nsterm.m for you, please?

-- 
Alan Third

[-- Attachment #2: 0001-Use-a-run-time-check-for-macOS-Sierra-tabbing-suppor.patch --]
[-- Type: text/plain, Size: 3081 bytes --]

From dd73f985107937ee1c593f0878bdd5b01178af1c Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Thu, 6 Jul 2017 23:10:49 +0100
Subject: [PATCH] Use a run-time check for macOS Sierra tabbing support

* src/nsterm.m (initFrameFromEmacs) [NS_IMPL_COCOA]: Switch from
compile-time check to run-time.
* src/nsterm.h (NSWindowTabbingMode): Define in pre-Sierra macOS.
(NS_SILENCE_MISSING_METHOD_WARNING_BEGIN,
NS_SILENCE_MISSING_METHOD_WARNING_BEGIN_END): Add #defines for
silencing known missing method warnings.
---
 src/nsterm.h | 33 +++++++++++++++++++++++++++++++++
 src/nsterm.m |  8 +++++---
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/src/nsterm.h b/src/nsterm.h
index 0f1b36db7b..c5b4b17eed 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -1317,6 +1317,39 @@ extern char gnustep_base_version[];  /* version tracking */
 #ifdef __OBJC__
 typedef NSUInteger NSWindowStyleMask;
 #endif
+
+/* Window tabbing mode enums are new too. */
+enum NSWindowTabbingMode
+  {
+    NSWindowTabbingModeAutomatic,
+    NSWindowTabbingModePreferred,
+    NSWindowTabbingModeDisallowed
+  };
+#endif
+
+/* Surround code with the following macros to silence a missing method
+ * warning from the compiler.  This is useful when testing whether a
+ * method is available on an object, and then calling it if so.
+ *
+ *       if ([obj respondsToSelector: @selector(methodName)])
+ *         [obj methodName];
+ *
+ * Where obj doesn't have a selector methodName, the compiler will
+ * issue a warning.  Sometimes we want to build against libraries
+ * where we don't know if the method exists, for example when a new
+ * version of Cocoa adds or removes a method.
+ */
+#if defined (__GNUC__) && !defined (__clang__)
+/* gcc doesn't seem to have any way to silence this warning. */
+#define NS_SILENCE_MISSING_METHOD_WARNING_BEGIN
+#define NS_SILENCE_MISSING_METHOD_WARNING_BEGIN_END
+
+#elif defined (__clang__)
+#define NS_SILENCE_MISSING_METHOD_WARNING_BEGIN                   \
+  _Pragma ("clang diagnostic push")                               \
+  _Pragma ("clang diagnostic ignored \"-Wobjc-method-access\"")
+#define NS_SILENCE_MISSING_METHOD_WARNING_BEGIN_END     \
+  _Pragma ("clang diagnostic pop")
 #endif
 
 #endif	/* HAVE_NS */
diff --git a/src/nsterm.m b/src/nsterm.m
index a3c7031331..e1ce05b978 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -7091,9 +7091,11 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   /* macOS Sierra automatically enables tabbed windows.  We can't
      allow this to be enabled until it's available on a Free system.
      Currently it only happens by accident and is buggy anyway. */
-#if defined (NS_IMPL_COCOA) && \
-  MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_12
-  [win setTabbingMode: NSWindowTabbingModeDisallowed];
+#ifdef NS_IMPL_COCOA
+  NS_SILENCE_MISSING_METHOD_WARNING_BEGIN
+  if ([win respondsToSelector: @selector(setTabbingMode:)])
+    [win setTabbingMode: NSWindowTabbingModeDisallowed];
+  NS_SILENCE_MISSING_METHOD_WARNING_BEGIN_END
 #endif
 
   ns_window_num++;
-- 
2.12.0


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-18 22:16                                 ` Alan Third
@ 2017-07-19  4:57                                   ` Charles A. Roelli
  2017-07-21 20:31                                     ` Anders Lindgren
  0 siblings, 1 reply; 63+ messages in thread
From: Charles A. Roelli @ 2017-07-19  4:57 UTC (permalink / raw)
  To: Alan Third
  Cc: Paul Michael Reilly, Jean-Christophe Helary, Anders Lindgren,
	Emacs-Devel devel

Yes:

   CC       nsterm.o
nsterm.m: In function ‘-[EmacsView initFrameFromEmacs:]’:
nsterm.m:7079: warning: ‘NSWindow’ may not respond to ‘-setTabbingMode:’
nsterm.m:7079: warning: (Messages without a matching method signature
nsterm.m:7079: warning: will be assumed to return ‘id’ and accept
nsterm.m:7079: warning: ‘...’ as arguments.)



On 19/07/2017 00:16, Alan Third wrote:
> On Tue, Jul 18, 2017 at 08:33:05PM +0200, Charles A. Roelli wrote:
>> It does seem to be gcc:
>>
>> (snippet from output of 'make V=1' when compiling src/nsterm.m):
>> gcc -std=gnu99 -c -Demacs -I. -I. -I../lib -I../lib -D_REENTRANT -I
>> [...]  -MMD -MF deps/nsterm.d -MP -O0 -g3 nsterm.m
> Can you check whether the attached patch results in a compilation
> warning from nsterm.m for you, please?
>




^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-19  4:57                                   ` Charles A. Roelli
@ 2017-07-21 20:31                                     ` Anders Lindgren
  2017-07-22 11:22                                       ` Alan Third
  0 siblings, 1 reply; 63+ messages in thread
From: Anders Lindgren @ 2017-07-21 20:31 UTC (permalink / raw)
  To: Charles A. Roelli
  Cc: Paul Michael Reilly, Alan Third, Jean-Christophe Helary,
	Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 1902 bytes --]

Hi!

I just gave this matter another think-through. I don't think we ever will
be able to build an Emacs on an old system like 10.6.8 that will be able to
use all the bells and whistles when executed on a new system. However, the
other way around could, at some point in time, be possible.

In other words, one solution would be something like:

    #ifdef NS_IMPL_COCOA
    #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_12
    if ([win respondsToSelector: @selector(setTabbingMode)])
      [win setTabbingMode: NSWindowTabbingModeDisallowed];
    #endif
    #endif

This should not generate any warning in any context (gcc or clang, macOS or
GNUStep). The resulting code will run correctly on the system it was built
for, and it will run correctly on older systems. The only thing that
doesn't work is when Emacs is built on an old system, features provided by
newer OS versions aren't included.

    -- Anders

On Wed, Jul 19, 2017 at 6:57 AM, Charles A. Roelli <charles@aurox.ch> wrote:

> Yes:
>
>   CC       nsterm.o
> nsterm.m: In function ‘-[EmacsView initFrameFromEmacs:]’:
> nsterm.m:7079: warning: ‘NSWindow’ may not respond to ‘-setTabbingMode:’
> nsterm.m:7079: warning: (Messages without a matching method signature
> nsterm.m:7079: warning: will be assumed to return ‘id’ and accept
> nsterm.m:7079: warning: ‘...’ as arguments.)
>
>
>
>
> On 19/07/2017 00:16, Alan Third wrote:
>
>> On Tue, Jul 18, 2017 at 08:33:05PM +0200, Charles A. Roelli wrote:
>>
>>> It does seem to be gcc:
>>>
>>> (snippet from output of 'make V=1' when compiling src/nsterm.m):
>>> gcc -std=gnu99 -c -Demacs -I. -I. -I../lib -I../lib -D_REENTRANT -I
>>> [...]  -MMD -MF deps/nsterm.d -MP -O0 -g3 nsterm.m
>>>
>> Can you check whether the attached patch results in a compilation
>> warning from nsterm.m for you, please?
>>
>>
>

[-- Attachment #2: Type: text/html, Size: 3009 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-21 20:31                                     ` Anders Lindgren
@ 2017-07-22 11:22                                       ` Alan Third
  2017-07-23 12:17                                         ` NS runtime feature detection (was: Mac OS Sierra tab feature breaks C-x 5 2) Alan Third
  2017-07-23 22:35                                         ` Mac OS Sierra tab feature breaks C-x 5 2 Tim Cross
  0 siblings, 2 replies; 63+ messages in thread
From: Alan Third @ 2017-07-22 11:22 UTC (permalink / raw)
  To: Anders Lindgren
  Cc: Paul Michael Reilly, Jean-Christophe Helary, Charles A. Roelli,
	Emacs-Devel devel

On Fri, Jul 21, 2017 at 10:31:27PM +0200, Anders Lindgren wrote:
> Hi!
> 
> I just gave this matter another think-through. I don't think we ever will
> be able to build an Emacs on an old system like 10.6.8 that will be able to
> use all the bells and whistles when executed on a new system. However, the
> other way around could, at some point in time, be possible.
> 
> In other words, one solution would be something like:
> 
>     #ifdef NS_IMPL_COCOA
>     #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_12
>     if ([win respondsToSelector: @selector(setTabbingMode)])
>       [win setTabbingMode: NSWindowTabbingModeDisallowed];
>     #endif
>     #endif
> 
> This should not generate any warning in any context (gcc or clang, macOS or
> GNUStep). The resulting code will run correctly on the system it was built
> for, and it will run correctly on older systems. The only thing that
> doesn't work is when Emacs is built on an old system, features provided by
> newer OS versions aren't included.

Unfortunately it doesn’t fix our immediate issue, which is that
setTabbingMode should be called on 10.12, but emacsformacosx.com
builds on 10.9.

I think you’re definitely right, though, that we’re not going to be
able to come up with a uniform solution. Something has to give
somewhere, and I’m tempted to say it should be 10.6. We could live
with build warnings on 10.6, or just say that code like the above
should be ifdef’d out on 10.6.

Or perhaps we provide a flag that enables a universal binary build
that doesn’t bother about hiding the warnings?

I suppose that would look like:

    ./configure --with-ns --universal-binary

    #ifdef NS_IMPL_COCOA
    #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_12 \
        || defined (UNIVERSAL_BINARY)
    if ([win respondsToSelector: @selector(setTabbingMode)])
      [win setTabbingMode: NSWindowTabbingModeDisallowed];
    #endif
    #endif

-- 
Alan Third



^ permalink raw reply	[flat|nested] 63+ messages in thread

* NS runtime feature detection (was: Mac OS Sierra tab feature breaks C-x 5 2)
  2017-07-22 11:22                                       ` Alan Third
@ 2017-07-23 12:17                                         ` Alan Third
  2017-07-24 19:02                                           ` NS runtime feature detection Charles A. Roelli
  2017-07-23 22:35                                         ` Mac OS Sierra tab feature breaks C-x 5 2 Tim Cross
  1 sibling, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-07-23 12:17 UTC (permalink / raw)
  To: Anders Lindgren
  Cc: Paul Michael Reilly, Jean-Christophe Helary, Charles A. Roelli,
	Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 728 bytes --]

On Sat, Jul 22, 2017 at 12:22:30PM +0100, Alan Third wrote:
> Or perhaps we provide a flag that enables a universal binary build
> that doesn’t bother about hiding the warnings?

Attached is a first attempt at this in nsterm.m. I think I’ve got
everything, but there may be some new variables and things defined
that I’ve missed which will throw up errors in older macOS versions.

It seems ‘universal binary’ means something specific, so I went with a
different name.

Use:

    ./configure --with-ns --enable-macos-runtime-feature-detection

It will give a lot of deprecation and unknown method warnings, but
hopefully that’s all.

If you build normally there should be no (new) warnings or errors.
-- 
Alan Third

[-- Attachment #2: 0001-Use-a-run-time-check-for-macOS-Sierra-tabbing-suppor.patch --]
[-- Type: text/plain, Size: 11489 bytes --]

From 5e2c7950c404de6be42f257adf927d71ca59507d Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Thu, 6 Jul 2017 23:10:49 +0100
Subject: [PATCH] Use a run-time check for macOS Sierra tabbing support

* configure.ac: Add --enable-macos-runtime-feature-detection flag.
* src/nsterm.h (NSWindowTabbingMode): Define in pre-Sierra macOS.
(MACOS_MIN_VERSION, MACOS_MAX_VERSION): Add new defines for version
detection.
* src/nsterm.m (colorForEmacsRed):
(colorUsingDefaultColorSpace):
(runAlertPanel):
(firstRectForCharacterRange):
(initFrameFromEmacs):
(windowDidEnterFullScreen):
(toggleFullScreen):
(constrainFrameRect):
(scrollerWidth): Switch from compile-time to run-time checks.
---
 configure.ac |   6 ++++
 src/nsterm.h |  16 ++++++++-
 src/nsterm.m | 113 ++++++++++++++++++++++++++++++++++++-----------------------
 3 files changed, 91 insertions(+), 44 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5e6dbda2b6..09ce9a851a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -426,6 +426,12 @@ AC_DEFUN
    EN_NS_SELF_CONTAINED=$enableval,
    EN_NS_SELF_CONTAINED=yes)
 
+AC_ARG_ENABLE(macos-runtime-feature-detection,
+[AS_HELP_STRING([--enable-macos-runtime-feature-detection],
+                [bypass compiler checks of macOS versions])],
+   AC_DEFINE(NS_RUNTIME_CHECKS, 1,
+             [Perform runtime checks of macOS features.]))
+
 locallisppathset=no
 AC_ARG_ENABLE(locallisppath,
 [AS_HELP_STRING([--enable-locallisppath=PATH],
diff --git a/src/nsterm.h b/src/nsterm.h
index 0f1b36db7b..9578591d9d 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -49,6 +49,13 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #endif /* NS_IMPL_COCOA */
 
+#define MACOS_MIN_VERSION(min) (defined (NS_IMPL_COCOA)                 \
+                                && (MAC_OS_X_VERSION_MAX_ALLOWED >= min \
+                                    || defined (NS_RUNTIME_CHECKS)))
+#define MACOS_MAX_VERSION(max) (defined (NS_IMPL_COCOA)                 \
+                                && (MAC_OS_X_VERSION_MAX_ALLOWED <= max \
+                                    || defined (NS_RUNTIME_CHECKS)))
+
 #ifdef __OBJC__
 
 /* CGFloat on GNUstep may be 4 or 8 byte, but functions expect float* for some
@@ -1317,6 +1324,13 @@ extern char gnustep_base_version[];  /* version tracking */
 #ifdef __OBJC__
 typedef NSUInteger NSWindowStyleMask;
 #endif
-#endif
 
+/* Window tabbing mode enums are new too. */
+enum NSWindowTabbingMode
+  {
+    NSWindowTabbingModeAutomatic,
+    NSWindowTabbingModePreferred,
+    NSWindowTabbingModeDisallowed
+  };
+#endif
 #endif	/* HAVE_NS */
diff --git a/src/nsterm.m b/src/nsterm.m
index a3c7031331..e57493afd4 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -136,15 +136,15 @@ @implementation NSColor (EmacsColor)
 + (NSColor *)colorForEmacsRed:(CGFloat)red green:(CGFloat)green
                          blue:(CGFloat)blue alpha:(CGFloat)alpha
 {
-#ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-  if (ns_use_srgb_colorspace)
+#if MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_7)
+  if (ns_use_srgb_colorspace
+      && [NSColor respondsToSelector:
+                    @selector(colorWithSRGBRed:green:blue:alpha:)])
       return [NSColor colorWithSRGBRed: red
                                  green: green
                                   blue: blue
                                  alpha: alpha];
 #endif
-#endif
   return [NSColor colorWithCalibratedRed: red
                                    green: green
                                     blue: blue
@@ -153,12 +153,15 @@ + (NSColor *)colorForEmacsRed:(CGFloat)red green:(CGFloat)green
 
 - (NSColor *)colorUsingDefaultColorSpace
 {
-#ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-  if (ns_use_srgb_colorspace)
+#if MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_7)
+  /* FIXMES: We're checking for colorWithSRGBRed here so this will
+     only work in the same place as in the method above.  It should
+     really be a check whether we're on macOS 10.7 or above. */
+  if (ns_use_srgb_colorspace
+      && [NSColor respondsToSelector:
+                    @selector(colorWithSRGBRed:green:blue:alpha:)])
     return [self colorUsingColorSpace: [NSColorSpace sRGBColorSpace]];
 #endif
-#endif
   return [self colorUsingColorSpaceName: NSCalibratedRGBColorSpace];
 }
 
@@ -5550,8 +5553,7 @@ - (void) terminate: (id)sender
               NSString *defaultButton,
               NSString *alternateButton)
 {
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
+#ifdef NS_IMPL_GNUSTEP
   return NSRunAlertPanel(title, msgFormat, defaultButton, alternateButton, nil)
     == NSAlertDefaultReturn;
 #else
@@ -6312,14 +6314,28 @@ - (NSRect)firstRectForCharacterRange: (NSRange)theRange
                                        +FRAME_LINE_HEIGHT (emacsframe));
 
   pt = [self convertPoint: pt toView: nil];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
-  pt = [[self window] convertBaseToScreen: pt];
-  rect.origin = pt;
-#else
-  rect.origin = pt;
-  rect = [[self window] convertRectToScreen: rect];
+
+#ifdef NS_RUNTIME_CHECKS
+  if ([[self window] respondsToSelector: @selector(convertRectToScreen:)])
+    {
+#endif
+#if MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_7)
+      rect.origin = pt;
+      rect = [[self window] convertRectToScreen: rect];
+#endif
+#ifdef NS_RUNTIME_CHECKS
+    }
+  else
+    {
+#endif
+#if MACOS_MAX_VERSION (MAC_OS_X_VERSION_10_6) || defined (NS_IMPL_GNUSTEP)
+      pt = [[self window] convertBaseToScreen: pt];
+      rect.origin = pt;
+#endif
+#ifdef NS_RUNTIME_CHECKS
+    }
 #endif
+
   return rect;
 }
 
@@ -7019,9 +7035,9 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 
   [win setAcceptsMouseMovedEvents: YES];
   [win setDelegate: self];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-  [win useOptimizedDrawing: YES];
+#if !defined (NS_IMPL_COCOA) || MACOS_MAX_VERSION (MAC_OS_X_VERSION_10_9)
+  if ([win respondsToSelector: @selector(useOptimizedDrawing:)])
+    [win useOptimizedDrawing: YES];
 #endif
 
   [[win contentView] addSubview: self];
@@ -7081,19 +7097,19 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   if ([col alphaComponent] != (EmacsCGFloat) 1.0)
     [win setOpaque: NO];
 
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-  [self allocateGState];
+#if !defined (NS_IMPL_COCOA) || MACOS_MAX_VERSION (MAC_OS_X_VERSION_10_9)
+  if ([self respondsToSelector: @selector(allocateGState)])
+    [self allocateGState];
 #endif
   [NSApp registerServicesMenuSendTypes: ns_send_types
                            returnTypes: [NSArray array]];
 
+#if MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_12)
   /* macOS Sierra automatically enables tabbed windows.  We can't
      allow this to be enabled until it's available on a Free system.
      Currently it only happens by accident and is buggy anyway. */
-#if defined (NS_IMPL_COCOA) && \
-  MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_12
-  [win setTabbingMode: NSWindowTabbingModeDisallowed];
+  if ([win respondsToSelector: @selector(setTabbingMode:)])
+    [win setTabbingMode: NSWindowTabbingModeDisallowed];
 #endif
 
   ns_window_num++;
@@ -7349,7 +7365,11 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
     {
       BOOL tbar_visible = FRAME_EXTERNAL_TOOL_BAR (emacsframe) ? YES : NO;
 #ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
+#if MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_6
+      /* These two values are only defined in 10.7 and above. */
+      int NSApplicationPresentationFullScreen = (1 << 10);
+      int NSApplicationPresentationAutoHideToolbar = (1 << 11);
+#endif
       unsigned val = (unsigned)[NSApp presentationOptions];
 
       // Mac OS X 10.7 bug fix, the menu won't appear without this.
@@ -7365,7 +7385,6 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
           [NSApp setPresentationOptions: options];
         }
 #endif
-#endif
       [toolbar setVisible:tbar_visible];
     }
 }
@@ -7499,10 +7518,10 @@ - (void)toggleFullScreen: (id)sender
     {
       NSScreen *screen = [w screen];
 
-#if defined (NS_IMPL_COCOA) && \
-  MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
+#if MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_9)
       /* Hide ghost menu bar on secondary monitor? */
-      if (! onFirstScreen)
+      if (! onFirstScreen
+          && [NSScreen respondsToSelector: @selector(screensHaveSeparateSpaces)])
         onFirstScreen = [NSScreen screensHaveSeparateSpaces];
 #endif
       /* Hide dock and menubar if we are on the primary screen.  */
@@ -7530,9 +7549,9 @@ - (void)toggleFullScreen: (id)sender
       [fw setTitle:[w title]];
       [fw setDelegate:self];
       [fw setAcceptsMouseMovedEvents: YES];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-      [fw useOptimizedDrawing: YES];
+#if !defined (NS_IMPL_COCOA) || MACOS_MAX_VERSION (MAC_OS_X_VERSION_10_9)
+      if ([fw respondsToSelector: @selector(useOptimizedDrawing:)])
+        [fw useOptimizedDrawing: YES];
 #endif
       [fw setBackgroundColor: col];
       if ([col alphaComponent] != (EmacsCGFloat) 1.0)
@@ -8093,10 +8112,11 @@ - (NSRect)constrainFrameRect:(NSRect)frameRect toScreen:(NSScreen *)screen
              NSTRACE_ARG_RECT (frameRect));
 
 #ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
+#if MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_9)
   // If separate spaces is on, it is like each screen is independent.  There is
   // no spanning of frames across screens.
-  if ([NSScreen screensHaveSeparateSpaces])
+  if ([NSScreen respondsToSelector: @selector(screensHaveSeparateSpaces)]
+      && [NSScreen screensHaveSeparateSpaces])
     {
       NSTRACE_MSG ("Screens have separate spaces");
       frameRect = [super constrainFrameRect:frameRect toScreen:screen];
@@ -8104,7 +8124,7 @@ - (NSRect)constrainFrameRect:(NSRect)frameRect toScreen:(NSScreen *)screen
       return frameRect;
     }
   else
-#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9 */
+#endif /* MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_9) */
     // Check that the proposed frameRect is visible in at least one
     // screen.  If it is not, ask the system to reposition it (only
     // for non-child windows).
@@ -8310,12 +8330,19 @@ + (CGFloat) scrollerWidth
   /* TODO: if we want to allow variable widths, this is the place to do it,
            however neither GNUstep nor Cocoa support it very well */
   CGFloat r;
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
-  r = [NSScroller scrollerWidth];
-#else
-  r = [NSScroller scrollerWidthForControlSize: NSControlSizeRegular
-                                scrollerStyle: NSScrollerStyleLegacy];
+#ifdef NS_RUNTIME_CHECKS
+  if ([NSScroller respondsToSelector:
+                    @selector(scrollerWidthForControlSize:scrollerStyle:)])
+#endif
+#if MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_7)
+    r = [NSScroller scrollerWidthForControlSize: NSControlSizeRegular
+                                  scrollerStyle: NSScrollerStyleLegacy];
+#endif
+#ifdef NS_RUNTIME_CHECKS
+  else
+#endif
+#if MACOS_MAX_VERSION (MAC_OS_X_VERSION_10_6)
+    r = [NSScroller scrollerWidth];
 #endif
   return r;
 }
-- 
2.12.0


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* Re: Mac OS Sierra tab feature breaks C-x 5 2
  2017-07-22 11:22                                       ` Alan Third
  2017-07-23 12:17                                         ` NS runtime feature detection (was: Mac OS Sierra tab feature breaks C-x 5 2) Alan Third
@ 2017-07-23 22:35                                         ` Tim Cross
  1 sibling, 0 replies; 63+ messages in thread
From: Tim Cross @ 2017-07-23 22:35 UTC (permalink / raw)
  To: Alan Third
  Cc: Paul Michael Reilly, Jean-Christophe Helary, Charles A. Roelli,
	Anders Lindgren, Emacs-Devel devel

[-- Attachment #1: Type: text/plain, Size: 2575 bytes --]

I think we should not put too much effort into avoiding warning on 10.6.
This is a very old version, does not receive security fixes and for the
protection of users, should not be encouraging use of 10.6. IOW if your
running 10.6, expect you will see warnings for things which have been added
since and which are not supported on that platform.

Tim

On 22 July 2017 at 21:22, Alan Third <alan@idiocy.org> wrote:

> On Fri, Jul 21, 2017 at 10:31:27PM +0200, Anders Lindgren wrote:
> > Hi!
> >
> > I just gave this matter another think-through. I don't think we ever will
> > be able to build an Emacs on an old system like 10.6.8 that will be able
> to
> > use all the bells and whistles when executed on a new system. However,
> the
> > other way around could, at some point in time, be possible.
> >
> > In other words, one solution would be something like:
> >
> >     #ifdef NS_IMPL_COCOA
> >     #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_12
> >     if ([win respondsToSelector: @selector(setTabbingMode)])
> >       [win setTabbingMode: NSWindowTabbingModeDisallowed];
> >     #endif
> >     #endif
> >
> > This should not generate any warning in any context (gcc or clang, macOS
> or
> > GNUStep). The resulting code will run correctly on the system it was
> built
> > for, and it will run correctly on older systems. The only thing that
> > doesn't work is when Emacs is built on an old system, features provided
> by
> > newer OS versions aren't included.
>
> Unfortunately it doesn’t fix our immediate issue, which is that
> setTabbingMode should be called on 10.12, but emacsformacosx.com
> builds on 10.9.
>
> I think you’re definitely right, though, that we’re not going to be
> able to come up with a uniform solution. Something has to give
> somewhere, and I’m tempted to say it should be 10.6. We could live
> with build warnings on 10.6, or just say that code like the above
> should be ifdef’d out on 10.6.
>
> Or perhaps we provide a flag that enables a universal binary build
> that doesn’t bother about hiding the warnings?
>
> I suppose that would look like:
>
>     ./configure --with-ns --universal-binary
>
>     #ifdef NS_IMPL_COCOA
>     #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_12 \
>         || defined (UNIVERSAL_BINARY)
>     if ([win respondsToSelector: @selector(setTabbingMode)])
>       [win setTabbingMode: NSWindowTabbingModeDisallowed];
>     #endif
>     #endif
>
> --
> Alan Third
>
>


-- 
regards,

Tim

--
Tim Cross

[-- Attachment #2: Type: text/html, Size: 3492 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: NS runtime feature detection
  2017-07-23 12:17                                         ` NS runtime feature detection (was: Mac OS Sierra tab feature breaks C-x 5 2) Alan Third
@ 2017-07-24 19:02                                           ` Charles A. Roelli
  2017-07-24 20:44                                             ` bug#27810: " Alan Third
  2017-07-24 20:45                                             ` Alan Third
  0 siblings, 2 replies; 63+ messages in thread
From: Charles A. Roelli @ 2017-07-24 19:02 UTC (permalink / raw)
  To: Alan Third, Anders Lindgren
  Cc: Paul Michael Reilly, Jean-Christophe Helary, Emacs-Devel devel

Thanks for taking the initiative with this.  Maybe it's time to open a bug?


When I try to compile with the patch applied, with this block:

@@ -7349,7 +7365,11 @@ - (void)windowDidEnterFullScreen /* provided for 
direct calls */
      {
        BOOL tbar_visible = FRAME_EXTERNAL_TOOL_BAR (emacsframe) ? YES : NO;
  #ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
+#if MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_6
+      /* These two values are only defined in 10.7 and above. */
+      int NSApplicationPresentationFullScreen = (1 << 10);
+      int NSApplicationPresentationAutoHideToolbar = (1 << 11);
+#endif
        unsigned val = (unsigned)[NSApp presentationOptions];

while building with:

./configure --with-ns --enable-macos-runtime-feature-detection && make

I got these errors:

nsterm.m: In function ‘-[EmacsView windowDidEnterFullScreen]’:
nsterm.m:7395: error: ‘NSApplicationPresentationFullScreen’ undeclared 
(first use in this function)
nsterm.m:7395: error: (Each undeclared identifier is reported only once
nsterm.m:7395: error: for each function it appears in.)
nsterm.m:7396: error: ‘NSApplicationPresentationAutoHideToolbar’ 
undeclared (first use in this function)

(line numbers are off due to an intervening commit)

I'm confused why the macro call you wrote doesn't prevent this. But
when I change it to #if MAC_OS_X_VERSION_MIN_ALLOWED <=
MAC_OS_X_VERSION_10_6, then it compiles.  This min/max stuff always
confuses me...



On 23/07/2017 14:17, Alan Third wrote:
> On Sat, Jul 22, 2017 at 12:22:30PM +0100, Alan Third wrote:
>> Or perhaps we provide a flag that enables a universal binary build
>> that doesn’t bother about hiding the warnings?
> Attached is a first attempt at this in nsterm.m. I think I’ve got
> everything, but there may be some new variables and things defined
> that I’ve missed which will throw up errors in older macOS versions.
>
> It seems ‘universal binary’ means something specific, so I went with a
> different name.
>
> Use:
>
>      ./configure --with-ns --enable-macos-runtime-feature-detection
>
> It will give a lot of deprecation and unknown method warnings, but
> hopefully that’s all.
>
> If you build normally there should be no (new) warnings or errors.




^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: macOS runtime feature detection
@ 2017-07-24 20:22 Alan Third
  2017-07-26  2:59 ` Richard Stallman
  0 siblings, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-07-24 20:22 UTC (permalink / raw)
  To: 27810

We should be able to build a single binary that can detect new OS
features at runtime rather than depend on compile‐time macros.

-- 
Alan Third





^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-07-24 19:02                                           ` NS runtime feature detection Charles A. Roelli
@ 2017-07-24 20:44                                             ` Alan Third
  2017-07-24 20:53                                               ` Glenn Morris
  2017-07-26 21:57                                               ` Alan Third
  2017-07-24 20:45                                             ` Alan Third
  1 sibling, 2 replies; 63+ messages in thread
From: Alan Third @ 2017-07-24 20:44 UTC (permalink / raw)
  To: Charles A. Roelli; +Cc: Anders Lindgren, 27810

[-- Attachment #1: Type: text/plain, Size: 1410 bytes --]

I wrote a big reply to this but seem to have lost it.

On Mon, Jul 24, 2017 at 09:02:57PM +0200, Charles A. Roelli wrote:
> nsterm.m: In function ‘-[EmacsView windowDidEnterFullScreen]’:
> nsterm.m:7395: error: ‘NSApplicationPresentationFullScreen’ undeclared
> (first use in this function)
> nsterm.m:7395: error: (Each undeclared identifier is reported only once
> nsterm.m:7395: error: for each function it appears in.)
> nsterm.m:7396: error: ‘NSApplicationPresentationAutoHideToolbar’ undeclared
> (first use in this function)

I think perhaps we need to just test ‘< 10.7’. I’ve attached a new
patch that deals with that and some other bits and pieces.

> I'm confused why the macro call you wrote doesn't prevent this. But
> when I change it to #if MAC_OS_X_VERSION_MIN_ALLOWED <=
> MAC_OS_X_VERSION_10_6, then it compiles.  This min/max stuff always
> confuses me...

I’m unclear where we should be using MIN_REQUIRED vs MAX_ALLOWED, but
I think we’re OK with MAX everywhere for what we’re doing. I’ve just
used MAX in both my new macros.

We need a proper runtime OS version check in a few places. I think
there are two ways of doing this depending on which OS version you’re
building on. It’s getting very close to circular. ;)

macfont.m looks like it could be a small nightmare as it has a LOT of
version dependent code.

Thanks for testing this patch.
-- 
Alan Third

[-- Attachment #2: 0001-Use-a-run-time-check-for-macOS-Sierra-tabbing-suppor.patch --]
[-- Type: text/plain, Size: 12355 bytes --]

From b26a3594f62544f3942e2eaceb4a1b1b92a2166d Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Thu, 6 Jul 2017 23:10:49 +0100
Subject: [PATCH] Use a run-time check for macOS Sierra tabbing support

* configure.ac: Add --enable-macos-runtime-feature-detection flag.
* src/nsterm.h (NSWindowTabbingMode): Define in pre-Sierra macOS.
(MACOS_MIN_VERSION, MACOS_MAX_VERSION): Add new defines for version
detection.
* src/nsterm.m (colorForEmacsRed):
(colorUsingDefaultColorSpace):
(runAlertPanel):
(firstRectForCharacterRange):
(initFrameFromEmacs):
(windowDidEnterFullScreen):
(toggleFullScreen):
(constrainFrameRect):
(scrollerWidth): Allow run-time checks.
* src/nsfns.m (ns_screen_name): Allow run-time checks.
---
 configure.ac |   6 ++++
 src/nsfns.m  |   7 ++--
 src/nsterm.h |  16 ++++++++-
 src/nsterm.m | 113 ++++++++++++++++++++++++++++++++++++-----------------------
 4 files changed, 96 insertions(+), 46 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5e6dbda2b6..09ce9a851a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -426,6 +426,12 @@ AC_DEFUN
    EN_NS_SELF_CONTAINED=$enableval,
    EN_NS_SELF_CONTAINED=yes)
 
+AC_ARG_ENABLE(macos-runtime-feature-detection,
+[AS_HELP_STRING([--enable-macos-runtime-feature-detection],
+                [bypass compiler checks of macOS versions])],
+   AC_DEFINE(NS_RUNTIME_CHECKS, 1,
+             [Perform runtime checks of macOS features.]))
+
 locallisppathset=no
 AC_ARG_ENABLE(locallisppath,
 [AS_HELP_STRING([--enable-locallisppath=PATH],
diff --git a/src/nsfns.m b/src/nsfns.m
index 36748cebb8..c5ff65806a 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -2512,12 +2512,15 @@ and GNUstep implementations ("distributor-specific release
 {
   char *name = NULL;
 
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
+#if MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_9)
   mach_port_t masterPort;
   io_iterator_t it;
   io_object_t obj;
 
-  // CGDisplayIOServicePort is deprecated.  Do it another (harder) way.
+  /* CGDisplayIOServicePort is deprecated.  Do it another (harder) way.
+
+     Is this code OK for macOS < 10.9, and GNUstep?  I suspect it is,
+     in which case is it worth keeping the other method in here? */
 
   if (IOMasterPort (MACH_PORT_NULL, &masterPort) != kIOReturnSuccess
       || IOServiceGetMatchingServices (masterPort,
diff --git a/src/nsterm.h b/src/nsterm.h
index 0f1b36db7b..9578591d9d 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -49,6 +49,13 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #endif /* NS_IMPL_COCOA */
 
+#define MACOS_MIN_VERSION(min) (defined (NS_IMPL_COCOA)                 \
+                                && (MAC_OS_X_VERSION_MAX_ALLOWED >= min \
+                                    || defined (NS_RUNTIME_CHECKS)))
+#define MACOS_MAX_VERSION(max) (defined (NS_IMPL_COCOA)                 \
+                                && (MAC_OS_X_VERSION_MAX_ALLOWED <= max \
+                                    || defined (NS_RUNTIME_CHECKS)))
+
 #ifdef __OBJC__
 
 /* CGFloat on GNUstep may be 4 or 8 byte, but functions expect float* for some
@@ -1317,6 +1324,13 @@ extern char gnustep_base_version[];  /* version tracking */
 #ifdef __OBJC__
 typedef NSUInteger NSWindowStyleMask;
 #endif
-#endif
 
+/* Window tabbing mode enums are new too. */
+enum NSWindowTabbingMode
+  {
+    NSWindowTabbingModeAutomatic,
+    NSWindowTabbingModePreferred,
+    NSWindowTabbingModeDisallowed
+  };
+#endif
 #endif	/* HAVE_NS */
diff --git a/src/nsterm.m b/src/nsterm.m
index a3c7031331..07ec899583 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -136,15 +136,15 @@ @implementation NSColor (EmacsColor)
 + (NSColor *)colorForEmacsRed:(CGFloat)red green:(CGFloat)green
                          blue:(CGFloat)blue alpha:(CGFloat)alpha
 {
-#ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-  if (ns_use_srgb_colorspace)
+#if MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_7)
+  if (ns_use_srgb_colorspace
+      && [NSColor respondsToSelector:
+                    @selector(colorWithSRGBRed:green:blue:alpha:)])
       return [NSColor colorWithSRGBRed: red
                                  green: green
                                   blue: blue
                                  alpha: alpha];
 #endif
-#endif
   return [NSColor colorWithCalibratedRed: red
                                    green: green
                                     blue: blue
@@ -153,12 +153,15 @@ + (NSColor *)colorForEmacsRed:(CGFloat)red green:(CGFloat)green
 
 - (NSColor *)colorUsingDefaultColorSpace
 {
-#ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-  if (ns_use_srgb_colorspace)
+#if MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_7)
+  /* FIXMES: We're checking for colorWithSRGBRed here so this will
+     only work in the same place as in the method above.  It should
+     really be a check whether we're on macOS 10.7 or above. */
+  if (ns_use_srgb_colorspace
+      && [NSColor respondsToSelector:
+                    @selector(colorWithSRGBRed:green:blue:alpha:)])
     return [self colorUsingColorSpace: [NSColorSpace sRGBColorSpace]];
 #endif
-#endif
   return [self colorUsingColorSpaceName: NSCalibratedRGBColorSpace];
 }
 
@@ -5550,8 +5553,7 @@ - (void) terminate: (id)sender
               NSString *defaultButton,
               NSString *alternateButton)
 {
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
+#ifdef NS_IMPL_GNUSTEP
   return NSRunAlertPanel(title, msgFormat, defaultButton, alternateButton, nil)
     == NSAlertDefaultReturn;
 #else
@@ -6312,14 +6314,28 @@ - (NSRect)firstRectForCharacterRange: (NSRange)theRange
                                        +FRAME_LINE_HEIGHT (emacsframe));
 
   pt = [self convertPoint: pt toView: nil];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
-  pt = [[self window] convertBaseToScreen: pt];
-  rect.origin = pt;
-#else
-  rect.origin = pt;
-  rect = [[self window] convertRectToScreen: rect];
+
+#ifdef NS_RUNTIME_CHECKS
+  if ([[self window] respondsToSelector: @selector(convertRectToScreen:)])
+    {
+#endif
+#if MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_7)
+      rect.origin = pt;
+      rect = [[self window] convertRectToScreen: rect];
+#endif
+#ifdef NS_RUNTIME_CHECKS
+    }
+  else
+    {
+#endif
+#if MACOS_MAX_VERSION (MAC_OS_X_VERSION_10_6) || defined (NS_IMPL_GNUSTEP)
+      pt = [[self window] convertBaseToScreen: pt];
+      rect.origin = pt;
+#endif
+#ifdef NS_RUNTIME_CHECKS
+    }
 #endif
+
   return rect;
 }
 
@@ -7019,9 +7035,9 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 
   [win setAcceptsMouseMovedEvents: YES];
   [win setDelegate: self];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-  [win useOptimizedDrawing: YES];
+#if !defined (NS_IMPL_COCOA) || MACOS_MAX_VERSION (MAC_OS_X_VERSION_10_9)
+  if ([win respondsToSelector: @selector(useOptimizedDrawing:)])
+    [win useOptimizedDrawing: YES];
 #endif
 
   [[win contentView] addSubview: self];
@@ -7081,19 +7097,19 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   if ([col alphaComponent] != (EmacsCGFloat) 1.0)
     [win setOpaque: NO];
 
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-  [self allocateGState];
+#if !defined (NS_IMPL_COCOA) || MACOS_MAX_VERSION (MAC_OS_X_VERSION_10_9)
+  if ([self respondsToSelector: @selector(allocateGState)])
+    [self allocateGState];
 #endif
   [NSApp registerServicesMenuSendTypes: ns_send_types
                            returnTypes: [NSArray array]];
 
+#if MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_12)
   /* macOS Sierra automatically enables tabbed windows.  We can't
      allow this to be enabled until it's available on a Free system.
      Currently it only happens by accident and is buggy anyway. */
-#if defined (NS_IMPL_COCOA) && \
-  MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_12
-  [win setTabbingMode: NSWindowTabbingModeDisallowed];
+  if ([win respondsToSelector: @selector(setTabbingMode:)])
+    [win setTabbingMode: NSWindowTabbingModeDisallowed];
 #endif
 
   ns_window_num++;
@@ -7349,7 +7365,11 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
     {
       BOOL tbar_visible = FRAME_EXTERNAL_TOOL_BAR (emacsframe) ? YES : NO;
 #ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
+#if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
+      /* These two values are only defined in 10.7 and above. */
+      int NSApplicationPresentationFullScreen = (1 << 10);
+      int NSApplicationPresentationAutoHideToolbar = (1 << 11);
+#endif
       unsigned val = (unsigned)[NSApp presentationOptions];
 
       // Mac OS X 10.7 bug fix, the menu won't appear without this.
@@ -7365,7 +7385,6 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
           [NSApp setPresentationOptions: options];
         }
 #endif
-#endif
       [toolbar setVisible:tbar_visible];
     }
 }
@@ -7499,10 +7518,10 @@ - (void)toggleFullScreen: (id)sender
     {
       NSScreen *screen = [w screen];
 
-#if defined (NS_IMPL_COCOA) && \
-  MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
+#if MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_9)
       /* Hide ghost menu bar on secondary monitor? */
-      if (! onFirstScreen)
+      if (! onFirstScreen
+          && [NSScreen respondsToSelector: @selector(screensHaveSeparateSpaces)])
         onFirstScreen = [NSScreen screensHaveSeparateSpaces];
 #endif
       /* Hide dock and menubar if we are on the primary screen.  */
@@ -7530,9 +7549,9 @@ - (void)toggleFullScreen: (id)sender
       [fw setTitle:[w title]];
       [fw setDelegate:self];
       [fw setAcceptsMouseMovedEvents: YES];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-      [fw useOptimizedDrawing: YES];
+#if !defined (NS_IMPL_COCOA) || MACOS_MAX_VERSION (MAC_OS_X_VERSION_10_9)
+      if ([fw respondsToSelector: @selector(useOptimizedDrawing:)])
+        [fw useOptimizedDrawing: YES];
 #endif
       [fw setBackgroundColor: col];
       if ([col alphaComponent] != (EmacsCGFloat) 1.0)
@@ -8093,10 +8112,11 @@ - (NSRect)constrainFrameRect:(NSRect)frameRect toScreen:(NSScreen *)screen
              NSTRACE_ARG_RECT (frameRect));
 
 #ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
+#if MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_9)
   // If separate spaces is on, it is like each screen is independent.  There is
   // no spanning of frames across screens.
-  if ([NSScreen screensHaveSeparateSpaces])
+  if ([NSScreen respondsToSelector: @selector(screensHaveSeparateSpaces)]
+      && [NSScreen screensHaveSeparateSpaces])
     {
       NSTRACE_MSG ("Screens have separate spaces");
       frameRect = [super constrainFrameRect:frameRect toScreen:screen];
@@ -8104,7 +8124,7 @@ - (NSRect)constrainFrameRect:(NSRect)frameRect toScreen:(NSScreen *)screen
       return frameRect;
     }
   else
-#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9 */
+#endif /* MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_9) */
     // Check that the proposed frameRect is visible in at least one
     // screen.  If it is not, ask the system to reposition it (only
     // for non-child windows).
@@ -8310,12 +8330,19 @@ + (CGFloat) scrollerWidth
   /* TODO: if we want to allow variable widths, this is the place to do it,
            however neither GNUstep nor Cocoa support it very well */
   CGFloat r;
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
-  r = [NSScroller scrollerWidth];
-#else
-  r = [NSScroller scrollerWidthForControlSize: NSControlSizeRegular
-                                scrollerStyle: NSScrollerStyleLegacy];
+#ifdef NS_RUNTIME_CHECKS
+  if ([NSScroller respondsToSelector:
+                    @selector(scrollerWidthForControlSize:scrollerStyle:)])
+#endif
+#if MACOS_MIN_VERSION (MAC_OS_X_VERSION_10_7)
+    r = [NSScroller scrollerWidthForControlSize: NSControlSizeRegular
+                                  scrollerStyle: NSScrollerStyleLegacy];
+#endif
+#ifdef NS_RUNTIME_CHECKS
+  else
+#endif
+#if MACOS_MAX_VERSION (MAC_OS_X_VERSION_10_6)
+    r = [NSScroller scrollerWidth];
 #endif
   return r;
 }
-- 
2.12.0


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* Re: NS runtime feature detection
  2017-07-24 19:02                                           ` NS runtime feature detection Charles A. Roelli
  2017-07-24 20:44                                             ` bug#27810: " Alan Third
@ 2017-07-24 20:45                                             ` Alan Third
  1 sibling, 0 replies; 63+ messages in thread
From: Alan Third @ 2017-07-24 20:45 UTC (permalink / raw)
  To: Charles A. Roelli
  Cc: Paul Michael Reilly, Jean-Christophe Helary, Anders Lindgren,
	Emacs-Devel devel

On Mon, Jul 24, 2017 at 09:02:57PM +0200, Charles A. Roelli wrote:
> Thanks for taking the initiative with this.  Maybe it's time to open a bug?

Continued over in bug#27810.
-- 
Alan Third



^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-07-24 20:44                                             ` bug#27810: " Alan Third
@ 2017-07-24 20:53                                               ` Glenn Morris
  2017-07-25 17:56                                                 ` Alan Third
  2017-07-26 21:57                                               ` Alan Third
  1 sibling, 1 reply; 63+ messages in thread
From: Glenn Morris @ 2017-07-24 20:53 UTC (permalink / raw)
  To: Alan Third; +Cc: Charles A. Roelli, Anders Lindgren, 27810

Alan Third wrote:

> I think perhaps we need to just test '< 10.7'. I've attached a new
> patch that deals with that and some other bits and pieces.
[...]
> I'm unclear where we should be using MIN_REQUIRED vs MAX_ALLOWED, but
> I think we're OK with MAX everywhere for what we're doing. I've just
> used MAX in both my new macros.
>
> We need a proper runtime OS version check in a few places. I think
> there are two ways of doing this depending on which OS version you're
> building on. It's getting very close to circular. ;)
>
> macfont.m looks like it could be a small nightmare as it has a LOT of
> version dependent code.


FWIW, a comment from someone who is disinterested in OS X:

You could probably make your life easier by not supporting more
than Apple does. Eg on the net I read:

    Apple is only supporting Mac OS X 10.9 – 10.11. Apple stopped
    providing security updates for Mac OS 10.8 and earlier versions with
    the release of Mac OS X 10.11 (El Capitan) in 2015.





^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-07-24 20:53                                               ` Glenn Morris
@ 2017-07-25 17:56                                                 ` Alan Third
  2017-07-25 18:22                                                   ` Charles A. Roelli
  0 siblings, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-07-25 17:56 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Charles A. Roelli, Anders Lindgren, 27810

On Mon, Jul 24, 2017 at 04:53:02PM -0400, Glenn Morris wrote:
> FWIW, a comment from someone who is disinterested in OS X:
> 
> You could probably make your life easier by not supporting more
> than Apple does. Eg on the net I read:
> 
>     Apple is only supporting Mac OS X 10.9 – 10.11. Apple stopped
>     providing security updates for Mac OS 10.8 and earlier versions with
>     the release of Mac OS X 10.11 (El Capitan) in 2015.

It would make life a lot easier, we could get rid of almost all
version dependent code. Just dropping support for 10.6 would remove a
lot too.

I doubt Charles would like that, though, and he’s the only other
person regularly working on the NS port.
-- 
Alan Third





^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-07-25 17:56                                                 ` Alan Third
@ 2017-07-25 18:22                                                   ` Charles A. Roelli
  2017-07-25 20:08                                                     ` Anders Lindgren
  0 siblings, 1 reply; 63+ messages in thread
From: Charles A. Roelli @ 2017-07-25 18:22 UTC (permalink / raw)
  To: Alan Third, Glenn Morris; +Cc: Anders Lindgren, 27810

I'm not attached to 10.6, but if we can keep supporting it, I'd like to.


And for what it's worth, there are 11 places in nsterm.m with
version-specific code (unless I've missed some):

12 matches for "MAC_OS_X_VERSION_MAX_ALLOWED" in buffer: nsterm.m
     140:#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
     157:#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
    5567:  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
    6329:  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
    7036:  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
    7098:  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
    7365:#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
    7516:  MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
    7547:  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
    8109:#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
    8120:#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9 */
    8327:  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7

which seems manageable.  But if and when support for earlier macOSen
causes problems, then I'd have no objections removing support.


On 25/07/2017 19:56, Alan Third wrote:
> On Mon, Jul 24, 2017 at 04:53:02PM -0400, Glenn Morris wrote:
>> FWIW, a comment from someone who is disinterested in OS X:
>>
>> You could probably make your life easier by not supporting more
>> than Apple does. Eg on the net I read:
>>
>>      Apple is only supporting Mac OS X 10.9 – 10.11. Apple stopped
>>      providing security updates for Mac OS 10.8 and earlier versions with
>>      the release of Mac OS X 10.11 (El Capitan) in 2015.
> It would make life a lot easier, we could get rid of almost all
> version dependent code. Just dropping support for 10.6 would remove a
> lot too.
>
> I doubt Charles would like that, though, and he’s the only other
> person regularly working on the NS port.






^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-07-25 18:22                                                   ` Charles A. Roelli
@ 2017-07-25 20:08                                                     ` Anders Lindgren
  0 siblings, 0 replies; 63+ messages in thread
From: Anders Lindgren @ 2017-07-25 20:08 UTC (permalink / raw)
  To: Charles A. Roelli; +Cc: Alan Third, 27810

[-- Attachment #1: Type: text/plain, Size: 2572 bytes --]

Hi!

There are a lot of Mac:s still used where the last supported OS version is
10.6.8. It would be a nice touch to support them, at least as long as all
we need to come up with is a good set of preprocessor tests to exclude
code. Of course, should we need to put a lot of effort into maintaining
support, I too would vote for dropping them (after all, there are fairly
modern versions of Emacs available for them).

    -- Anders

On Tue, Jul 25, 2017 at 8:22 PM, Charles A. Roelli <charles@aurox.ch> wrote:

> I'm not attached to 10.6, but if we can keep supporting it, I'd like to.
>
>
> And for what it's worth, there are 11 places in nsterm.m with
> version-specific code (unless I've missed some):
>
> 12 matches for "MAC_OS_X_VERSION_MAX_ALLOWED" in buffer: nsterm.m
>     140:#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
>     157:#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
>    5567:  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
>    6329:  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
>    7036:  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
>    7098:  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
>    7365:#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
>    7516:  MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
>    7547:  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
>    8109:#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
>    8120:#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9 */
>    8327:  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
>
> which seems manageable.  But if and when support for earlier macOSen
> causes problems, then I'd have no objections removing support.
>
>
>
> On 25/07/2017 19:56, Alan Third wrote:
>
>> On Mon, Jul 24, 2017 at 04:53:02PM -0400, Glenn Morris wrote:
>>
>>> FWIW, a comment from someone who is disinterested in OS X:
>>>
>>> You could probably make your life easier by not supporting more
>>> than Apple does. Eg on the net I read:
>>>
>>>      Apple is only supporting Mac OS X 10.9 – 10.11. Apple stopped
>>>      providing security updates for Mac OS 10.8 and earlier versions with
>>>      the release of Mac OS X 10.11 (El Capitan) in 2015.
>>>
>> It would make life a lot easier, we could get rid of almost all
>> version dependent code. Just dropping support for 10.6 would remove a
>> lot too.
>>
>> I doubt Charles would like that, though, and he’s the only other
>> person regularly working on the NS port.
>>
>
>

[-- Attachment #2: Type: text/html, Size: 3340 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: macOS runtime feature detection
  2017-07-24 20:22 bug#27810: macOS runtime feature detection Alan Third
@ 2017-07-26  2:59 ` Richard Stallman
  2017-07-26 16:06   ` Alan Third
  0 siblings, 1 reply; 63+ messages in thread
From: Richard Stallman @ 2017-07-26  2:59 UTC (permalink / raw)
  To: Alan Third; +Cc: 27810

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > We should be able to build a single binary that can detect new OS
  > features at runtime rather than depend on compile‐time macros.

One reason to avoid doing it that way
is for the sake of cross-compilation.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.






^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: macOS runtime feature detection
  2017-07-26  2:59 ` Richard Stallman
@ 2017-07-26 16:06   ` Alan Third
  2017-07-27  1:43     ` Richard Stallman
  2017-07-29 19:07     ` Richard Stallman
  0 siblings, 2 replies; 63+ messages in thread
From: Alan Third @ 2017-07-26 16:06 UTC (permalink / raw)
  To: Richard Stallman; +Cc: 27810

On Tue, Jul 25, 2017 at 10:59:38PM -0400, Richard Stallman wrote:
> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
> 
>   > We should be able to build a single binary that can detect new OS
>   > features at runtime rather than depend on compile‐time macros.
> 
> One reason to avoid doing it that way
> is for the sake of cross-compilation.

I’m afraid I don’t understand why it’s a problem for cross
compilation. At the moment we don’t even provide any way of building
Emacs on one version of macOS that is guaranteed to work on another
version of macOS, never mind cross‐compiling the NS port from another
platform.

For example, macOS 10.12 requires us to disable ‘tabbing mode’,
otherwise it causes problems with fullscreen. However, the way we do
that is incompatible with every previous version of macOS, and GNUstep
too. Currently we just wrap the code in a compile‐time version check,
but this means that to run Emacs on 10.12, it has to have been built
on 10.12, and an Emacs built on 10.12 cannot run on 10.11.

Objective C gives us a couple of ways of checking whether we can run
the code at runtime, though. For example we can check whether an
object has a particular method before calling it:

    if ([win respondsToSelector: @selector(setTabbingMode:)])
      [win setTabbingMode: NSWindowTabbingModeDisallowed];

This allows us to bypass the problem with having to build on the
target OS version.

I hope this explains what I’m trying to achieve.
-- 
Alan Third





^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-07-24 20:44                                             ` bug#27810: " Alan Third
  2017-07-24 20:53                                               ` Glenn Morris
@ 2017-07-26 21:57                                               ` Alan Third
  2017-07-31 19:05                                                 ` Charles A. Roelli
  1 sibling, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-07-26 21:57 UTC (permalink / raw)
  To: Charles A. Roelli; +Cc: Anders Lindgren, 27810

[-- Attachment #1: Type: text/plain, Size: 1394 bytes --]

On Mon, Jul 24, 2017 at 09:44:04PM +0100, Alan Third wrote:
> > I'm confused why the macro call you wrote doesn't prevent this. But
> > when I change it to #if MAC_OS_X_VERSION_MIN_ALLOWED <=
> > MAC_OS_X_VERSION_10_6, then it compiles.  This min/max stuff always
> > confuses me...
> 
> I’m unclear where we should be using MIN_REQUIRED vs MAX_ALLOWED, but
> I think we’re OK with MAX everywhere for what we’re doing. I’ve just
> used MAX in both my new macros.

I think I finally worked it out by reading macfont.m. I’ve attached
YET ANOTHER version of this which doesn't use any custom macros.

To compile for multiple versions you do something like:

./configure --with-ns CFLAGS="-DMAC_OS_X_VERSION_MIN_REQUIRED=1070 -DMAC_OS_X_VERSION_MAX_ALLOWED=101200 -g -O3"

By default max and min are set to the version you’re running on,
afaict. Please be aware that 10.6 isn’t fully compatible with this at
the moment, as there is at least one place (nsmenu.m:535) where
there’s a bug fix for it that I can’t see an immediate way of making
dynamic.

Additionally the native fullscreen stuff is based on a simple check
against MAX. I expect I’ll be able to fix that with a bit of work,
though.

I’d be interested to see if this builds on systems lower than 10.10
with the above configure command. There are probably bits and pieces
I’ve not quite got right.

-- 
Alan Third

[-- Attachment #2: 0001-Allow-use-of-run-time-OS-version-checks-on-macOS-bug.patch --]
[-- Type: text/plain, Size: 16587 bytes --]

From 44d3d2d20789997505a93101d6d14e6c854dbd50 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Thu, 6 Jul 2017 23:10:49 +0100
Subject: [PATCH] Allow use of run-time OS version checks on macOS (bug#27810)

* src/nsterm.h (NSWindowTabbingMode): Define in pre-Sierra macOS.
* src/nsterm.m (colorForEmacsRed):
(colorUsingDefaultColorSpace):
(runAlertPanel):
(firstRectForCharacterRange):
(initFrameFromEmacs):
(windowDidEnterFullScreen):
(toggleFullScreen):
(constrainFrameRect):
(scrollerWidth): Allow run-time checks.
* src/nsfns.m (ns_screen_name): Use run-time OS version checks.
* src/macfont.m (macfont_draw): Use run-time OS version checks.
---
 src/macfont.m |  16 +++++--
 src/nsfns.m   |  79 ++++++++++++++++++--------------
 src/nsterm.h  |   9 +++-
 src/nsterm.m  | 144 ++++++++++++++++++++++++++++++++++++++++------------------
 4 files changed, 164 insertions(+), 84 deletions(-)

diff --git a/src/macfont.m b/src/macfont.m
index 4d310e47ae..d373e391f9 100644
--- a/src/macfont.m
+++ b/src/macfont.m
@@ -2870,10 +2870,18 @@ So we use CTFontDescriptorCreateMatchingFontDescriptor (no
              Apple says there are no plans to address this issue
              (rdar://11644870) currently.  So we add a workaround.  */
 #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-          CGContextSetLineWidth (context, synthetic_bold_factor * font_size
-                                 * [[FRAME_NS_VIEW(f) window] backingScaleFactor]);
-#else
-          CGContextSetLineWidth (context, synthetic_bold_factor * font_size);
+#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_7
+          if ([[FRAME_NS_VIEW(f) window] respondsToSelector:
+                                           @selector(backingScaleFactor)])
+#endif
+            CGContextSetLineWidth (context, synthetic_bold_factor * font_size
+                                   * [[FRAME_NS_VIEW(f) window] backingScaleFactor]);
+#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_7
+          else
+#endif
+#endif
+#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_7
+            CGContextSetLineWidth (context, synthetic_bold_factor * font_size);
 #endif
           CG_SET_STROKE_COLOR_WITH_FACE_FOREGROUND (context, face, f);
         }
diff --git a/src/nsfns.m b/src/nsfns.m
index 36748cebb8..62c25d5e87 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -2513,51 +2513,60 @@ and GNUstep implementations ("distributor-specific release
   char *name = NULL;
 
 #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
-  mach_port_t masterPort;
-  io_iterator_t it;
-  io_object_t obj;
+#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_9
+  if (CGDisplayIOServicePort == NULL)
+#endif
+    {
+      mach_port_t masterPort;
+      io_iterator_t it;
+      io_object_t obj;
 
-  // CGDisplayIOServicePort is deprecated.  Do it another (harder) way.
+      /* CGDisplayIOServicePort is deprecated.  Do it another (harder) way.
 
-  if (IOMasterPort (MACH_PORT_NULL, &masterPort) != kIOReturnSuccess
-      || IOServiceGetMatchingServices (masterPort,
-                                       IOServiceMatching ("IONDRVDevice"),
-                                       &it) != kIOReturnSuccess)
-    return name;
+         Is this code OK for macOS < 10.9, and GNUstep?  I suspect it is,
+         in which case is it worth keeping the other method in here? */
 
-  /* Must loop until we find a name.  Many devices can have the same unit
-     number (represents different GPU parts), but only one has a name.  */
-  while (! name && (obj = IOIteratorNext (it)))
-    {
-      CFMutableDictionaryRef props;
-      const void *val;
-
-      if (IORegistryEntryCreateCFProperties (obj,
-                                             &props,
-                                             kCFAllocatorDefault,
-                                             kNilOptions) == kIOReturnSuccess
-          && props != nil
-          && (val = CFDictionaryGetValue(props, @"IOFBDependentIndex")))
+      if (IOMasterPort (MACH_PORT_NULL, &masterPort) != kIOReturnSuccess
+          || IOServiceGetMatchingServices (masterPort,
+                                           IOServiceMatching ("IONDRVDevice"),
+                                           &it) != kIOReturnSuccess)
+        return name;
+
+      /* Must loop until we find a name.  Many devices can have the same unit
+         number (represents different GPU parts), but only one has a name.  */
+      while (! name && (obj = IOIteratorNext (it)))
         {
-          unsigned nr = [(NSNumber *)val unsignedIntegerValue];
-          if (nr == CGDisplayUnitNumber (did))
-            name = ns_get_name_from_ioreg (obj);
+          CFMutableDictionaryRef props;
+          const void *val;
+
+          if (IORegistryEntryCreateCFProperties (obj,
+                                                 &props,
+                                                 kCFAllocatorDefault,
+                                                 kNilOptions) == kIOReturnSuccess
+              && props != nil
+              && (val = CFDictionaryGetValue(props, @"IOFBDependentIndex")))
+            {
+              unsigned nr = [(NSNumber *)val unsignedIntegerValue];
+              if (nr == CGDisplayUnitNumber (did))
+                name = ns_get_name_from_ioreg (obj);
+            }
+
+          CFRelease (props);
+          IOObjectRelease (obj);
         }
 
-      CFRelease (props);
-      IOObjectRelease (obj);
+      IOObjectRelease (it);
     }
-
-  IOObjectRelease (it);
-
-#else
-
-  name = ns_get_name_from_ioreg (CGDisplayIOServicePort (did));
-
+#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_9
+  else
+#endif
+#endif /* #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9 */
+#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_9
+    name = ns_get_name_from_ioreg (CGDisplayIOServicePort (did));
 #endif
   return name;
 }
-#endif
+#endif /* NS_IMPL_COCOA */
 
 static Lisp_Object
 ns_make_monitor_attribute_list (struct MonitorInfo *monitors,
diff --git a/src/nsterm.h b/src/nsterm.h
index 0f1b36db7b..e1dae28111 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -1317,6 +1317,13 @@ extern char gnustep_base_version[];  /* version tracking */
 #ifdef __OBJC__
 typedef NSUInteger NSWindowStyleMask;
 #endif
-#endif
 
+/* Window tabbing mode enums are new too. */
+enum NSWindowTabbingMode
+  {
+    NSWindowTabbingModeAutomatic,
+    NSWindowTabbingModePreferred,
+    NSWindowTabbingModeDisallowed
+  };
+#endif
 #endif	/* HAVE_NS */
diff --git a/src/nsterm.m b/src/nsterm.m
index 36d906a7ce..10f726be86 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -136,14 +136,18 @@ @implementation NSColor (EmacsColor)
 + (NSColor *)colorForEmacsRed:(CGFloat)red green:(CGFloat)green
                          blue:(CGFloat)blue alpha:(CGFloat)alpha
 {
-#ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-  if (ns_use_srgb_colorspace)
-      return [NSColor colorWithSRGBRed: red
-                                 green: green
-                                  blue: blue
-                                 alpha: alpha];
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
+  if (ns_use_srgb_colorspace
+#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_7
+      && [NSColor respondsToSelector:
+                    @selector(colorWithSRGBRed:green:blue:alpha:)]
 #endif
+      )
+    return [NSColor colorWithSRGBRed: red
+                               green: green
+                                blue: blue
+                               alpha: alpha];
 #endif
   return [NSColor colorWithCalibratedRed: red
                                    green: green
@@ -153,11 +157,18 @@ + (NSColor *)colorForEmacsRed:(CGFloat)red green:(CGFloat)green
 
 - (NSColor *)colorUsingDefaultColorSpace
 {
-#ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-  if (ns_use_srgb_colorspace)
-    return [self colorUsingColorSpace: [NSColorSpace sRGBColorSpace]];
+  /* FIXMES: We're checking for colorWithSRGBRed here so this will
+     only work in the same place as in the method above.  It should
+     really be a check whether we're on macOS 10.7 or above. */
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
+  if (ns_use_srgb_colorspace
+#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_7
+      && [NSColor respondsToSelector:
+                    @selector(colorWithSRGBRed:green:blue:alpha:)]
 #endif
+      )
+    return [self colorUsingColorSpace: [NSColorSpace sRGBColorSpace]];
 #endif
   return [self colorUsingColorSpaceName: NSCalibratedRGBColorSpace];
 }
@@ -5563,8 +5574,7 @@ - (void) terminate: (id)sender
               NSString *defaultButton,
               NSString *alternateButton)
 {
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
+#ifdef NS_IMPL_GNUSTEP
   return NSRunAlertPanel(title, msgFormat, defaultButton, alternateButton, nil)
     == NSAlertDefaultReturn;
 #else
@@ -6325,14 +6335,27 @@ - (NSRect)firstRectForCharacterRange: (NSRange)theRange
                                        +FRAME_LINE_HEIGHT (emacsframe));
 
   pt = [self convertPoint: pt toView: nil];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
-  pt = [[self window] convertBaseToScreen: pt];
-  rect.origin = pt;
-#else
-  rect.origin = pt;
-  rect = [[self window] convertRectToScreen: rect];
+
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
+#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_7
+  if ([[self window] respondsToSelector: @selector(convertRectToScreen:)])
+    {
+#endif
+      rect.origin = pt;
+      rect = [[self window] convertRectToScreen: rect];
+#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_7
+    }
+  else
+#endif
+#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7 */
+#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_7 \
+  || defined (NS_IMPL_GNUSTEP)
+    {
+      pt = [[self window] convertBaseToScreen: pt];
+      rect.origin = pt;
+    }
 #endif
+
   return rect;
 }
 
@@ -7032,9 +7055,12 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 
   [win setAcceptsMouseMovedEvents: YES];
   [win setDelegate: self];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-  [win useOptimizedDrawing: YES];
+#if !defined (NS_IMPL_COCOA) \
+  || MAC_OS_X_VERSION_MIN_REQUIRED <= MAC_OS_X_VERSION_10_9
+#if MAC_OS_X_VERSION_MAX_ALLOWED > MAC_OS_X_VERSION_10_9
+  if ([win respondsToSelector: @selector(useOptimizedDrawing:)])
+#endif
+    [win useOptimizedDrawing: YES];
 #endif
 
   [[win contentView] addSubview: self];
@@ -7094,9 +7120,12 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   if ([col alphaComponent] != (EmacsCGFloat) 1.0)
     [win setOpaque: NO];
 
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-  [self allocateGState];
+#if !defined (NS_IMPL_COCOA) \
+  || MAC_OS_X_VERSION_MIN_REQUIRED <= MAC_OS_X_VERSION_10_9
+#if MAC_OS_X_VERSION_MAX_ALLOWED > MAC_OS_X_VERSION_10_9
+  if ([self respondsToSelector: @selector(allocateGState)])
+#endif
+    [self allocateGState];
 #endif
   [NSApp registerServicesMenuSendTypes: ns_send_types
                            returnTypes: [NSArray array]];
@@ -7104,9 +7133,12 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   /* macOS Sierra automatically enables tabbed windows.  We can't
      allow this to be enabled until it's available on a Free system.
      Currently it only happens by accident and is buggy anyway. */
-#if defined (NS_IMPL_COCOA) && \
-  MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_12
-  [win setTabbingMode: NSWindowTabbingModeDisallowed];
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_12
+#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_12
+  if ([win respondsToSelector: @selector(setTabbingMode:)])
+#endif
+    [win setTabbingMode: NSWindowTabbingModeDisallowed];
 #endif
 
   ns_window_num++;
@@ -7362,7 +7394,11 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
     {
       BOOL tbar_visible = FRAME_EXTERNAL_TOOL_BAR (emacsframe) ? YES : NO;
 #ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
+#if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
+      /* These two values are only defined in 10.7 and above. */
+      int NSApplicationPresentationFullScreen = (1 << 10);
+      int NSApplicationPresentationAutoHideToolbar = (1 << 11);
+#endif
       unsigned val = (unsigned)[NSApp presentationOptions];
 
       // Mac OS X 10.7 bug fix, the menu won't appear without this.
@@ -7378,7 +7414,6 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
           [NSApp setPresentationOptions: options];
         }
 #endif
-#endif
       [toolbar setVisible:tbar_visible];
     }
 }
@@ -7512,10 +7547,14 @@ - (void)toggleFullScreen: (id)sender
     {
       NSScreen *screen = [w screen];
 
-#if defined (NS_IMPL_COCOA) && \
-  MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
       /* Hide ghost menu bar on secondary monitor? */
-      if (! onFirstScreen)
+      if (! onFirstScreen
+#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_9
+          && [NSScreen respondsToSelector: @selector(screensHaveSeparateSpaces)]
+#endif
+          )
         onFirstScreen = [NSScreen screensHaveSeparateSpaces];
 #endif
       /* Hide dock and menubar if we are on the primary screen.  */
@@ -7543,9 +7582,12 @@ - (void)toggleFullScreen: (id)sender
       [fw setTitle:[w title]];
       [fw setDelegate:self];
       [fw setAcceptsMouseMovedEvents: YES];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-      [fw useOptimizedDrawing: YES];
+#if !defined (NS_IMPL_COCOA) \
+  || MAC_OS_X_VERSION_MIN_ALLOWED <= MAC_OS_X_VERSION_10_9
+#if MAC_OS_X_VERSION_MAX_ALLOWED > MAC_OS_X_VERSION_10_9
+      if ([fw respondsToSelector: @selector(useOptimizedDrawing:)])
+#endif
+        [fw useOptimizedDrawing: YES];
 #endif
       [fw setBackgroundColor: col];
       if ([col alphaComponent] != (EmacsCGFloat) 1.0)
@@ -8109,7 +8151,11 @@ - (NSRect)constrainFrameRect:(NSRect)frameRect toScreen:(NSScreen *)screen
 #if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
   // If separate spaces is on, it is like each screen is independent.  There is
   // no spanning of frames across screens.
-  if ([NSScreen screensHaveSeparateSpaces])
+  if (
+#if MAC_OS_X_VERSION_MIN_ALLOWED < MAC_OS_X_VERSION_10_9
+      [NSScreen respondsToSelector: @selector(screensHaveSeparateSpaces)] &&
+#endif
+      [NSScreen screensHaveSeparateSpaces])
     {
       NSTRACE_MSG ("Screens have separate spaces");
       frameRect = [super constrainFrameRect:frameRect toScreen:screen];
@@ -8118,6 +8164,7 @@ - (NSRect)constrainFrameRect:(NSRect)frameRect toScreen:(NSScreen *)screen
     }
   else
 #endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9 */
+
     // Check that the proposed frameRect is visible in at least one
     // screen.  If it is not, ask the system to reposition it (only
     // for non-child windows).
@@ -8323,12 +8370,21 @@ + (CGFloat) scrollerWidth
   /* TODO: if we want to allow variable widths, this is the place to do it,
            however neither GNUstep nor Cocoa support it very well */
   CGFloat r;
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
-  r = [NSScroller scrollerWidth];
-#else
-  r = [NSScroller scrollerWidthForControlSize: NSControlSizeRegular
-                                scrollerStyle: NSScrollerStyleLegacy];
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
+#if MAC_OS_X_VERSION_MIN_ALLOWED < MAC_OS_X_VERSION_10_7
+  if ([NSScroller respondsToSelector:
+                    @selector(scrollerWidthForControlSize:scrollerStyle:)])
+#endif
+    r = [NSScroller scrollerWidthForControlSize: NSControlSizeRegular
+                                  scrollerStyle: NSScrollerStyleLegacy];
+#if MAC_OS_X_VERSION_MIN_ALLOWED < MAC_OS_X_VERSION_10_7
+  else
+#endif
+#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7 */
+#if MAC_OS_X_VERSION_MIN_ALLOWED < MAC_OS_X_VERSION_10_7 \
+  || defined (NS_IMPL_GNUSTEP)
+    r = [NSScroller scrollerWidth];
 #endif
   return r;
 }
-- 
2.12.0


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* bug#27810: macOS runtime feature detection
  2017-07-26 16:06   ` Alan Third
@ 2017-07-27  1:43     ` Richard Stallman
  2017-07-27 17:31       ` Eli Zaretskii
  2017-07-29 19:07     ` Richard Stallman
  1 sibling, 1 reply; 63+ messages in thread
From: Richard Stallman @ 2017-07-27  1:43 UTC (permalink / raw)
  To: Alan Third; +Cc: 27810

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > >   > We should be able to build a single binary that can detect new OS
  > >   > features at runtime rather than depend on compile‐time macros.
  > > 
  > > One reason to avoid doing it that way
  > > is for the sake of cross-compilation.

  > I’m afraid I don’t understand why it’s a problem for cross
  > compilation.

Autoconf tests that test the compilation environment
will automatically do the right thing in cross-compilation.
That's why we try to implement ALL tests that way.

  > At the moment we don’t even provide any way of building
  > Emacs on one version of macOS that is guaranteed to work on another
  > version of macOS, never mind cross‐compiling the NS port from another
  > platform.

Ideally cross-compiling should work from any platform to any platform
if you have installed suitable cross-compilation tools.  If this
does not work, in principle that is a bug in Emacs.  I am not saying
it is an extremely important bug to fix, but we should not introduce
more of them.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.






^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: macOS runtime feature detection
  2017-07-27  1:43     ` Richard Stallman
@ 2017-07-27 17:31       ` Eli Zaretskii
  2017-07-28 17:14         ` Richard Stallman
  0 siblings, 1 reply; 63+ messages in thread
From: Eli Zaretskii @ 2017-07-27 17:31 UTC (permalink / raw)
  To: rms; +Cc: alan, 27810

> From: Richard Stallman <rms@gnu.org>
> Date: Wed, 26 Jul 2017 21:43:44 -0400
> Cc: 27810@debbugs.gnu.org
> 
> Ideally cross-compiling should work from any platform to any platform
> if you have installed suitable cross-compilation tools.  If this
> does not work, in principle that is a bug in Emacs.

AFAIU, cross-compiling Emacs cannot work because building Emacs
includes dumping, which needs to run the produced binary.





^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: macOS runtime feature detection
  2017-07-27 17:31       ` Eli Zaretskii
@ 2017-07-28 17:14         ` Richard Stallman
  2017-07-28 17:36           ` Eli Zaretskii
  0 siblings, 1 reply; 63+ messages in thread
From: Richard Stallman @ 2017-07-28 17:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: alan, 27810

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > AFAIU, cross-compiling Emacs cannot work because building Emacs
  > includes dumping, which needs to run the produced binary.

'make emacs' can't run to completion in a cross-environment, but
cross-compilation can work well enough in practice.  Precisely, 'make
temacs' can work with a cross compiler; then you can dump with temacs
on the target platform.


-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.






^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: macOS runtime feature detection
  2017-07-28 17:14         ` Richard Stallman
@ 2017-07-28 17:36           ` Eli Zaretskii
  2017-07-29 19:04             ` Richard Stallman
  0 siblings, 1 reply; 63+ messages in thread
From: Eli Zaretskii @ 2017-07-28 17:36 UTC (permalink / raw)
  To: rms; +Cc: alan, 27810

> From: Richard Stallman <rms@gnu.org>
> CC: alan@idiocy.org, 27810@debbugs.gnu.org
> Date: Fri, 28 Jul 2017 13:14:19 -0400
> 
>   > AFAIU, cross-compiling Emacs cannot work because building Emacs
>   > includes dumping, which needs to run the produced binary.
> 
> 'make emacs' can't run to completion in a cross-environment, but
> cross-compilation can work well enough in practice.  Precisely, 'make
> temacs' can work with a cross compiler; then you can dump with temacs
> on the target platform.

You'd need to copy the entire build tree to the target platform for
that to work, so it's not really a cross-build anymore, not in the
usual sense.





^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: macOS runtime feature detection
  2017-07-28 17:36           ` Eli Zaretskii
@ 2017-07-29 19:04             ` Richard Stallman
  2017-07-31  0:45               ` Richard Stallman
  0 siblings, 1 reply; 63+ messages in thread
From: Richard Stallman @ 2017-07-29 19:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: alan, 27810

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > 'make emacs' can't run to completion in a cross-environment, but
  > > cross-compilation can work well enough in practice.  Precisely, 'make
  > > temacs' can work with a cross compiler; then you can dump with temacs
  > > on the target platform.

  > You'd need to copy the entire build tree to the target platform for
  > that to work,

Yes, but why is that a problem?

		  so it's not really a cross-build anymore, not in the
  > usual sense.

It is not 100% a cross-build.  But it means you don't need a compiler
for the target machine.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.






^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: macOS runtime feature detection
  2017-07-26 16:06   ` Alan Third
  2017-07-27  1:43     ` Richard Stallman
@ 2017-07-29 19:07     ` Richard Stallman
  2017-07-30 12:12       ` Alan Third
  1 sibling, 1 reply; 63+ messages in thread
From: Richard Stallman @ 2017-07-29 19:07 UTC (permalink / raw)
  To: Alan Third; +Cc: 27810

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > I’m afraid I don’t understand why it’s a problem for cross
  > compilation. At the moment we don’t even provide any way of building
  > Emacs on one version of macOS that is guaranteed to work on another
  > version of macOS, never mind cross‐compiling the NS port from another
  > platform.

Supporting cross-compilation is a general goal for GNU packages.
That's why we make a point, in general, to use compile time
for autoconf tests.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.






^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: macOS runtime feature detection
  2017-07-29 19:07     ` Richard Stallman
@ 2017-07-30 12:12       ` Alan Third
  2017-07-30 14:15         ` Eli Zaretskii
  2017-07-31  0:47         ` Richard Stallman
  0 siblings, 2 replies; 63+ messages in thread
From: Alan Third @ 2017-07-30 12:12 UTC (permalink / raw)
  To: Richard Stallman; +Cc: 27810

On Sat, Jul 29, 2017 at 03:07:05PM -0400, Richard Stallman wrote:
>   > I’m afraid I don’t understand why it’s a problem for cross
>   > compilation. At the moment we don’t even provide any way of building
>   > Emacs on one version of macOS that is guaranteed to work on another
>   > version of macOS, never mind cross‐compiling the NS port from another
>   > platform.
> 
> Supporting cross-compilation is a general goal for GNU packages.
> That's why we make a point, in general, to use compile time
> for autoconf tests.

Should I stop working on this? I honestly don’t believe it will have
any effect on cross compilation, and the default is to not include the
feature detection code unless you ask for it explicitly.

Not including my changes makes it harder to target macOS versions
other than that of the build system.
-- 
Alan Third





^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: macOS runtime feature detection
  2017-07-30 12:12       ` Alan Third
@ 2017-07-30 14:15         ` Eli Zaretskii
  2017-07-31  0:47         ` Richard Stallman
  1 sibling, 0 replies; 63+ messages in thread
From: Eli Zaretskii @ 2017-07-30 14:15 UTC (permalink / raw)
  To: Alan Third; +Cc: rms, 27810

> Date: Sun, 30 Jul 2017 13:12:59 +0100
> From: Alan Third <alan@idiocy.org>
> Cc: 27810@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> 
> Should I stop working on this? I honestly don’t believe it will have
> any effect on cross compilation, and the default is to not include the
> feature detection code unless you ask for it explicitly.

As long as cross-compilation is not affected, there's no reasons for
you to stop working on this.  So please go ahead, just make sure
version-dependent stuff is only used when necessary, i.e. when there
are no reasonable fallbacks that are back-compatible.

Thanks.





^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: macOS runtime feature detection
  2017-07-29 19:04             ` Richard Stallman
@ 2017-07-31  0:45               ` Richard Stallman
  0 siblings, 0 replies; 63+ messages in thread
From: Richard Stallman @ 2017-07-31  0:45 UTC (permalink / raw)
  To: eliz, alan, 27810; +Cc: rms

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > 		  so it's not really a cross-build anymore, not in the
  >   > usual sense.

  > It is not 100% a cross-build.  But it means you don't need a compiler
  > for the target machine.

I mean, you don't need to do compilation ON the target machine.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.






^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: macOS runtime feature detection
  2017-07-30 12:12       ` Alan Third
  2017-07-30 14:15         ` Eli Zaretskii
@ 2017-07-31  0:47         ` Richard Stallman
  1 sibling, 0 replies; 63+ messages in thread
From: Richard Stallman @ 2017-07-31  0:47 UTC (permalink / raw)
  To: Alan Third; +Cc: 27810

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > Should I stop working on this? I honestly don’t believe it will have
  > any effect on cross compilation, and the default is to not include the
  > feature detection code unless you ask for it explicitly.

I wasn't trying to make a decision about this particular question.
I was disagreeing with the general idea that it was better to do
Autoconf tests by running programs rather than by just compiling them.

If your code would have no effect at all other than on MacOS,
it can't do much harm.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.






^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-07-26 21:57                                               ` Alan Third
@ 2017-07-31 19:05                                                 ` Charles A. Roelli
  2017-08-01 15:38                                                   ` Anders Lindgren
  0 siblings, 1 reply; 63+ messages in thread
From: Charles A. Roelli @ 2017-07-31 19:05 UTC (permalink / raw)
  To: Alan Third; +Cc: Anders Lindgren, 27810

Shouldn't MIN_ALLOWED be MIN_REQUIRED?  As in here:

+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
+#if MAC_OS_X_VERSION_MIN_ALLOWED < MAC_OS_X_VERSION_10_7

There are a few more places that have a MIN_ALLOWED thing.  This
always trips me up, so I'm not sure.

And also, it's apparently feasible to do a runtime check for a
specific macOS, according to this:

https://developer.apple.com/library/content/documentation/DeveloperTools/Conceptual/cross_development/Using/using.html#//apple_ref/doc/uid/20002000-SW7

which mentions APPKIT_EXTERN double NSAppKitVersionNumber.  I'm not
sure if they still define this on Sierra, though.  But if they do,
then we can use this to fix the nsmenu.m problem.


On 26/07/2017 23:57, Alan Third wrote:
> On Mon, Jul 24, 2017 at 09:44:04PM +0100, Alan Third wrote:
>>> I'm confused why the macro call you wrote doesn't prevent this. But
>>> when I change it to #if MAC_OS_X_VERSION_MIN_ALLOWED <=
>>> MAC_OS_X_VERSION_10_6, then it compiles.  This min/max stuff always
>>> confuses me...
>> I’m unclear where we should be using MIN_REQUIRED vs MAX_ALLOWED, but
>> I think we’re OK with MAX everywhere for what we’re doing. I’ve just
>> used MAX in both my new macros.
> I think I finally worked it out by reading macfont.m. I’ve attached
> YET ANOTHER version of this which doesn't use any custom macros.
>
> To compile for multiple versions you do something like:
>
> ./configure --with-ns CFLAGS="-DMAC_OS_X_VERSION_MIN_REQUIRED=1070 -DMAC_OS_X_VERSION_MAX_ALLOWED=101200 -g -O3"
>
> By default max and min are set to the version you’re running on,
> afaict. Please be aware that 10.6 isn’t fully compatible with this at
> the moment, as there is at least one place (nsmenu.m:535) where
> there’s a bug fix for it that I can’t see an immediate way of making
> dynamic.
>
> Additionally the native fullscreen stuff is based on a simple check
> against MAX. I expect I’ll be able to fix that with a bit of work,
> though.
>
> I’d be interested to see if this builds on systems lower than 10.10
> with the above configure command. There are probably bits and pieces
> I’ve not quite got right.
>






^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-07-31 19:05                                                 ` Charles A. Roelli
@ 2017-08-01 15:38                                                   ` Anders Lindgren
  2017-08-01 22:03                                                     ` Alan Third
  0 siblings, 1 reply; 63+ messages in thread
From: Anders Lindgren @ 2017-08-01 15:38 UTC (permalink / raw)
  To: Charles A. Roelli; +Cc: Alan Third, 27810

[-- Attachment #1: Type: text/plain, Size: 2598 bytes --]

It's always a good idea to enable warnings when undefined preprocessor
symbols are used. In gcc this is -Wundef and I gess it's the same in clang.

    -- Anders

On Mon, Jul 31, 2017 at 9:05 PM, Charles A. Roelli <charles@aurox.ch> wrote:

> Shouldn't MIN_ALLOWED be MIN_REQUIRED?  As in here:
>
> +#if defined (NS_IMPL_COCOA) \
> +  && MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
> +#if MAC_OS_X_VERSION_MIN_ALLOWED < MAC_OS_X_VERSION_10_7
>
> There are a few more places that have a MIN_ALLOWED thing.  This
> always trips me up, so I'm not sure.
>
> And also, it's apparently feasible to do a runtime check for a
> specific macOS, according to this:
>
> https://developer.apple.com/library/content/documentation/De
> veloperTools/Conceptual/cross_development/Using/using.html#/
> /apple_ref/doc/uid/20002000-SW7
>
> which mentions APPKIT_EXTERN double NSAppKitVersionNumber.  I'm not
> sure if they still define this on Sierra, though.  But if they do,
> then we can use this to fix the nsmenu.m problem.
>
>
>
> On 26/07/2017 23:57, Alan Third wrote:
>
>> On Mon, Jul 24, 2017 at 09:44:04PM +0100, Alan Third wrote:
>>
>>> I'm confused why the macro call you wrote doesn't prevent this. But
>>>> when I change it to #if MAC_OS_X_VERSION_MIN_ALLOWED <=
>>>> MAC_OS_X_VERSION_10_6, then it compiles.  This min/max stuff always
>>>> confuses me...
>>>>
>>> I’m unclear where we should be using MIN_REQUIRED vs MAX_ALLOWED, but
>>> I think we’re OK with MAX everywhere for what we’re doing. I’ve just
>>> used MAX in both my new macros.
>>>
>> I think I finally worked it out by reading macfont.m. I’ve attached
>> YET ANOTHER version of this which doesn't use any custom macros.
>>
>> To compile for multiple versions you do something like:
>>
>> ./configure --with-ns CFLAGS="-DMAC_OS_X_VERSION_MIN_REQUIRED=1070
>> -DMAC_OS_X_VERSION_MAX_ALLOWED=101200 -g -O3"
>>
>> By default max and min are set to the version you’re running on,
>> afaict. Please be aware that 10.6 isn’t fully compatible with this at
>> the moment, as there is at least one place (nsmenu.m:535) where
>> there’s a bug fix for it that I can’t see an immediate way of making
>> dynamic.
>>
>> Additionally the native fullscreen stuff is based on a simple check
>> against MAX. I expect I’ll be able to fix that with a bit of work,
>> though.
>>
>> I’d be interested to see if this builds on systems lower than 10.10
>> with the above configure command. There are probably bits and pieces
>> I’ve not quite got right.
>>
>>
>

[-- Attachment #2: Type: text/html, Size: 3653 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-08-01 15:38                                                   ` Anders Lindgren
@ 2017-08-01 22:03                                                     ` Alan Third
  2017-08-06 20:29                                                       ` Charles A. Roelli
  0 siblings, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-08-01 22:03 UTC (permalink / raw)
  To: Anders Lindgren; +Cc: Charles A. Roelli, 27810

[-- Attachment #1: Type: text/plain, Size: 1312 bytes --]

On Tue, Aug 01, 2017 at 05:38:03PM +0200, Anders Lindgren wrote:
> It's always a good idea to enable warnings when undefined preprocessor
> symbols are used. In gcc this is -Wundef and I gess it's the same in clang.

Unfortunately this produces an absolute ton of spurious warnings which
scroll off my terminal buffer. I think I’ve caught all the important
ones now, though.

I’ve attached my latest go with this. I’ve removed the
MAC_OS_X_VERSION_10_XX macros with their numbers, as we can use the
existence of the macros to tell what platform we’re compiling on. Eg.

    #if !defined (MAC_OS_X_VERSION_10_7) \
      && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070

I’ve also used the NSAppKitVersionNumber macros in a few places to do
runtime version detection. I’m not sure this is reliable. There are
two methods of finding the OS version definitively, which we can use
if we need to.

I’ve got rid of the HAS_NATIVE_FS macro and tried to replace it with
other checks where required. It compiles on 10.12, but I’m not at all
convinced it will compile on 10.6.

New 10.7 variables that need to be available on 10.6 when we’re
compiling for 10.7+ have been added near the bottom of nsterm.h.

Overall it looks quite different in places, but the functionality
hasn’t really changed.
-- 
Alan Third

[-- Attachment #2: 0001-Allow-use-of-run-time-OS-version-checks-on-macOS-bug.patch --]
[-- Type: text/plain, Size: 24231 bytes --]

From f014f12082b6e9651a331134ee5a683c720fc4fb Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Thu, 6 Jul 2017 23:10:49 +0100
Subject: [PATCH] Allow use of run-time OS version checks on macOS (bug#27810)

* src/nsterm.h (NSWindowTabbingMode): Define in pre-Sierra macOS.
(MAC_OS_X_VERSION_10_6, MAC_OS_X_VERSION_10_7, MAC_OS_X_VERSION_10_8,
MAC_OS_X_VERSION_10_9, MAC_OS_X_VERSION_10_12, HAVE_NATIVE_FS): Remove
defines.
(NSWindowStyleMaskFullScreen,
NSWindowCollectionBehaviorFullScreenPrimary,
NSApplicationPresentationFullScreen,
NSApplicationPresentationAutoHideToolbar): Define in macOS 10.6.
* src/nsterm.m (colorForEmacsRed, colorUsingDefaultColorSpace,
check_native_fs, ns_read_socket, ns_select, runAlertPanel,
firstRectForCharacterRange, initFrameFromEmacs, windowDidMiniaturize,
windowDidEnterFullScreen, windowDidExitFullScreen, isFullscreen,
updateCollectionBehavior, toggleFullScreen, constrainFrameRect,
scrollerWidth, syms_of_nsterm): Allow use of run-time checks and
replace version check macros.
* src/nsfns.m (ns_screen_name): Use run-time OS version checks.
* src/macfont.m (macfont_draw): Use run-time OS version checks.
* src/nsmenu.m (menuWillOpen): Use run-time OS version checks.
---
 src/macfont.m |  18 ++++--
 src/nsfns.m   |  83 +++++++++++++------------
 src/nsmenu.m  |   9 ++-
 src/nsterm.h  |  45 ++++++--------
 src/nsterm.m  | 191 ++++++++++++++++++++++++++++++++++++++--------------------
 5 files changed, 209 insertions(+), 137 deletions(-)

diff --git a/src/macfont.m b/src/macfont.m
index 4d310e47ae..19145f92c0 100644
--- a/src/macfont.m
+++ b/src/macfont.m
@@ -2869,11 +2869,19 @@ So we use CTFontDescriptorCreateMatchingFontDescriptor (no
              and synthetic bold looks thinner on such environments.
              Apple says there are no plans to address this issue
              (rdar://11644870) currently.  So we add a workaround.  */
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-          CGContextSetLineWidth (context, synthetic_bold_factor * font_size
-                                 * [[FRAME_NS_VIEW(f) window] backingScaleFactor]);
-#else
-          CGContextSetLineWidth (context, synthetic_bold_factor * font_size);
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+          if ([[FRAME_NS_VIEW(f) window] respondsToSelector:
+                                           @selector(backingScaleFactor)])
+#endif
+            CGContextSetLineWidth (context, synthetic_bold_factor * font_size
+                                   * [[FRAME_NS_VIEW(f) window] backingScaleFactor]);
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+          else
+#endif
+#endif
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+            CGContextSetLineWidth (context, synthetic_bold_factor * font_size);
 #endif
           CG_SET_STROKE_COLOR_WITH_FACE_FOREGROUND (context, face, f);
         }
diff --git a/src/nsfns.m b/src/nsfns.m
index 36748cebb8..e19e4e2641 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -1592,7 +1592,7 @@ Frames are listed from topmost (first) to bottommost (last).  */)
 }
 
 #ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED > MAC_OS_X_VERSION_10_9
+#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090
 #define MODAL_OK_RESPONSE NSModalResponseOK
 #endif
 #endif
@@ -2512,52 +2512,61 @@ and GNUstep implementations ("distributor-specific release
 {
   char *name = NULL;
 
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
-  mach_port_t masterPort;
-  io_iterator_t it;
-  io_object_t obj;
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1090
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
+  if (CGDisplayIOServicePort == NULL)
+#endif
+    {
+      mach_port_t masterPort;
+      io_iterator_t it;
+      io_object_t obj;
 
-  // CGDisplayIOServicePort is deprecated.  Do it another (harder) way.
+      /* CGDisplayIOServicePort is deprecated.  Do it another (harder) way.
 
-  if (IOMasterPort (MACH_PORT_NULL, &masterPort) != kIOReturnSuccess
-      || IOServiceGetMatchingServices (masterPort,
-                                       IOServiceMatching ("IONDRVDevice"),
-                                       &it) != kIOReturnSuccess)
-    return name;
+         Is this code OK for macOS < 10.9, and GNUstep?  I suspect it is,
+         in which case is it worth keeping the other method in here? */
 
-  /* Must loop until we find a name.  Many devices can have the same unit
-     number (represents different GPU parts), but only one has a name.  */
-  while (! name && (obj = IOIteratorNext (it)))
-    {
-      CFMutableDictionaryRef props;
-      const void *val;
-
-      if (IORegistryEntryCreateCFProperties (obj,
-                                             &props,
-                                             kCFAllocatorDefault,
-                                             kNilOptions) == kIOReturnSuccess
-          && props != nil
-          && (val = CFDictionaryGetValue(props, @"IOFBDependentIndex")))
+      if (IOMasterPort (MACH_PORT_NULL, &masterPort) != kIOReturnSuccess
+          || IOServiceGetMatchingServices (masterPort,
+                                           IOServiceMatching ("IONDRVDevice"),
+                                           &it) != kIOReturnSuccess)
+        return name;
+
+      /* Must loop until we find a name.  Many devices can have the same unit
+         number (represents different GPU parts), but only one has a name.  */
+      while (! name && (obj = IOIteratorNext (it)))
         {
-          unsigned nr = [(NSNumber *)val unsignedIntegerValue];
-          if (nr == CGDisplayUnitNumber (did))
-            name = ns_get_name_from_ioreg (obj);
+          CFMutableDictionaryRef props;
+          const void *val;
+
+          if (IORegistryEntryCreateCFProperties (obj,
+                                                 &props,
+                                                 kCFAllocatorDefault,
+                                                 kNilOptions) == kIOReturnSuccess
+              && props != nil
+              && (val = CFDictionaryGetValue(props, @"IOFBDependentIndex")))
+            {
+              unsigned nr = [(NSNumber *)val unsignedIntegerValue];
+              if (nr == CGDisplayUnitNumber (did))
+                name = ns_get_name_from_ioreg (obj);
+            }
+
+          CFRelease (props);
+          IOObjectRelease (obj);
         }
 
-      CFRelease (props);
-      IOObjectRelease (obj);
+      IOObjectRelease (it);
     }
-
-  IOObjectRelease (it);
-
-#else
-
-  name = ns_get_name_from_ioreg (CGDisplayIOServicePort (did));
-
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
+  else
+#endif
+#endif /* #if MAC_OS_X_VERSION_MAX_ALLOWED >= 1090 */
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
+    name = ns_get_name_from_ioreg (CGDisplayIOServicePort (did));
 #endif
   return name;
 }
-#endif
+#endif /* NS_IMPL_COCOA */
 
 static Lisp_Object
 ns_make_monitor_attribute_list (struct MonitorInfo *monitors,
diff --git a/src/nsmenu.m b/src/nsmenu.m
index 37a1a62d6d..93e06707c0 100644
--- a/src/nsmenu.m
+++ b/src/nsmenu.m
@@ -532,9 +532,14 @@ - (void)menuWillOpen:(NSMenu *)menu
 {
   ++trackingMenu;
 
-#if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
   // On 10.6 we get repeated calls, only the one for NSSystemDefined is "real".
-  if ([[NSApp currentEvent] type] != NSSystemDefined) return;
+  if (
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+      NSAppKitVersionNumber < NSAppKitVersionNumber10_7 &&
+#endif
+      [[NSApp currentEvent] type] != NSEventTypeSystemDefined)
+    return;
 #endif
 
   /* When dragging from one menu to another, we get willOpen followed by didClose,
diff --git a/src/nsterm.h b/src/nsterm.h
index 0f1b36db7b..a60f94ef64 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -25,30 +25,6 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "sysselect.h"
 
 #ifdef HAVE_NS
-
-#ifdef NS_IMPL_COCOA
-#ifndef MAC_OS_X_VERSION_10_6
-#define MAC_OS_X_VERSION_10_6 1060
-#endif
-#ifndef MAC_OS_X_VERSION_10_7
-#define MAC_OS_X_VERSION_10_7 1070
-#endif
-#ifndef MAC_OS_X_VERSION_10_8
-#define MAC_OS_X_VERSION_10_8 1080
-#endif
-#ifndef MAC_OS_X_VERSION_10_9
-#define MAC_OS_X_VERSION_10_9 1090
-#endif
-#ifndef MAC_OS_X_VERSION_10_12
-#define MAC_OS_X_VERSION_10_12 101200
-#endif
-
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-#define HAVE_NATIVE_FS
-#endif
-
-#endif /* NS_IMPL_COCOA */
-
 #ifdef __OBJC__
 
 /* CGFloat on GNUstep may be 4 or 8 byte, but functions expect float* for some
@@ -1275,9 +1251,16 @@ extern char gnustep_base_version[];  /* version tracking */
                                 ? (min) : (((x)>(max)) ? (max) : (x)))
 #define SCREENMAXBOUND(x) (IN_BOUND (-SCREENMAX, x, SCREENMAX))
 
+/* macOS 10.7 introduces some new constants.  */
+#if !defined (MAC_OS_X_VERSION_10_7) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#define NSWindowStyleMaskFullScreen                 (1 << 14)
+#define NSWindowCollectionBehaviorFullScreenPrimary (1 << 7)
+#define NSApplicationPresentationFullScreen         (1 << 10)
+#define NSApplicationPresentationAutoHideToolbar    (1 << 11)
+#endif
+
 /* macOS 10.12 deprecates a bunch of constants. */
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_12
+#if !defined (NS_IMPL_COCOA) || !defined (MAC_OS_X_VERSION_10_12)
 #define NSEventModifierFlagCommand         NSCommandKeyMask
 #define NSEventModifierFlagControl         NSControlKeyMask
 #define NSEventModifierFlagHelp            NSHelpKeyMask
@@ -1303,6 +1286,7 @@ extern char gnustep_base_version[];  /* version tracking */
 #define NSEventTypeKeyUp                   NSKeyUp
 #define NSEventTypeFlagsChanged            NSFlagsChanged
 #define NSEventMaskAny                     NSAnyEventMask
+#define NSEventTypeSystemDefined           NSSystemDefined
 #define NSWindowStyleMaskBorderless        NSBorderlessWindowMask
 #define NSWindowStyleMaskClosable          NSClosableWindowMask
 #define NSWindowStyleMaskFullScreen        NSFullScreenWindowMask
@@ -1317,6 +1301,13 @@ extern char gnustep_base_version[];  /* version tracking */
 #ifdef __OBJC__
 typedef NSUInteger NSWindowStyleMask;
 #endif
-#endif
 
+/* Window tabbing mode enums are new too. */
+enum NSWindowTabbingMode
+  {
+    NSWindowTabbingModeAutomatic,
+    NSWindowTabbingModePreferred,
+    NSWindowTabbingModeDisallowed
+  };
+#endif
 #endif	/* HAVE_NS */
diff --git a/src/nsterm.m b/src/nsterm.m
index 36d906a7ce..1eaf94ad88 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -136,14 +136,18 @@ @implementation NSColor (EmacsColor)
 + (NSColor *)colorForEmacsRed:(CGFloat)red green:(CGFloat)green
                          blue:(CGFloat)blue alpha:(CGFloat)alpha
 {
-#ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-  if (ns_use_srgb_colorspace)
-      return [NSColor colorWithSRGBRed: red
-                                 green: green
-                                  blue: blue
-                                 alpha: alpha];
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+  if (ns_use_srgb_colorspace
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+      && [NSColor respondsToSelector:
+                    @selector(colorWithSRGBRed:green:blue:alpha:)]
 #endif
+      )
+    return [NSColor colorWithSRGBRed: red
+                               green: green
+                                blue: blue
+                               alpha: alpha];
 #endif
   return [NSColor colorWithCalibratedRed: red
                                    green: green
@@ -153,11 +157,18 @@ + (NSColor *)colorForEmacsRed:(CGFloat)red green:(CGFloat)green
 
 - (NSColor *)colorUsingDefaultColorSpace
 {
-#ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-  if (ns_use_srgb_colorspace)
-    return [self colorUsingColorSpace: [NSColorSpace sRGBColorSpace]];
+  /* FIXMES: We're checking for colorWithSRGBRed here so this will
+     only work in the same place as in the method above.  It should
+     really be a check whether we're on macOS 10.7 or above. */
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+  if (ns_use_srgb_colorspace
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+      && [NSColor respondsToSelector:
+                    @selector(colorWithSRGBRed:green:blue:alpha:)]
 #endif
+      )
+    return [self colorUsingColorSpace: [NSColorSpace sRGBColorSpace]];
 #endif
   return [self colorUsingColorSpaceName: NSCalibratedRGBColorSpace];
 }
@@ -4140,7 +4151,7 @@ in certain situations (rapid incoming events).
     }
 }
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 static void
 check_native_fs ()
 {
@@ -4242,7 +4253,7 @@ in certain situations (rapid incoming events).
 
   NSTRACE_WHEN (NSTRACE_GROUP_EVENTS, "ns_read_socket");
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
   check_native_fs ();
 #endif
 
@@ -4324,7 +4335,7 @@ in certain situations (rapid incoming events).
 
   NSTRACE_WHEN (NSTRACE_GROUP_EVENTS, "ns_select");
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
   check_native_fs ();
 #endif
 
@@ -5563,8 +5574,7 @@ - (void) terminate: (id)sender
               NSString *defaultButton,
               NSString *alternateButton)
 {
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
+#ifdef NS_IMPL_GNUSTEP
   return NSRunAlertPanel(title, msgFormat, defaultButton, alternateButton, nil)
     == NSAlertDefaultReturn;
 #else
@@ -6325,14 +6335,27 @@ - (NSRect)firstRectForCharacterRange: (NSRange)theRange
                                        +FRAME_LINE_HEIGHT (emacsframe));
 
   pt = [self convertPoint: pt toView: nil];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
-  pt = [[self window] convertBaseToScreen: pt];
-  rect.origin = pt;
-#else
-  rect.origin = pt;
-  rect = [[self window] convertRectToScreen: rect];
+
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+  if ([[self window] respondsToSelector: @selector(convertRectToScreen:)])
+    {
 #endif
+      rect.origin = pt;
+      rect = [[self window] convertRectToScreen: rect];
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+    }
+  else
+#endif
+#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 */
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070 \
+  || defined (NS_IMPL_GNUSTEP)
+    {
+      pt = [[self window] convertBaseToScreen: pt];
+      rect.origin = pt;
+    }
+#endif
+
   return rect;
 }
 
@@ -6988,11 +7011,15 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   scrollbarsNeedingUpdate = 0;
   fs_state = FULLSCREEN_NONE;
   fs_before_fs = next_maximized = -1;
-#ifdef HAVE_NATIVE_FS
-  fs_is_native = ns_use_native_fullscreen;
-#else
+
   fs_is_native = NO;
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+  if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
 #endif
+    fs_is_native = ns_use_native_fullscreen;
+#endif
+
   maximized_width = maximized_height = -1;
   nonfs_window = nil;
 
@@ -7023,7 +7050,10 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
                         backing: NSBackingStoreBuffered
                           defer: YES];
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+  if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
+#endif
     [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
 #endif
 
@@ -7032,9 +7062,11 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 
   [win setAcceptsMouseMovedEvents: YES];
   [win setDelegate: self];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-  [win useOptimizedDrawing: YES];
+#if !defined (NS_IMPL_COCOA) || MAC_OS_X_VERSION_MIN_REQUIRED <= 1090
+#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090
+  if ([win respondsToSelector: @selector(useOptimizedDrawing:)])
+#endif
+    [win useOptimizedDrawing: YES];
 #endif
 
   [[win contentView] addSubview: self];
@@ -7094,9 +7126,12 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   if ([col alphaComponent] != (EmacsCGFloat) 1.0)
     [win setOpaque: NO];
 
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-  [self allocateGState];
+#if !defined (NS_IMPL_COCOA) \
+  || MAC_OS_X_VERSION_MIN_REQUIRED <= 1090
+#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090
+  if ([self respondsToSelector: @selector(allocateGState)])
+#endif
+    [self allocateGState];
 #endif
   [NSApp registerServicesMenuSendTypes: ns_send_types
                            returnTypes: [NSArray array]];
@@ -7104,9 +7139,12 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   /* macOS Sierra automatically enables tabbed windows.  We can't
      allow this to be enabled until it's available on a Free system.
      Currently it only happens by accident and is buggy anyway. */
-#if defined (NS_IMPL_COCOA) && \
-  MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_12
-  [win setTabbingMode: NSWindowTabbingModeDisallowed];
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= 101200
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 101200
+  if ([win respondsToSelector: @selector(setTabbingMode:)])
+#endif
+    [win setTabbingMode: NSWindowTabbingModeDisallowed];
 #endif
 
   ns_window_num++;
@@ -7323,7 +7361,7 @@ - (void)windowDidMiniaturize: sender
     }
 }
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 - (NSApplicationPresentationOptions)window:(NSWindow *)window
       willUseFullScreenPresentationOptions:
   (NSApplicationPresentationOptions)proposedOptions
@@ -7361,8 +7399,8 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
   else
     {
       BOOL tbar_visible = FRAME_EXTERNAL_TOOL_BAR (emacsframe) ? YES : NO;
-#ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070        \
+  && MAC_OS_X_VERSION_MIN_REQUIRED <= 1070
       unsigned val = (unsigned)[NSApp presentationOptions];
 
       // Mac OS X 10.7 bug fix, the menu won't appear without this.
@@ -7378,7 +7416,6 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
           [NSApp setPresentationOptions: options];
         }
 #endif
-#endif
       [toolbar setVisible:tbar_visible];
     }
 }
@@ -7417,7 +7454,7 @@ - (void)windowDidExitFullScreen /* provided for direct calls */
     }
   [self setFSValue: fs_before_fs];
   fs_before_fs = -1;
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
   [self updateCollectionBehavior];
 #endif
   if (FRAME_EXTERNAL_TOOL_BAR (emacsframe))
@@ -7449,7 +7486,7 @@ - (BOOL)isFullscreen
     }
   else
     {
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
       res = (([[self window] styleMask] & NSWindowStyleMaskFullScreen) != 0);
 #else
       res = NO;
@@ -7462,7 +7499,7 @@ - (BOOL)isFullscreen
   return res;
 }
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 - (void)updateCollectionBehavior
 {
   NSTRACE ("[EmacsView updateCollectionBehavior]");
@@ -7477,7 +7514,10 @@ - (void)updateCollectionBehavior
         b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
 
       [win setCollectionBehavior: b];
-      fs_is_native = ns_use_native_fullscreen;
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+      if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
+#endif
+        fs_is_native = ns_use_native_fullscreen;
     }
 }
 #endif
@@ -7494,8 +7534,11 @@ - (void)toggleFullScreen: (id)sender
 
   if (fs_is_native)
     {
-#ifdef HAVE_NATIVE_FS
-      [[self window] toggleFullScreen:sender];
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+      if ([[self window] respondsToSelector: @selector(toggleFullScreen:)])
+#endif
+        [[self window] toggleFullScreen:sender];
 #endif
       return;
     }
@@ -7512,10 +7555,13 @@ - (void)toggleFullScreen: (id)sender
     {
       NSScreen *screen = [w screen];
 
-#if defined (NS_IMPL_COCOA) && \
-  MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1090
       /* Hide ghost menu bar on secondary monitor? */
-      if (! onFirstScreen)
+      if (! onFirstScreen
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
+          && [NSScreen respondsToSelector: @selector(screensHaveSeparateSpaces)]
+#endif
+          )
         onFirstScreen = [NSScreen screensHaveSeparateSpaces];
 #endif
       /* Hide dock and menubar if we are on the primary screen.  */
@@ -7543,9 +7589,12 @@ - (void)toggleFullScreen: (id)sender
       [fw setTitle:[w title]];
       [fw setDelegate:self];
       [fw setAcceptsMouseMovedEvents: YES];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-      [fw useOptimizedDrawing: YES];
+#if !defined (NS_IMPL_COCOA) \
+  || MAC_OS_X_VERSION_MIN_REQUIRED <= 1090
+#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090
+      if ([fw respondsToSelector: @selector(useOptimizedDrawing:)])
+#endif
+        [fw useOptimizedDrawing: YES];
 #endif
       [fw setBackgroundColor: col];
       if ([col alphaComponent] != (EmacsCGFloat) 1.0)
@@ -8106,10 +8155,14 @@ - (NSRect)constrainFrameRect:(NSRect)frameRect toScreen:(NSScreen *)screen
              NSTRACE_ARG_RECT (frameRect));
 
 #ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1090
   // If separate spaces is on, it is like each screen is independent.  There is
   // no spanning of frames across screens.
-  if ([NSScreen screensHaveSeparateSpaces])
+  if (
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
+      [NSScreen respondsToSelector: @selector(screensHaveSeparateSpaces)] &&
+#endif
+      [NSScreen screensHaveSeparateSpaces])
     {
       NSTRACE_MSG ("Screens have separate spaces");
       frameRect = [super constrainFrameRect:frameRect toScreen:screen];
@@ -8117,7 +8170,8 @@ - (NSRect)constrainFrameRect:(NSRect)frameRect toScreen:(NSScreen *)screen
       return frameRect;
     }
   else
-#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9 */
+#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= 1090 */
+
     // Check that the proposed frameRect is visible in at least one
     // screen.  If it is not, ask the system to reposition it (only
     // for non-child windows).
@@ -8323,12 +8377,21 @@ + (CGFloat) scrollerWidth
   /* TODO: if we want to allow variable widths, this is the place to do it,
            however neither GNUstep nor Cocoa support it very well */
   CGFloat r;
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
-  r = [NSScroller scrollerWidth];
-#else
-  r = [NSScroller scrollerWidthForControlSize: NSControlSizeRegular
-                                scrollerStyle: NSScrollerStyleLegacy];
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+  if ([NSScroller respondsToSelector:
+                    @selector(scrollerWidthForControlSize:scrollerStyle:)])
+#endif
+    r = [NSScroller scrollerWidthForControlSize: NSControlSizeRegular
+                                  scrollerStyle: NSScrollerStyleLegacy];
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+  else
+#endif
+#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 */
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070 \
+  || defined (NS_IMPL_GNUSTEP)
+    r = [NSScroller scrollerWidth];
 #endif
   return r;
 }
@@ -9015,12 +9078,8 @@ Convert an X font name (XLFD) to an NS font name.
      doc: /*Non-nil means to use native fullscreen on Mac OS X 10.7 and later.
 Nil means use fullscreen the old (< 10.7) way.  The old way works better with
 multiple monitors, but lacks tool bar.  This variable is ignored on
-Mac OS X < 10.7.  Default is t for 10.7 and later, nil otherwise.  */);
-#ifdef HAVE_NATIVE_FS
+Mac OS X < 10.7.  Default is t.  */);
   ns_use_native_fullscreen = YES;
-#else
-  ns_use_native_fullscreen = NO;
-#endif
   ns_last_use_native_fullscreen = ns_use_native_fullscreen;
 
   DEFVAR_BOOL ("ns-use-fullscreen-animation", ns_use_fullscreen_animation,
-- 
2.12.0


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-08-01 22:03                                                     ` Alan Third
@ 2017-08-06 20:29                                                       ` Charles A. Roelli
  2017-08-06 21:29                                                         ` Alan Third
  0 siblings, 1 reply; 63+ messages in thread
From: Charles A. Roelli @ 2017-08-06 20:29 UTC (permalink / raw)
  To: Alan Third, Anders Lindgren; +Cc: 27810

[-- Attachment #1: Type: text/plain, Size: 5448 bytes --]

I ran a simple configure/compile with your patch installed, which worked 
fine.
I then tried:

./configure --with-ns CFLAGS=-DMAC_OS_X_VERSION_MIN_REQUIRED=1060 
-DMAC_OS_X_VERSION_MAX_ALLOWED=101200 -g -O3

and ran into a few errors, which should be fixed with the attached
patch applied on top of yours.  I've written notes on some of the
changed parts below.

--- a/src/macfont.h
+++ b/src/macfont.h
@@ -45,12 +45,12 @@ struct mac_glyph_layout
    CGGlyph glyph_id;
  };

-#if MAC_OS_X_VERSION_MAX_ALLOWED < 1080
+#if !defined (MAC_OS_X_VERSION_10_8)

We have to define these constants when compiling on macOS < 10.8,
since they're used by macfont.m and only available on 10.8+.

-#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 && defined (MAC_OS_X_VERSION_10_7)
    kCTFontTraitColorGlyphs = kCTFontColorGlyphsTrait
  #else
    kCTFontTraitColorGlyphs = (1 << 13)

kCTFontColorGlyphsTrait is only defined on macOS 10.7+.

--- a/src/macfont.m
+++ b/src/macfont.m
@@ -2875,7 +2875,7 @@ So we use 
CTFontDescriptorCreateMatchingFontDescriptor (no
@selector(backingScaleFactor)])
  #endif
              CGContextSetLineWidth (context, synthetic_bold_factor * 
font_size
-                                   * [[FRAME_NS_VIEW(f) window] 
backingScaleFactor]);
+                                   * [(EmacsWindow *) [FRAME_NS_VIEW(f) 
window] backingScaleFactor]);

Compiler needs a cast to EmacsWindow * here.  I add the backing scale
factor to the interface declaration of EmacsWindow here in nsterm.h:

@@ -470,6 +499,10 @@ @interface EmacsWindow : NSWindow
  {
    NSPoint grabOffset;
  }
+#if !defined (MAC_OS_X_VERSION_10_7) && MAC_OS_X_VERSION_MAX_ALLOWED >= 
1070
+- (NSRect)convertRectToScreen:(NSRect)rect;
+@property(readonly) CGFloat backingScaleFactor;
+#endif
  @end

Next, in nsfns.m:

--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -1592,7 +1592,7 @@ Frames are listed from topmost (first) to 
bottommost (last).  */)
  }

  #ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090
+#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090 && defined (MAC_OS_X_VERSION_10_9)
  #define MODAL_OK_RESPONSE NSModalResponseOK
  #endif
  #endif

NSModelResponseOK is only defined on macOS 10.9+.  The next ifndef
clause takes care of the right define on macOS below 10.9.

Next, in src/nsterm.h:

+#define NSAppKitVersionNumber10_7 1138

New define, since it's referenced verbatim in nsterm.m:7017:

#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
   if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
#endif

New scroll styles in 10.7+:

+enum {
+    NSScrollerStyleLegacy = 0,
+    NSScrollerStyleOverlay = 1
+};
+typedef NSInteger NSScrollerStyle;

Add-on to the class declaration of NSScroller (not sure if this is the
right way to do it).  Otherwise the compiler errors out on compiling
the 10.7+ call to scrollerWidthForControlSize:

+@interface NSScroller(NSObject)
++ (CGFloat)scrollerWidthForControlSize:(NSControlSize)controlSize 
scrollerStyle:(NSScrollerStyle)scrollerStyle;
+@end

Forward declarations for functions used by macfont.m (declared as weak
imports since they won't all be available unless we're on 10.8+):

+void CTFontDrawGlyphs(CTFontRef font, const CGGlyph *glyphs, const 
CGPoint *positions, size_t count, CGContextRef context) 
__attribute__((weak_import));
+#endif
+
+#if !defined (MAC_OS_X_VERSION_10_8) && MAC_OS_X_VERSION_MAX_ALLOWED >= 
1080
+extern CFArrayRef CTFontCopyDefaultCascadeListForLanguages(CTFontRef 
font, CFArrayRef languagePrefList) __attribute__((weak_import));

The compiler issued no complaints here, but the linker would not link
temacs unless the symbols were listed as permitted to be undefined,
using this in src/Makefile:

## System-specific LDFLAGS.
LD_SWITCH_SYSTEM= -Wl,-U,_CTFontCopyDefaultCascadeListForLanguages 
-Wl,-U,_CTFontDrawGlyphs

I'm not sure where to integrate this in the source tree (and if we can
conditionalize it based on what version of macOS we're building for).

If they look okay, could you please integrate these changes into your
patch?  Thanks a lot for your help on this.


On 02/08/2017 00:03, Alan Third wrote:
> On Tue, Aug 01, 2017 at 05:38:03PM +0200, Anders Lindgren wrote:
>> It's always a good idea to enable warnings when undefined preprocessor
>> symbols are used. In gcc this is -Wundef and I gess it's the same in clang.
> Unfortunately this produces an absolute ton of spurious warnings which
> scroll off my terminal buffer. I think I’ve caught all the important
> ones now, though.
>
> I’ve attached my latest go with this. I’ve removed the
> MAC_OS_X_VERSION_10_XX macros with their numbers, as we can use the
> existence of the macros to tell what platform we’re compiling on. Eg.
>
>      #if !defined (MAC_OS_X_VERSION_10_7) \
>        && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
>
> I’ve also used the NSAppKitVersionNumber macros in a few places to do
> runtime version detection. I’m not sure this is reliable. There are
> two methods of finding the OS version definitively, which we can use
> if we need to.
>
> I’ve got rid of the HAS_NATIVE_FS macro and tried to replace it with
> other checks where required. It compiles on 10.12, but I’m not at all
> convinced it will compile on 10.6.
>
> New 10.7 variables that need to be available on 10.6 when we’re
> compiling for 10.7+ have been added near the bottom of nsterm.h.
>
> Overall it looks quite different in places, but the functionality
> hasn’t really changed.


[-- Attachment #2: 0001-draft-macos-runtime-check.patch --]
[-- Type: text/x-patch, Size: 4692 bytes --]

diff --git a/src/macfont.h b/src/macfont.h
index 3289990..abb37cd 100644
--- a/src/macfont.h
+++ b/src/macfont.h
@@ -45,12 +45,12 @@ struct mac_glyph_layout
   CGGlyph glyph_id;
 };
 
-#if MAC_OS_X_VERSION_MAX_ALLOWED < 1080
+#if !defined (MAC_OS_X_VERSION_10_8)
 enum {
   kCTFontTraitItalic = kCTFontItalicTrait,
   kCTFontTraitBold = kCTFontBoldTrait,
   kCTFontTraitMonoSpace = kCTFontMonoSpaceTrait,
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 && defined (MAC_OS_X_VERSION_10_7)
   kCTFontTraitColorGlyphs = kCTFontColorGlyphsTrait
 #else
   kCTFontTraitColorGlyphs = (1 << 13)
diff --git a/src/macfont.m b/src/macfont.m
index 19145f9..91ad520 100644
--- a/src/macfont.m
+++ b/src/macfont.m
@@ -2875,7 +2875,7 @@ So we use CTFontDescriptorCreateMatchingFontDescriptor (no
                                            @selector(backingScaleFactor)])
 #endif
             CGContextSetLineWidth (context, synthetic_bold_factor * font_size
-                                   * [[FRAME_NS_VIEW(f) window] backingScaleFactor]);
+                                   * [(EmacsWindow *) [FRAME_NS_VIEW(f) window] backingScaleFactor]);
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
           else
 #endif
diff --git a/src/nsfns.m b/src/nsfns.m
index e19e4e2..3f20c8e 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -1592,7 +1592,7 @@ Frames are listed from topmost (first) to bottommost (last).  */)
 }
 
 #ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090
+#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090 && defined (MAC_OS_X_VERSION_10_9)
 #define MODAL_OK_RESPONSE NSModalResponseOK
 #endif
 #endif
diff --git a/src/nsterm.h b/src/nsterm.h
index a60f94e..7969a90 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -38,6 +38,35 @@ typedef CGFloat EmacsCGFloat;
 typedef float EmacsCGFloat;
 #endif
 
+/* macOS 10.7 introduces some new constants, enums and methods.
+   Forward declare them when building on macOS 10.6.  */
+#if !defined (MAC_OS_X_VERSION_10_7) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#define NSFullScreenWindowMask                      (1 << 14)
+#define NSWindowCollectionBehaviorFullScreenPrimary (1 << 7)
+#define NSApplicationPresentationFullScreen         (1 << 10)
+#define NSApplicationPresentationAutoHideToolbar    (1 << 11)
+#define NSAppKitVersionNumber10_7 1138
+
+// As in https://chromium.googlesource.com/chromium/blink/+/master/Source/platform/mac/NSScrollerImpDetails.h.
+enum {
+    NSScrollerStyleLegacy = 0,
+    NSScrollerStyleOverlay = 1
+};
+typedef NSInteger NSScrollerStyle;
+
+@interface NSScroller(NSObject)
++ (CGFloat)scrollerWidthForControlSize:(NSControlSize)controlSize scrollerStyle:(NSScrollerStyle)scrollerStyle;
+@end
+
+void CTFontDrawGlyphs(CTFontRef font, const CGGlyph *glyphs, const CGPoint *positions, size_t count, CGContextRef context) __attribute__((weak_import));
+#endif
+
+#if !defined (MAC_OS_X_VERSION_10_8) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1080
+extern CFArrayRef CTFontCopyDefaultCascadeListForLanguages(CTFontRef font, CFArrayRef languagePrefList) __attribute__((weak_import));
+#define kCTFontUIFontUser 0
+
+#endif
+
 /* ==========================================================================
 
    Trace support
@@ -470,6 +499,10 @@ typedef id instancetype;
 {
   NSPoint grabOffset;
 }
+#if !defined (MAC_OS_X_VERSION_10_7) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+- (NSRect)convertRectToScreen:(NSRect)rect;
+@property(readonly) CGFloat backingScaleFactor;
+#endif
 @end
 
 
@@ -1251,14 +1284,6 @@ extern char gnustep_base_version[];  /* version tracking */
                                 ? (min) : (((x)>(max)) ? (max) : (x)))
 #define SCREENMAXBOUND(x) (IN_BOUND (-SCREENMAX, x, SCREENMAX))
 
-/* macOS 10.7 introduces some new constants.  */
-#if !defined (MAC_OS_X_VERSION_10_7) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
-#define NSWindowStyleMaskFullScreen                 (1 << 14)
-#define NSWindowCollectionBehaviorFullScreenPrimary (1 << 7)
-#define NSApplicationPresentationFullScreen         (1 << 10)
-#define NSApplicationPresentationAutoHideToolbar    (1 << 11)
-#endif
-
 /* macOS 10.12 deprecates a bunch of constants. */
 #if !defined (NS_IMPL_COCOA) || !defined (MAC_OS_X_VERSION_10_12)
 #define NSEventModifierFlagCommand         NSCommandKeyMask
diff --git a/src/nsterm.m b/src/nsterm.m
index 1eaf94a..e0a977b 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -6342,7 +6342,7 @@ - (NSRect)firstRectForCharacterRange: (NSRange)theRange
     {
 #endif
       rect.origin = pt;
-      rect = [[self window] convertRectToScreen: rect];
+      rect = [(EmacsWindow *) [self window] convertRectToScreen: rect];
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
     }
   else

^ permalink raw reply related	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-08-06 20:29                                                       ` Charles A. Roelli
@ 2017-08-06 21:29                                                         ` Alan Third
  2017-08-07 19:23                                                           ` Charles A. Roelli
  0 siblings, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-08-06 21:29 UTC (permalink / raw)
  To: Charles A. Roelli; +Cc: Anders Lindgren, 27810

On Sun, Aug 06, 2017 at 10:29:49PM +0200, Charles A. Roelli wrote:
> I ran a simple configure/compile with your patch installed, which worked
> fine.
> I then tried:
> 
> ./configure --with-ns CFLAGS=-DMAC_OS_X_VERSION_MIN_REQUIRED=1060
> -DMAC_OS_X_VERSION_MAX_ALLOWED=101200 -g -O3
> 
> and ran into a few errors, which should be fixed with the attached
> patch applied on top of yours.  I've written notes on some of the
> changed parts below.

Thanks! These are exactly the kinds of errors I was expecting to see.

> Next, in src/nsterm.h:
> 
> +#define NSAppKitVersionNumber10_7 1138
> 
> New define, since it's referenced verbatim in nsterm.m:7017:

Ah, I didn’t even think of that. :)

> New scroll styles in 10.7+:
> 
> +enum {
> +    NSScrollerStyleLegacy = 0,
> +    NSScrollerStyleOverlay = 1
> +};
> +typedef NSInteger NSScrollerStyle;

I believe we can make this slightly neater:

    enum NSScrollerStyle {
      NSScrollerStyleLegacy = 0,
      NSScrollerStyleOverlay = 1
    };

> Add-on to the class declaration of NSScroller (not sure if this is the
> right way to do it).  Otherwise the compiler errors out on compiling
> the 10.7+ call to scrollerWidthForControlSize:
> 
> +@interface NSScroller(NSObject)
> ++ (CGFloat)scrollerWidthForControlSize:(NSControlSize)controlSize
> scrollerStyle:(NSScrollerStyle)scrollerStyle;
> +@end

Strange... Are you sure this one was an error and not just a warning?
We’re expecting warnings for this type of thing.

> Forward declarations for functions used by macfont.m (declared as weak
> imports since they won't all be available unless we're on 10.8+):
> 
> +void CTFontDrawGlyphs(CTFontRef font, const CGGlyph *glyphs, const CGPoint
> *positions, size_t count, CGContextRef context)
> __attribute__((weak_import));
> +#endif
> +
> +#if !defined (MAC_OS_X_VERSION_10_8) && MAC_OS_X_VERSION_MAX_ALLOWED >=
> 1080
> +extern CFArrayRef CTFontCopyDefaultCascadeListForLanguages(CTFontRef font,
> CFArrayRef languagePrefList) __attribute__((weak_import));
> 
> The compiler issued no complaints here, but the linker would not link
> temacs unless the symbols were listed as permitted to be undefined,
> using this in src/Makefile:
> 
> ## System-specific LDFLAGS.
> LD_SWITCH_SYSTEM= -Wl,-U,_CTFontCopyDefaultCascadeListForLanguages
> -Wl,-U,_CTFontDrawGlyphs

I’ve done a bit more reading up on this and I think I’ve misunderstood
how this works, and probably mislead you.

It seems these functions need to be declared as weak in the definition
of the library they’re supposed to be in. If we declare them in the
Emacs code‐base then the linker, reasonably, expects the functions to
be in the Emacs code‐base.

The top answer here might explain it better:

https://stackoverflow.com/questions/274753/how-to-make-weak-linking-work-with-gcc

I suppose this means we simply can’t guarantee perfect forward
compatibility. It’s a shame, but I don’t see a way round it.

> If they look okay, could you please integrate these changes into your
> patch?  Thanks a lot for your help on this.

If you can confirm the scrollerWidthForControlSize thing, I’ll
incorporate everything else.

Thank you for your help.

Oh, by the way, are you able to check whether the .app built on 10.6
actually runs on something higher?
-- 
Alan Third





^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-08-06 21:29                                                         ` Alan Third
@ 2017-08-07 19:23                                                           ` Charles A. Roelli
  2017-08-10 21:04                                                             ` Alan Third
  0 siblings, 1 reply; 63+ messages in thread
From: Charles A. Roelli @ 2017-08-07 19:23 UTC (permalink / raw)
  To: Alan Third; +Cc: Anders Lindgren, 27810

On 06/08/2017 23:29, Alan Third wrote:
>> New scroll styles in 10.7+:
>>
>> +enum {
>> +    NSScrollerStyleLegacy = 0,
>> +    NSScrollerStyleOverlay = 1
>> +};
>> +typedef NSInteger NSScrollerStyle;
> I believe we can make this slightly neater:
>
>      enum NSScrollerStyle {
>        NSScrollerStyleLegacy = 0,
>        NSScrollerStyleOverlay = 1
>      };

Strange, it doesn't work here:

 > nsterm.h:58: error: expected ‘)’ before ‘NSScrollerStyle’

>> Add-on to the class declaration of NSScroller (not sure if this is the
>> right way to do it).  Otherwise the compiler errors out on compiling
>> the 10.7+ call to scrollerWidthForControlSize:
>>
>> +@interface NSScroller(NSObject)
>> ++ (CGFloat)scrollerWidthForControlSize:(NSControlSize)controlSize
>> scrollerStyle:(NSScrollerStyle)scrollerStyle;
>> +@end
> Strange... Are you sure this one was an error and not just a warning?
> We’re expecting warnings for this type of thing.

Without it, I get this:

 > nsterm.m: In function ‘+[EmacsScroller scrollerWidth]’:
 > nsterm.m:8387: warning: ‘NSScroller’ may not respond to 
‘+scrollerWidthForControlSize:scrollerStyle:’
 > nsterm.m:8387: error: incompatible types in assignment

I think the compiler has to verify somehow that the result of the call 
is a CGFloat.
Casting to CGFloat gives:

 > nsterm.m:8387: error: pointer value used where a floating point value 
was expected

>> Forward declarations for functions used by macfont.m (declared as weak
>> imports since they won't all be available unless we're on 10.8+):
>>
>> +void CTFontDrawGlyphs(CTFontRef font, const CGGlyph *glyphs, const CGPoint
>> *positions, size_t count, CGContextRef context)
>> __attribute__((weak_import));
>> +#endif
>> +
>> +#if !defined (MAC_OS_X_VERSION_10_8) && MAC_OS_X_VERSION_MAX_ALLOWED >=
>> 1080
>> +extern CFArrayRef CTFontCopyDefaultCascadeListForLanguages(CTFontRef font,
>> CFArrayRef languagePrefList) __attribute__((weak_import));
>>
>> The compiler issued no complaints here, but the linker would not link
>> temacs unless the symbols were listed as permitted to be undefined,
>> using this in src/Makefile:
>>
>> ## System-specific LDFLAGS.
>> LD_SWITCH_SYSTEM= -Wl,-U,_CTFontCopyDefaultCascadeListForLanguages
>> -Wl,-U,_CTFontDrawGlyphs
> I’ve done a bit more reading up on this and I think I’ve misunderstood
> how this works, and probably mislead you.
>
> It seems these functions need to be declared as weak in the definition
> of the library they’re supposed to be in. If we declare them in the
> Emacs code‐base then the linker, reasonably, expects the functions to
> be in the Emacs code‐base.

Maybe I'm also confused.  I thought we would be able to do this,
since:

   - At link time, the symbol is marked as a weak reference, to be
     resolved at runtime.

   - At runtime, the dynamic linker resolves the reference to the weak
     symbol, setting it to NULL if it isn't available.  Normally the
     definition of the function will be found in a dynamic library that
     is part of macOS (as far as I understand).

The Apple compiler/linker should be capable of doing this, supposedly,
as long as you give the magical -Wl,-U,_symbol command line arguments
to the linker.  See also https://stackoverflow.com/a/34983229.


>> If they look okay, could you please integrate these changes into your
>> patch?  Thanks a lot for your help on this.
> If you can confirm the scrollerWidthForControlSize thing, I’ll
> incorporate everything else.
>
> Thank you for your help.
>
> Oh, by the way, are you able to check whether the .app built on 10.6
> actually runs on something higher?

I'd like to check, but wouldn't I need to either:

a) Statically link libraries Emacs depends on, or
b) Include the dependent libraries in the app bundle?

By the libraries Emacs depends on, I mean the ones starting with $HOME here:

$ otool -L src/emacs
src/emacs:
     /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit 
(compatibility version 45.0.0, current version 1038.36.0)
     /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit 
(compatibility version 1.0.0, current version 275.0.0)
     $HOME/Build/SnowLeopard/lib/libjpeg.9.dylib (compatibility version 
12.0.0, current version 12.0.0)
     $HOME/Build/SnowLeopard/lib/librsvg-2.2.dylib (compatibility 
version 43.0.0, current version 43.16.0)
     /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current 
version 125.2.11)
     $HOME/Build/SnowLeopard/lib/libgio-2.0.0.dylib (compatibility 
version 4503.0.0, current version 4503.0.0)
     $HOME/Build/SnowLeopard/lib/libgdk_pixbuf-2.0.0.dylib 
(compatibility version 3401.0.0, current version 3401.0.0)
     $HOME/Build/SnowLeopard/lib/libgobject-2.0.0.dylib (compatibility 
version 4503.0.0, current version 4503.0.0)
     $HOME/Build/SnowLeopard/lib/libglib-2.0.0.dylib (compatibility 
version 4503.0.0, current version 4503.0.0)
     $HOME/Build/SnowLeopard/lib/libintl.9.dylib (compatibility version 
11.0.0, current version 11.4.0)
     $HOME/Build/SnowLeopard/lib/libcairo.2.dylib (compatibility version 
11403.0.0, current version 11403.6.0)
     $HOME/Build/SnowLeopard/lib/libxml2.2.dylib (compatibility version 
12.0.0, current version 12.4.0)
     /usr/lib/libncurses.5.4.dylib (compatibility version 5.4.0, current 
version 5.4.0)
     $HOME/Build/SnowLeopard/lib/libgnutls.30.dylib (compatibility 
version 40.0.0, current version 40.0.0)
     /usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 
1.2.3)
     /usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current 
version 227.0.0)
/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation 
(compatibility version 150.0.0, current version 550.44.0)
/System/Library/Frameworks/ApplicationServices.framework/Versions/A/ApplicationServices 
(compatibility version 1.0.0, current version 38.0.0)
/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation 
(compatibility version 300.0.0, current version 751.63.0)

Do either of these sound feasible?





^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-08-07 19:23                                                           ` Charles A. Roelli
@ 2017-08-10 21:04                                                             ` Alan Third
  2017-08-12 11:13                                                               ` Charles A. Roelli
  0 siblings, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-08-10 21:04 UTC (permalink / raw)
  To: Charles A. Roelli; +Cc: Anders Lindgren, 27810

[-- Attachment #1: Type: text/plain, Size: 2244 bytes --]

On Mon, Aug 07, 2017 at 09:23:10PM +0200, Charles A. Roelli wrote:
> On 06/08/2017 23:29, Alan Third wrote:
> > I believe we can make this slightly neater:
> > 
> >      enum NSScrollerStyle {
> >        NSScrollerStyleLegacy = 0,
> >        NSScrollerStyleOverlay = 1
> >      };
> 
> Strange, it doesn't work here:

We’ll just go with what works, then.

> > I’ve done a bit more reading up on this and I think I’ve misunderstood
> > how this works, and probably mislead you.
> > 
> > It seems these functions need to be declared as weak in the definition
> > of the library they’re supposed to be in. If we declare them in the
> > Emacs code‐base then the linker, reasonably, expects the functions to
> > be in the Emacs code‐base.
> 
> Maybe I'm also confused.  I thought we would be able to do this,
> since:
> 
>   - At link time, the symbol is marked as a weak reference, to be
>     resolved at runtime.
> 
>   - At runtime, the dynamic linker resolves the reference to the weak
>     symbol, setting it to NULL if it isn't available.  Normally the
>     definition of the function will be found in a dynamic library that
>     is part of macOS (as far as I understand).
> 
> The Apple compiler/linker should be capable of doing this, supposedly,
> as long as you give the magical -Wl,-U,_symbol command line arguments
> to the linker.  See also https://stackoverflow.com/a/34983229.

That’s quite a good description. I guess that we want to do what
you’re suggesting, then. I’m not sure how, though. I’ll try to have a
look through configure.ac to see if I can work it out sometime over
the weekend.

> I'd like to check, but wouldn't I need to either:
> 
> a) Statically link libraries Emacs depends on, or
> b) Include the dependent libraries in the app bundle?

Yes, I suppose so. I kind of assumed it would statically link at least
some of them, but I guess not.

I’ve had a look at the build scripts for emacsformacosx.com, but I
don’t understand what they’re doing.

I’ve attached what I have so far, which I think includes all your
changes except for the requirements for linker arguments.
Unfortunately master doesn’t build here now because of some other
problem so it’s untested.
-- 
Alan Third

[-- Attachment #2: 0001-Allow-use-of-run-time-OS-version-checks-on-macOS-bug.patch --]
[-- Type: text/plain, Size: 27410 bytes --]

From c4945387576a4fa80f6d4daf34199deba003f171 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Thu, 6 Jul 2017 23:10:49 +0100
Subject: [PATCH] Allow use of run-time OS version checks on macOS (bug#27810)

* src/nsterm.h (NSWindowTabbingMode): Define in pre-Sierra macOS.
(MAC_OS_X_VERSION_10_6, MAC_OS_X_VERSION_10_7, MAC_OS_X_VERSION_10_8,
MAC_OS_X_VERSION_10_9, MAC_OS_X_VERSION_10_12, HAVE_NATIVE_FS): Remove
defines.
(NSWindowStyleMaskFullScreen,
NSWindowCollectionBehaviorFullScreenPrimary,
NSApplicationPresentationFullScreen,
NSApplicationPresentationAutoHideToolbar): Define in macOS 10.6.
* src/nsterm.m (colorForEmacsRed, colorUsingDefaultColorSpace,
check_native_fs, ns_read_socket, ns_select, runAlertPanel,
initFrameFromEmacs, windowDidMiniaturize, windowDidEnterFullScreen,
windowDidExitFullScreen, isFullscreen, updateCollectionBehavior,
toggleFullScreen, constrainFrameRect, scrollerWidth, syms_of_nsterm):
Allow use of run-time checks and replace version check macros.
(firstRectForCharacterRange): Explicit cast to (EmacsWindow *).
* src/nsfns.m (ns_screen_name): Use run-time OS version checks.
* src/macfont.m (macfont_draw): Use run-time OS version checks and
Explicit cast to (EmacsWindow *).
* src/nsmenu.m (menuWillOpen): Use run-time OS version checks.
---
 src/macfont.h |   4 +-
 src/macfont.m |  24 +++++---
 src/nsfns.m   |  83 +++++++++++++------------
 src/nsmenu.m  |   9 ++-
 src/nsterm.h  |  64 +++++++++++---------
 src/nsterm.m  | 191 ++++++++++++++++++++++++++++++++++++++--------------------
 6 files changed, 232 insertions(+), 143 deletions(-)

diff --git a/src/macfont.h b/src/macfont.h
index 32899908be..abb37cd8a6 100644
--- a/src/macfont.h
+++ b/src/macfont.h
@@ -45,12 +45,12 @@ struct mac_glyph_layout
   CGGlyph glyph_id;
 };
 
-#if MAC_OS_X_VERSION_MAX_ALLOWED < 1080
+#if !defined (MAC_OS_X_VERSION_10_8)
 enum {
   kCTFontTraitItalic = kCTFontItalicTrait,
   kCTFontTraitBold = kCTFontBoldTrait,
   kCTFontTraitMonoSpace = kCTFontMonoSpaceTrait,
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 && defined (MAC_OS_X_VERSION_10_7)
   kCTFontTraitColorGlyphs = kCTFontColorGlyphsTrait
 #else
   kCTFontTraitColorGlyphs = (1 << 13)
diff --git a/src/macfont.m b/src/macfont.m
index 4d310e47ae..a79feac8f0 100644
--- a/src/macfont.m
+++ b/src/macfont.m
@@ -2869,11 +2869,19 @@ So we use CTFontDescriptorCreateMatchingFontDescriptor (no
              and synthetic bold looks thinner on such environments.
              Apple says there are no plans to address this issue
              (rdar://11644870) currently.  So we add a workaround.  */
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-          CGContextSetLineWidth (context, synthetic_bold_factor * font_size
-                                 * [[FRAME_NS_VIEW(f) window] backingScaleFactor]);
-#else
-          CGContextSetLineWidth (context, synthetic_bold_factor * font_size);
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+          if ([[FRAME_NS_VIEW(f) window] respondsToSelector:
+                                           @selector(backingScaleFactor)])
+#endif
+            CGContextSetLineWidth (context, synthetic_bold_factor * font_size
+                                   * [(EmacsWindow *) [FRAME_NS_VIEW(f) window] backingScaleFactor]);
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+          else
+#endif
+#endif
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+            CGContextSetLineWidth (context, synthetic_bold_factor * font_size);
 #endif
           CG_SET_STROKE_COLOR_WITH_FACE_FOREGROUND (context, face, f);
         }
@@ -2883,7 +2891,7 @@ So we use CTFontDescriptorCreateMatchingFontDescriptor (no
       CGContextSetTextMatrix (context, atfm);
       CGContextSetTextPosition (context, text_position.x, text_position.y);
 
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if defined (MAC_OS_X_VERSION_10_7) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
       if (macfont_info->color_bitmap_p
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
           && CTFontDrawGlyphs != NULL
@@ -2897,7 +2905,7 @@ So we use CTFontDescriptorCreateMatchingFontDescriptor (no
             }
         }
       else
-#endif	/* MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 */
+#endif	/* defined (MAC_OS_X_VERSION_10_7) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 */
         {
           CGContextSetFont (context, macfont_info->cgfont);
           CGContextSetFontSize (context, font_size);
@@ -3892,7 +3900,7 @@ So we use CTFontDescriptorCreateMatchingFontDescriptor (no
 {
   CFArrayRef result = NULL;
 
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1080
+#if defined (MAC_OS_X_VERSION_10_8) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1080
 #if MAC_OS_X_VERSION_MIN_REQUIRED < 1080
   if (CTFontCopyDefaultCascadeListForLanguages != NULL)
 #endif
diff --git a/src/nsfns.m b/src/nsfns.m
index 36748cebb8..3f20c8e9e9 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -1592,7 +1592,7 @@ Frames are listed from topmost (first) to bottommost (last).  */)
 }
 
 #ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED > MAC_OS_X_VERSION_10_9
+#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090 && defined (MAC_OS_X_VERSION_10_9)
 #define MODAL_OK_RESPONSE NSModalResponseOK
 #endif
 #endif
@@ -2512,52 +2512,61 @@ and GNUstep implementations ("distributor-specific release
 {
   char *name = NULL;
 
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
-  mach_port_t masterPort;
-  io_iterator_t it;
-  io_object_t obj;
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1090
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
+  if (CGDisplayIOServicePort == NULL)
+#endif
+    {
+      mach_port_t masterPort;
+      io_iterator_t it;
+      io_object_t obj;
 
-  // CGDisplayIOServicePort is deprecated.  Do it another (harder) way.
+      /* CGDisplayIOServicePort is deprecated.  Do it another (harder) way.
 
-  if (IOMasterPort (MACH_PORT_NULL, &masterPort) != kIOReturnSuccess
-      || IOServiceGetMatchingServices (masterPort,
-                                       IOServiceMatching ("IONDRVDevice"),
-                                       &it) != kIOReturnSuccess)
-    return name;
+         Is this code OK for macOS < 10.9, and GNUstep?  I suspect it is,
+         in which case is it worth keeping the other method in here? */
 
-  /* Must loop until we find a name.  Many devices can have the same unit
-     number (represents different GPU parts), but only one has a name.  */
-  while (! name && (obj = IOIteratorNext (it)))
-    {
-      CFMutableDictionaryRef props;
-      const void *val;
-
-      if (IORegistryEntryCreateCFProperties (obj,
-                                             &props,
-                                             kCFAllocatorDefault,
-                                             kNilOptions) == kIOReturnSuccess
-          && props != nil
-          && (val = CFDictionaryGetValue(props, @"IOFBDependentIndex")))
+      if (IOMasterPort (MACH_PORT_NULL, &masterPort) != kIOReturnSuccess
+          || IOServiceGetMatchingServices (masterPort,
+                                           IOServiceMatching ("IONDRVDevice"),
+                                           &it) != kIOReturnSuccess)
+        return name;
+
+      /* Must loop until we find a name.  Many devices can have the same unit
+         number (represents different GPU parts), but only one has a name.  */
+      while (! name && (obj = IOIteratorNext (it)))
         {
-          unsigned nr = [(NSNumber *)val unsignedIntegerValue];
-          if (nr == CGDisplayUnitNumber (did))
-            name = ns_get_name_from_ioreg (obj);
+          CFMutableDictionaryRef props;
+          const void *val;
+
+          if (IORegistryEntryCreateCFProperties (obj,
+                                                 &props,
+                                                 kCFAllocatorDefault,
+                                                 kNilOptions) == kIOReturnSuccess
+              && props != nil
+              && (val = CFDictionaryGetValue(props, @"IOFBDependentIndex")))
+            {
+              unsigned nr = [(NSNumber *)val unsignedIntegerValue];
+              if (nr == CGDisplayUnitNumber (did))
+                name = ns_get_name_from_ioreg (obj);
+            }
+
+          CFRelease (props);
+          IOObjectRelease (obj);
         }
 
-      CFRelease (props);
-      IOObjectRelease (obj);
+      IOObjectRelease (it);
     }
-
-  IOObjectRelease (it);
-
-#else
-
-  name = ns_get_name_from_ioreg (CGDisplayIOServicePort (did));
-
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
+  else
+#endif
+#endif /* #if MAC_OS_X_VERSION_MAX_ALLOWED >= 1090 */
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
+    name = ns_get_name_from_ioreg (CGDisplayIOServicePort (did));
 #endif
   return name;
 }
-#endif
+#endif /* NS_IMPL_COCOA */
 
 static Lisp_Object
 ns_make_monitor_attribute_list (struct MonitorInfo *monitors,
diff --git a/src/nsmenu.m b/src/nsmenu.m
index 37a1a62d6d..93e06707c0 100644
--- a/src/nsmenu.m
+++ b/src/nsmenu.m
@@ -532,9 +532,14 @@ - (void)menuWillOpen:(NSMenu *)menu
 {
   ++trackingMenu;
 
-#if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
   // On 10.6 we get repeated calls, only the one for NSSystemDefined is "real".
-  if ([[NSApp currentEvent] type] != NSSystemDefined) return;
+  if (
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+      NSAppKitVersionNumber < NSAppKitVersionNumber10_7 &&
+#endif
+      [[NSApp currentEvent] type] != NSEventTypeSystemDefined)
+    return;
 #endif
 
   /* When dragging from one menu to another, we get willOpen followed by didClose,
diff --git a/src/nsterm.h b/src/nsterm.h
index 0f1b36db7b..750b6fe1fe 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -25,30 +25,6 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "sysselect.h"
 
 #ifdef HAVE_NS
-
-#ifdef NS_IMPL_COCOA
-#ifndef MAC_OS_X_VERSION_10_6
-#define MAC_OS_X_VERSION_10_6 1060
-#endif
-#ifndef MAC_OS_X_VERSION_10_7
-#define MAC_OS_X_VERSION_10_7 1070
-#endif
-#ifndef MAC_OS_X_VERSION_10_8
-#define MAC_OS_X_VERSION_10_8 1080
-#endif
-#ifndef MAC_OS_X_VERSION_10_9
-#define MAC_OS_X_VERSION_10_9 1090
-#endif
-#ifndef MAC_OS_X_VERSION_10_12
-#define MAC_OS_X_VERSION_10_12 101200
-#endif
-
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-#define HAVE_NATIVE_FS
-#endif
-
-#endif /* NS_IMPL_COCOA */
-
 #ifdef __OBJC__
 
 /* CGFloat on GNUstep may be 4 or 8 byte, but functions expect float* for some
@@ -62,6 +38,27 @@ typedef CGFloat EmacsCGFloat;
 typedef float EmacsCGFloat;
 #endif
 
+/* macOS 10.7 introduces some new constants, enums and methods.
+   Forward declare them when building on macOS 10.6.  */
+#if !defined (MAC_OS_X_VERSION_10_7) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#define NSFullScreenWindowMask                      (1 << 14)
+#define NSWindowCollectionBehaviorFullScreenPrimary (1 << 7)
+#define NSApplicationPresentationFullScreen         (1 << 10)
+#define NSApplicationPresentationAutoHideToolbar    (1 << 11)
+#define NSAppKitVersionNumber10_7                   1138
+
+// As in https://chromium.googlesource.com/chromium/blink/+/master/Source/platform/mac/NSScrollerImpDetails.h.
+enum {
+  NSScrollerStyleLegacy = 0,
+  NSScrollerStyleOverlay = 1
+};
+typedef NSInteger NSScrollerStyle;
+
+@interface NSScroller(NSObject)
++ (CGFloat)scrollerWidthForControlSize:(NSControlSize)controlSize scrollerStyle:(NSScrollerStyle)scrollerStyle;
+@end
+#endif /* !defined (MAC_OS_X_VERSION_10_7) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 */
+
 /* ==========================================================================
 
    Trace support
@@ -471,7 +468,7 @@ typedef id instancetype;
 - (void) toggleFullScreen: (id) sender;
 - (BOOL) fsIsNative;
 - (BOOL) isFullscreen;
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 - (void) updateCollectionBehavior;
 #endif
 
@@ -494,6 +491,10 @@ typedef id instancetype;
 {
   NSPoint grabOffset;
 }
+#if !defined (MAC_OS_X_VERSION_10_7) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+- (NSRect)convertRectToScreen:(NSRect)rect;
+@property(readonly) CGFloat backingScaleFactor;
+#endif
 @end
 
 
@@ -1276,8 +1277,7 @@ extern char gnustep_base_version[];  /* version tracking */
 #define SCREENMAXBOUND(x) (IN_BOUND (-SCREENMAX, x, SCREENMAX))
 
 /* macOS 10.12 deprecates a bunch of constants. */
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_12
+#if !defined (NS_IMPL_COCOA) || !defined (MAC_OS_X_VERSION_10_12)
 #define NSEventModifierFlagCommand         NSCommandKeyMask
 #define NSEventModifierFlagControl         NSControlKeyMask
 #define NSEventModifierFlagHelp            NSHelpKeyMask
@@ -1303,6 +1303,7 @@ extern char gnustep_base_version[];  /* version tracking */
 #define NSEventTypeKeyUp                   NSKeyUp
 #define NSEventTypeFlagsChanged            NSFlagsChanged
 #define NSEventMaskAny                     NSAnyEventMask
+#define NSEventTypeSystemDefined           NSSystemDefined
 #define NSWindowStyleMaskBorderless        NSBorderlessWindowMask
 #define NSWindowStyleMaskClosable          NSClosableWindowMask
 #define NSWindowStyleMaskFullScreen        NSFullScreenWindowMask
@@ -1317,6 +1318,13 @@ extern char gnustep_base_version[];  /* version tracking */
 #ifdef __OBJC__
 typedef NSUInteger NSWindowStyleMask;
 #endif
-#endif
 
+/* Window tabbing mode enums are new too. */
+enum NSWindowTabbingMode
+  {
+    NSWindowTabbingModeAutomatic,
+    NSWindowTabbingModePreferred,
+    NSWindowTabbingModeDisallowed
+  };
+#endif
 #endif	/* HAVE_NS */
diff --git a/src/nsterm.m b/src/nsterm.m
index 36d906a7ce..e0a977b927 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -136,14 +136,18 @@ @implementation NSColor (EmacsColor)
 + (NSColor *)colorForEmacsRed:(CGFloat)red green:(CGFloat)green
                          blue:(CGFloat)blue alpha:(CGFloat)alpha
 {
-#ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-  if (ns_use_srgb_colorspace)
-      return [NSColor colorWithSRGBRed: red
-                                 green: green
-                                  blue: blue
-                                 alpha: alpha];
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+  if (ns_use_srgb_colorspace
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+      && [NSColor respondsToSelector:
+                    @selector(colorWithSRGBRed:green:blue:alpha:)]
 #endif
+      )
+    return [NSColor colorWithSRGBRed: red
+                               green: green
+                                blue: blue
+                               alpha: alpha];
 #endif
   return [NSColor colorWithCalibratedRed: red
                                    green: green
@@ -153,11 +157,18 @@ + (NSColor *)colorForEmacsRed:(CGFloat)red green:(CGFloat)green
 
 - (NSColor *)colorUsingDefaultColorSpace
 {
-#ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-  if (ns_use_srgb_colorspace)
-    return [self colorUsingColorSpace: [NSColorSpace sRGBColorSpace]];
+  /* FIXMES: We're checking for colorWithSRGBRed here so this will
+     only work in the same place as in the method above.  It should
+     really be a check whether we're on macOS 10.7 or above. */
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+  if (ns_use_srgb_colorspace
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+      && [NSColor respondsToSelector:
+                    @selector(colorWithSRGBRed:green:blue:alpha:)]
 #endif
+      )
+    return [self colorUsingColorSpace: [NSColorSpace sRGBColorSpace]];
 #endif
   return [self colorUsingColorSpaceName: NSCalibratedRGBColorSpace];
 }
@@ -4140,7 +4151,7 @@ in certain situations (rapid incoming events).
     }
 }
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 static void
 check_native_fs ()
 {
@@ -4242,7 +4253,7 @@ in certain situations (rapid incoming events).
 
   NSTRACE_WHEN (NSTRACE_GROUP_EVENTS, "ns_read_socket");
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
   check_native_fs ();
 #endif
 
@@ -4324,7 +4335,7 @@ in certain situations (rapid incoming events).
 
   NSTRACE_WHEN (NSTRACE_GROUP_EVENTS, "ns_select");
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
   check_native_fs ();
 #endif
 
@@ -5563,8 +5574,7 @@ - (void) terminate: (id)sender
               NSString *defaultButton,
               NSString *alternateButton)
 {
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
+#ifdef NS_IMPL_GNUSTEP
   return NSRunAlertPanel(title, msgFormat, defaultButton, alternateButton, nil)
     == NSAlertDefaultReturn;
 #else
@@ -6325,14 +6335,27 @@ - (NSRect)firstRectForCharacterRange: (NSRange)theRange
                                        +FRAME_LINE_HEIGHT (emacsframe));
 
   pt = [self convertPoint: pt toView: nil];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
-  pt = [[self window] convertBaseToScreen: pt];
-  rect.origin = pt;
-#else
-  rect.origin = pt;
-  rect = [[self window] convertRectToScreen: rect];
+
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+  if ([[self window] respondsToSelector: @selector(convertRectToScreen:)])
+    {
 #endif
+      rect.origin = pt;
+      rect = [(EmacsWindow *) [self window] convertRectToScreen: rect];
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+    }
+  else
+#endif
+#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 */
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070 \
+  || defined (NS_IMPL_GNUSTEP)
+    {
+      pt = [[self window] convertBaseToScreen: pt];
+      rect.origin = pt;
+    }
+#endif
+
   return rect;
 }
 
@@ -6988,11 +7011,15 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   scrollbarsNeedingUpdate = 0;
   fs_state = FULLSCREEN_NONE;
   fs_before_fs = next_maximized = -1;
-#ifdef HAVE_NATIVE_FS
-  fs_is_native = ns_use_native_fullscreen;
-#else
+
   fs_is_native = NO;
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+  if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
 #endif
+    fs_is_native = ns_use_native_fullscreen;
+#endif
+
   maximized_width = maximized_height = -1;
   nonfs_window = nil;
 
@@ -7023,7 +7050,10 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
                         backing: NSBackingStoreBuffered
                           defer: YES];
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+  if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
+#endif
     [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
 #endif
 
@@ -7032,9 +7062,11 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 
   [win setAcceptsMouseMovedEvents: YES];
   [win setDelegate: self];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-  [win useOptimizedDrawing: YES];
+#if !defined (NS_IMPL_COCOA) || MAC_OS_X_VERSION_MIN_REQUIRED <= 1090
+#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090
+  if ([win respondsToSelector: @selector(useOptimizedDrawing:)])
+#endif
+    [win useOptimizedDrawing: YES];
 #endif
 
   [[win contentView] addSubview: self];
@@ -7094,9 +7126,12 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   if ([col alphaComponent] != (EmacsCGFloat) 1.0)
     [win setOpaque: NO];
 
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-  [self allocateGState];
+#if !defined (NS_IMPL_COCOA) \
+  || MAC_OS_X_VERSION_MIN_REQUIRED <= 1090
+#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090
+  if ([self respondsToSelector: @selector(allocateGState)])
+#endif
+    [self allocateGState];
 #endif
   [NSApp registerServicesMenuSendTypes: ns_send_types
                            returnTypes: [NSArray array]];
@@ -7104,9 +7139,12 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   /* macOS Sierra automatically enables tabbed windows.  We can't
      allow this to be enabled until it's available on a Free system.
      Currently it only happens by accident and is buggy anyway. */
-#if defined (NS_IMPL_COCOA) && \
-  MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_12
-  [win setTabbingMode: NSWindowTabbingModeDisallowed];
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= 101200
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 101200
+  if ([win respondsToSelector: @selector(setTabbingMode:)])
+#endif
+    [win setTabbingMode: NSWindowTabbingModeDisallowed];
 #endif
 
   ns_window_num++;
@@ -7323,7 +7361,7 @@ - (void)windowDidMiniaturize: sender
     }
 }
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 - (NSApplicationPresentationOptions)window:(NSWindow *)window
       willUseFullScreenPresentationOptions:
   (NSApplicationPresentationOptions)proposedOptions
@@ -7361,8 +7399,8 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
   else
     {
       BOOL tbar_visible = FRAME_EXTERNAL_TOOL_BAR (emacsframe) ? YES : NO;
-#ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070        \
+  && MAC_OS_X_VERSION_MIN_REQUIRED <= 1070
       unsigned val = (unsigned)[NSApp presentationOptions];
 
       // Mac OS X 10.7 bug fix, the menu won't appear without this.
@@ -7378,7 +7416,6 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
           [NSApp setPresentationOptions: options];
         }
 #endif
-#endif
       [toolbar setVisible:tbar_visible];
     }
 }
@@ -7417,7 +7454,7 @@ - (void)windowDidExitFullScreen /* provided for direct calls */
     }
   [self setFSValue: fs_before_fs];
   fs_before_fs = -1;
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
   [self updateCollectionBehavior];
 #endif
   if (FRAME_EXTERNAL_TOOL_BAR (emacsframe))
@@ -7449,7 +7486,7 @@ - (BOOL)isFullscreen
     }
   else
     {
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
       res = (([[self window] styleMask] & NSWindowStyleMaskFullScreen) != 0);
 #else
       res = NO;
@@ -7462,7 +7499,7 @@ - (BOOL)isFullscreen
   return res;
 }
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 - (void)updateCollectionBehavior
 {
   NSTRACE ("[EmacsView updateCollectionBehavior]");
@@ -7477,7 +7514,10 @@ - (void)updateCollectionBehavior
         b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
 
       [win setCollectionBehavior: b];
-      fs_is_native = ns_use_native_fullscreen;
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+      if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
+#endif
+        fs_is_native = ns_use_native_fullscreen;
     }
 }
 #endif
@@ -7494,8 +7534,11 @@ - (void)toggleFullScreen: (id)sender
 
   if (fs_is_native)
     {
-#ifdef HAVE_NATIVE_FS
-      [[self window] toggleFullScreen:sender];
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+      if ([[self window] respondsToSelector: @selector(toggleFullScreen:)])
+#endif
+        [[self window] toggleFullScreen:sender];
 #endif
       return;
     }
@@ -7512,10 +7555,13 @@ - (void)toggleFullScreen: (id)sender
     {
       NSScreen *screen = [w screen];
 
-#if defined (NS_IMPL_COCOA) && \
-  MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1090
       /* Hide ghost menu bar on secondary monitor? */
-      if (! onFirstScreen)
+      if (! onFirstScreen
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
+          && [NSScreen respondsToSelector: @selector(screensHaveSeparateSpaces)]
+#endif
+          )
         onFirstScreen = [NSScreen screensHaveSeparateSpaces];
 #endif
       /* Hide dock and menubar if we are on the primary screen.  */
@@ -7543,9 +7589,12 @@ - (void)toggleFullScreen: (id)sender
       [fw setTitle:[w title]];
       [fw setDelegate:self];
       [fw setAcceptsMouseMovedEvents: YES];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-      [fw useOptimizedDrawing: YES];
+#if !defined (NS_IMPL_COCOA) \
+  || MAC_OS_X_VERSION_MIN_REQUIRED <= 1090
+#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090
+      if ([fw respondsToSelector: @selector(useOptimizedDrawing:)])
+#endif
+        [fw useOptimizedDrawing: YES];
 #endif
       [fw setBackgroundColor: col];
       if ([col alphaComponent] != (EmacsCGFloat) 1.0)
@@ -8106,10 +8155,14 @@ - (NSRect)constrainFrameRect:(NSRect)frameRect toScreen:(NSScreen *)screen
              NSTRACE_ARG_RECT (frameRect));
 
 #ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1090
   // If separate spaces is on, it is like each screen is independent.  There is
   // no spanning of frames across screens.
-  if ([NSScreen screensHaveSeparateSpaces])
+  if (
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
+      [NSScreen respondsToSelector: @selector(screensHaveSeparateSpaces)] &&
+#endif
+      [NSScreen screensHaveSeparateSpaces])
     {
       NSTRACE_MSG ("Screens have separate spaces");
       frameRect = [super constrainFrameRect:frameRect toScreen:screen];
@@ -8117,7 +8170,8 @@ - (NSRect)constrainFrameRect:(NSRect)frameRect toScreen:(NSScreen *)screen
       return frameRect;
     }
   else
-#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9 */
+#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= 1090 */
+
     // Check that the proposed frameRect is visible in at least one
     // screen.  If it is not, ask the system to reposition it (only
     // for non-child windows).
@@ -8323,12 +8377,21 @@ + (CGFloat) scrollerWidth
   /* TODO: if we want to allow variable widths, this is the place to do it,
            however neither GNUstep nor Cocoa support it very well */
   CGFloat r;
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
-  r = [NSScroller scrollerWidth];
-#else
-  r = [NSScroller scrollerWidthForControlSize: NSControlSizeRegular
-                                scrollerStyle: NSScrollerStyleLegacy];
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+  if ([NSScroller respondsToSelector:
+                    @selector(scrollerWidthForControlSize:scrollerStyle:)])
+#endif
+    r = [NSScroller scrollerWidthForControlSize: NSControlSizeRegular
+                                  scrollerStyle: NSScrollerStyleLegacy];
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+  else
+#endif
+#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 */
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070 \
+  || defined (NS_IMPL_GNUSTEP)
+    r = [NSScroller scrollerWidth];
 #endif
   return r;
 }
@@ -9015,12 +9078,8 @@ Convert an X font name (XLFD) to an NS font name.
      doc: /*Non-nil means to use native fullscreen on Mac OS X 10.7 and later.
 Nil means use fullscreen the old (< 10.7) way.  The old way works better with
 multiple monitors, but lacks tool bar.  This variable is ignored on
-Mac OS X < 10.7.  Default is t for 10.7 and later, nil otherwise.  */);
-#ifdef HAVE_NATIVE_FS
+Mac OS X < 10.7.  Default is t.  */);
   ns_use_native_fullscreen = YES;
-#else
-  ns_use_native_fullscreen = NO;
-#endif
   ns_last_use_native_fullscreen = ns_use_native_fullscreen;
 
   DEFVAR_BOOL ("ns-use-fullscreen-animation", ns_use_fullscreen_animation,
-- 
2.12.0


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-08-10 21:04                                                             ` Alan Third
@ 2017-08-12 11:13                                                               ` Charles A. Roelli
  2017-08-12 13:02                                                                 ` Charles A. Roelli
  2017-08-12 15:51                                                                 ` Alan Third
  0 siblings, 2 replies; 63+ messages in thread
From: Charles A. Roelli @ 2017-08-12 11:13 UTC (permalink / raw)
  To: Alan Third; +Cc: Anders Lindgren, 27810

Hm, on second thoughts, it seems a bit overwrought to try doing this
weak linking only for the sake of forward-compatible builds.  It
should be enough to support only backward-compatible builds, so that
we might one day distribute Emacs as a .dmg (built on the latest macOS
and backwards-compatible with the oldest version of macOS that we
support).  I don't want to waste time adding code that will hardly
ever be run, so we can take out those forward declarations.  Sorry for
the trouble!  I will send a patch on top of your new one that removes
them (I'm also having build trouble at the moment).

I also looked at the emacsformacosx.com build scripts, and it seems
like they're making copies of Emacs' dependent dynamic libraries,
including them in the application bundle, then using
install_name_tool(1) to patch the Emacs binary to depend on them (I
don't understand, though, how those scripts resolve dependencies
between the dynamic libraries themselves).


On 10/08/2017 23:04, Alan Third wrote:
> On Mon, Aug 07, 2017 at 09:23:10PM +0200, Charles A. Roelli wrote:
>> On 06/08/2017 23:29, Alan Third wrote:
>>> I believe we can make this slightly neater:
>>>
>>>       enum NSScrollerStyle {
>>>         NSScrollerStyleLegacy = 0,
>>>         NSScrollerStyleOverlay = 1
>>>       };
>> Strange, it doesn't work here:
> We’ll just go with what works, then.
>
>>> I’ve done a bit more reading up on this and I think I’ve misunderstood
>>> how this works, and probably mislead you.
>>>
>>> It seems these functions need to be declared as weak in the definition
>>> of the library they’re supposed to be in. If we declare them in the
>>> Emacs code‐base then the linker, reasonably, expects the functions to
>>> be in the Emacs code‐base.
>> Maybe I'm also confused.  I thought we would be able to do this,
>> since:
>>
>>    - At link time, the symbol is marked as a weak reference, to be
>>      resolved at runtime.
>>
>>    - At runtime, the dynamic linker resolves the reference to the weak
>>      symbol, setting it to NULL if it isn't available.  Normally the
>>      definition of the function will be found in a dynamic library that
>>      is part of macOS (as far as I understand).
>>
>> The Apple compiler/linker should be capable of doing this, supposedly,
>> as long as you give the magical -Wl,-U,_symbol command line arguments
>> to the linker.  See also https://stackoverflow.com/a/34983229.
> That’s quite a good description. I guess that we want to do what
> you’re suggesting, then. I’m not sure how, though. I’ll try to have a
> look through configure.ac to see if I can work it out sometime over
> the weekend.
>
>> I'd like to check, but wouldn't I need to either:
>>
>> a) Statically link libraries Emacs depends on, or
>> b) Include the dependent libraries in the app bundle?
> Yes, I suppose so. I kind of assumed it would statically link at least
> some of them, but I guess not.
>
> I’ve had a look at the build scripts for emacsformacosx.com, but I
> don’t understand what they’re doing.
>
> I’ve attached what I have so far, which I think includes all your
> changes except for the requirements for linker arguments.
> Unfortunately master doesn’t build here now because of some other
> problem so it’s untested.






^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-08-12 11:13                                                               ` Charles A. Roelli
@ 2017-08-12 13:02                                                                 ` Charles A. Roelli
  2017-08-16 20:31                                                                   ` Alan Third
  2017-08-12 15:51                                                                 ` Alan Third
  1 sibling, 1 reply; 63+ messages in thread
From: Charles A. Roelli @ 2017-08-12 13:02 UTC (permalink / raw)
  To: Alan Third; +Cc: Anders Lindgren, 27810

[-- Attachment #1: Type: text/plain, Size: 3510 bytes --]

(I fixed the build issue with 'make bootstrap')

Attached is the patch to revert my changes.


On 12/08/2017 13:13, Charles A. Roelli wrote:
> Hm, on second thoughts, it seems a bit overwrought to try doing this
> weak linking only for the sake of forward-compatible builds.  It
> should be enough to support only backward-compatible builds, so that
> we might one day distribute Emacs as a .dmg (built on the latest macOS
> and backwards-compatible with the oldest version of macOS that we
> support).  I don't want to waste time adding code that will hardly
> ever be run, so we can take out those forward declarations.  Sorry for
> the trouble!  I will send a patch on top of your new one that removes
> them (I'm also having build trouble at the moment).
>
> I also looked at the emacsformacosx.com build scripts, and it seems
> like they're making copies of Emacs' dependent dynamic libraries,
> including them in the application bundle, then using
> install_name_tool(1) to patch the Emacs binary to depend on them (I
> don't understand, though, how those scripts resolve dependencies
> between the dynamic libraries themselves).
>
>
> On 10/08/2017 23:04, Alan Third wrote:
>> On Mon, Aug 07, 2017 at 09:23:10PM +0200, Charles A. Roelli wrote:
>>> On 06/08/2017 23:29, Alan Third wrote:
>>>> I believe we can make this slightly neater:
>>>>
>>>>       enum NSScrollerStyle {
>>>>         NSScrollerStyleLegacy = 0,
>>>>         NSScrollerStyleOverlay = 1
>>>>       };
>>> Strange, it doesn't work here:
>> We’ll just go with what works, then.
>>
>>>> I’ve done a bit more reading up on this and I think I’ve misunderstood
>>>> how this works, and probably mislead you.
>>>>
>>>> It seems these functions need to be declared as weak in the definition
>>>> of the library they’re supposed to be in. If we declare them in the
>>>> Emacs code‐base then the linker, reasonably, expects the functions to
>>>> be in the Emacs code‐base.
>>> Maybe I'm also confused.  I thought we would be able to do this,
>>> since:
>>>
>>>    - At link time, the symbol is marked as a weak reference, to be
>>>      resolved at runtime.
>>>
>>>    - At runtime, the dynamic linker resolves the reference to the weak
>>>      symbol, setting it to NULL if it isn't available. Normally the
>>>      definition of the function will be found in a dynamic library that
>>>      is part of macOS (as far as I understand).
>>>
>>> The Apple compiler/linker should be capable of doing this, supposedly,
>>> as long as you give the magical -Wl,-U,_symbol command line arguments
>>> to the linker.  See also https://stackoverflow.com/a/34983229.
>> That’s quite a good description. I guess that we want to do what
>> you’re suggesting, then. I’m not sure how, though. I’ll try to have a
>> look through configure.ac to see if I can work it out sometime over
>> the weekend.
>>
>>> I'd like to check, but wouldn't I need to either:
>>>
>>> a) Statically link libraries Emacs depends on, or
>>> b) Include the dependent libraries in the app bundle?
>> Yes, I suppose so. I kind of assumed it would statically link at least
>> some of them, but I guess not.
>>
>> I’ve had a look at the build scripts for emacsformacosx.com, but I
>> don’t understand what they’re doing.
>>
>> I’ve attached what I have so far, which I think includes all your
>> changes except for the requirements for linker arguments.
>> Unfortunately master doesn’t build here now because of some other
>> problem so it’s untested.
>


[-- Attachment #2: remove-forward-compatibility-changes.diff --]
[-- Type: application/diff, Size: 4320 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-08-12 11:13                                                               ` Charles A. Roelli
  2017-08-12 13:02                                                                 ` Charles A. Roelli
@ 2017-08-12 15:51                                                                 ` Alan Third
  2017-09-12 20:01                                                                   ` David Caldwell
  1 sibling, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-08-12 15:51 UTC (permalink / raw)
  To: Charles A. Roelli
  Cc: David Reitter, david+emacsformacosx, Anders Lindgren, 27810

[-- Attachment #1: Type: text/plain, Size: 2287 bytes --]

On Sat, Aug 12, 2017 at 01:13:56PM +0200, Charles A. Roelli wrote:
> Hm, on second thoughts, it seems a bit overwrought to try doing this
> weak linking only for the sake of forward-compatible builds.  It
> should be enough to support only backward-compatible builds, so that
> we might one day distribute Emacs as a .dmg (built on the latest macOS
> and backwards-compatible with the oldest version of macOS that we
> support).

I think this makes sense. Especially given we’re not able to actually
create a stand‐alone .app without implementing something like...

> I also looked at the emacsformacosx.com build scripts, and it seems
> like they're making copies of Emacs' dependent dynamic libraries,
> including them in the application bundle, then using
> install_name_tool(1) to patch the Emacs binary to depend on them (I
> don't understand, though, how those scripts resolve dependencies
> between the dynamic libraries themselves).

I wouldn’t have a problem with putting this capability in, but I don’t
have the knowledge nor the inclination to do it myself.

I was going to write something up in INSTALL about building with
feature detection, but I really don’t know how to put it. I don’t want
to give the impression that if you use
-DMAC_OS_X_VERSION_MIN_ALLOWED=1060 that it will magically build a
portable .app. I began to wonder if it’s worth mentioning at all since
I doubt any more than a handful of people will be interested in
building with this option.

David and David, I hope it’s OK to include you in this. I thought you
might be interested and perhaps have some thoughts on what we’re
doing.

Basically, instead of detecting all macOS features at compile‐time,
we’ve stuck in some code to detect them at run‐time. It causes
compiler warnings, so by default it still limits features to those
available at build‐time, but if you do something like:

    ./configure --with-ns CFLAGS="-DMAC_OS_X_VERSION_MIN_REQUIRED=1060 -O3 -g"

when building on macOS Sierra, you should, in theory, end up with an
executable that will work correctly on every version of macOS back to
10.6, inclusive. We haven’t been able to properly test portability yet
as it requires including dynamic libraries.

The patch is attached to this email.
-- 
Alan Third

[-- Attachment #2: 0001-Allow-use-of-run-time-OS-version-checks-on-macOS-bug.patch --]
[-- Type: text/plain, Size: 24519 bytes --]

From bf69eba8c699b8d2ed4e4c7a18496a74444739fe Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Thu, 6 Jul 2017 23:10:49 +0100
Subject: [PATCH] Allow use of run-time OS version checks on macOS (bug#27810)

* src/nsterm.h (NSWindowTabbingMode): Define in pre-Sierra macOS.
(MAC_OS_X_VERSION_10_6, MAC_OS_X_VERSION_10_7, MAC_OS_X_VERSION_10_8,
MAC_OS_X_VERSION_10_9, MAC_OS_X_VERSION_10_12, HAVE_NATIVE_FS): Remove
defines.
(NSWindowStyleMaskFullScreen,
NSWindowCollectionBehaviorFullScreenPrimary,
NSApplicationPresentationFullScreen,
NSApplicationPresentationAutoHideToolbar): Define in macOS 10.6.
* src/nsterm.m (colorForEmacsRed, colorUsingDefaultColorSpace,
check_native_fs, ns_read_socket, ns_select, runAlertPanel,
initFrameFromEmacs, windowDidMiniaturize, windowDidEnterFullScreen,
windowDidExitFullScreen, isFullscreen, updateCollectionBehavior,
toggleFullScreen, constrainFrameRect, scrollerWidth, syms_of_nsterm):
Allow use of run-time checks and replace version check macros.
* src/nsfns.m (ns_screen_name): Use run-time OS version checks.
* src/macfont.m (macfont_draw): Use run-time OS version checks.
* src/nsmenu.m (menuWillOpen): Use run-time OS version checks.
---
 src/macfont.m |  18 ++++--
 src/nsfns.m   |  83 +++++++++++++------------
 src/nsmenu.m  |   9 ++-
 src/nsterm.h  |  48 ++++++---------
 src/nsterm.m  | 191 ++++++++++++++++++++++++++++++++++++++--------------------
 5 files changed, 211 insertions(+), 138 deletions(-)

diff --git a/src/macfont.m b/src/macfont.m
index 4d310e47ae..19145f92c0 100644
--- a/src/macfont.m
+++ b/src/macfont.m
@@ -2869,11 +2869,19 @@ So we use CTFontDescriptorCreateMatchingFontDescriptor (no
              and synthetic bold looks thinner on such environments.
              Apple says there are no plans to address this issue
              (rdar://11644870) currently.  So we add a workaround.  */
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-          CGContextSetLineWidth (context, synthetic_bold_factor * font_size
-                                 * [[FRAME_NS_VIEW(f) window] backingScaleFactor]);
-#else
-          CGContextSetLineWidth (context, synthetic_bold_factor * font_size);
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+          if ([[FRAME_NS_VIEW(f) window] respondsToSelector:
+                                           @selector(backingScaleFactor)])
+#endif
+            CGContextSetLineWidth (context, synthetic_bold_factor * font_size
+                                   * [[FRAME_NS_VIEW(f) window] backingScaleFactor]);
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+          else
+#endif
+#endif
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+            CGContextSetLineWidth (context, synthetic_bold_factor * font_size);
 #endif
           CG_SET_STROKE_COLOR_WITH_FACE_FOREGROUND (context, face, f);
         }
diff --git a/src/nsfns.m b/src/nsfns.m
index 36748cebb8..e19e4e2641 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -1592,7 +1592,7 @@ Frames are listed from topmost (first) to bottommost (last).  */)
 }
 
 #ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED > MAC_OS_X_VERSION_10_9
+#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090
 #define MODAL_OK_RESPONSE NSModalResponseOK
 #endif
 #endif
@@ -2512,52 +2512,61 @@ and GNUstep implementations ("distributor-specific release
 {
   char *name = NULL;
 
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
-  mach_port_t masterPort;
-  io_iterator_t it;
-  io_object_t obj;
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1090
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
+  if (CGDisplayIOServicePort == NULL)
+#endif
+    {
+      mach_port_t masterPort;
+      io_iterator_t it;
+      io_object_t obj;
 
-  // CGDisplayIOServicePort is deprecated.  Do it another (harder) way.
+      /* CGDisplayIOServicePort is deprecated.  Do it another (harder) way.
 
-  if (IOMasterPort (MACH_PORT_NULL, &masterPort) != kIOReturnSuccess
-      || IOServiceGetMatchingServices (masterPort,
-                                       IOServiceMatching ("IONDRVDevice"),
-                                       &it) != kIOReturnSuccess)
-    return name;
+         Is this code OK for macOS < 10.9, and GNUstep?  I suspect it is,
+         in which case is it worth keeping the other method in here? */
 
-  /* Must loop until we find a name.  Many devices can have the same unit
-     number (represents different GPU parts), but only one has a name.  */
-  while (! name && (obj = IOIteratorNext (it)))
-    {
-      CFMutableDictionaryRef props;
-      const void *val;
-
-      if (IORegistryEntryCreateCFProperties (obj,
-                                             &props,
-                                             kCFAllocatorDefault,
-                                             kNilOptions) == kIOReturnSuccess
-          && props != nil
-          && (val = CFDictionaryGetValue(props, @"IOFBDependentIndex")))
+      if (IOMasterPort (MACH_PORT_NULL, &masterPort) != kIOReturnSuccess
+          || IOServiceGetMatchingServices (masterPort,
+                                           IOServiceMatching ("IONDRVDevice"),
+                                           &it) != kIOReturnSuccess)
+        return name;
+
+      /* Must loop until we find a name.  Many devices can have the same unit
+         number (represents different GPU parts), but only one has a name.  */
+      while (! name && (obj = IOIteratorNext (it)))
         {
-          unsigned nr = [(NSNumber *)val unsignedIntegerValue];
-          if (nr == CGDisplayUnitNumber (did))
-            name = ns_get_name_from_ioreg (obj);
+          CFMutableDictionaryRef props;
+          const void *val;
+
+          if (IORegistryEntryCreateCFProperties (obj,
+                                                 &props,
+                                                 kCFAllocatorDefault,
+                                                 kNilOptions) == kIOReturnSuccess
+              && props != nil
+              && (val = CFDictionaryGetValue(props, @"IOFBDependentIndex")))
+            {
+              unsigned nr = [(NSNumber *)val unsignedIntegerValue];
+              if (nr == CGDisplayUnitNumber (did))
+                name = ns_get_name_from_ioreg (obj);
+            }
+
+          CFRelease (props);
+          IOObjectRelease (obj);
         }
 
-      CFRelease (props);
-      IOObjectRelease (obj);
+      IOObjectRelease (it);
     }
-
-  IOObjectRelease (it);
-
-#else
-
-  name = ns_get_name_from_ioreg (CGDisplayIOServicePort (did));
-
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
+  else
+#endif
+#endif /* #if MAC_OS_X_VERSION_MAX_ALLOWED >= 1090 */
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
+    name = ns_get_name_from_ioreg (CGDisplayIOServicePort (did));
 #endif
   return name;
 }
-#endif
+#endif /* NS_IMPL_COCOA */
 
 static Lisp_Object
 ns_make_monitor_attribute_list (struct MonitorInfo *monitors,
diff --git a/src/nsmenu.m b/src/nsmenu.m
index 37a1a62d6d..93e06707c0 100644
--- a/src/nsmenu.m
+++ b/src/nsmenu.m
@@ -532,9 +532,14 @@ - (void)menuWillOpen:(NSMenu *)menu
 {
   ++trackingMenu;
 
-#if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
   // On 10.6 we get repeated calls, only the one for NSSystemDefined is "real".
-  if ([[NSApp currentEvent] type] != NSSystemDefined) return;
+  if (
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+      NSAppKitVersionNumber < NSAppKitVersionNumber10_7 &&
+#endif
+      [[NSApp currentEvent] type] != NSEventTypeSystemDefined)
+    return;
 #endif
 
   /* When dragging from one menu to another, we get willOpen followed by didClose,
diff --git a/src/nsterm.h b/src/nsterm.h
index 0f1b36db7b..ac6fc32324 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -25,30 +25,6 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "sysselect.h"
 
 #ifdef HAVE_NS
-
-#ifdef NS_IMPL_COCOA
-#ifndef MAC_OS_X_VERSION_10_6
-#define MAC_OS_X_VERSION_10_6 1060
-#endif
-#ifndef MAC_OS_X_VERSION_10_7
-#define MAC_OS_X_VERSION_10_7 1070
-#endif
-#ifndef MAC_OS_X_VERSION_10_8
-#define MAC_OS_X_VERSION_10_8 1080
-#endif
-#ifndef MAC_OS_X_VERSION_10_9
-#define MAC_OS_X_VERSION_10_9 1090
-#endif
-#ifndef MAC_OS_X_VERSION_10_12
-#define MAC_OS_X_VERSION_10_12 101200
-#endif
-
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-#define HAVE_NATIVE_FS
-#endif
-
-#endif /* NS_IMPL_COCOA */
-
 #ifdef __OBJC__
 
 /* CGFloat on GNUstep may be 4 or 8 byte, but functions expect float* for some
@@ -471,7 +447,7 @@ typedef id instancetype;
 - (void) toggleFullScreen: (id) sender;
 - (BOOL) fsIsNative;
 - (BOOL) isFullscreen;
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 - (void) updateCollectionBehavior;
 #endif
 
@@ -1275,9 +1251,17 @@ extern char gnustep_base_version[];  /* version tracking */
                                 ? (min) : (((x)>(max)) ? (max) : (x)))
 #define SCREENMAXBOUND(x) (IN_BOUND (-SCREENMAX, x, SCREENMAX))
 
+/* macOS 10.7 introduces some new constants. */
+#if !defined (MAC_OS_X_VERSION_10_7)
+#define NSFullScreenWindowMask                      (1 << 14)
+#define NSWindowCollectionBehaviorFullScreenPrimary (1 << 7)
+#define NSApplicationPresentationFullScreen         (1 << 10)
+#define NSApplicationPresentationAutoHideToolbar    (1 << 11)
+#define NSAppKitVersionNumber10_7                   1138
+#endif /* !defined (MAC_OS_X_VERSION_10_7) */
+
 /* macOS 10.12 deprecates a bunch of constants. */
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_12
+#if !defined (NS_IMPL_COCOA) || !defined (MAC_OS_X_VERSION_10_12)
 #define NSEventModifierFlagCommand         NSCommandKeyMask
 #define NSEventModifierFlagControl         NSControlKeyMask
 #define NSEventModifierFlagHelp            NSHelpKeyMask
@@ -1303,6 +1287,7 @@ extern char gnustep_base_version[];  /* version tracking */
 #define NSEventTypeKeyUp                   NSKeyUp
 #define NSEventTypeFlagsChanged            NSFlagsChanged
 #define NSEventMaskAny                     NSAnyEventMask
+#define NSEventTypeSystemDefined           NSSystemDefined
 #define NSWindowStyleMaskBorderless        NSBorderlessWindowMask
 #define NSWindowStyleMaskClosable          NSClosableWindowMask
 #define NSWindowStyleMaskFullScreen        NSFullScreenWindowMask
@@ -1317,6 +1302,13 @@ extern char gnustep_base_version[];  /* version tracking */
 #ifdef __OBJC__
 typedef NSUInteger NSWindowStyleMask;
 #endif
-#endif
 
+/* Window tabbing mode enums are new too. */
+enum NSWindowTabbingMode
+  {
+    NSWindowTabbingModeAutomatic,
+    NSWindowTabbingModePreferred,
+    NSWindowTabbingModeDisallowed
+  };
+#endif
 #endif	/* HAVE_NS */
diff --git a/src/nsterm.m b/src/nsterm.m
index 36d906a7ce..e0a977b927 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -136,14 +136,18 @@ @implementation NSColor (EmacsColor)
 + (NSColor *)colorForEmacsRed:(CGFloat)red green:(CGFloat)green
                          blue:(CGFloat)blue alpha:(CGFloat)alpha
 {
-#ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-  if (ns_use_srgb_colorspace)
-      return [NSColor colorWithSRGBRed: red
-                                 green: green
-                                  blue: blue
-                                 alpha: alpha];
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+  if (ns_use_srgb_colorspace
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+      && [NSColor respondsToSelector:
+                    @selector(colorWithSRGBRed:green:blue:alpha:)]
 #endif
+      )
+    return [NSColor colorWithSRGBRed: red
+                               green: green
+                                blue: blue
+                               alpha: alpha];
 #endif
   return [NSColor colorWithCalibratedRed: red
                                    green: green
@@ -153,11 +157,18 @@ + (NSColor *)colorForEmacsRed:(CGFloat)red green:(CGFloat)green
 
 - (NSColor *)colorUsingDefaultColorSpace
 {
-#ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
-  if (ns_use_srgb_colorspace)
-    return [self colorUsingColorSpace: [NSColorSpace sRGBColorSpace]];
+  /* FIXMES: We're checking for colorWithSRGBRed here so this will
+     only work in the same place as in the method above.  It should
+     really be a check whether we're on macOS 10.7 or above. */
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+  if (ns_use_srgb_colorspace
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+      && [NSColor respondsToSelector:
+                    @selector(colorWithSRGBRed:green:blue:alpha:)]
 #endif
+      )
+    return [self colorUsingColorSpace: [NSColorSpace sRGBColorSpace]];
 #endif
   return [self colorUsingColorSpaceName: NSCalibratedRGBColorSpace];
 }
@@ -4140,7 +4151,7 @@ in certain situations (rapid incoming events).
     }
 }
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 static void
 check_native_fs ()
 {
@@ -4242,7 +4253,7 @@ in certain situations (rapid incoming events).
 
   NSTRACE_WHEN (NSTRACE_GROUP_EVENTS, "ns_read_socket");
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
   check_native_fs ();
 #endif
 
@@ -4324,7 +4335,7 @@ in certain situations (rapid incoming events).
 
   NSTRACE_WHEN (NSTRACE_GROUP_EVENTS, "ns_select");
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
   check_native_fs ();
 #endif
 
@@ -5563,8 +5574,7 @@ - (void) terminate: (id)sender
               NSString *defaultButton,
               NSString *alternateButton)
 {
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
+#ifdef NS_IMPL_GNUSTEP
   return NSRunAlertPanel(title, msgFormat, defaultButton, alternateButton, nil)
     == NSAlertDefaultReturn;
 #else
@@ -6325,14 +6335,27 @@ - (NSRect)firstRectForCharacterRange: (NSRange)theRange
                                        +FRAME_LINE_HEIGHT (emacsframe));
 
   pt = [self convertPoint: pt toView: nil];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
-  pt = [[self window] convertBaseToScreen: pt];
-  rect.origin = pt;
-#else
-  rect.origin = pt;
-  rect = [[self window] convertRectToScreen: rect];
+
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+  if ([[self window] respondsToSelector: @selector(convertRectToScreen:)])
+    {
 #endif
+      rect.origin = pt;
+      rect = [(EmacsWindow *) [self window] convertRectToScreen: rect];
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+    }
+  else
+#endif
+#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 */
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070 \
+  || defined (NS_IMPL_GNUSTEP)
+    {
+      pt = [[self window] convertBaseToScreen: pt];
+      rect.origin = pt;
+    }
+#endif
+
   return rect;
 }
 
@@ -6988,11 +7011,15 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   scrollbarsNeedingUpdate = 0;
   fs_state = FULLSCREEN_NONE;
   fs_before_fs = next_maximized = -1;
-#ifdef HAVE_NATIVE_FS
-  fs_is_native = ns_use_native_fullscreen;
-#else
+
   fs_is_native = NO;
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+  if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
 #endif
+    fs_is_native = ns_use_native_fullscreen;
+#endif
+
   maximized_width = maximized_height = -1;
   nonfs_window = nil;
 
@@ -7023,7 +7050,10 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
                         backing: NSBackingStoreBuffered
                           defer: YES];
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+  if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
+#endif
     [win setCollectionBehavior:NSWindowCollectionBehaviorFullScreenPrimary];
 #endif
 
@@ -7032,9 +7062,11 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
 
   [win setAcceptsMouseMovedEvents: YES];
   [win setDelegate: self];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-  [win useOptimizedDrawing: YES];
+#if !defined (NS_IMPL_COCOA) || MAC_OS_X_VERSION_MIN_REQUIRED <= 1090
+#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090
+  if ([win respondsToSelector: @selector(useOptimizedDrawing:)])
+#endif
+    [win useOptimizedDrawing: YES];
 #endif
 
   [[win contentView] addSubview: self];
@@ -7094,9 +7126,12 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   if ([col alphaComponent] != (EmacsCGFloat) 1.0)
     [win setOpaque: NO];
 
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-  [self allocateGState];
+#if !defined (NS_IMPL_COCOA) \
+  || MAC_OS_X_VERSION_MIN_REQUIRED <= 1090
+#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090
+  if ([self respondsToSelector: @selector(allocateGState)])
+#endif
+    [self allocateGState];
 #endif
   [NSApp registerServicesMenuSendTypes: ns_send_types
                            returnTypes: [NSArray array]];
@@ -7104,9 +7139,12 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   /* macOS Sierra automatically enables tabbed windows.  We can't
      allow this to be enabled until it's available on a Free system.
      Currently it only happens by accident and is buggy anyway. */
-#if defined (NS_IMPL_COCOA) && \
-  MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_12
-  [win setTabbingMode: NSWindowTabbingModeDisallowed];
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= 101200
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 101200
+  if ([win respondsToSelector: @selector(setTabbingMode:)])
+#endif
+    [win setTabbingMode: NSWindowTabbingModeDisallowed];
 #endif
 
   ns_window_num++;
@@ -7323,7 +7361,7 @@ - (void)windowDidMiniaturize: sender
     }
 }
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 - (NSApplicationPresentationOptions)window:(NSWindow *)window
       willUseFullScreenPresentationOptions:
   (NSApplicationPresentationOptions)proposedOptions
@@ -7361,8 +7399,8 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
   else
     {
       BOOL tbar_visible = FRAME_EXTERNAL_TOOL_BAR (emacsframe) ? YES : NO;
-#ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070        \
+  && MAC_OS_X_VERSION_MIN_REQUIRED <= 1070
       unsigned val = (unsigned)[NSApp presentationOptions];
 
       // Mac OS X 10.7 bug fix, the menu won't appear without this.
@@ -7378,7 +7416,6 @@ - (void)windowDidEnterFullScreen /* provided for direct calls */
           [NSApp setPresentationOptions: options];
         }
 #endif
-#endif
       [toolbar setVisible:tbar_visible];
     }
 }
@@ -7417,7 +7454,7 @@ - (void)windowDidExitFullScreen /* provided for direct calls */
     }
   [self setFSValue: fs_before_fs];
   fs_before_fs = -1;
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
   [self updateCollectionBehavior];
 #endif
   if (FRAME_EXTERNAL_TOOL_BAR (emacsframe))
@@ -7449,7 +7486,7 @@ - (BOOL)isFullscreen
     }
   else
     {
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
       res = (([[self window] styleMask] & NSWindowStyleMaskFullScreen) != 0);
 #else
       res = NO;
@@ -7462,7 +7499,7 @@ - (BOOL)isFullscreen
   return res;
 }
 
-#ifdef HAVE_NATIVE_FS
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
 - (void)updateCollectionBehavior
 {
   NSTRACE ("[EmacsView updateCollectionBehavior]");
@@ -7477,7 +7514,10 @@ - (void)updateCollectionBehavior
         b &= ~NSWindowCollectionBehaviorFullScreenPrimary;
 
       [win setCollectionBehavior: b];
-      fs_is_native = ns_use_native_fullscreen;
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+      if (NSAppKitVersionNumber >= NSAppKitVersionNumber10_7)
+#endif
+        fs_is_native = ns_use_native_fullscreen;
     }
 }
 #endif
@@ -7494,8 +7534,11 @@ - (void)toggleFullScreen: (id)sender
 
   if (fs_is_native)
     {
-#ifdef HAVE_NATIVE_FS
-      [[self window] toggleFullScreen:sender];
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+      if ([[self window] respondsToSelector: @selector(toggleFullScreen:)])
+#endif
+        [[self window] toggleFullScreen:sender];
 #endif
       return;
     }
@@ -7512,10 +7555,13 @@ - (void)toggleFullScreen: (id)sender
     {
       NSScreen *screen = [w screen];
 
-#if defined (NS_IMPL_COCOA) && \
-  MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
+#if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1090
       /* Hide ghost menu bar on secondary monitor? */
-      if (! onFirstScreen)
+      if (! onFirstScreen
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
+          && [NSScreen respondsToSelector: @selector(screensHaveSeparateSpaces)]
+#endif
+          )
         onFirstScreen = [NSScreen screensHaveSeparateSpaces];
 #endif
       /* Hide dock and menubar if we are on the primary screen.  */
@@ -7543,9 +7589,12 @@ - (void)toggleFullScreen: (id)sender
       [fw setTitle:[w title]];
       [fw setDelegate:self];
       [fw setAcceptsMouseMovedEvents: YES];
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_9
-      [fw useOptimizedDrawing: YES];
+#if !defined (NS_IMPL_COCOA) \
+  || MAC_OS_X_VERSION_MIN_REQUIRED <= 1090
+#if MAC_OS_X_VERSION_MAX_ALLOWED > 1090
+      if ([fw respondsToSelector: @selector(useOptimizedDrawing:)])
+#endif
+        [fw useOptimizedDrawing: YES];
 #endif
       [fw setBackgroundColor: col];
       if ([col alphaComponent] != (EmacsCGFloat) 1.0)
@@ -8106,10 +8155,14 @@ - (NSRect)constrainFrameRect:(NSRect)frameRect toScreen:(NSScreen *)screen
              NSTRACE_ARG_RECT (frameRect));
 
 #ifdef NS_IMPL_COCOA
-#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9
+#if MAC_OS_X_VERSION_MAX_ALLOWED >= 1090
   // If separate spaces is on, it is like each screen is independent.  There is
   // no spanning of frames across screens.
-  if ([NSScreen screensHaveSeparateSpaces])
+  if (
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1090
+      [NSScreen respondsToSelector: @selector(screensHaveSeparateSpaces)] &&
+#endif
+      [NSScreen screensHaveSeparateSpaces])
     {
       NSTRACE_MSG ("Screens have separate spaces");
       frameRect = [super constrainFrameRect:frameRect toScreen:screen];
@@ -8117,7 +8170,8 @@ - (NSRect)constrainFrameRect:(NSRect)frameRect toScreen:(NSScreen *)screen
       return frameRect;
     }
   else
-#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9 */
+#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= 1090 */
+
     // Check that the proposed frameRect is visible in at least one
     // screen.  If it is not, ask the system to reposition it (only
     // for non-child windows).
@@ -8323,12 +8377,21 @@ + (CGFloat) scrollerWidth
   /* TODO: if we want to allow variable widths, this is the place to do it,
            however neither GNUstep nor Cocoa support it very well */
   CGFloat r;
-#if !defined (NS_IMPL_COCOA) || \
-  MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
-  r = [NSScroller scrollerWidth];
-#else
-  r = [NSScroller scrollerWidthForControlSize: NSControlSizeRegular
-                                scrollerStyle: NSScrollerStyleLegacy];
+#if defined (NS_IMPL_COCOA) \
+  && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+  if ([NSScroller respondsToSelector:
+                    @selector(scrollerWidthForControlSize:scrollerStyle:)])
+#endif
+    r = [NSScroller scrollerWidthForControlSize: NSControlSizeRegular
+                                  scrollerStyle: NSScrollerStyleLegacy];
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070
+  else
+#endif
+#endif /* MAC_OS_X_VERSION_MAX_ALLOWED >= 1070 */
+#if MAC_OS_X_VERSION_MIN_REQUIRED < 1070 \
+  || defined (NS_IMPL_GNUSTEP)
+    r = [NSScroller scrollerWidth];
 #endif
   return r;
 }
@@ -9015,12 +9078,8 @@ Convert an X font name (XLFD) to an NS font name.
      doc: /*Non-nil means to use native fullscreen on Mac OS X 10.7 and later.
 Nil means use fullscreen the old (< 10.7) way.  The old way works better with
 multiple monitors, but lacks tool bar.  This variable is ignored on
-Mac OS X < 10.7.  Default is t for 10.7 and later, nil otherwise.  */);
-#ifdef HAVE_NATIVE_FS
+Mac OS X < 10.7.  Default is t.  */);
   ns_use_native_fullscreen = YES;
-#else
-  ns_use_native_fullscreen = NO;
-#endif
   ns_last_use_native_fullscreen = ns_use_native_fullscreen;
 
   DEFVAR_BOOL ("ns-use-fullscreen-animation", ns_use_fullscreen_animation,
-- 
2.12.0


^ permalink raw reply related	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-08-12 13:02                                                                 ` Charles A. Roelli
@ 2017-08-16 20:31                                                                   ` Alan Third
  0 siblings, 0 replies; 63+ messages in thread
From: Alan Third @ 2017-08-16 20:31 UTC (permalink / raw)
  To: Charles A. Roelli; +Cc: 27810-done, Anders Lindgren

On Sat, Aug 12, 2017 at 03:02:07PM +0200, Charles A. Roelli wrote:
> Attached is the patch to revert my changes.

I’ve pushed it to master now.
-- 
Alan Third





^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-08-12 15:51                                                                 ` Alan Third
@ 2017-09-12 20:01                                                                   ` David Caldwell
  2017-09-12 20:06                                                                     ` David Reitter
  2017-09-12 20:29                                                                     ` Alan Third
  0 siblings, 2 replies; 63+ messages in thread
From: David Caldwell @ 2017-09-12 20:01 UTC (permalink / raw)
  To: Alan Third, Charles A. Roelli
  Cc: David Reitter, david+emacsformacosx, Anders Lindgren, 27810


[-- Attachment #1.1: Type: text/plain, Size: 4468 bytes --]

On 8/12/17 8:51 AM, Alan Third wrote:
> On Sat, Aug 12, 2017 at 01:13:56PM +0200, Charles A. Roelli wrote:
>> Hm, on second thoughts, it seems a bit overwrought to try doing this
>> weak linking only for the sake of forward-compatible builds.  It
>> should be enough to support only backward-compatible builds, so that
>> we might one day distribute Emacs as a .dmg (built on the latest macOS
>> and backwards-compatible with the oldest version of macOS that we
>> support).
> 
> I think this makes sense. Especially given we’re not able to actually
> create a stand‐alone .app without implementing something like...
> 
>> I also looked at the emacsformacosx.com build scripts, and it seems
>> like they're making copies of Emacs' dependent dynamic libraries,
>> including them in the application bundle, then using
>> install_name_tool(1) to patch the Emacs binary to depend on them (I
>> don't understand, though, how those scripts resolve dependencies
>> between the dynamic libraries themselves).

The install_name_tool stuff is to change the absolute paths in the
dynamic library load paths to application relative paths. And only for
extra dependency stuff that gets installed like gnutls (and eventually
more—I'd like ImageMagick, but I haven't been able to get relative paths
working with its plugin-architecture).

The relative paths still include the OS that they were compiled on so
the dylibs can find each other but not conflict with the other OS dylibs.

For instance, Emacs-Emacs.app/Contents/MacOS/Emacs-x86_64-10_9 has this
as one of it's load paths:

  @executable_path/lib-x86_64-10_9/libgnutls.30.dylib

and Emacs.app/Contents/MacOS/lib-x86_64-10_9/libgnutls.30.dylib has
stuff like:

  @executable_path/lib-x86_64-10_9/libp11-kit.0.dylib

It's pretty straightforward in the end, but it's not documented super
well, and it's sort of tricky that you have to manipulate all these
things manually.

> I wouldn’t have a problem with putting this capability in, but I don’t
> have the knowledge nor the inclination to do it myself.
> 
> I was going to write something up in INSTALL about building with
> feature detection, but I really don’t know how to put it. I don’t want
> to give the impression that if you use
> -DMAC_OS_X_VERSION_MIN_ALLOWED=1060 that it will magically build a
> portable .app. I began to wonder if it’s worth mentioning at all since
> I doubt any more than a handful of people will be interested in
> building with this option.
> 
> David and David, I hope it’s OK to include you in this. I thought you
> might be interested and perhaps have some thoughts on what we’re
> doing.
> 
> Basically, instead of detecting all macOS features at compile‐time,
> we’ve stuck in some code to detect them at run‐time. It causes
> compiler warnings, so by default it still limits features to those
> available at build‐time, but if you do something like:
> 
>     ./configure --with-ns CFLAGS="-DMAC_OS_X_VERSION_MIN_REQUIRED=1060 -O3 -g"
> 
> when building on macOS Sierra, you should, in theory, end up with an
> executable that will work correctly on every version of macOS back to
> 10.6, inclusive. We haven’t been able to properly test portability yet
> as it requires including dynamic libraries.

I would love that. As far as I know, that's the way Apple recommends
doing feature detection. Many of the complexities of the
EmacsForMacOS.com builds (like all the individual OS compiled versions)
are because Emacs does compile-time feature detection instead of
run-time. Compiling on old systems means having to jump through hoops
sometimes as different software stops being supported on them. I have to
download a "portable ruby" binary that homebrew made so that I can run
something newer than 1.8 on old OSes. For master branch builds I have to
run the autoconf stuff on an up-to-date Debian machine before handing it
off to the Mac build machines because it's really hard to automate
installing the latest autoconf in a way that supports stuff back to 10.6
(Homebrew is not available on 10.6, for instance).

So having a single backwards compatible version of Emacs would be very
very nice. The build-system is rickety and this would make it more
streamlined. And much easier for a random person to build a universal
app (btw I'd be happy to help produce official Mac builds if that is
ever wanted).

-David


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-09-12 20:01                                                                   ` David Caldwell
@ 2017-09-12 20:06                                                                     ` David Reitter
  2017-09-12 20:34                                                                       ` Alan Third
  2017-09-12 20:29                                                                     ` Alan Third
  1 sibling, 1 reply; 63+ messages in thread
From: David Reitter @ 2017-09-12 20:06 UTC (permalink / raw)
  To: Alan Third
  Cc: Charles A. Roelli, 27810, david+emacsformacosx, Anders Lindgren,
	David Caldwell

All,
If I haven’t replied to this earlier (sorry), I second David’s comments.  
The distributed binary version of Aquamacs is built with an environment that hides installed libraries as far as possible, and with patches that add runtime checks for the availability of APIs.

I never understood why the Emacs build on Macs does so much compile-time checking, and I can only suspect that there are historical reasons from GNU/Linux systems where you have source distributions, build locally, using package managers to take care of dependencies.

- David


> On Sep 12, 2017, at 4:01 PM, David Caldwell <david@porkrind.org> wrote:
> 
> On 8/12/17 8:51 AM, Alan Third wrote:
>> On Sat, Aug 12, 2017 at 01:13:56PM +0200, Charles A. Roelli wrote:
>>> Hm, on second thoughts, it seems a bit overwrought to try doing this
>>> weak linking only for the sake of forward-compatible builds.  It
>>> should be enough to support only backward-compatible builds, so that
>>> we might one day distribute Emacs as a .dmg (built on the latest macOS
>>> and backwards-compatible with the oldest version of macOS that we
>>> support).
>> 
>> I think this makes sense. Especially given we’re not able to actually
>> create a stand‐alone .app without implementing something like...
>> 
>>> I also looked at the emacsformacosx.com build scripts, and it seems
>>> like they're making copies of Emacs' dependent dynamic libraries,
>>> including them in the application bundle, then using
>>> install_name_tool(1) to patch the Emacs binary to depend on them (I
>>> don't understand, though, how those scripts resolve dependencies
>>> between the dynamic libraries themselves).
> 
> The install_name_tool stuff is to change the absolute paths in the
> dynamic library load paths to application relative paths. And only for
> extra dependency stuff that gets installed like gnutls (and eventually
> more—I'd like ImageMagick, but I haven't been able to get relative paths
> working with its plugin-architecture).
> 
> The relative paths still include the OS that they were compiled on so
> the dylibs can find each other but not conflict with the other OS dylibs.
> 
> For instance, Emacs-Emacs.app/Contents/MacOS/Emacs-x86_64-10_9 has this
> as one of it's load paths:
> 
>  @executable_path/lib-x86_64-10_9/libgnutls.30.dylib
> 
> and Emacs.app/Contents/MacOS/lib-x86_64-10_9/libgnutls.30.dylib has
> stuff like:
> 
>  @executable_path/lib-x86_64-10_9/libp11-kit.0.dylib
> 
> It's pretty straightforward in the end, but it's not documented super
> well, and it's sort of tricky that you have to manipulate all these
> things manually.
> 
>> I wouldn’t have a problem with putting this capability in, but I don’t
>> have the knowledge nor the inclination to do it myself.
>> 
>> I was going to write something up in INSTALL about building with
>> feature detection, but I really don’t know how to put it. I don’t want
>> to give the impression that if you use
>> -DMAC_OS_X_VERSION_MIN_ALLOWED=1060 that it will magically build a
>> portable .app. I began to wonder if it’s worth mentioning at all since
>> I doubt any more than a handful of people will be interested in
>> building with this option.
>> 
>> David and David, I hope it’s OK to include you in this. I thought you
>> might be interested and perhaps have some thoughts on what we’re
>> doing.
>> 
>> Basically, instead of detecting all macOS features at compile‐time,
>> we’ve stuck in some code to detect them at run‐time. It causes
>> compiler warnings, so by default it still limits features to those
>> available at build‐time, but if you do something like:
>> 
>>    ./configure --with-ns CFLAGS="-DMAC_OS_X_VERSION_MIN_REQUIRED=1060 -O3 -g"
>> 
>> when building on macOS Sierra, you should, in theory, end up with an
>> executable that will work correctly on every version of macOS back to
>> 10.6, inclusive. We haven’t been able to properly test portability yet
>> as it requires including dynamic libraries.
> 
> I would love that. As far as I know, that's the way Apple recommends
> doing feature detection. Many of the complexities of the
> EmacsForMacOS.com builds (like all the individual OS compiled versions)
> are because Emacs does compile-time feature detection instead of
> run-time. Compiling on old systems means having to jump through hoops
> sometimes as different software stops being supported on them. I have to
> download a "portable ruby" binary that homebrew made so that I can run
> something newer than 1.8 on old OSes. For master branch builds I have to
> run the autoconf stuff on an up-to-date Debian machine before handing it
> off to the Mac build machines because it's really hard to automate
> installing the latest autoconf in a way that supports stuff back to 10.6
> (Homebrew is not available on 10.6, for instance).
> 
> So having a single backwards compatible version of Emacs would be very
> very nice. The build-system is rickety and this would make it more
> streamlined. And much easier for a random person to build a universal
> app (btw I'd be happy to help produce official Mac builds if that is
> ever wanted).
> 
> -David






^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-09-12 20:01                                                                   ` David Caldwell
  2017-09-12 20:06                                                                     ` David Reitter
@ 2017-09-12 20:29                                                                     ` Alan Third
  2017-09-13 19:03                                                                       ` Charles A. Roelli
  1 sibling, 1 reply; 63+ messages in thread
From: Alan Third @ 2017-09-12 20:29 UTC (permalink / raw)
  To: David Caldwell; +Cc: David Reitter, Charles A. Roelli, Anders Lindgren, 27810

On Tue, Sep 12, 2017 at 01:01:56PM -0700, David Caldwell wrote:
> On 8/12/17 8:51 AM, Alan Third wrote:
> > Basically, instead of detecting all macOS features at compile‐time,
> > we’ve stuck in some code to detect them at run‐time. It causes
> > compiler warnings, so by default it still limits features to those
> > available at build‐time, but if you do something like:
> > 
> >     ./configure --with-ns CFLAGS="-DMAC_OS_X_VERSION_MIN_REQUIRED=1060 -O3 -g"
> > 
> > when building on macOS Sierra, you should, in theory, end up with an
> > executable that will work correctly on every version of macOS back to
> > 10.6, inclusive. We haven’t been able to properly test portability yet
> > as it requires including dynamic libraries.
> 
> I would love that. As far as I know, that's the way Apple recommends
> doing feature detection.

Well, the good news is that the patch was committed to master a wee
while ago and the above command should build the dynamic version. It
builds here on 10.12 with just a few deprecation warnings.

We’ve still not been able to confirm whether it works properly, so if
you’d be willing to help us test it that would be great.

-- 
Alan Third





^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-09-12 20:06                                                                     ` David Reitter
@ 2017-09-12 20:34                                                                       ` Alan Third
  0 siblings, 0 replies; 63+ messages in thread
From: Alan Third @ 2017-09-12 20:34 UTC (permalink / raw)
  To: David Reitter
  Cc: Charles A. Roelli, 27810, david+emacsformacosx, Anders Lindgren,
	David Caldwell

On Tue, Sep 12, 2017 at 04:06:20PM -0400, David Reitter wrote:
> I never understood why the Emacs build on Macs does so much
> compile-time checking, and I can only suspect that there are
> historical reasons from GNU/Linux systems where you have source
> distributions, build locally, using package managers to take care of
> dependencies.

RMS has some concerns about cross compiling which may explain it, but
I don’t think it really makes any difference for macOS.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27810#23
-- 
Alan Third





^ permalink raw reply	[flat|nested] 63+ messages in thread

* bug#27810: NS runtime feature detection
  2017-09-12 20:29                                                                     ` Alan Third
@ 2017-09-13 19:03                                                                       ` Charles A. Roelli
  0 siblings, 0 replies; 63+ messages in thread
From: Charles A. Roelli @ 2017-09-13 19:03 UTC (permalink / raw)
  To: Alan Third; +Cc: david.reitter, david, andlind, 27810

> Date: Tue, 12 Sep 2017 21:29:43 +0100
> From: Alan Third <alan@idiocy.org>
> 
> Well, the good news is that the patch was committed to master a wee
> while ago and the above command should build the dynamic version. It
> builds here on 10.12 with just a few deprecation warnings.
> 
> We’ve still not been able to confirm whether it works properly, so if
> you’d be willing to help us test it that would be great.

I found a little tool to help automate the dylib collection process
for creating a standalone app bundle:

  https://github.com/auriamg/macdylibbundler/

I haven't gotten it to work on my 10.6 system yet, since I have a bit
of a dylib hell from some botched library install attempts, that I've
never properly fixed -- and the tool can't fix it for me.  If I get
some time soon I'll try to build the master branch on a different
10.12 machine, then bundle the dylibs with the above tool, and try the
result on 10.6.





^ permalink raw reply	[flat|nested] 63+ messages in thread

end of thread, other threads:[~2017-09-13 19:03 UTC | newest]

Thread overview: 63+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-06 11:29 Mac OS Sierra tab feature breaks C-x 5 2 Paul Michael Reilly
2017-07-06 12:14 ` Jean-Christophe Helary
2017-07-06 12:46   ` Sebastian Christ
2017-07-06 12:53 ` Alan Third
2017-07-06 14:35   ` Alan Third
2017-07-06 15:05     ` Jean-Christophe Helary
2017-07-06 17:42       ` Alan Third
2017-07-06 22:16         ` Alan Third
2017-07-10 19:17           ` Anders Lindgren
2017-07-10 19:52             ` Alan Third
2017-07-10 20:22               ` Anders Lindgren
2017-07-12 18:23                 ` Alan Third
2017-07-12 21:20                   ` Anders Lindgren
2017-07-13 20:22                     ` Alan Third
2017-07-16 18:43                       ` Anders Lindgren
2017-07-16 23:01                         ` Alan Third
2017-07-17 20:09                           ` Charles A. Roelli
2017-07-18  6:06                             ` Anders Lindgren
2017-07-18 18:33                               ` Charles A. Roelli
2017-07-18 22:16                                 ` Alan Third
2017-07-19  4:57                                   ` Charles A. Roelli
2017-07-21 20:31                                     ` Anders Lindgren
2017-07-22 11:22                                       ` Alan Third
2017-07-23 12:17                                         ` NS runtime feature detection (was: Mac OS Sierra tab feature breaks C-x 5 2) Alan Third
2017-07-24 19:02                                           ` NS runtime feature detection Charles A. Roelli
2017-07-24 20:44                                             ` bug#27810: " Alan Third
2017-07-24 20:53                                               ` Glenn Morris
2017-07-25 17:56                                                 ` Alan Third
2017-07-25 18:22                                                   ` Charles A. Roelli
2017-07-25 20:08                                                     ` Anders Lindgren
2017-07-26 21:57                                               ` Alan Third
2017-07-31 19:05                                                 ` Charles A. Roelli
2017-08-01 15:38                                                   ` Anders Lindgren
2017-08-01 22:03                                                     ` Alan Third
2017-08-06 20:29                                                       ` Charles A. Roelli
2017-08-06 21:29                                                         ` Alan Third
2017-08-07 19:23                                                           ` Charles A. Roelli
2017-08-10 21:04                                                             ` Alan Third
2017-08-12 11:13                                                               ` Charles A. Roelli
2017-08-12 13:02                                                                 ` Charles A. Roelli
2017-08-16 20:31                                                                   ` Alan Third
2017-08-12 15:51                                                                 ` Alan Third
2017-09-12 20:01                                                                   ` David Caldwell
2017-09-12 20:06                                                                     ` David Reitter
2017-09-12 20:34                                                                       ` Alan Third
2017-09-12 20:29                                                                     ` Alan Third
2017-09-13 19:03                                                                       ` Charles A. Roelli
2017-07-24 20:45                                             ` Alan Third
2017-07-23 22:35                                         ` Mac OS Sierra tab feature breaks C-x 5 2 Tim Cross
  -- strict thread matches above, loose matches on Subject: below --
2017-07-06 17:24 Matthew Bauer
2017-07-24 20:22 bug#27810: macOS runtime feature detection Alan Third
2017-07-26  2:59 ` Richard Stallman
2017-07-26 16:06   ` Alan Third
2017-07-27  1:43     ` Richard Stallman
2017-07-27 17:31       ` Eli Zaretskii
2017-07-28 17:14         ` Richard Stallman
2017-07-28 17:36           ` Eli Zaretskii
2017-07-29 19:04             ` Richard Stallman
2017-07-31  0:45               ` Richard Stallman
2017-07-29 19:07     ` Richard Stallman
2017-07-30 12:12       ` Alan Third
2017-07-30 14:15         ` Eli Zaretskii
2017-07-31  0:47         ` Richard Stallman

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.