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

Can't compile for SwiftUI Previews #949

Merged
merged 2 commits into from Aug 2, 2022
Merged

Conversation

dimitribouniol
Copy link
Contributor

@dimitribouniol dimitribouniol commented Jul 28, 2022

What and why?

This package is the only one that cannot be used if our own project uses SwiftUI Previews in our own packages (ie. Xcode Project links a single SPM package that links all our other dependencies, though I'd tried other arrangements unsuccessfully). This is because all library products in this package explicitly specify either .static or .dynamic, which does not play well with Xcode's build system when it needs to wiggle things around for previews.

Specifically, when importing the static variant, previews fail with:

HumanReadableSwiftError

SettingsError: noExecutablePath(<IDESwiftPackageStaticLibraryProductBuildable:ObjectIdentifier(0x00006000069648d0):'DatadogStatic'>)

and when it's dynamic:

PotentialCrashError: Update failed

XCPreviewAgent may have crashed. Check ~/Library/Logs/DiagnosticReports for any crash logs from your application.

==================================

|  RemoteHumanReadableError
|  
|  LoadingError: failed to load library at path "/Users/<omitted>": Optional(dlopen(/Users/<omitted>, 0x0000): Library not loaded: @rpath/Datadog.framework/Datadog
|    Referenced from: <3AFEA85D-E7EE-3D6F-9C82-72984B024CB3> /Users/<omitted>
|    Reason: tried: '/Users/<omitted>/Products/Debug-iphonesimulator/Datadog.framework/Datadog' (no such file), '/Applications/Xcode-beta-4.app/Contents/Developer/Platforms/iPhoneOS.platform/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS.simruntime/Contents/Resources/RuntimeRoot/System/Library/Frameworks/Datadog.framework/Datadog' (no such file))
|  
|  ==================================
|  
|  |  MessageSendFailure: Message send failure for <ServiceMessage 3: update>

This fails both in Xcode 13 and all Xcode 14 betas that I've tried.

How?

As packaging as a dynamic library is an optimization step for when users don't want to bloat their binary (which they could better accomplish by including their own wrapper product specific to their project), this reverts the unqualified products to not specify a library type which is the norm for the vast majority of projects, and introduces a new set of qualified types for the dynamic use case.

Ideally, the Static and Dynamic variants should be removed or deprecated, but I'm unaware of a method of signaling that, so I added an appropriate todo to remove at least the static variant in the next major version.

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
  • Add CHANGELOG entry for user facing changes

Custom CI job configuration (optional)

  • Run unit tests
  • Run integration tests
  • Run smoke tests

@dimitribouniol dimitribouniol requested a review from a team as a code owner July 28, 2022 21:47
@ncreated ncreated self-assigned this Aug 1, 2022
Copy link
Collaborator

@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.

@dimitribouniol thanks a lot for tracking this down and clear explanation of the problem. I was able to reproduce it and indeed, this fixes the issue 👍.

I wasn't aware of other impacts, hence did some test in scenarios that we already patched Package.swift for, notably:

Ideally, the Static and Dynamic variants should be removed or deprecated, but I'm unaware of a method of signaling that, so I added an appropriate todo to remove at least the static variant in the next major version.

This is my feeling too, however given that Xcode's SPM isn't yet perfect (see this swift.org thread), I'm not entirely sure if we can solely rely on Xcode and "automatic" linking (I remember this to be broken in previous versions of Xcode, but appears to be fixed now). As we plan changes in our modules layout, I linked appropriate tasks in TODOs to look into this closer.

@ncreated
Copy link
Collaborator

ncreated commented Aug 2, 2022

CI is green, Bitrise checks fail to report proper status. Merging with admin right.

Screenshot 2022-08-02 at 12 02 42

@ncreated ncreated merged commit 7c6bd63 into DataDog:develop Aug 2, 2022
@dimitribouniol
Copy link
Contributor Author

Thanks for getting this in so quickly @ncreated! Do you know roughly when a version will be tagged with this change so we can switch to it?

@ncreated
Copy link
Collaborator

ncreated commented Aug 2, 2022

Thanks for getting this in so quickly @ncreated! Do you know roughly when a version will be tagged with this change so we can switch to it?

Hey @dimitribouniol - we plan on releasing 1.12.0-beta1 this week. We don't yet know when it will become GA - depends if we spot any blockers.

@dimitribouniol
Copy link
Contributor Author

Thanks for the update, looking forward to it!

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.

None yet

2 participants