feat: Add support for dynamic library #1726
Conversation
|
Thanks, @mrtnrst, for opening this PR. After reading the thread you pointed out I'm not convinced that adding a new |
I can get a sample application tonight to help demonstrate what I am talking about. For other repos that support this, here are two that my team is actively using: |
|
Thanks for the update, @mrtnrst. A sample application would be excellent. Ideally, we would like to validate the dynamic library in CI similar to sentry-cocoa/.github/workflows/build.yml Lines 109 to 124 in 2e5882c Your sample app could maybe be a starting point to achieve this. |
|
@philipphofmann sorry for the delay here, work is a little busy. I will try to get back to this later in the week 🙏🏽 |
|
That's actually the only way to support dynamic frameworks via SPM:
|
|
We're using a dynamic library in Unity, for macOS: getsentry/sentry-unity#710 |
Yeah I've had a look at this PR but didn't get what this actually changes - the |
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you label it "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
Hey @mrtnrst, do you have time to add the sample application for validation in CI, maybe? |
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you label it "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
Hi! Apologies for the terrible delay (I kept staring at this notification and just scared myself into not opening it 😅). What kind of example project were you wanting? Something that passes and runs through CI? Or something that demonstrates the issue I am having? |
|
Hey @mrtnrst, please see my comment above #1726 (comment). |
|
Thanks, @mrtnrst, for adding the sample. I will get back to this PR later. |
|
Hi @mrtnrst, I'm just looking for some clarification on how you have your project set up (and per #1726 (comment), I think it would be helpful to have a MVP that mimics your actual setup, which can then also be used to validate). AIUI, you have an app target and a static library target, and both of them link in Sentry, so that statically linking your library into your app results in duplicate Sentry symbols from both targets. You'd like to be able to declare your app and library to link Sentry dynamically at runtime instead of at build time to avoid the duplication. Is that right? graph LR;
App--static-->Lib;
App--static-->Sentry;
Lib--static-->Sentry;
I tried setting this up using only Xcode targets but couldn't force a duplicate symbol error, so it must be something specific to how SPM does things. I tried using SPM based on the example you provided but the project it creates for me crashes Xcode as soon as I try to build 😰 |
I'll see if I can get a more exact replica of what I am experiencing here. And to answer you other question, that is correct. The framework project we create cannot import the Sentry target without creating the duplicate references so the change to make it dynamic (to allow the import without embed) gives us that freedom. We would still want to use the static embedded framework on the main project, not the dynamic library. |
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you label it "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
There was a problem hiding this comment.
Thanks for the effort, @mrtnrst. Sorry for the delay, I was on paternity leave.
| - feat: Add more info to touch event breadcrumbs (#1724) | ||
| - feat: Add support for dynamic library (#1726) |
There was a problem hiding this comment.
I'm going to fix that for you 😁 after merging this PR.
|
I'm going to merge this after releasing the hotfix 7.24.1. |
* master: (73 commits) ref: Fix typo in measurement (#2154) Fix typos in changelog (#2153) docs: Add a note on flaky tests to Contributing (#2161) docs: fix PR reference in CHANGELOG (#2157) test: generate graphs for benchmarks (#2158) release: 7.25.1 fix: Prewarmed app start detection (#2151) ci: temporarily disable scheduled CI runs uploading profiles/symbols (#2152) Run tests on iOS 16 using Xcode 14.0 (#2147) ref: Fix Xcode 14 compile issues (#2145) Provide fallbacks for assertions in production (#2141) Fix compile errors in Xcode 14 (#2146) release: 7.25.0 fix: setting SDK name through options[sdk][name] shouldn't clear version (#2139) fix: SentryTracer instances should be called "tracer", not "transaction" (#2137) ref: functions to convert to/from enum cases (#2108) fix: Crash with screenshot is reported twice (#2134) meta: Clarify PR rules in Contributing (#2133) meta: Fix Changelog (#2136) feat: Add support for dynamic library (#1726) ...
|
It seems to me that Sentry-Dynamic is building an empty framework? Build of Sentry-Dynamic: Causes a failure trying to use the framework: |
|
@lhunath, please open a new issue for your problem above. |
|
I tried yesterday, but half-way through collecting facts around the issue my problem disappeared and the Sentry-Dynamic framework started receiving its binary. I could not find out what the repro is. I will come back to a new issue if I have more reliable details. |
|
I'm happy that the problem disappeared. Thanks for the update, @lhunath 🙏. |
📜 Description
This PR adds support for using a dynamic library. Keeping it an additive change, and not breaking, I added a new library product that generates
Sentry-Dynamic.💡 Motivation and Context
The motivation behind this work is personal. I am working on a project that utilizes a private framework within the same Xcode project. With nested frameworks like this, Xcode fails to compile the duplication.
Relevant Swift forum thread: https://forums.swift.org/t/swift-packages-in-multiple-targets-results-in-this-will-result-in-duplication-of-library-code-errors/34892
💚 How did you test it?
After adding in the project there are two frameworks now listed under the Sentry category: Sentry and Sentry-Dynamic
📝 Checklist
🔮 Next steps