[PATCH] ui/cocoa: Drop workarounds for pre-10.12 OSX

[PATCH] ui/cocoa: Drop workarounds for pre-10.12 OSX

[Peter Maydell]

Our official OSX support policy covers the last two released versions.
Currently that is 10.14 and 10.15.  We also may work on older versions, but
don't guarantee it.

In commit 50290c002c045280f8d in mid-2019 we introduced some uses of
CLOCK_MONOTONIC which incidentally broke compilation for pre-10.12 OSX
versions (see LP:1861551). We don't intend to fix that, so we might
as well drop the code in ui/cocoa.m which caters for pre-10.12
versions as well. (For reference, 10.11 fell out of Apple extended
security support in September 2018.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
The bug report is recent, but this was also pointed out on
the mailing list back in June 2019. Since nobody has cared
to try to fix it, we clearly don't care about 10.11 in
practice as well as in theory.]
---
 ui/cocoa.m | 59 ------------------------------------------------------
 1 file changed, 59 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index fbb5b1b45f..f9945bc712 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -42,60 +42,10 @@
 #include <Carbon/Carbon.h>
 #include "hw/core/cpu.h"
 
-#ifndef MAC_OS_X_VERSION_10_5
-#define MAC_OS_X_VERSION_10_5 1050
-#endif
-#ifndef MAC_OS_X_VERSION_10_6
-#define MAC_OS_X_VERSION_10_6 1060
-#endif
-#ifndef MAC_OS_X_VERSION_10_9
-#define MAC_OS_X_VERSION_10_9 1090
-#endif
-#ifndef MAC_OS_X_VERSION_10_10
-#define MAC_OS_X_VERSION_10_10 101000
-#endif
-#ifndef MAC_OS_X_VERSION_10_12
-#define MAC_OS_X_VERSION_10_12 101200
-#endif
 #ifndef MAC_OS_X_VERSION_10_13
 #define MAC_OS_X_VERSION_10_13 101300
 #endif
 
-/* macOS 10.12 deprecated many constants, #define the new names for older SDKs */
-#if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_12
-#define NSEventMaskAny                  NSAnyEventMask
-#define NSEventModifierFlagCapsLock     NSAlphaShiftKeyMask
-#define NSEventModifierFlagShift        NSShiftKeyMask
-#define NSEventModifierFlagCommand      NSCommandKeyMask
-#define NSEventModifierFlagControl      NSControlKeyMask
-#define NSEventModifierFlagOption       NSAlternateKeyMask
-#define NSEventTypeFlagsChanged         NSFlagsChanged
-#define NSEventTypeKeyUp                NSKeyUp
-#define NSEventTypeKeyDown              NSKeyDown
-#define NSEventTypeMouseMoved           NSMouseMoved
-#define NSEventTypeLeftMouseDown        NSLeftMouseDown
-#define NSEventTypeRightMouseDown       NSRightMouseDown
-#define NSEventTypeOtherMouseDown       NSOtherMouseDown
-#define NSEventTypeLeftMouseDragged     NSLeftMouseDragged
-#define NSEventTypeRightMouseDragged    NSRightMouseDragged
-#define NSEventTypeOtherMouseDragged    NSOtherMouseDragged
-#define NSEventTypeLeftMouseUp          NSLeftMouseUp
-#define NSEventTypeRightMouseUp         NSRightMouseUp
-#define NSEventTypeOtherMouseUp         NSOtherMouseUp
-#define NSEventTypeScrollWheel          NSScrollWheel
-#define NSTextAlignmentCenter           NSCenterTextAlignment
-#define NSWindowStyleMaskBorderless     NSBorderlessWindowMask
-#define NSWindowStyleMaskClosable       NSClosableWindowMask
-#define NSWindowStyleMaskMiniaturizable NSMiniaturizableWindowMask
-#define NSWindowStyleMaskTitled         NSTitledWindowMask
-#endif
-/* 10.13 deprecates NSFileHandlingPanelOKButton in favour of
- * NSModalResponseOK, which was introduced in 10.9. Define
- * it for older versions.
- */
-#if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_9
-#define NSModalResponseOK NSFileHandlingPanelOKButton
-#endif
 /* 10.14 deprecates NSOnState and NSOffState in favor of
  * NSControlStateValueOn/Off, which were introduced in 10.13.
  * Define for older versions
@@ -465,11 +415,7 @@ - (void) drawRect:(NSRect) rect
     COCOA_DEBUG("QemuCocoaView: drawRect\n");
 
     // get CoreGraphic context
-#if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_10
-    CGContextRef viewContextRef = [[NSGraphicsContext currentContext] graphicsPort];
-#else
     CGContextRef viewContextRef = [[NSGraphicsContext currentContext] CGContext];
-#endif
 
     CGContextSetInterpolationQuality (viewContextRef, kCGInterpolationNone);
     CGContextSetShouldAntialias (viewContextRef, NO);
@@ -1075,9 +1021,7 @@ - (void) raiseAllKeys
  ------------------------------------------------------
 */
 @interface QemuCocoaAppController : NSObject
-#if (MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_6)
                                        <NSWindowDelegate, NSApplicationDelegate>
-#endif
 {
 }
 - (void)doToggleFullScreen:(id)sender;
@@ -1126,9 +1070,6 @@ - (id) init
         [normalWindow setAcceptsMouseMovedEvents:YES];
         [normalWindow setTitle:@"QEMU"];
         [normalWindow setContentView:cocoaView];
-#if (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_10)
-        [normalWindow useOptimizedDrawing:YES];
-#endif
         [normalWindow makeKeyAndOrderFront:self];
         [normalWindow center];
         [normalWindow setDelegate: self];
-- 
2.17.2 (Apple Git-113)

Re: [PATCH] ui/cocoa: Drop workarounds for pre-10.12 OSX

[Gerd Hoffmann]

On Sat, Feb 01, 2020 at 05:05:34PM +0000, Peter Maydell wrote:
> Our official OSX support policy covers the last two released versions.
> Currently that is 10.14 and 10.15.  We also may work on older versions, but
> don't guarantee it.
> 
> In commit 50290c002c045280f8d in mid-2019 we introduced some uses of
> CLOCK_MONOTONIC which incidentally broke compilation for pre-10.12 OSX
> versions (see LP:1861551). We don't intend to fix that, so we might
> as well drop the code in ui/cocoa.m which caters for pre-10.12
> versions as well. (For reference, 10.11 fell out of Apple extended
> security support in September 2018.)

Added to UI patch queue.

thanks,
  Gerd

Re: [PATCH] ui/cocoa: Drop workarounds for pre-10.12 OSX

[Christian Schoenebeck]

On Dienstag, 4. Februar 2020 14:13:35 CET Peter Maydell wrote:
> > How about handling this with an error message at configure stage instead?
> > Removing this code does not increase the chance for somebody going to fix
> > pre-10.12 support.
> 
> But is anybody actually going to fix pre-10.12 support? They've
> had six months so far... If we had a good sized body of developers
> taking active care of how well QEMU on OSX works, I would be
> entirely happy for them to decide amongst themselves what they
> thought was a reasonable version support policy for OSX. But
> we just don't.

No idea of course whether somebody will fix it. It's just that young people 
tend to have fairly old Macbooks (i.e. >= 10 years) since younger models in 
that universe are often outside of their budget.

> I wouldn't object to a configure check if anybody wanted to write
> a patch to add one.

Like I said, I don't care much, but if you want I can send a small configure 
script patch of course. That's not a big deal. So roughly you want a warning 
for pre-10.14 and error out on pre-10.12, right?

G3, if you want to take care about sending this patch instead, very much 
appreciated of course. :)

> > > "Last two versions only" is our support policy for OSX for a reason,
> > > and the reason is that there just aren't very many people running
> > > OSX and actively working on QEMU. Of those, even fewer will
> > > be still running OSX versions that are no longer being supported
> > > for security fixes by Apple.
> > 
> > Apple currently seems to maintain 4 macOS release branches with security
> > fixes (10.15.x, 10.14.x, 10.13.x, 10.12.x):
> > https://en.wikipedia.org/wiki/MacOS_Sierra#Release_history
> 
> Indeed, and none of those is the 10.11-and-earlier that this patch
> drops attempts to cater for. The patch does still keep 10.12 and
> 10.13 support alive, even though it's not strictly required by
> our support policy, because building for those host versions does
> in practice still work.

Sure, I just wanted to bring that to your attention. In fact security support 
policy by Apple is somewhat unpredictable. There is no official statement 
which branches are covered by security fixes. They simply decide on a case-to-
case basis whether or not to release updates for certain old release branches.

Best regards,
Christian Schoenebeck

Re: [PATCH] ui/cocoa: Drop workarounds for pre-10.12 OSX

[Peter Maydell]

On Tue, 4 Feb 2020 at 13:03, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
> Not that I really care, but I also wonder whether that would be an appropriate
> action to remove this code, as it does not seem to impose anything negative to
> retain it.

Dead code is always a maintenance burden, even if it's not a very
large one. It means that somebody reading the code and working
on it has to think "maybe I need to do something to cater for these older
versions in my new feature I'm working on, maybe I need to test on
older versions" when they don't. It makes code harder to read when
it's got a bunch of ifdefs in it.

> > The rationale is in the commit message -- QEMU *already* does not
> > build on 10.11, and it has not done so for more than six months, and
> > none of these Mac users has submitted a patch to try to fix it.
>
> How about handling this with an error message at configure stage instead?
> Removing this code does not increase the chance for somebody going to fix
> pre-10.12 support.

But is anybody actually going to fix pre-10.12 support? They've
had six months so far... If we had a good sized body of developers
taking active care of how well QEMU on OSX works, I would be
entirely happy for them to decide amongst themselves what they
thought was a reasonable version support policy for OSX. But
we just don't.

I wouldn't object to a configure check if anybody wanted to write
a patch to add one.

> > "Last two versions only" is our support policy for OSX for a reason,
> > and the reason is that there just aren't very many people running
> > OSX and actively working on QEMU. Of those, even fewer will
> > be still running OSX versions that are no longer being supported
> > for security fixes by Apple.
>
> Apple currently seems to maintain 4 macOS release branches with security fixes
> (10.15.x, 10.14.x, 10.13.x, 10.12.x):
> https://en.wikipedia.org/wiki/MacOS_Sierra#Release_history

Indeed, and none of those is the 10.11-and-earlier that this patch
drops attempts to cater for. The patch does still keep 10.12 and
10.13 support alive, even though it's not strictly required by
our support policy, because building for those host versions does
in practice still work.

thanks
-- PMM

Re: [PATCH] ui/cocoa: Drop workarounds for pre-10.12 OSX

[Christian Schoenebeck]

On Dienstag, 4. Februar 2020 11:15:36 CET Peter Maydell wrote:
> On Mon, 3 Feb 2020 at 19:59, G 3 <programmingkidx@gmail.com> wrote:
> >> Our official OSX support policy covers the last two released versions.
> >> Currently that is 10.14 and 10.15.  We also may work on older versions,
> >> but
> >> don't guarantee it.
> >> 
> >> In commit 50290c002c045280f8d in mid-2019 we introduced some uses of
> >> CLOCK_MONOTONIC which incidentally broke compilation for pre-10.12 OSX
> >> versions (see LP:1861551). We don't intend to fix that, so we might
> >> as well drop the code in ui/cocoa.m which caters for pre-10.12
> >> versions as well. (For reference, 10.11 fell out of Apple extended
> >> security support in September 2018.)
> >> 
> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >> ---
> >> The bug report is recent, but this was also pointed out on
> >> the mailing list back in June 2019. Since nobody has cared
> >> to try to fix it, we clearly don't care about 10.11 in
> >> practice as well as in theory.]
> > 
> > Why would you want to inconvenience the Mac users? This is unacceptable.
> > I and many other Mac users would not be able to use the newer versions of
> > QEMU anymore. Keeping Mac OS 10.10 and up support doesn't seem like a
> > burden. This patch should not be applied.

Not that I really care, but I also wonder whether that would be an appropriate 
action to remove this code, as it does not seem to impose anything negative to 
retain it.

> The rationale is in the commit message -- QEMU *already* does not
> build on 10.11, and it has not done so for more than six months, and
> none of these Mac users has submitted a patch to try to fix it.

How about handling this with an error message at configure stage instead? 
Removing this code does not increase the chance for somebody going to fix 
pre-10.12 support.

> "Last two versions only" is our support policy for OSX for a reason,
> and the reason is that there just aren't very many people running
> OSX and actively working on QEMU. Of those, even fewer will
> be still running OSX versions that are no longer being supported
> for security fixes by Apple.

Apple currently seems to maintain 4 macOS release branches with security fixes 
(10.15.x, 10.14.x, 10.13.x, 10.12.x):
https://en.wikipedia.org/wiki/MacOS_Sierra#Release_history

Best regards,
Christian Schoenebeck

Re: [PATCH] ui/cocoa: Drop workarounds for pre-10.12 OSX

[Peter Maydell]

On Mon, 3 Feb 2020 at 19:59, G 3 <programmingkidx@gmail.com> wrote:
>> Our official OSX support policy covers the last two released versions.
>> Currently that is 10.14 and 10.15.  We also may work on older versions, but
>> don't guarantee it.
>>
>> In commit 50290c002c045280f8d in mid-2019 we introduced some uses of
>> CLOCK_MONOTONIC which incidentally broke compilation for pre-10.12 OSX
>> versions (see LP:1861551). We don't intend to fix that, so we might
>> as well drop the code in ui/cocoa.m which caters for pre-10.12
>> versions as well. (For reference, 10.11 fell out of Apple extended
>> security support in September 2018.)
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> The bug report is recent, but this was also pointed out on
>> the mailing list back in June 2019. Since nobody has cared
>> to try to fix it, we clearly don't care about 10.11 in
>> practice as well as in theory.]

> Why would you want to inconvenience the Mac users? This is unacceptable.
> I and many other Mac users would not be able to use the newer versions of QEMU
> anymore. Keeping Mac OS 10.10 and up support doesn't seem like a burden. This
> patch should not be applied.

The rationale is in the commit message -- QEMU *already* does not
build on 10.11, and it has not done so for more than six months, and
none of these Mac users has submitted a patch to try to fix it.
"Last two versions only" is our support policy for OSX for a reason,
and the reason is that there just aren't very many people running
OSX and actively working on QEMU. Of those, even fewer will
be still running OSX versions that are no longer being supported
for security fixes by Apple.

thanks
-- PMM

Re: [PATCH] ui/cocoa: Drop workarounds for pre-10.12 OSX

[G 3]

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

> Date: Sat,  1 Feb 2020 17:05:34 +0000
> From: Peter Maydell <peter.maydell@linaro.org>
> To: qemu-devel@nongnu.org
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Subject: [PATCH] ui/cocoa: Drop workarounds for pre-10.12 OSX
> Message-ID: <20200201170534.22123-1-peter.maydell@linaro.org>
>
> Our official OSX support policy covers the last two released versions.
> Currently that is 10.14 and 10.15.  We also may work on older versions, but
> don't guarantee it.
>
> In commit 50290c002c045280f8d in mid-2019 we introduced some uses of
> CLOCK_MONOTONIC which incidentally broke compilation for pre-10.12 OSX
> versions (see LP:1861551). We don't intend to fix that, so we might
> as well drop the code in ui/cocoa.m which caters for pre-10.12
> versions as well. (For reference, 10.11 fell out of Apple extended
> security support in September 2018.)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> The bug report is recent, but this was also pointed out on
> the mailing list back in June 2019. Since nobody has cared
> to try to fix it, we clearly don't care about 10.11 in
> practice as well as in theory.]
> ---
>  ui/cocoa.m | 59 ------------------------------------------------------
>  1 file changed, 59 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index fbb5b1b45f..f9945bc712 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -42,60 +42,10 @@
>  #include <Carbon/Carbon.h>
>  #include "hw/core/cpu.h"
>
> -#ifndef MAC_OS_X_VERSION_10_5
> -#define MAC_OS_X_VERSION_10_5 1050
> -#endif
> -#ifndef MAC_OS_X_VERSION_10_6
> -#define MAC_OS_X_VERSION_10_6 1060
> -#endif
> -#ifndef MAC_OS_X_VERSION_10_9
> -#define MAC_OS_X_VERSION_10_9 1090
> -#endif
> -#ifndef MAC_OS_X_VERSION_10_10
> -#define MAC_OS_X_VERSION_10_10 101000
> -#endif
> -#ifndef MAC_OS_X_VERSION_10_12
> -#define MAC_OS_X_VERSION_10_12 101200
> -#endif
>  #ifndef MAC_OS_X_VERSION_10_13
>  #define MAC_OS_X_VERSION_10_13 101300
>  #endif
>
> -/* macOS 10.12 deprecated many constants, #define the new names for older
> SDKs */
> -#if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_12
> -#define NSEventMaskAny                  NSAnyEventMask
> -#define NSEventModifierFlagCapsLock     NSAlphaShiftKeyMask
> -#define NSEventModifierFlagShift        NSShiftKeyMask
> -#define NSEventModifierFlagCommand      NSCommandKeyMask
> -#define NSEventModifierFlagControl      NSControlKeyMask
> -#define NSEventModifierFlagOption       NSAlternateKeyMask
> -#define NSEventTypeFlagsChanged         NSFlagsChanged
> -#define NSEventTypeKeyUp                NSKeyUp
> -#define NSEventTypeKeyDown              NSKeyDown
> -#define NSEventTypeMouseMoved           NSMouseMoved
> -#define NSEventTypeLeftMouseDown        NSLeftMouseDown
> -#define NSEventTypeRightMouseDown       NSRightMouseDown
> -#define NSEventTypeOtherMouseDown       NSOtherMouseDown
> -#define NSEventTypeLeftMouseDragged     NSLeftMouseDragged
> -#define NSEventTypeRightMouseDragged    NSRightMouseDragged
> -#define NSEventTypeOtherMouseDragged    NSOtherMouseDragged
> -#define NSEventTypeLeftMouseUp          NSLeftMouseUp
> -#define NSEventTypeRightMouseUp         NSRightMouseUp
> -#define NSEventTypeOtherMouseUp         NSOtherMouseUp
> -#define NSEventTypeScrollWheel          NSScrollWheel
> -#define NSTextAlignmentCenter           NSCenterTextAlignment
> -#define NSWindowStyleMaskBorderless     NSBorderlessWindowMask
> -#define NSWindowStyleMaskClosable       NSClosableWindowMask
> -#define NSWindowStyleMaskMiniaturizable NSMiniaturizableWindowMask
> -#define NSWindowStyleMaskTitled         NSTitledWindowMask
> -#endif
> -/* 10.13 deprecates NSFileHandlingPanelOKButton in favour of
> - * NSModalResponseOK, which was introduced in 10.9. Define
> - * it for older versions.
> - */
> -#if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_9
> -#define NSModalResponseOK NSFileHandlingPanelOKButton
> -#endif
>  /* 10.14 deprecates NSOnState and NSOffState in favor of
>   * NSControlStateValueOn/Off, which were introduced in 10.13.
>   * Define for older versions
> @@ -465,11 +415,7 @@ - (void) drawRect:(NSRect) rect
>      COCOA_DEBUG("QemuCocoaView: drawRect\n");
>
>      // get CoreGraphic context
> -#if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_10
> -    CGContextRef viewContextRef = [[NSGraphicsContext currentContext]
> graphicsPort];
> -#else
>      CGContextRef viewContextRef = [[NSGraphicsContext currentContext]
> CGContext];
> -#endif
>
>      CGContextSetInterpolationQuality (viewContextRef,
> kCGInterpolationNone);
>      CGContextSetShouldAntialias (viewContextRef, NO);
> @@ -1075,9 +1021,7 @@ - (void) raiseAllKeys
>   ------------------------------------------------------
>  */
>  @interface QemuCocoaAppController : NSObject
> -#if (MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_6)
>                                         <NSWindowDelegate,
> NSApplicationDelegate>
> -#endif
>  {
>  }
>  - (void)doToggleFullScreen:(id)sender;
> @@ -1126,9 +1070,6 @@ - (id) init
>          [normalWindow setAcceptsMouseMovedEvents:YES];
>          [normalWindow setTitle:@"QEMU"];
>          [normalWindow setContentView:cocoaView];
> -#if (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_10)
> -        [normalWindow useOptimizedDrawing:YES];
> -#endif
>          [normalWindow makeKeyAndOrderFront:self];
>          [normalWindow center];
>          [normalWindow setDelegate: self];
> --
> 2.17.2 (Apple Git-113)
>
>
> Why would you want to inconvenience the Mac users? This is unacceptable. I
and many other Mac users would not be able to use the newer versions of
QEMU anymore. Keeping Mac OS 10.10 and up support doesn't seem like a
burden. This patch should not be applied.

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