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

Do not warn for missing arch if $ONLY_ACTIVE_ARCH is set to NO. #10114

Closed

Conversation

dnkoutso
Copy link
Contributor

@dnkoutso dnkoutso commented Oct 2, 2020

@@ -33,7 +33,7 @@ module ScriptPhaseConstants
intersected_archs="$(echo ${ARCHS[@]} ${binary_archs[@]} | tr ' ' '\\n' | sort | uniq -d)"
# If there are no archs supported by this binary then warn the user
if [[ -z "$intersected_archs" ]]; then
if [[ "$warn_missing_arch" == "true" ]]; then
if [[ "$warn_missing_arch" == "true" ]] && [[ "$ONLY_ACTIVE_ARCH" == "NO" ]]; then

Choose a reason for hiding this comment

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

Replying to @dnkoutso's #10076 (comment).

I confirm that the warning is indeed gone for debug builds, but it is still present for release builds. I see the following warning when I archive in Xcode 12 (stable) for "Any iOS Device (arm64)":

warning: [CP] Vendored binary '/Users/radazzouz/Library/Developer/Xcode/DerivedData/Build/Intermediates.noindex/ArchiveIntermediates/Minimal/IntermediateBuildFilesPath/Pods.build/Release-iphoneos/PSPDFKit.build/DerivedSources/PSPDFKitUI.framework.ios-x86_64-maccatalyst.dSYM/Contents/Resources/DWARF/PSPDFKitUI' contains architectures (x86_64) none of which match the current build architectures (arm64).

I don't think we should see this warning when archiving for iOS because it refers to the Mac Catalyst architecture.

I suspect that this happens because CocoaPods copies all the dSYMs (including the simulator and Mac Catalyst) into the iOS archive.

Screen Shot 2020-10-02 at 2 46 27 PM

BTW, the same thing happens (warnings about the iOS dSYMs) when I archive for Mac Catalyst.

I hope that this helps!

Copy link
Contributor Author

@dnkoutso dnkoutso Oct 2, 2020

Choose a reason for hiding this comment

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

@radazzouz hmmm for release ONLY_ACTIVE_ARCH is set to YES therefore cocoapods should warn that you are missing an architecture for the archs that you are building for right?

How can this be avoided in this case?

Copy link

@radazzouz radazzouz Oct 2, 2020

Choose a reason for hiding this comment

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

Yeah. That's strange. Could it be because $intersected_archs (from a few lines above) includes all the architectures (including architecture that we we're not building/archiving for)? If that's the case, then I think that $intersected_archs should not include the Mac Catalyst architecture when archiving for iOS and vice-versa.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmmm I think I see the issue let me try a few things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CocoaPods will install all dSYMs in the shell script

install_dsym "/Users/dimitris/Downloads/Minimal-CocoaPods/Pods/PSPDFKit/PSPDFKit.dSYMs/PSPDFKit.framework.ios-x86_64-maccatalyst.dSYM"
install_dsym "/Users/dimitris/Downloads/Minimal-CocoaPods/Pods/PSPDFKit/PSPDFKit.dSYMs/PSPDFKit.framework.ios-arm64.dSYM"
install_dsym "/Users/dimitris/Downloads/Minimal-CocoaPods/Pods/PSPDFKit/PSPDFKit.dSYMs/PSPDFKit.framework.ios-arm64_x86_64-simulator.dSYM"
install_dsym "/Users/dimitris/Downloads/Minimal-CocoaPods/Pods/PSPDFKit/PSPDFKitUI.dSYMs/PSPDFKitUI.framework.ios-x86_64-maccatalyst.dSYM"
install_dsym "/Users/dimitris/Downloads/Minimal-CocoaPods/Pods/PSPDFKit/PSPDFKitUI.dSYMs/PSPDFKitUI.framework.ios-arm64.dSYM"
install_dsym "/Users/dimitris/Downloads/Minimal-CocoaPods/Pods/PSPDFKit/PSPDFKitUI.dSYMs/PSPDFKitUI.framework.ios-arm64_x86_64-simulator.dSYM"

We either need to update this logic to be backed by ARCHS or update the warning to handle this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Considering alternative path again around the warning logic...

Choose a reason for hiding this comment

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

@radazzouz do xcframeworks contain all dSYMs always?

Yes. We bundle a dSYM for each platform (iOS device, simulator, and Mac Catalyst). We bundle the dSYMs for our XCFrameworks in the ${framework_name}.dSYMs directory next to the XCFramework itself, as recommended by CocoaPods.

Copy link
Contributor

Choose a reason for hiding this comment

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

@radazzouz I stumbled onto your PR while doing some testing regarding xcframework debug symbols with Xcode 12 and wanted to bring the work I'm doing to your attention (#10111 and PR #10122).

In short, Xcode 12 now supports an Apple-recommended approach for where debug symbols are placed with XCFrameworks. When using the new structure, manually dragged in XCFrameworks will automatically have their relevant debug symbols included in app archives, so I've been working a PR to ensure that CocoaPods also supports including symbols that are placed based on the new structure.

@dnkoutso your question of "Do xcframeworks always include those and are they named always by architecture? Is that standard in the xcframework structure?" relates to the work I'm doing as well... there's only an official placement for where they should go as of Xcode 12, but we also have the approach @radazzouz is describing which was the CocoaPods recommended placement when XCFrameworks were originally introduced.

I believe that the warning that is being hit here is only relevant in the context of the Xcode 11 style symbols where the MyLib-copy-dsyms.sh script is used and is copying all dSYMs. This script is not generated when using Xcode 12 style XCFramework debug symbols.

Copy link

@radazzouz radazzouz Oct 8, 2020

Choose a reason for hiding this comment

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

Thank you so much for the heads up @johntmcintosh! ❤️ I’ll take #10122 for a spin some time next week and I’ll make sure that the new PSPDFKit XCFrameworks for CocoaPods will be updated after the PR lands.

Copy link
Member

Choose a reason for hiding this comment

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

Catching up - I don't think explicitly removing the warning in this case is the right way to go. As @radazzouz suggested we should catch this earlier and not install those dSYMS at all unless they are appropriate for the current architecture

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.

Wrong warning is emitted about the dSYMs architectures not matching when using XCFrameworks
4 participants