fix(appkit): add NSSavePanel tag-name compatibility (#112)

Co-authored-by: Kalah <[email protected]>

docs/TESTING.md

@@ -222,6 +222,31 @@ and prints `OPENPANEL_CHECK mode=interactive result=RETURNED code=... url=...`
 once it closes. As with the other probes, check the printed line rather than
 only the launcher exit status.
 
+### NSSavePanel Finder tags (`setTagNames:` / `tagNames`)
+
+`python3 tools/build-riot-save-panel-tag-check.py` builds
+`build/riot-save-panel-tag-check`. Install the patched AppKit first, with clients
+and the game closed. The probe checks selector availability, inheritance, and
+safe no-op semantics without launching any client or creating window fixtures:
+
+```sh
+P=/Volumes/SystemRoot/path/to/darling-lol/build/riot-save-panel-tag-check
+E="env DYLD_LIBRARY_PATH=/Users/me/RuntimeLibraries DYLD_FRAMEWORK_PATH=/Users/me/RuntimeFrameworks"
+darling shell $E $P
+```
+
+Expected output:
+
+```
+SAVE_PANEL_TAG class=NSSavePanel selectors=1 inherited=1 values=1 pass=1
+SAVE_PANEL_TAG class=NSOpenPanel selectors=1 inherited=1 values=1 pass=1
+SAVE_PANEL_TAG_CHECK pass=1
+```
+
+Cocotron does not persist Finder tag metadata or provide a tag UI. `setTagNames:`
+safely ignores input and `tagNames` returns an empty `NSArray` to prevent
+unrecognized-selector crashes from callers (such as Electron file dialogs).
+
 ### Presentation publisher handoff
 
 With clients closed, run `build/riot-client-resize-check --handoff` through Darling with the normal runtime overrides. This creates an owned window and two fixture frame slots. The live software owner first supplies a 1280×720 main frame, then switches to a 262×131 popup; another slot supplies the resized main frame without the software flag. The test samples its own window beyond the original frame extent and checks generation colors across growth and shrinkage. The old selector reports black at the 1600×900 sample; the correction passes all four sizes. Repeat with `RCLOL_XSHM=0` and with `DARLING_XSHM_LIBRARY` pointing to the normal transport to cover both paths. The fixtures remove their own frame slots on exit; they do not read client pixels or send input.

patches/cocotron.patch

@@ -11408,7 +11408,7 @@ diff --git a/AppKit/NSSavePanel.subproj/NSSavePanel.m b/AppKit/NSSavePanel.subpr
 index 50c3d59e..be5a8463 100644
 --- a/AppKit/NSSavePanel.subproj/NSSavePanel.m
 +++ b/AppKit/NSSavePanel.subproj/NSSavePanel.m
-@@ -113,19 +113,108 @@
+@@ -113,19 +113,116 @@
      }
  }
  
@@ -11510,6 +11510,14 @@ index 50c3d59e..be5a8463 100644
 +    return NO;
 +}
 +
++- (void) setTagNames: (NSArray *) names {
++    (void)names;
++}
++
++- (NSArray *) tagNames {
++    return [NSArray array];
++}
++
 +- (void) setExtensionHidden: (BOOL) flag {
 +}
 +
@@ -11580,7 +11588,7 @@ diff --git a/AppKit/include/AppKit/NSSavePanel.h b/AppKit/include/AppKit/NSSaveP
 index 53bacc46..3c6adfb7 100644
 --- a/AppKit/include/AppKit/NSSavePanel.h
 +++ b/AppKit/include/AppKit/NSSavePanel.h
-@@ -88,6 +88,18 @@ enum {
+@@ -88,6 +88,20 @@ enum {
  - (void) setPrompt: (NSString *) message;
  - (NSString *) prompt;
  
@@ -11588,6 +11596,8 @@ index 53bacc46..3c6adfb7 100644
 +- (NSString *) nameFieldLabel;
 +- (void) setShowsTagField: (BOOL) flag;
 +- (BOOL) showsTagField;
++- (void) setTagNames: (NSArray *) names;
++- (NSArray *) tagNames;
 +- (void) setExtensionHidden: (BOOL) flag;
 +- (BOOL) isExtensionHidden;
 +- (void) setCanSelectHiddenExtension: (BOOL) flag;

tests/test_save_panel_tags.py

@@ -0,0 +1,169 @@
+"""Offline contracts for NSSavePanel's unsupported Finder-tag compatibility."""
+import json
+from pathlib import Path
+import re
+import runpy
+import shutil
+import subprocess
+import tempfile
+import unittest
+from unittest.mock import patch
+
+
+ROOT = Path(__file__).resolve().parents[1]
+IMPLEMENTATION = 'AppKit/NSSavePanel.subproj/NSSavePanel.m'
+HEADER = 'AppKit/include/AppKit/NSSavePanel.h'
+PROBE_SOURCE = 'tools/riot-save-panel-tag-check.m'
+BUILD_HELPER = 'tools/build-riot-save-panel-tag-check.py'
+
+
+def added_source(path):
+    marker = f'diff --git a/{path} b/{path}\n'
+    section = (ROOT / 'patches/cocotron.patch').read_text().split(marker, 1)[1]
+    section = section.split('\ndiff --git ', 1)[0]
+    return '\n'.join(line[1:] for line in section.splitlines()
+                     if line.startswith('+') and not line.startswith('+++'))
+
+
+def extract_patch(path):
+    marker = f'diff --git a/{path} b/{path}\n'
+    patch_text = (ROOT / 'patches/cocotron.patch').read_text()
+    if marker not in patch_text:
+        raise AssertionError(f'Missing patch section for {path}')
+    section = patch_text.split(marker, 1)[1].split('\ndiff --git ', 1)[0]
+    return marker + section + '\n'
+
+
+def method(source, signature):
+    match = re.search(re.escape(signature) + r'\s*\{([^}]+)\}', source)
+    if match is None:
+        raise AssertionError(f'Missing implementation: {signature}')
+    return match.group(1)
+
+
+class SavePanelTagTests(unittest.TestCase):
+    def test_tag_names_setter_accepts_unsupported_metadata_without_storing_it(self):
+        body = method(added_source(IMPLEMENTATION), '- (void) setTagNames: (NSArray *) names')
+        self.assertIn('(void)names;', body)
+        self.assertNotIn('retain', body)
+        self.assertNotIn('copy', body)
+
+    def test_tag_names_getter_returns_an_empty_array_not_nil(self):
+        body = method(added_source(IMPLEMENTATION), '- (NSArray *) tagNames')
+        self.assertIn('return [NSArray array];', body)
+
+    def test_public_header_declares_both_tag_name_methods(self):
+        header = added_source(HEADER)
+        self.assertIn('- (void) setTagNames: (NSArray *) names;', header)
+        self.assertIn('- (NSArray *) tagNames;', header)
+
+    def test_existing_tag_field_and_modal_fixes_are_retained(self):
+        source = added_source(IMPLEMENTATION)
+        self.assertIn('- (void) setShowsTagField: (BOOL) flag', source)
+        self.assertIn('- (BOOL) showsTagField', source)
+        self.assertIn('- (void) _dismissWithCode: (NSModalResponse) code', source)
+        self.assertNotIn('setShowsTags:', source)
+
+    def test_patch_applies_cleanly_to_pinned_upstream_cocotron(self):
+        if not shutil.which('patch'):
+            self.skipTest('patch executable required')
+        manifest = json.loads((ROOT / 'upstream/manifest.json').read_text())
+        component = next(c for c in manifest['components'] if c['patch'] == 'patches/cocotron.patch')
+        repo = ROOT / 'build/darling-src' / component['path']
+        if not (repo / '.git').exists():
+            self.skipTest('prepared Darling sources required')
+
+        revision = component['revision']
+        for relpath in [IMPLEMENTATION, HEADER]:
+            filename = Path(relpath).name
+            original = subprocess.check_output(['git', '-C', str(repo), 'show', f'{revision}:{relpath}'])
+            with tempfile.TemporaryDirectory() as td:
+                work_file = Path(td) / filename
+                work_file.write_bytes(original)
+                patch_file = Path(td) / f'{filename}.patch'
+                patch_file.write_text(extract_patch(relpath))
+                res = subprocess.run(
+                    ['patch', '--dry-run', '--fuzz=0', str(work_file), str(patch_file)],
+                    capture_output=True,
+                    text=True,
+                )
+                self.assertEqual(res.returncode, 0, f'Patch failed to apply to {relpath}: {res.stderr}\n{res.stdout}')
+
+    def test_extracted_method_bodies_compile_with_clang(self):
+        clang = shutil.which('clang')
+        if not clang:
+            self.skipTest('clang not available')
+        impl = added_source(IMPLEMENTATION)
+        setter_body = method(impl, '- (void) setTagNames: (NSArray *) names')
+        getter_body = method(impl, '- (NSArray *) tagNames')
+        snippet = f'''
+@interface NSObject
++ (instancetype)alloc;
+- (instancetype)init;
+@end
+
+@interface NSArray : NSObject
++ (instancetype)array;
+@end
+
+@interface NSSavePanel : NSObject
+- (void) setTagNames: (NSArray *) names;
+- (NSArray *) tagNames;
+@end
+
+@implementation NSSavePanel
+- (void) setTagNames: (NSArray *) names {{{setter_body}}}
+- (NSArray *) tagNames {{{getter_body}}}
+@end
+'''
+        with tempfile.NamedTemporaryFile(suffix='.m', mode='w') as f:
+            f.write(snippet)
+            f.flush()
+            res = subprocess.run(
+                [clang, '-Qunused-arguments', '-x', 'objective-c', '-fsyntax-only', '-Werror', f.name],
+                capture_output=True,
+                text=True,
+            )
+            self.assertEqual(res.returncode, 0, f'Clang syntax check failed: {res.stderr}')
+
+    def test_builder_compiles_and_links_probe_without_embedding_shim(self):
+        source = ROOT / BUILD_HELPER
+        self.assertTrue(source.exists(), f'Missing build helper: {BUILD_HELPER}')
+        with tempfile.TemporaryDirectory() as temporary:
+            project = Path(temporary)
+            driver = project / 'tools' / source.name
+            driver.parent.mkdir()
+            driver.write_text(source.read_text())
+            framework = project / 'build/darling-src/build/src/external/cocotron/AppKit'
+            cmake = framework / 'CMakeFiles/X11_backend.dir'
+            cmake.mkdir(parents=True)
+            (cmake / 'flags.make').write_text('C_DEFINES = -DTEST\nC_INCLUDES = -Itest\nC_FLAGS = -O0\n')
+            calls = []
+            with patch('subprocess.run', side_effect=lambda argv, **kw: calls.append((argv, kw))):
+                runpy.run_path(str(driver), run_name='__main__')
+            probe_name = 'riot-save-panel-tag-check'
+            compiles = [argv for argv, _ in calls if str(project / f'tools/{probe_name}.m') in argv]
+            self.assertEqual(len(compiles), 1, 'probe must be compiled')
+            self.assertEqual(compiles[0][compiles[0].index('-o') + 1], str(project / f'build/{probe_name}.o'))
+            links = [argv for argv, _ in calls if str(project / f'build/{probe_name}') in argv]
+            self.assertEqual(len(links), 1, 'probe must be linked')
+            self.assertIn(str(project / f'build/{probe_name}.o'), links[0])
+            self.assertIn('AppKit', links[0])
+            for argv, _ in calls:
+                for arg in argv:
+                    self.assertNotIn('compat', arg)
+
+    def test_probe_declares_only_and_implements_no_shim(self):
+        source = (ROOT / PROBE_SOURCE).read_text()
+        self.assertIn('@interface NSSavePanel (RiotTagProbeDeclarations)', source)
+        self.assertIn('- (void)setTagNames:(NSArray *)names;', source)
+        self.assertIn('- (NSArray *)tagNames;', source)
+        self.assertIn('- (void)setShowsTagField:(BOOL)value;', source)
+        self.assertIn('- (BOOL)showsTagField;', source)
+        self.assertNotIn('@implementation NSSavePanel', source, 'probe must not contain compatibility shim')
+        self.assertIn('checkPanel([NSSavePanel class])', source)
+        self.assertIn('checkPanel([NSOpenPanel class])', source)
+
+
+if __name__ == '__main__':
+    unittest.main()

tools/build-riot-save-panel-tag-check.py

@@ -0,0 +1,24 @@
+#!/usr/bin/env python3
+"""Build the standalone NSSavePanel Finder-tag compatibility check."""
+from pathlib import Path
+import json
+import shlex
+import subprocess
+
+root = Path(__file__).resolve().parent.parent
+folder = root / 'build/darling-src/build/src/external/cocotron/AppKit'
+config = root / 'build/config.json'
+sdk = json.loads(config.read_text())['sdk'] if config.exists() else '/usr/local/libexec/darling'
+flags = {}
+for line in (folder / 'CMakeFiles/X11_backend.dir/flags.make').read_text().splitlines():
+    if line.startswith('C_') and ' = ' in line:
+        key, value = line.split(' = ', 1)
+        flags[key] = shlex.split(value)
+obj = root / 'build/riot-save-panel-tag-check.o'
+subprocess.run(['clang', *flags['C_DEFINES'], *flags['C_INCLUDES'], *flags['C_FLAGS'],
+                '-O2', '-fblocks', '-UNDEBUG', '-Wno-macro-redefined', '-c', str(root / 'tools/riot-save-panel-tag-check.m'),
+                '-o', str(obj)], check=True)
+subprocess.run(['clang', '-target', 'x86_64-apple-macosx11.0', '-fuse-ld=lld', '-nostdlib',
+                '-isysroot', sdk, str(obj), '-lSystem', '-lobjc',
+                '-framework', 'AppKit', '-framework', 'CoreFoundation', '-framework', 'Foundation',
+                '-o', str(root / 'build/riot-save-panel-tag-check')], check=True)

tools/riot-save-panel-tag-check.m

@@ -0,0 +1,74 @@
+#import <AppKit/NSApplication.h>
+#import <AppKit/NSSavePanel.h>
+#import <AppKit/NSOpenPanel.h>
+#import <Foundation/Foundation.h>
+#import <objc/runtime.h>
+#include <stdio.h>
+
+/* Declarations only: implementations must come from the loaded AppKit, not
+ * from linking a compatibility category directly into this probe. */
+@interface NSSavePanel (RiotTagProbeDeclarations)
+- (void)setShowsTagField:(BOOL)value;
+- (BOOL)showsTagField;
+- (void)setTagNames:(NSArray *)names;
+- (NSArray *)tagNames;
+@end
+
+static BOOL emptyTags(NSSavePanel *panel) {
+    NSArray *tags = [panel tagNames];
+    return tags != nil && [tags isKindOfClass:[NSArray class]] && [tags count] == 0;
+}
+
+static BOOL checkPanel(Class panelClass) {
+    SEL selectors[] = {@selector(setShowsTagField:), @selector(showsTagField),
+                       @selector(setTagNames:), @selector(tagNames)};
+    BOOL available = YES, inherited = YES, values = NO;
+    /* This stateless probe must work without panel initialization. Bare runtime
+     * allocation avoids NSWindow/nib/display setup; it does NOT test the chooser.
+     * Dispose without running NSWindow's deallocator on an uninitialized panel. */
+    NSSavePanel *panel = (NSSavePanel *)class_createInstance(panelClass, 0);
+    for (unsigned i = 0; i < sizeof(selectors) / sizeof(selectors[0]); ++i) {
+        Method base = class_getInstanceMethod([NSSavePanel class], selectors[i]);
+        Method method = class_getInstanceMethod(panelClass, selectors[i]);
+        available &= panel != nil && method != NULL && [panel respondsToSelector:selectors[i]];
+        inherited &= base != NULL && method != NULL &&
+            method_getImplementation(base) == method_getImplementation(method);
+    }
+    if (panelClass == [NSOpenPanel class])
+        inherited &= class_getSuperclass(panelClass) == [NSSavePanel class];
+    if (available) {
+        @try {
+            values = ![panel showsTagField] && emptyTags(panel);
+            [panel setShowsTagField:YES];
+            values &= ![panel showsTagField];
+            [panel setShowsTagField:NO];
+            values &= ![panel showsTagField];
+            NSMutableArray *names = [NSMutableArray arrayWithObject:@"probe-tag"];
+            [panel setTagNames:names];
+            [names addObject:@"second-tag"];
+            values &= emptyTags(panel);
+            [panel setTagNames:[NSArray array]];
+            values &= emptyTags(panel);
+            [panel setTagNames:nil];
+            values &= emptyTags(panel);
+        } @catch (NSException *exception) {
+            fprintf(stderr, "SAVE_PANEL_TAG exception=%s\n", [[exception name] UTF8String]);
+            values = NO;
+        }
+    }
+    if (panel != nil)
+        object_dispose(panel);
+    BOOL pass = available && inherited && values;
+    printf("SAVE_PANEL_TAG class=%s selectors=%d inherited=%d values=%d pass=%d\n",
+           class_getName(panelClass), available, inherited, values, pass);
+    return pass;
+}
+
+int main(void) {
+    @autoreleasepool {
+        BOOL save = checkPanel([NSSavePanel class]);
+        BOOL open = checkPanel([NSOpenPanel class]);
+        printf("SAVE_PANEL_TAG_CHECK pass=%d\n", save && open);
+        return save && open ? 0 : 1;
+    }
+}