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

Xcframework debug symbols xcode12 example #10137

Conversation

johntmcintosh
Copy link
Contributor

@amorde as discussed in the other PR, here's the separate PR with the example that I setup for testing #10111.

The one change I'll call out that was outside of adding the new example is that I checked in the AppUsingStaticLibraries.xcscheme for the existing Vendored XCFramework Example. I think it was probably an oversight that it wasn't checked in before (sometimes Xcode is finicky about not creating the file at first for a scheme even though it's marked as shared). Let me know if that's not the case though, and I can remove it.

@johntmcintosh
Copy link
Contributor Author

Hmmm, looks like CI failed on my new test with

Example: examples/Vendored-XCFramework12-Example
    Installing Pods
    Building scheme: ArchiveAndValidate
rake aborted!
Build settings from command line:
    ONLY_ACTIVE_ARCH = NO

xcodebuild: error: Unable to find a destination matching the provided destination specifier:
		{ platform:iOS Simulator, OS:latest, name:iPhone 11 Pro }

	Available destinations for the "ArchiveAndValidate" scheme:
		{ platform:macOS, arch:x86_64, variant:Mac Catalyst, id:4203018E-580F-C1B5-9525-B745CECA79EB }

	Ineligible destinations for the "ArchiveAndValidate" scheme:
		{ platform:iOS, id:dvtdevice-DVTiPhonePlaceholder-iphoneos:placeholder, name:Generic iOS Device }
		{ platform:iOS Simulator, id:dvtdevice-DVTiOSDeviceSimulatorPlaceholder-iphonesimulator:placeholder, name:Generic iOS Simulator Device }
   ...

I had been running with Xcode 11.7 locally and I see CI is using 11.3.1. I can try that (hopefully tomorrow) and see if it's apparent what device designation change is needed to get that running.

@@ -0,0 +1,57 @@
#!/usr/bin/env sh
Copy link
Member

Choose a reason for hiding this comment

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

any way this could be shared between this example and the existing one here? https://github.com/CocoaPods/CocoaPods/blob/master/examples/Vendored%20XCFramework%20Example/CoconutLib/build.sh? Not a blocker for this PR, but would be nice to not have to maintain both

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amorde The scripts are similar, but there are a few differences between each and I worried a little about overcomplicating the setup by trying too hard to avoid the duplication.

The new Banana script that's compiling the Xcode 11 format xcframework is the closest to the existing script: both collect the dSYMs at the root and manually stuff the BCSymbolMaps down inside the .frameworks. The current differences are:

  • Location of xcarchives (new in build/DerivedData/, old in build/)
  • The new script does not have a need for static linking or library-style output. (I'm not as familiar with the setup for static linking, so I'm a little hesitant to refactor in a way that affects that portion of the existing script)

The new Coconut script is the Xcode 12 style compilation that removes the manual manipulation of debug symbols and instead uses the new xcodebuild -create-xcframework -debug-symbols option.

So, I think if we were to consolidate we'd need to consider:

  • Where does the master copy of the script live where it can be called for both example projects?
  • Fully generalize the script so it can be argument-controlled to pass in:
    • Source project/directory to compile
    • Output directory
    • Static/Dynamic Library/Framework output
    • Xcode 11 vs Xcode 12 debug symbol collection
  • Maybe setup new wrapper build scripts that call into the master script so that there's less of a chance of passing in the wrong arguments when re-running.

You're definitely right that there's a good bit of similarity between parts of the script, I just fear that to properly setup a consolidated script that replicates what these do, we'd end up losing some of the more straightforward traceability that the current ones have. Let me know what you think!

@@ -0,0 +1,78 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Member

Choose a reason for hiding this comment

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

thanks for adding this, yes this was meant to be commited

@@ -0,0 +1,49 @@
#!/usr/bin/env sh
Copy link
Member

Choose a reason for hiding this comment

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

ah same question here re:sharing, there's now 3 copies of this script

@@ -231,7 +231,7 @@
);
runOnlyForDeploymentPostprocessing = 0;
shellPath = /bin/sh;
shellScript = "archivePath=\"${BUILD_DIR}/VendoredXCFrameworkExample.xcarchive\"\n\nxcodebuild archive \\\n -workspace \"${PROJECT_DIR}/Examples.xcworkspace\" \\\n -scheme \"VendoredXCFrameworkExample\" \\\n -configuration \"Release\" \\\n -archivePath \"${archivePath}\" \\\n CODE_SIGN_IDENTITY=\"\" \\\n CODE_SIGNING_REQUIRED=NO \\\n CODE_SIGN_ENTITLEMENTS=\"\" \\\n CODE_SIGNING_ALLOWED=\"NO\"\n\n# ---------------------------------------\n# Collecting debug symbols in archive\n# ---------------------------------------\nexitCode=0\n\n# Ensure the expected BCSYmbolMaps are in the root of the archive\nbcSymbolMapsPath=\"${archivePath}/BCSymbolMaps\"\n\n# Ensure the expected SDK12 BCSYmbolMaps are in the root of the archive\nlibraryBCSymbolMapDir=\"${PROJECT_DIR}/CoconutLib/build/CoconutLib.xcframework/ios-arm64/BCSymbolMaps\"\nfor file in \"${libraryBCSymbolMapDir}\"/*; do\n filename=\"${file##*/}\"\n if ! test -f \"${bcSymbolMapsPath}/${filename}\"; then\n echo \"error: Missing BCSymbolMap for CoconutLib: ${filename}\"\n exitCode=1\n fi\ndone\n\n# Ensure the expected dSYMs are in the root of the archive\ndsymsPath=\"${archivePath}/dSYMs\"\n\nif ! test -d \"$dsymsPath/VendoredXCFrameworkExample.app.dSYM\"; then\n echo \"error: Missing dSYM: VendoredXCFrameworkExample.app.dSYM\"\n exitCode=1\nfi\n\nif ! test -d \"$dsymsPath/CoconutLib.framework.dSYM\"; then\n echo \"error: Missing dSYM: CoconutLib.framework.dSYM\"\n exitCode=1\nfi\n\nexit $exitCode\n";
shellScript = "archivePath=\"${BUILD_DIR}/VendoredXCFrameworkExample.xcarchive\"\n\nxcodebuild archive \\\n -workspace \"${PROJECT_DIR}/Examples.xcworkspace\" \\\n -scheme \"VendoredXCFrameworkExample\" \\\n -configuration \"Release\" \\\n -archivePath \"${archivePath}\" \\\n CODE_SIGN_IDENTITY=\"\" \\\n CODE_SIGNING_REQUIRED=NO \\\n CODE_SIGN_ENTITLEMENTS=\"\" \\\n CODE_SIGNING_ALLOWED=\"NO\"\n\n# ---------------------------------------\n# Collecting debug symbols in archive\n# ---------------------------------------\nexitCode=0\n\nbcSymbolMapsPath=\"${archivePath}/BCSymbolMaps\"\n\n# Ensure the expected Xcode 11-style BananaLib BCSymbolMaps are in the root of the archive\nlibraryBCSymbolMapDir=\"${PROJECT_DIR}/BananaLib/build/BananaLib.xcframework/ios-arm64/BananaLib.framework/BCSymbolMaps\"\nfor file in \"${libraryBCSymbolMapDir}\"/*; do\n filename=\"${file##*/}\"\n if ! test -f \"${bcSymbolMapsPath}/${filename}\"; then\n echo \"error: Missing BCSymbolMap for BananaLib: ${filename}\"\n exitCode=1\n fi\ndone\n\n# Ensure the expected Xcode 12-style CoconutLib BCSymbolMaps are in the root of the archive\nlibraryBCSymbolMapDir=\"${PROJECT_DIR}/CoconutLib/build/CoconutLib.xcframework/ios-arm64/BCSymbolMaps\"\nfor file in \"${libraryBCSymbolMapDir}\"/*; do\n filename=\"${file##*/}\"\n if ! test -f \"${bcSymbolMapsPath}/${filename}\"; then\n echo \"error: Missing BCSymbolMap for CoconutLib: ${filename}\"\n exitCode=1\n fi\ndone\n\n# Ensure the expected dSYMs are in the root of the archive\ndsymsPath=\"${archivePath}/dSYMs\"\n\nif ! test -d \"$dsymsPath/VendoredXCFrameworkExample.app.dSYM\"; then\n echo \"error: Missing dSYM: VendoredXCFrameworkExample.app.dSYM\"\n exitCode=1\nfi\n\nif ! test -d \"$dsymsPath/CoconutLib.framework.dSYM\"; then\n echo \"error: Missing dSYM: CoconutLib.framework.dSYM\"\n exitCode=1\nfi\n\n# ------------------------------------------------------\n# Stripping embedded Xcode 11-style BananaLib framework\n# ------------------------------------------------------\n\n# Ensure BCSymbolMaps have been stripped from the embedded Xcode 11-style BananaLib framework\n\nappPath=\"${archivePath}/Products/Applications/VendoredXCFrameworkExample.app\"\nframeworkPath=\"${appPath}/Frameworks/BananaLib.framework\"\n\nif test -d \"${xcode11FrameworkPath}/BCSymbolMaps\"; then\n echo \"error: Embedded BCSymbolMap not removed: BananaLib.framework/BCSymbolMaps\"\n exitCode=1\nfi\n\nexit $exitCode\n";
Copy link
Member

Choose a reason for hiding this comment

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

can you pull this out into a separate file? it's difficult to review as a single line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, yep will do.

@@ -0,0 +1,20 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Member

Choose a reason for hiding this comment

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

does the build folder need to be checked in? particularly the xcarchives, it doesn't look like they are used or compared against the generated output

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amorde you're right -- the xcarchives do not need to be checked in. I'm making a tweak to the build scripts to push the generated archives down a level into build/DerivedData/ which will get them ignored by the global gitignore.

Copy link
Member

@amorde amorde left a comment

Choose a reason for hiding this comment

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

Didn't mean to approve immediately

@amorde amorde self-requested a review October 9, 2020 19:18
@dnkoutso dnkoutso added this to the 1.10.0 milestone Oct 17, 2020
@dnkoutso
Copy link
Contributor

@amorde @johntmcintosh we can still merge this right? its just an added example

@johntmcintosh
Copy link
Contributor Author

@dnkoutso - yep, no functional changes here, just the example that's used to validate the changes from the original PR. I've either addressed or responded to all the feedback so far, but if anything else comes up I should be able to take a look on Monday.

@dnkoutso dnkoutso merged commit 6a4047b into CocoaPods:1-10-stable Oct 17, 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.

None yet

3 participants