Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separately named modules for Sentry with and without UIKit #4086

Open
mike-at-home opened this issue Jun 18, 2024 · 9 comments · Fixed by #4140
Open

Separately named modules for Sentry with and without UIKit #4086

mike-at-home opened this issue Jun 18, 2024 · 9 comments · Fixed by #4140
Assignees
Labels
Platform: Cocoa Sync: Jira apply to auto-create a Jira shadow ticket Type: Feature Request

Comments

@mike-at-home
Copy link

mike-at-home commented Jun 18, 2024

Problem Statement

I'm having some difficulty migrating our build to use both of the binary released sentry-cocoa frameworks (Sentry and Sentry-WithoutUIKitOrAppKit)

The framework without UIKit has the same module name as the one with. I'm not entirely sure if it's possible to get it working with our build system, but I've spent enough time to say that diagnosing why it's not working is difficult enough without the duplicate names.

Would it be possible to get a "no-UIKit" release with a distinct module name or separate out the UIKit in a different way?
Thanks!

Solution Brainstorm

No response

Are you willing to submit a PR?

No response

┆Issue is synchronized with this Jira Improvement by Unito

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jun 18, 2024
@brustolin brustolin moved this from Needs Discussion to Needs Investigation in Mobile & Cross Platform SDK Jun 19, 2024
@irodrigues-git irodrigues-git added the Sync: Jira apply to auto-create a Jira shadow ticket label Jun 19, 2024
@brustolin
Copy link
Contributor

We're looking into it, no ETA though.

@armcknight
Copy link
Member

Hi @mike-at-home , thanks for reporting. We built this special compilation mode specifically for FileProvider extensions which cannot link UIKit. So I guess that if you are delivering such an extension, that your total build process is building regular Sentry (with UIKit) for your app, and then Sentry-without-UIKit for your extension, and because they wind up with the same name, the collision breaks things.

When we set up this new config, I tried creating a test FileProvider extension, but never was able to get it to work. Would you by any chance be able to provide a test project that tests such an integration?

In any case, I think that if we just change the module name setting from this:
image
to this:
image
that should get us most of the way there. We'd just need to validate it doesn't break other things, like package management.

@armcknight armcknight self-assigned this Jun 20, 2024
@mike-at-home
Copy link
Author

Something along those lines should resolve the issue. If the resulting zips can be extracted side by side (ie don't have conflicting Sentry.framework folders) then I think we're good.

Yes, I'm working on Dropbox's codebase which has a file provider. Currently, since we're building both Sentry targets from source, the modules were able to be named separately. But now I'm trying to replace that with the released binaries, so the same flexibility isn't there.

I've never made a file provider project. It's my understanding that it simply cannot link or access UI things. I think you can approximate the same effect by making a command line Mac project.

(FWIW, in an ideal world, it'd be great to have a core Sentry module and then have the UIKit one depend on that, if possible, but this works well enough right now)

@armcknight
Copy link
Member

(FWIW, in an ideal world, it'd be great to have a core Sentry module and then have the UIKit one depend on that, if possible, but this works well enough right now)

Absolutely agreed there. See #3219

@armcknight
Copy link
Member

Hi @mike-at-home, I spoke with @aaronsky and we looked at a couple things, including a test project going: https://github.com/armcknight/sentry-cocoa-bazel

But before that is even useful, I need to fix the module issue you all are having. He showed me a commit you made on your fork: mike-at-home@fad4f5d

I've made the changes I described here in #4140, which wound up breaking things and I've tried to make a few fixes, including renaming Debug_without_UIKIt to DebugWithoutUIKit (and likewise for its release counterpart) due to underscores not being valid in some of the other build settings I'm using that naming scheme in, but can't fully get the build to work again for those configs. Currently hitting this kind of error:

<unknown>:0: error: underlying Objective-C module 'SentryWithoutUIKit' not found
<unknown>:0: error: underlying Objective-C module 'SentryWithoutUIKit' not found
<unknown>:0: error: underlying Objective-C module 'SentryWithoutUIKit' not found
<unknown>:0: error: module 'SentryWithoutUIKit' in AST file '/Users/andrewmcknight/Library/Developer/Xcode/DerivedData/ModuleCache.noindex/22JF8W0XJRMRE/SentryWithoutUIKit-1Z1Q6V1DCHSHF.pcm' (imported by AST file '/Users/andrewmcknight/Library/Developer/Xcode/DerivedData/ModuleCache.noindex/22JF8W0XJRMRE/_SentryPrivate-23Z4ZY3NE0LI4.pcm') is not defined in any loaded module map file; maybe you need to load '/Users/andrewmcknight/Library/Developer/Xcode/DerivedData/Sentry-gnnqqmcfqpqmmhcjxfzwadgjajnt/Build/Products/DebugWithoutUIKit-iphonesimulator/Sentry.framework/Modules/module.modulemap'?
<unknown>:0: note: imported by module '_SentryPrivate' in '/Users/andrewmcknight/Library/Developer/Xcode/DerivedData/ModuleCache.noindex/22JF8W0XJRMRE/_SentryPrivate-23Z4ZY3NE0LI4.pcm'
/Users/andrewmcknight/Code/organization/getsentry/repos/public/sentry-cocoa/Sources/Swift/Integrations/SessionReplay/RRWeb/SentryRRWebEvent.swift:1:29: error: no such module '_SentryPrivate'
@_implementationOnly import _SentryPrivate

Looking at your commit, it looks like you might've already solved this module issue, because you moved on to importing the generated -Swift.h header file. When I look in my build products, there are only a couple things missing: SentryWithoutUIKit-Swift.h, the Sentry.framework/Modules/Sentry.swiftmodule directory (which should be named SentryWithoutUIKit.swiftmodule, I guess), and the binary itself.

Did you make any other changes to the build to get that working?

@aaronsky
Copy link

aaronsky commented Jul 2, 2024

Left you a patch on the PR to try. Works in the Sentry xcodeproj in my own testing, though may be onerous for y'all to have to maintain two umbrella headers (maybe good motivation to break out the UIKit bits from Sentry-Core? 🙏🏼)

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jul 2, 2024
@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Jul 8, 2024
@kahest kahest moved this from Needs Investigation to Done in Mobile & Cross Platform SDK Jul 10, 2024
@kahest
Copy link
Member

kahest commented Jul 10, 2024

this has been fixed via #4140 - the fix is included in release 8.30.1

@kahest kahest closed this as completed Jul 10, 2024
@armcknight
Copy link
Member

armcknight commented Jul 26, 2024

I heard from another customer that this doesn't quite fix the issue in all cases:

#if __has_include(<Sentry/Sentry.h>) and #if __has_include(<SentryWithoutUIKit/Sentry.h>) lines in the umbrella header create an implicit relationship between the two frameworks, only in the Xcode build system, resulting in a dependency cycle

They are using a niche build system that we don't officially support, but I just wanted to add that as context. I'm not sure what other build systems are affected, since I did not observe that problem in my test project at https://github.com/armcknight/sentry-cocoa-bazel, which uses Bazel to manage the target builds, and Carthage to bring in Sentry as a dependency.

@armcknight armcknight reopened this Jul 26, 2024
@aaronsky
Copy link

@armcknight the issue we were running into was under the Xcode Build System (building with Buck or Bazel each work fine). Under Xcode for an app that links Sentry and has an extension that links -WithoutUIKit, both frameworks are being copied into the same intermediate directory, which causes the __has_include branches in the umbrella headers to always be followed, and Clang chooses to describe this phenomenon as a dependency cycle. The workaround we came to, since we only ever use Xcode for local development, was to always use regular Sentry under Xcode and only use -WithoutUIKit in Buck builds.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jul 29, 2024
@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Aug 6, 2024
@kahest kahest moved this from Done to In Progress in Mobile & Cross Platform SDK Aug 6, 2024
@armcknight armcknight moved this from In Progress to Done in Mobile & Cross Platform SDK Aug 10, 2024
@kahest kahest moved this from Done to Backlog in Mobile & Cross Platform SDK Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: Cocoa Sync: Jira apply to auto-create a Jira shadow ticket Type: Feature Request
Projects
Status: No status
Status: Backlog
Development

Successfully merging a pull request may close this issue.

6 participants