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

chore: introduce script to make a single XCFramework #881

Closed
wants to merge 1 commit into from

Conversation

jfro
Copy link
Contributor

@jfro jfro commented Dec 15, 2020

📜 Description

  1. Turns SKIP_INSTALL to no, this I'm wondering might be an issue for other distribution, open to suggestions to work around if so
  2. Adds scripts/build-xcf.sh which builds an XCFramework for all destinations

💡 Motivation and Context

Ran into recently released Xcode 12.3 being strict about iOS frameworks not allowing both iOS & iOS Simulator to be present. Ran across this forum post which suggests pre-built frameworks should be XCFramework

💚 How did you test it?

I've tested the XCFramework output on iOS without issue, expect the only concern is the SKIP_INSTALL change with Cocoapods/Carthage methods, so wondering if that might have to be tweaked (maybe script can somehow force that setting?)

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the CHANGELOG
  • I updated the docs if needed
  • Review from the native team if needed
  • No breaking changes

🔮 Next steps

Just needs someone familiar with the project to confirm if this won't break anything (SKIP_INSTALL) as otherwise this should leave everything else as is and is a script addition that could be added to release procedures in future.

@philipphofmann
Copy link
Member

Hi @jfro,
thanks for opening up this PR. Can you please point out how to reproduce the warning you got from Xcode 12.3? Was it with our sample projects or how do you include Sentry into your own project?

How does adding the build-xcf.sh solve the issue? Shouldn't we call that somewhere?

@jfro
Copy link
Contributor Author

jfro commented Jan 4, 2021

Sorry for the delay in response. So think we're odd ones out so understandable of these changes doesn't fit with how you do releases. We don't use Cocoapods or Carthage currently, so we were just downloading the Carthage build and removing some of the directory structure, and just linking it like normal in Xcode.

I believe Carthage might be working on a solution as well for this problem, but linking the frameworks directly from your releases page got us the error/warning.

@philipphofmann
Copy link
Member

No problem @jfro, thanks for the update. Yes, Carthage recently merged Carthage/Carthage#3071, which addresses this issue I think. We are just waiting for a new release.

@bruno-garcia
Copy link
Member

We just got a new release today btw: https://github.com/getsentry/sentry-cocoa/releases/tag/6.1.0

@philipphofmann
Copy link
Member

@bruno-garcia I was referring to a release of Carthage 😁.

@philipphofmann
Copy link
Member

We solve this with #1175.

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.

4 participants