chore: pin platform target for all plugins - #2848
Merged
bitsandfoxes merged 11 commits intoSep 14, 2026
Merged
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
`Sentry.Unity.iOS` holds the managed side of the Cocoa bridge, declared with
`DllImport("__Internal")`. That binds at link time against a symbol inside the
player executable, which only exists where Unity compiles the native bridge in:
iOS via `Plugins/iOS/SentryNativeBridge.m` and macOS via the macOS equivalent.
On Windows and Linux a native plugin is always a separate shared library, so
those imports can never resolve.
The plugin importer nevertheless enabled Standalone Win, Win64 and Linux64,
making it the only platform-specific assembly in the package that was not scoped
to its platform. It stayed invisible because nothing in a desktop player
references the bridge, so the UnityLinker stripped the types before IL2CPP could
emit the imports. Correctness depended on stripping: any consumer that preserves
the assembly, for instance another package contributing a `link.xml`, gets 21
unresolved `SentryNativeBridge*` externals and a failed link instead.
`package-dev` keeps the Editor entry, because the editor-only
`Sentry.Unity.iOS.Tests` assembly references the plugin and cannot load without
it. The release excludes `Tests`, so `package/Runtime` now carries an override
that drops the Editor too, leaving the shipped plugin scoped to iOS and macOS
like `Sentry.Unity.MacOS`, `Sentry.Unity.Native.Switch` and
`Sentry.Unity.Native.PlayStation`.
`test-plugin-platforms.ps1` guards the invariant: no managed plugin declaring
`__Internal` imports may target a desktop standalone player. It runs in the
`package-validation` job against the packed artifact, so it checks what ships.
Extends the plugin platform test from one rule to a hardcoded expectation for every plugin importer and every `.asmdef` in the package, so a scope cannot change without editing the table in the same commit. It catches a platform gained, a platform lost, a plugin added without a pinned scope, a plugin removed, and an `.asmdef` flipping between an include allowlist and an exclude list, which is the shape of the change that widened the runtime assembly's platform set. The release and `package-dev` expectations are separate, because `pack.ps1` drops the test assemblies and `package/` overrides the iOS bridge meta. The `__Internal` rule still applies on top of the tables and cannot be waived by editing them. Plugin `.meta` files are tracked while the binaries they describe are generated, so scope checks are strict whether the test runs against the packed artifact or the working tree. The `__Internal` rule needs the binary and so covers whatever is present, which in CI is everything. Pins 27 scopes today. Three existing ones are recorded as they are rather than changed, since correcting them is a separate decision: both `Plugins/iOS` Objective-C sources enable tvOS and not iOS, the macOS source enables tvOS alongside OSXUniversal, and `Plugins/PS5/sentry_utils.c` enables both GameCore targets while its own comment describes it as a Windows, Linux and PlayStation wrapper.
Three scopes that were wider than the code behind them. `Plugins/PS5/sentry_utils.c` defines `vsnprintf_sentry` and was enabled for PS5 plus both GameCore Xbox targets. `SentryNativeBridge` only declares that import under `SENTRY_NATIVE_PLAYSTATION` or `SENTRY_NATIVE_SWITCH`; the Xbox build takes the `#else` branch and reaches `vsnprintf` through `msvcrt`, and the Switch gets the symbol from its own stubs or from sentry-switch. So the file was compiled into Xbox players that never call it. Now PS5 only. Its legacy `Any` block also left Windows and Linux un-excluded, which was inert while `Any` stayed disabled but is the same trap that made the iOS bridge ship to desktop, so those are excluded now too. `Plugins/macOS/SentryNativeBridge.m` enabled tvOS alongside OSXUniversal, and both `Plugins/iOS` bridge sources enabled tvOS and nothing else. tvOS is in the runtime assembly's exclude list, so the SDK does not run there at all. The iOS sources deliberately target no platform, so Unity never copies them into the generated Xcode project. `BuildPostProcess` copies whichever one applies to `Libraries/<package>/SentryNativeBridge.m` and `AddSentryNativeBridge` adds that path to the target, so enabling a platform here would collide with the SDK's own copy. `.gitignore` records the same intent where it un-ignores these metas to control their target platforms. `SentryCxaThrowHook.cpp` keeps iOS, because nothing copies it by hand and it does rely on the importer. The pinned expectations move with the metas, so the table still describes what ships.
Every other platform's native SDK folder is ignored by name, but macOS was missing, so `package-dev/Plugins/macOS/Sentry~` and `SentryNative~` were only partly covered by the extension rules. `libsentry.dylib` matched `*.dylib` while `sentry-crash`, which has no extension, showed up as untracked and was easy to commit by accident when staging a directory.
Rebased onto `fix/native-library-name-clash`, which splits the desktop and Android builds of `Sentry.Unity.Native`. Desktop binds to `sentry-native` so Mono cannot resolve the name to the managed `Sentry.dll` on a case-insensitive file system, while Android keeps `sentry` from the `.aar`. So `Sentry.Unity.Native.Android.dll` joins the table at Android, and the desktop assembly drops Android from its scope. Also records two things that make a scope or an import table move on their own, so neither reads as a regression: `SwitchNativePluginBuildPreProcess` flips the Switch stub's importer during a build, disabling it once the consumer supplies the real static libraries. That entry is build-time mutable by design, and a local Switch build can leave the meta changed. The binaries under `package-dev` are whatever was last built there, which need not match the checked-out sources. Reading them can therefore describe a different branch entirely, which is why the packed artifact is the authority for the `__Internal` rule.
The entries were written before the pull request existed and guessed 2847 from the highest number then open. It landed as 2848.
The convention asked every agent commit to carry a `Co-Authored-By` trailer. It is noise in the history, so the rule now says the opposite, which also stops an agent re-adding the trailer from the file rather than from its own defaults.
bitsandfoxes
force-pushed
the
fix/ios-plugin-platform-scope
branch
from
September 11, 2026 14:31
6dec7ca to
9e99544
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| return !stubNeeded; | ||
| } | ||
|
|
||
| return stubNeeded && Matches(File.ReadAllText(stubPath), File.ReadAllText(TemplatePath())); |
There was a problem hiding this comment.
Bug: The build process can crash with an unhandled FileNotFoundException if the sentry_native_stubs.c template file is missing, as File.ReadAllText is called without error handling.
Severity: LOW
Suggested Fix
Wrap the File.ReadAllText(TemplatePath()) calls within a try...catch block to handle a potential FileNotFoundException. If the exception is caught, throw a BuildFailedException with a clear error message explaining that the template file is missing and the SDK might be corrupted, guiding the user to reinstall the package.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/Sentry.Unity.Editor/Native/SwitchNativeStub.cs#L69
Potential issue: The methods `IsInSync()` and `AddStub()` read the
`sentry_native_stubs.c` template file using `File.ReadAllText(TemplatePath())` without
any defensive checks. If the SDK package is corrupted or the template file is missing
for any reason, this will result in an unhandled `FileNotFoundException`. This crashes
the build process with a raw .NET exception instead of a more user-friendly
`BuildFailedException` that would provide a clear, actionable error message to the
developer.
Also affects:
src/Sentry.Unity.Editor/Native/SwitchNativeStub.cs:129~129
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5ea1a39. Configure here.
| } | ||
|
|
||
| return stubNeeded && Matches(File.ReadAllText(stubPath), File.ReadAllText(TemplatePath())); | ||
| } |
There was a problem hiding this comment.
Stub importer settings never verified
Medium Severity
IsInSync only checks that the stub file exists and matches the template, so the build preprocess returns success even when ConfigureImporter skipped setup. A newly written .c defaults to every platform and can compile the no-op stubs into desktop and mobile players.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 5ea1a39. Configure here.
tustanivsky
approved these changes
Sep 14, 2026
| @@ -352,9 +348,10 @@ int sentry_clear_crashed_last_run(void) | |||
| return 0; | |||
| } | |||
|
|
|||
| void sentry_reinstall_backend(void) | |||
Collaborator
There was a problem hiding this comment.
Could it be a problem that SentryNativeBridge's declaration of sentry_reinstall_backend still returns void instead of int?
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We keep running into issues where plugins are enabled for platforms where they have no business running at. Or where they should be running but are not. This is partly due to the split between
package-dev/andpackage/.This has me doubly nervous since #2834. We had to flip the explicit include into an explicit exclude to make it work for older Unity versions that do not have the Switch 2 as an valid platform.
So with this we have a hardcoded table setting the expectations of every plugin within the SDK, preventing future regressions.