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

RUMM-392 SDK is set dynamic in SPM #82

Merged
merged 1 commit into from
Apr 21, 2020

Conversation

buranmert
Copy link
Contributor

@buranmert buranmert commented Apr 20, 2020

What and why?

Swift Package Manager encourages us to leave library type for itself to decide either dynamic or static
Yet, it doesn't change to use dynamic when needed.

Example case

We have 3 targets:

  1. App: links Datadog, Framework1 and Framework2
  2. Framework1: links Datadog
  3. Framework2: links Datadog

When we build App, Xcode emits the following error:

Swift package product 'Datadog' is linked as a static library by 'Framework1' and 'Framework2'. This will result in duplication of library code

Solution

Either

  1. we can change our SDK's type in Package.swift to dynamic or
  2. our users can use DISABLE_DIAMOND_PROBLEM_DIAGNOSTIC build setting to silence the error

We implemented the change in Package.swift in this PR

Important note

Xcode builds App Extensions and alikes without this problem (unless symbols are duplicated in binaries)

That makes the example case above an edge case IMO

Conclusion

Given that the error occurs only in an edge case, giving up on static linking may not be the best solution.
I suggest not fixing the issue and

  1. waiting for Xcode/SwiftPackageManager to fix the issue on their end
  2. providing information about possible workarounds to our users (eg: in README.md or TROUBLESHOOT.md)

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference

@buranmert buranmert added the question Further information is requested label Apr 20, 2020
@buranmert buranmert requested a review from a team April 20, 2020 15:03
@buranmert buranmert self-assigned this Apr 20, 2020
@buranmert buranmert marked this pull request as ready for review April 20, 2020 15:44
Copy link
Member

@ncreated ncreated left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it is better to provide any solution that we can later iterate on, instead of having no SPM support for modular architectures at all. Let's start with dynamic framework and we'll see what future versions of SPM will bring 🙂.

@buranmert buranmert added this to the next-version milestone Apr 21, 2020
@buranmert buranmert removed the question Further information is requested label Apr 21, 2020
@buranmert buranmert changed the title SDK is set dynamic in SPM RUMM-392 SDK is set dynamic in SPM Apr 21, 2020
@buranmert buranmert merged commit 77e36ee into master Apr 21, 2020
@buranmert buranmert deleted the buranmert/RUMM392-dynamic-framework branch April 21, 2020 09:46
@ncreated ncreated removed this from the next-version milestone May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants