fix: MSB4006 circular dependency involving target "SetupCocoaSDK"#4956
fix: MSB4006 circular dependency involving target "SetupCocoaSDK"#4956jamescrosswell wants to merge 4 commits intomainfrom
Conversation
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog. This PR will not appear in the changelog. 🤖 This preview updates automatically when you update the PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4956 +/- ##
==========================================
- Coverage 73.89% 73.88% -0.02%
==========================================
Files 496 497 +1
Lines 17951 17957 +6
Branches 3516 3516
==========================================
+ Hits 13265 13267 +2
- Misses 3825 3832 +7
+ Partials 861 858 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| Condition="$([MSBuild]::IsOSPlatform('OSX')) and Exists('$(SentryCocoaFramework)') and !Exists('$(SentryCocoaFramework).sanitized.stamp')"> | ||
| <Message Importance="High" Text="Sanitizing $(SentryCocoaFramework): removing dSYMs, Headers, Modules, PrivateHeaders (including symlinks)." /> | ||
| <Exec Command="find "$(SentryCocoaFramework)" -depth \( -name dSYMs -o -name Headers -o -name Modules -o -name PrivateHeaders \) -exec echo Removing {} \; -exec rm -rf {} + 2>/dev/null || true" /> | ||
| <Touch Files="$(SentryCocoaFramework).sanitized.stamp" AlwaysCreate="true" /> |
There was a problem hiding this comment.
question: just to be sure ... this file doesn't get packed or published when an app is built, right?
There was a problem hiding this comment.
No the framework directory/file is specified here:
By appending .sanitized.stamp we effectively end up with a stamp file beside that directory/file.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Stale stamp file can skip sanitization after re-extraction
- Deleted the sanitization stamp at the start of _DownloadCocoaSDK so any re-extracted framework is always sanitized again.
Or push these changes by commenting:
@cursor push 584ff6f330
Preview (584ff6f330)
diff --git a/src/Sentry.Bindings.Cocoa/Sentry.Bindings.Cocoa.csproj b/src/Sentry.Bindings.Cocoa/Sentry.Bindings.Cocoa.csproj
--- a/src/Sentry.Bindings.Cocoa/Sentry.Bindings.Cocoa.csproj
+++ b/src/Sentry.Bindings.Cocoa/Sentry.Bindings.Cocoa.csproj
@@ -62,6 +62,9 @@
<Message Importance="High" Text="Setting up the Cocoa SDK version '$(SentryCocoaVersion)'." />
+ <!-- Ensure sanitization reruns after re-extracting an absent framework -->
+ <Delete Files="$(SentryCocoaFramework).sanitized.stamp" />
+
<!-- Clean cache if version does not exist to get rid of old versions -->
<RemoveDir
Condition="!Exists('$(SentryCocoaFramework).zip')"This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
| Condition="$([MSBuild]::IsOSPlatform('OSX')) and Exists('$(SentryCocoaFramework)') and !Exists('$(SentryCocoaFramework).sanitized.stamp')"> | ||
| <Message Importance="High" Text="Sanitizing $(SentryCocoaFramework): removing dSYMs, Headers, Modules, PrivateHeaders (including symlinks)." /> | ||
| <Exec Command="find "$(SentryCocoaFramework)" -depth \( -name dSYMs -o -name Headers -o -name Modules -o -name PrivateHeaders \) -exec echo Removing {} \; -exec rm -rf {} + 2>/dev/null || true" /> | ||
| <Touch Files="$(SentryCocoaFramework).sanitized.stamp" AlwaysCreate="true" /> |
There was a problem hiding this comment.
Stale stamp file can skip sanitization after re-extraction
Low Severity
If the xcframework directory is removed (manually or by an external process) while its .zip file remains cached, _DownloadCocoaSDK re-extracts the framework but the RemoveDir step is skipped (it only runs when the zip is missing). The old $(SentryCocoaFramework).sanitized.stamp survives, causing SanitizeSentryCocoaFramework to be skipped entirely. This leaves an unsanitized framework (with dSYMs, Headers, Modules, PrivateHeaders still present) used for the build. Deleting the stamp file during re-extraction (or at the start of _DownloadCocoaSDK) would close this gap.
Additional Locations (1)
2 participants
Resolves #4955:
Explanation
Me and Claude spent a bit of time looking at this. I think we now have a solution that is OK.
What the original code did
The recursive MSBuild call with
RemoveProperties="TargetFramework"meant that during an inner build, instead of running_SetupCocoaSDKdirectly, it spawned a separate MSBuild process on the same project with no TargetFramework set. That separate process ran_SetupCocoaSDKonce in isolation. So across both inner TFM builds, the actual work happened in one subprocess — not in the inner builds themselves.What the new code does
_SetupCocoaSDKnow runs once per inner build (once fornet9.0-ios18.0, once fornet9.0-maccatalyst18.0).However, every sub-target of
_SetupCocoaSDKis idempotent and has checks to make sure we're not double handling:_DownloadCocoaSDK— has Condition="... And !Exists('$(SentryCocoaFramework)')", so ,skips if already downloaded_BuildCocoaSDK— has Inputs/Outputs incremental build guards, skips if up-to-date_GenerateSentryCocoaBindings— has Inputs/Outputs, skips if up-to-dateSanitizeSentryCocoaFramework— has "!Exists('$(SentryCocoaFramework).sanitized.stamp"#skip-changelog