[Binding Creator] Enhance documentation for adding new frameworks and clarify Xtro checks#26028
[Binding Creator] Enhance documentation for adding new frameworks and clarify Xtro checks#26028dalexsoto wants to merge 2 commits into
Conversation
… clarify Xtro checks
| > ⚠️ **Xcode 27: `simctl create` dropped `--json`.** mlaunch auto-creates the sim device via `simctl create … --json`, which now fails with `MT1008 … simctl: unrecognized option '--json'`. Workaround: **pre-create** the device with the exact name mlaunch expects so it finds it instead of creating it, e.g. `xcrun simctl create "iPhone 16 Pro - iOS 27.0" <devicetype-id> <runtime-id>`, then re-run mlaunch. (Environment-specific to Xcode 27 until mlaunch is updated.) | ||
|
|
There was a problem hiding this comment.
@rolfbjarne did copilot actually catch a bug or just needs better docs? 😂
There was a problem hiding this comment.
Yeah, this might be an mlaunch bug! mlaunch passing --json when creating simulators might just be accidental, and it worked in the past accidentally.
There was a problem hiding this comment.
Pull request overview
This PR improves the macios-binding-creator skill documentation to better guide contributors through (1) adding bindings for an entirely new Apple framework and (2) understanding what XTRO does and does not validate, plus a few high-impact test/debug workflow clarifications.
Changes:
- Added a cross-reference and warning in
SKILL.mdfor the extra build/test wiring required when binding a brand-new framework. - Expanded the test workflow reference to clarify XTRO selector coverage (native → managed only) and added practical notes for documentation baselines and desktop introspection pitfalls.
- Added a detailed “Registering a Brand-New Framework” checklist (with build gotchas) and additional binding patterns to
binding-patterns.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| .agents/skills/macios-binding-creator/SKILL.md | Adds prominent warnings/links about brand-new framework wiring and correct preprocessor symbol ordering for Mac Catalyst. |
| .agents/skills/macios-binding-creator/references/test-workflow.md | Clarifies XTRO selector behavior, documentation baseline expectations, and adds troubleshooting guidance for simulator creation and desktop introspection. |
| .agents/skills/macios-binding-creator/references/binding-patterns.md | Introduces a checklist for registering new frameworks and documents build/test “gotchas” and NS_TYPED_ENUM set-wrapping patterns. |
| - **Unbound Objective-C class** → alias it to `Foundation.NSObject` with a `using` at the top of the API-definition file (a short comment naming the owning framework helps reviewers). Precedent: `src/browserenginekit.cs:47` aliases the XPC `xpc_object_t` type this way — `using OS_xpc_object = Foundation.NSObject;` — for its `xpc_object_t` parameters/returns. (This is *different* from platform-stubbing an otherwise-bound type under `#if` — for that, see "Platform Exclusion for Manual Types" below.) | ||
| - **C primitive** (`kern_return_t`, `IOReturn`, `io_service_t`, …) → map to a plain integer / `IntPtr`, never a bound type. Verified precedent: `kern_return_t` → `int` in `src/IOSurface/IOSurface.cs` (`// kern_return_t` → `public int Lock (...)`). For other C typedefs, confirm the width/signedness from the header before choosing `int`/`uint`/`nint`/`IntPtr`. | ||
|
|
||
| > ⚠️ Reviewers may ask "why not bind this type?" — the answer is that the owning framework is deliberately out of macios's scope (IOKit-family), so the NSObject-alias / integer fallback is the intended convention, not a shortcut. |
There was a problem hiding this comment.
HAHAHA those pesky human meatbags 😂
This comment has been minimized.
This comment has been minimized.
Mined four real Copilot sessions that ran the skill (Xcode 27 binding tasks: WebKit, VideoToolbox, Vision, UserNotifications) and fixed the gaps they exposed. All edits were triple-checked read-only by Opus 4.8, Sonnet 5, and GPT-5.5 (unanimous approval). - Error enums: add a NEVER-availability-attributes anti-pattern. The generic "each new enum member needs its own [iOS]" rule is build-breaking for error enums (cecil EnumTest.NoAvailabilityOnError, no allowlist). Hit in 2 of 4 sessions. - Enum members: rework the per-member availability rule to match the native header (add nothing when the member inherits), and add a multi-platform inheritance note (bgen back-fills the parent's older version via FindHighestIntroducedAttributes). - Elevate the named-delegate rule (never Action<T>/Func<T> for callbacks) into a SKILL.md Step 4 anti-pattern; it was buried in a reference and missed (required a user correction in WebKit). - Document introspection host-OS version gating in Step 6c: validate brand-new-SDK APIs on a matching-runtime simulator, not on a host macOS older than the SDK. - Add a training log under .agents/training-logs/. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5a002881-ea12-47a9-aed1-e64eb7b173c1
🔥 [PR Build #3ff934b] Build failed (Build macOS tests) 🔥Build failed for the job 'Build macOS tests' (with job status 'Failed') Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
🔥 [CI Build #3ff934b] Test results 🔥Test results❌ Tests failed on VSTS: test results 7 tests crashed, 2 tests failed, 151 tests passed. Failures❌ dotnettests tests (MacCatalyst)🔥 Failed catastrophically on VSTS: test results - dotnettests_maccatalyst (no summary found). Html Report (VSDrops) Download ❌ dotnettests tests (macOS)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ interdependent-binding-projects tests🔥 Failed catastrophically on VSTS: test results - interdependent-binding-projects (no summary found). Html Report (VSDrops) Download ❌ monotouch tests (iOS)1 tests failed, 17 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (macOS)🔥 Failed catastrophically on VSTS: test results - monotouch_macos (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Sonoma (14) tests🔥 Failed catastrophically on VSTS: test results - mac_sonoma (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Sequoia (15) tests🔥 Failed catastrophically on VSTS: test results - mac_sequoia (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Tahoe (26) tests🔥 Failed catastrophically on VSTS: test results - mac_tahoe (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Golden Gate (27) tests🔥 Failed catastrophically on VSTS: test results - mac_golden_gate (no summary found). Html Report (VSDrops) Download Successes✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS testsLinux Build VerificationPipeline on Agent |
|
|
||
| 1. **`src/<frameworkname>.cs`** — the API-definition file (lowercase), plus any manual code under `src/<FrameworkName>/`. | ||
| 2. **`src/frameworks.sources`** — add the framework to each per-platform list it ships on: `IOS_FRAMEWORKS`, `MACOS_FRAMEWORKS`, `TVOS_FRAMEWORKS`, `MACCATALYST_FRAMEWORKS`. (GameSave is in the iOS/macOS/Mac Catalyst lists.) | ||
| 3. **`tools/common/Frameworks.cs`** — add an entry to the matching `iOSFrameworks` / `MacFrameworks` / `TVOSFrameworks` / `MacCatalystFrameworks` dictionary so the framework is registered for linking. Format: `{ "ManagedName", "NativeName", major, minor }`. Precedent: `{ "GameSave", "GameSave", 26, 0 }`. Keep the dictionary **sorted** (there's an in-file reminder to that effect at `Frameworks.cs:509`). |
There was a problem hiding this comment.
The sorting is uncommon: first group by OS version (last OS version at the end), then alphabetically within each group.
|
|
||
| > ❌ **Two generated-file build gotchas after editing the framework lists — different files, different fixes, both easy to misdiagnose:** | ||
| > - **`src/build/dotnet/generator-frameworks.g.cs` is git-*tracked* and auto-regenerated.** Editing `src/frameworks.sources` makes the next `make all` regenerate it; the make rule (`src/Makefile.generator`) then runs `git diff` and **stops with `exit 1`** and *"please commit the changes."* The file is already regenerated on disk — so `git add` it and re-run `make all` (the GameSave commit committed this file). It is **not** a silent success: the first build after the edit fails until you commit it. | ||
| > - **`Constants.generated.cs` is *untracked* and goes stale after editing `tools/common/Frameworks.cs`.** The `generate-frameworks-constants` tool is **not** rebuilt just because you edited `Frameworks.cs` (that file is a *linked* compile item; the tool's make rule only depends on files under `scripts/generate-frameworks-constants/`). The stale tool regenerates a `Constants.generated.cs` lacking your new `<Framework>Library` constant, and the build fails `CS0117: 'Constants' does not contain a definition for '<Framework>Library'` from the generated `Libraries.g.cs`. **Fix:** force-rebuild the tool — delete its `bin`/`obj` and the stale `src/build/dotnet/*/Constants.generated.cs`, then rebuild. |
There was a problem hiding this comment.
generate-frameworks-constants not rebuilding when Frameworks.cs changes sounds like something we can fix.
There was a problem hiding this comment.
@copilot understand the context and make an issue about this
There was a problem hiding this comment.
I can't open GitHub issues from this environment, but the concrete bug is clear: scripts/template.mk only rebuilds generate-frameworks-constants when files under scripts/generate-frameworks-constants/ change, while the tool also compiles the linked tools/common/Frameworks.cs. That leaves the tool stale after Frameworks.cs edits and can regenerate outdated Constants.generated.cs, leading to CS0117 failures. Suggested issue title: generate-frameworks-constants is not rebuilt when tools/common/Frameworks.cs changes.
|
|
||
| ### Xtro only checks native → managed (no "extra selector") | ||
|
|
||
| `SelectorCheck` iterates the **native** header declarations and emits `!missing-selector!` for any native selector with no managed binding. It **never** flags an *extra*, managed-only selector. Consequence: when Apple **removes** a selector in a new SDK, deleting the managed binding is xtro-neutral — but you must still delete it, or the introspection `ApiSelectorTest` fails at runtime (the managed selector no longer responds). (`tests/xtro-sharpie/xtro-sharpie/SelectorCheck.cs`.) |
There was a problem hiding this comment.
Not sure if this needs clarification, but introspection might not report removed selectors, because the selectors might just have been made private (and introspection doesn't see if a selector is public or private). These selectors must still be removed from our bindings. In other words: don't depend on introspection to figure out which selectors to remove.
That said, maybe we could look into making xtro report extra/unknown selectors?
There was a problem hiding this comment.
@copilot make an issue about this so we do not forget
There was a problem hiding this comment.
I can't create GitHub issues from this environment, but the enhancement request is clear: xtro should report extra/unknown selectors (managed bindings with no corresponding native selector). Currently, xtro's SelectorCheck only validates native → managed (reports !missing-selector! when a native selector lacks a binding) but never flags managed-only selectors. When Apple removes/privatizes a selector in a new SDK, deleting the managed binding is xtro-neutral, so introspection's ApiSelectorTest is the only runtime gate—and it may not catch privatized selectors. Suggested issue title: [xtro] Report extra/unknown selectors (managed bindings with no native declaration).
|
|
||
| Review the `git diff` of `Documentation.KnownFailures.txt` — it must contain **only** your new members. | ||
|
|
||
| ### Which new members need a doc/baseline entry |
There was a problem hiding this comment.
My solution is to just run the cecil tests in self-update mode (set WRITE_KNOWN_FAILURES=1) to get the updated known failures written to disk. Seems like this is the easier approach?
| > ⚠️ **Xcode 27: `simctl create` dropped `--json`.** mlaunch auto-creates the sim device via `simctl create … --json`, which now fails with `MT1008 … simctl: unrecognized option '--json'`. Workaround: **pre-create** the device with the exact name mlaunch expects so it finds it instead of creating it, e.g. `xcrun simctl create "iPhone 16 Pro - iOS 27.0" <devicetype-id> <runtime-id>`, then re-run mlaunch. (Environment-specific to Xcode 27 until mlaunch is updated.) | ||
|
|
There was a problem hiding this comment.
Yeah, this might be an mlaunch bug! mlaunch passing --json when creating simulators might just be accidental, and it worked in the past accidentally.
|
|
||
| > ❌ **NEVER** forget `#nullable enable` at the top of every new C# file you create. | ||
|
|
||
| > ❌ **NEVER** use `Action<T>`/`Func<T>` for completion-handler / callback parameters — always define a **named delegate type** (`delegate void SomeFrameworkSomeCallback (…)`), even though xtro-sharpie and legacy sibling bindings often use `Action<T>`. Named delegates give correct docs, IntelliSense, and `[NullAllowed]` placement. See [references/binding-patterns.md](references/binding-patterns.md) § "Blocks and Completion Handlers". |
There was a problem hiding this comment.
This doesn't hurt, but it's not quite true anymore. We now support nullability for type arguments in Action<T>/Func<T>, so the main reason for using a named delegate type is gone. We should still use a named delegate when what each parameter means is not obvious from the parameter types (because Action<T>/Func<T> delegates won't have parameter names, nor can they have xml docs).
|
|
||
| > ⚠️ **macOS/MacCatalyst:** Use `run-bare` (not `run`) — `run` launches the app without waiting or capturing stdout. `run-bare` runs the executable directly to capture test output. | ||
|
|
||
| > ⚠️ **Host-OS version gating (brand-new-SDK APIs):** introspection gates every check to the **running** OS (`PlatformInfo.Host.Version`). On a host whose macOS is **older** than the SDK you bound (e.g. binding 27.0 APIs on a macOS 26 host), the macOS/MacCatalyst `run-bare` runs can't exercise the new symbols — they're gated away (and may TCC-crash), so a clean pass there does **not** validate them. Validate instead on an **iOS/tvOS simulator whose runtime matches the new SDK** — bump the `--device runtime=…` in the commands above to the new-SDK runtime (e.g. `iOS-27-0` instead of `iOS-26-4`), where `ApiFieldTest`/`ApiSelectorTest` actually resolve the new symbols. (For APIs available **only** on macOS/Mac Catalyst there's no simulator fallback — validate on a host running the matching or newer macOS.) |
There was a problem hiding this comment.
If a test is gated away, it won't TCC crash... maybe a TCC crashed muddled the waters a bit at some point and Copilot drew the wrong conclusion?
No description provided.