-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
dnkoutso
wants to merge
1
commit into
CocoaPods:1-10-stable
from
dnkoutso:only_active_arch_no_warning
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Submodule cocoapods-integration-specs
updated
34 files
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.
BTW, the same thing happens (warnings about the iOS dSYMs) when I archive for Mac Catalyst.
I hope that this helps!
There was a problem hiding this comment.
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 toYES
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?
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
We either need to update this logic to be backed by
ARCHS
or update the warning to handle this case.There was a problem hiding this comment.
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...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.There was a problem hiding this comment.
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.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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