qemu @5.2.0: build failure with +cocoa

[Ionic (Mihai Moldovan)]

This targets legacy OS versions only, so Raim doesn't need to care.

Some relevant excerpts from the build log on 10.9:

In file included from ../qemu-5.2.0/ui/cocoa.m:25:
In file included from /opt/local/var/macports/build/_opt_local_var_macports_sources_macports.rsync.ionic.de_release_ports_emulators_qemu/qemu/work/qemu-5.2.0/include/qemu/osdep.h:93:
/opt/local/include/LegacySupport/unistd.h:34:5: warning: '__DISABLE_MP_LEGACY_SUPPORT_SYSCONF_WRAP__' is not defined, evaluates to 0 [-Wundef]
/opt/local/include/LegacySupport/MacportsLegacySupport.h:107:12: note: expanded from macro '__ENABLE_MP_LEGACY_SUPPORT_SYSCONF_WRAP__'
                                                    !__DISABLE_MP_LEGACY_SUPPORT_SYSCONF_WRAP__       && \
                                                     ^
../qemu-5.2.0/ui/cocoa.m:421:71: warning: instance method '-CGContext' not found (return type defaults to 'id'); did you mean '-CIContext'? [-Wobjc-method-access]
    CGContextRef viewContextRef = [[NSGraphicsContext currentContext] CGContext];
                                                                      ^~~~~~~~~
                                                                      CIContext
/System/Library/Frameworks/AppKit.framework/Headers/NSGraphicsContext.h:39:12: note: receiver is instance of class declared here
@interface NSGraphicsContext : NSObject {
           ^
../qemu-5.2.0/ui/cocoa.m:421:18: warning: incompatible pointer types initializing 'CGContextRef' (aka 'struct CGContext *') with an expression of type 'id' [-Wincompatible-pointer-types]
    CGContextRef viewContextRef = [[NSGraphicsContext currentContext] CGContext];
                 ^                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../qemu-5.2.0/ui/cocoa.m:588:27: error: use of undeclared identifier 'NSWindowStyleMaskBorderless'
                styleMask:NSWindowStyleMaskBorderless
                          ^
../qemu-5.2.0/ui/cocoa.m:623:33: error: use of undeclared identifier 'NSEventModifierFlagControl'
    if ([event modifierFlags] & NSEventModifierFlagControl) {
                                ^
../qemu-5.2.0/ui/cocoa.m:709:14: error: use of undeclared identifier 'NSEventTypeFlagsChanged'; did you mean 'kCGEventFlagsChanged'?
        case NSEventTypeFlagsChanged:
             ^~~~~~~~~~~~~~~~~~~~~~~
             kCGEventFlagsChanged
/System/Library/Frameworks/CoreGraphics.framework/Headers/CGEventTypes.h:117:3: note: 'kCGEventFlagsChanged' declared here
  kCGEventFlagsChanged = NX_FLAGSCHANGED,
  ^
../qemu-5.2.0/ui/cocoa.m:718:40: error: use of undeclared identifier 'NSEventModifierFlagCapsLock'
                    if (!!(modifiers & NSEventModifierFlagCapsLock) != !!modifiers_state[Q_KEY_CODE_CAPS_LOCK]) {
                                       ^
../qemu-5.2.0/ui/cocoa.m:721:40: error: use of undeclared identifier 'NSEventModifierFlagShift'
                    if (!!(modifiers & NSEventModifierFlagShift) != !!modifiers_state[Q_KEY_CODE_SHIFT]) {
                                       ^
../qemu-5.2.0/ui/cocoa.m:724:40: error: use of undeclared identifier 'NSEventModifierFlagControl'
                    if (!!(modifiers & NSEventModifierFlagControl) != !!modifiers_state[Q_KEY_CODE_CTRL]) {
                                       ^
../qemu-5.2.0/ui/cocoa.m:727:40: error: use of undeclared identifier 'NSEventModifierFlagOption'
                    if (!!(modifiers & NSEventModifierFlagOption) != !!modifiers_state[Q_KEY_CODE_ALT]) {
                                       ^
../qemu-5.2.0/ui/cocoa.m:730:40: error: use of undeclared identifier 'NSEventModifierFlagCommand'
                    if (!!(modifiers & NSEventModifierFlagCommand) != !!modifiers_state[Q_KEY_CODE_META_L]) {
                                       ^
../qemu-5.2.0/ui/cocoa.m:759:14: error: use of undeclared identifier 'NSEventTypeKeyDown'
        case NSEventTypeKeyDown:
             ^
../qemu-5.2.0/ui/cocoa.m:763:61: error: use of undeclared identifier 'NSEventModifierFlagCommand'
            if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) {
                                                            ^
../qemu-5.2.0/ui/cocoa.m:777:42: error: use of undeclared identifier 'NSEventModifierFlagControl'
            if (([event modifierFlags] & NSEventModifierFlagControl) && ([event modifierFlags] & NSEventModifierFlagOption)) {
                                         ^
../qemu-5.2.0/ui/cocoa.m:777:98: error: use of undeclared identifier 'NSEventModifierFlagOption'
            if (([event modifierFlags] & NSEventModifierFlagControl) && ([event modifierFlags] & NSEventModifierFlagOption)) {
                                                                                                 ^
../qemu-5.2.0/ui/cocoa.m:802:14: error: use of undeclared identifier 'NSEventTypeKeyUp'; did you mean 'NSEventTypeSwipe'?
        case NSEventTypeKeyUp:
             ^~~~~~~~~~~~~~~~
             NSEventTypeSwipe

Ken, what do you think? Back then, in #59257, your fork included a downgrade of the cocoa interface from 4.1.0 to 4.0.0. We probably can't keep up with that going forward, since we'd lose any new changes to the UI (and, also, because features within qemu might be removed that the old UI references/uses).

Do we want to try to make the code compatible with older operating systems, or just take the easy way out and let the cocoa variant throw an error for anything below 10.12?

I don't currently know how much effort it would be to rewrite the cocoa code to make it work on older systems. A quick glance revealed that some errors are very easy to fix (for instance, NSEventTypeKeyUp was introduced in 10.12, while older systems provide the same functionality as NSKeyDown), while others might be more difficult to rewrite or may even not have a direct deprecated alternative we could use instead. Going forward, it will probably get ever more difficult to keep the code compatible.

I guess I'll try to make it compatible, as far as I can. My target is 10.9, though.

[Ionic (Mihai Moldovan)]

build log on 10.9.

[kencu (Ken)]

There is a nice compatibility #include file that defines all the new NS names to the old NS names. let me see if I can find it.

[kencu (Ken)]

Here it is -- maybe this makes life easier?

<​https://github.com/macports/macports-ports/blob/master/graphics/blender/files/old_appkit_compat/AppKit/AppKit.h>

[Ionic (Mihai Moldovan)]

Ah, that would have worked, yeah.

Too late now, I did all of this manually now. Let's see if it works.

Still, the real question is if it's worth the effort. We'll at some point hit a new feature for which no deprecated version exists and we likely won't be able to get around that... naturally, at that point, we can still error out the cocoa variant and say that we just won't support it.

[Ionic (Mihai Moldovan)]

Oh, and that file actually does the opposite, if I understand it correctly. It defines deprecated versions which probably were removed in newer OS versions. We'd need the other way around, providing the new stuff mapped to the old, deprecated one.

[kencu (Ken)]

It was written to allow new code to compile with old SDKs...on the left is the new name, on the right is the old name.

I haven't used it, but it looks right way around to me ... have to try.

[kencu (Ken)]

what we will have to do, if we want this to work long term, is to do it properly.

Build against MacOS10.14.sdk or some such, and use the correct Apple __availability setups in the actual code, with fallbacks where needed.

Upstream would (possibly) accept that.

[Ionic (Mihai Moldovan)]

This seems to compile, but somehow doesn't work properly. I cannot input anything. Whenever I press a key, I just get an audio bell signal. Also, I cannot uncapture the mouse pointer once captured (well, naturally, because the keyboard input doesn't seem to be getting processed correctly).

[Ionic (Mihai Moldovan)]

Rewriting that code with availability macros and pushing it upstream might work.

But once qemu starts using new features for which no deprecated alternative exists, we'd have to give it up.

[kencu (Ken)]

I don't immediately see the problem with your patch, sorry.

The availability stuff, I'm sure you know, looks like ​this now:

void my_function(NSSomeClass* var) {
  if (@available(macOS 10.12, *)) {
    [var fancyNewMethod];
  } else {
    // Put fallback behavior for old macOS versions (and for non-mac
    // platforms) here.
  }
}

[Ionic (Mihai Moldovan)]

Nope, never used it in ObjC code, good to know. :)

Yeah, the patch looks fine and there are no warnings, apart from the usual legacy-support ones. We'd better fix those, they are very distracting. But they won't cause any bad behavior, so that's okay.

Maybe the culprit is the part that you reverted, i.e., the new method that is used to get the pointer location. Will have to look into that.

[jmroot (Joshua Root)]

Failing with a slightly different error as of 6.1:

../qemu-6.1.0/ui/cocoa.m:1766:52: error: unknown type name 'NSPasteboardTypeOwner'; did you mean 'NSPasteboardType'?
@interface QemuCocoaPasteboardTypeOwner : NSObject<NSPasteboardTypeOwner>
                                                   ^~~~~~~~~~~~~~~~~~~~~
                                                   NSPasteboardType
/System/Library/Frameworks/AppKit.framework/Headers/NSPasteboard.h:22:20: note: 'NSPasteboardType' declared here
typedef NSString * NSPasteboardType NS_EXTENSIBLE_STRING_ENUM;
                   ^
../qemu-6.1.0/ui/cocoa.m:1766:43: error: type arguments cannot be applied to non-parameterized class 'NSObject'
@interface QemuCocoaPasteboardTypeOwner : NSObject<NSPasteboardTypeOwner>
                                          ^       ~~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.

​https://build.macports.org/builders/ports-10.13_x86_64-builder/builds/122976/steps/install-port/logs/stdio

I guess at minimum, +cocoa should not be on by default on <= 10.13, if it can't be fixed?

[jasonliu-- (Jason Liu)]

Replying to Ionic:

Oh, and that file actually does the opposite, if I understand it correctly. It defines deprecated versions which probably were removed in newer OS versions. We'd need the other way around, providing the new stuff mapped to the old, deprecated one.

The file that Ken point to, which was written by me, #defines the old names to the new names. This is because my file intervenes and patches the system-wide AppKit framework, and makes it look like a "newer" (i.e. post-10.12) AppKit framework. The project's source code is left unchanged, because from the perspective of the source code, it is #include-ing an AppKit that has all of the new entities.

The technique that you are describing, where the new names get mapped to the old names, is the technique that is used if you are patching the entities in the project's source code, but leaving the system-wide AppKit framework untouched.

I hope the reasoning behind each technique is clear.

[jasonliu-- (Jason Liu)]

Replying to kencu:

I don't immediately see the problem with your patch, sorry.

The availability stuff, I'm sure you know, looks like ​this now:

void my_function(NSSomeClass* var) {
  if (@available(macOS 10.12, *)) {
    [var fancyNewMethod];
  } else {
    // Put fallback behavior for old macOS versions (and for non-mac
    // platforms) here.
  }
}

Note that this technique has one very notable disadvantage when compared with the older, more traditional technique of the availability macros, like MAC_OS_X_VERSION_MAX_ALLOWED, MAC_OS_X_VERSION_MIN_REQUIRED, etc. Using @available allows code that has been compiled on newer systems to be run on older systems, but @available does not have the ability to hide any code from being seen by the compiler. On the other hand, when you use the availability macros, the preprocessor will actually hide the code from the compiler. This means that if you try to compile code that uses @available on an older macOS, but [var fancyNewMethod]; only exists in a newer version of macOS, you will still get a compiler error.

References:

• ​https://github.com/macports/macports-ports/blob/master/python/py-matplotlib/files/patch-v34-src-macosx.m.diff
• ​https://github.com/macports/macports-ports/commit/b07e0eb6440b1499bfd2157e19db86fc79984471#comments

[jasonliu-- (Jason Liu)]

Replying to jmroot:

Failing with a slightly different error as of 6.1:

../qemu-6.1.0/ui/cocoa.m:1766:52: error: unknown type name 'NSPasteboardTypeOwner'; did you mean 'NSPasteboardType'?
@interface QemuCocoaPasteboardTypeOwner : NSObject<NSPasteboardTypeOwner>
                                                   ^~~~~~~~~~~~~~~~~~~~~
                                                   NSPasteboardType
/System/Library/Frameworks/AppKit.framework/Headers/NSPasteboard.h:22:20: note: 'NSPasteboardType' declared here
typedef NSString * NSPasteboardType NS_EXTENSIBLE_STRING_ENUM;
                   ^
../qemu-6.1.0/ui/cocoa.m:1766:43: error: type arguments cannot be applied to non-parameterized class 'NSObject'
@interface QemuCocoaPasteboardTypeOwner : NSObject<NSPasteboardTypeOwner>
                                          ^       ~~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.

​https://build.macports.org/builders/ports-10.13_x86_64-builder/builds/122976/steps/install-port/logs/stdio

I guess at minimum, +cocoa should not be on by default on <= 10.13, if it can't be fixed?

Interestingly, I tried grepping for NSPasteboardTypeOwner in my copy of the 10.15 AppKit framework, and wasn't able to find it anywhere in the entire framework. However, Apple's own documentation clearly states that it is something that came into existence in AppKit starting in macOS 10.14+: ​https://developer.apple.com/documentation/appkit/nspasteboardtypeowner, so I'm not really sure what's going on with that one.

[evanmiller (Evan Miller)]

In c131d83ff9c95e756451e72714f1998600e80092/macports-ports (master):

qemu: default to +curses (not +cocoa) pre-10.14

See: #56742
See: #62164