Repository navigation
fix: correct Sentry Podfile workaround comment for MSDK-4862 weak-linking - #252
islameldesoky95 wants to merge 1 commit into
Conversation
…king mobile-sdk's Usercentrics.xcframework now links Sentry weakly (MSDK-4862), not eagerly/required — a missing or mismatched Sentry.framework no longer crashes the app at launch. This comment described the old required-link behavior and its crash consequence, which no longer applies. The force-dynamic-Sentry-pod workaround itself is unchanged and still needed: under `use_frameworks! :linkage => :static`, CocoaPods would still build Sentry as a static archive with no real Sentry.framework bundle, which would now silently leave SdkMonitor's crash reporting non-functional instead of crashing — a quieter failure mode, but still worth avoiding. NOT YET MERGED — holding until mobile-sdk's MSDK-4862 fix is actually published in a real release (currently only merged to mobile-sdk's develop branch). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
PR Summary: Update comment in sample/ios/Podfile to accurately describe why we force Sentry to build as a dynamic framework (MSDK-4862 weak-linking), without changing behavior.
|
| # WORKAROUND: Usercentrics.xcframework links Sentry weakly (mobile-sdk MSDK-4862 — a missing/ | ||
| # mismatched Sentry.framework no longer crashes the app at launch), but still expects a real | ||
| # dynamic Sentry.framework to actually be present for SdkMonitor's crash reporting to function. | ||
| # `use_frameworks! :linkage => :static` above makes CocoaPods build every pod — including | ||
| # Sentry — as a static archive with no Sentry.framework bundle ever embedded in the app, which | ||
| # would silently leave SdkMonitor's crash reporting non-functional (no crash, no error — it | ||
| # just never sends anything). Force only the Sentry pod to build as a dynamic framework so the | ||
| # real Sentry.framework exists and gets embedded, matching what the binary expects. |
There was a problem hiding this comment.
[VALIDATION] The new comment correctly describes the weak-link behavior introduced by MSDK-4862, but it's important to make the comment actionable and traceable: add an explicit link or commit/PR reference to the mobile-sdk change (e.g. PR #2544 or commit SHA) and the target released version that will contain the fix. Also add a TODO that documents when this Podfile workaround should be removed (e.g. "TODO: remove this workaround once mobile-sdk vX.Y.Z with MSDK-4862 is published"). This prevents the comment from becoming stale and helps reviewers know when it's safe to remove the workaround.
# WORKAROUND: Usercentrics.xcframework links Sentry weakly (mobile-sdk MSDK-4862 —
# see https://bitbucket.org/usercentricscode/mobile-sdk/pull-requests/2544)
# A missing/mismatched Sentry.framework no longer crashes the app at launch, but
# SdkMonitor’s crash reporting still requires a real dynamic Sentry.framework.
# `use_frameworks! :linkage => :static` above makes CocoaPods build every pod —
# including Sentry — as a static archive with no Sentry.framework bundle ever
# embedded in the app, which would silently leave SdkMonitor’s crash reporting
# non-functional (no crash, no error — it just never sends anything).
# TODO(MSDK-4862): remove this workaround once a released mobile-sdk version
# including this fix is consumed in this sample app.
# Force only the Sentry pod to build as a dynamic framework so the real
# Sentry.framework exists and gets embedded, matching what the binary expects.
pre_install do |installer|
installer.pod_targets.each do |pod_target|
if pod_target.name&.start_with?('Sentry')
def pod_target.build_type
Pod::BuildType.dynamic_framework
end
end
end
end|
Reviewed up to commit:838bee874d3688c1e2317656a646ed7f142b5cab Additional Suggestionsample/ios/Podfile, line:32-37The pre_install hook forces Sentry pods to be built as dynamic frameworks. Consider making the matcher slightly stricter/clearer to avoid surprising matches (and to document intent). For example, match Sentry root spec names or use a regex anchored to the start: if pod_target.name && pod_target.name.match?(/^Sentry/) then set Pod::BuildType.dynamic_framework. This is more explicit than a plain start_with? and documents that we mean any pod beginning with "Sentry" (SentrySwift/SentryRTC/etc.). Example replacement:installer.pod_targets.each do |pod_target| (Also consider adding a short comment explaining why we only touch Sentry-named targets.) pre_install do |installer|
installer.pod_targets.each do |pod_target|
# Force all Sentry-related pods to build as dynamic frameworks so that a
# real Sentry.framework bundle exists and can be weak-linked by
# Usercentrics.xcframework for SdkMonitor crash reporting.
if pod_target.name && pod_target.name.match?(/^Sentry/)
def pod_target.build_type
Pod::BuildType.dynamic_framework
end
end
end
endOthers- This PR is described as a comment-only correction, but the repository-level guidance states: DO NOT MERGE until the mobile-sdk MSDK-4862 fix is published in a real release. Please add an explicit gating note to the PR (and/or a checklist item) so maintainers do not merge this change before the mobile-sdk release exists. If possible, add a CI or release-check (or require the specific mobile-sdk version) before removing this placeholder or updating the comment again.- [ ] Do **not** merge until the mobile-sdk MSDK-4862 weak-linking fix is
available in a released version and the sample app is updated to depend
on that version.
- [ ] Once the release exists, update this PR description with the exact
mobile-sdk version (e.g. `mobile-sdk >= x.y.z`) and remove this gating
checklist item. |
Summary
mobile-sdk's MSDK-4862 fix switches
Usercentrics.xcframework's Sentry dependency from required (-framework Sentry) to weak linking (-weak_framework Sentry) — a missing or mismatchedSentry.frameworkno longer crashes the app at launch. Merged into mobile-sdk'sdevelopbranch (PR #2544), verified with real crash-report evidence both ways (required-link + missing Sentry → confirmedDYLD/Library not loadedcrash; weak-link + missing/mismatched Sentry → launches successfully).This PR only corrects
sample/ios/Podfile's comment, which described the old required-link crash behavior. No logic change — the force-dynamic-Sentry-pod workaround itself is unchanged and still needed: underuse_frameworks! :linkage => :static, CocoaPods would still build Sentry as a static archive with no realSentry.frameworkbundle, which would now silently leaveSdkMonitor's crash reporting non-functional instead of crashing — a quieter failure mode, but still worth avoiding.Holding until mobile-sdk's MSDK-4862 fix is actually published in a real release (currently only merged to mobile-sdk's
developbranch, not yet released).Test plan
ruby -c sample/ios/Podfile— syntax valid.🤖 Generated with Claude Code