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

Re-implement dSYM copying and stripping to avoid duplicate outputs. #9547

Merged
merged 1 commit into from Feb 21, 2020

Conversation

dnkoutso
Copy link
Contributor

@dnkoutso dnkoutso commented Feb 20, 2020

dSYMs should have never been treated as objects to "embed" like .frameworks all they need is copying and stripping. This PR move those steps into a single target and avoids multiple targets being built producing the same "output" in which Xcode 11 errors out.

closes #9185

integration specs: CocoaPods/cocoapods-integration-specs#269

Copy link
Member

@amorde amorde left a comment

looking great! nice work

@dnkoutso dnkoutso force-pushed the dsym_revamp branch 6 times, most recently from ad190fa to 6e2b0c2 Compare Feb 20, 2020
@dnkoutso dnkoutso changed the title WIP - Revamp dSYM integration Revamp dSYM integration Feb 20, 2020
@dnkoutso dnkoutso force-pushed the dsym_revamp branch 2 times, most recently from 028e1bb to b10dabb Compare Feb 20, 2020
@@ -263,7 +267,9 @@ def create_or_update_shell_script_build_phase(native_target, script_phase_name,
def create_or_update_user_script_phases(script_phases, native_target)
script_phase_names = script_phases.map { |k| k[:name] }
# Delete script phases no longer present in the target.
native_target_script_phases = native_target.shell_script_build_phases.select { |bp| !bp.name.nil? && bp.name.start_with?(USER_BUILD_PHASE_PREFIX) }
native_target_script_phases = native_target.shell_script_build_phases.select do |bp|
Copy link
Contributor Author

@dnkoutso dnkoutso Feb 20, 2020

Choose a reason for hiding this comment

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

just formatting as line was too long. no changes.

@@ -59,113 +59,103 @@ def generate
#
def script
script = <<-SH.strip_heredoc
#!/bin/sh
Copy link
Contributor Author

@dnkoutso dnkoutso Feb 20, 2020

Choose a reason for hiding this comment

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

just updated indentation. I think because I added string interpolation to the mix it broke indentation. Either way no changes here except using the script constants instead.

@dnkoutso dnkoutso force-pushed the dsym_revamp branch 3 times, most recently from b983379 to db2e3d7 Compare Feb 21, 2020
Copy link
Contributor

@nikolaykasyanov nikolaykasyanov left a comment

👏

@nikolaykasyanov
Copy link
Contributor

@nikolaykasyanov nikolaykasyanov commented Feb 21, 2020

I'm getting a build error using New Build System (Xcode 11.3.111.4 beta 2) on this branch:

Showing All Messages
Build system information
error: Cycle in dependencies between targets 'A-Unit-Tests' and 'A'; building could produce unreliable results.
Cycle path: A-Unit-Tests → A → A-Unit-Tests
Cycle details:
→ That command depends on command in Target 'A-Unit-Tests': script phase “[CP] Embed Pods Frameworks”
→ Target 'A-Unit-Tests' has target dependency on Target 'A'
→ Target 'A' has target dependency on Target 'B'
○ That command depends on command in Target 'B': script phase “[CP] Copy dSYMs”

Target name explanation.

UPD there's also a warning from the build system, not sure if related:

Showing All Messages
missing creator for mutated node: ('/Users/user/Library/Developer/Xcode/DerivedData/project-bfjsebfzwmezwxetcmlmiduwobqw/Build/Products/Debug-iphonesimulator/B/B.framework.dSYM')

@nikolaykasyanov
Copy link
Contributor

@nikolaykasyanov nikolaykasyanov commented Feb 21, 2020

Looks like [CP] Copy dSYMs output file list refernce is incorrect (equal to the input list):
${PODS_ROOT}/Target Support Files/B/B-strip-dsym-input-files.xcfilelist

@dnkoutso dnkoutso force-pushed the dsym_revamp branch 3 times, most recently from 9cf0b0f to f339988 Compare Feb 21, 2020
@dnkoutso dnkoutso force-pushed the dsym_revamp branch 5 times, most recently from 9f55cde to babcc11 Compare Feb 21, 2020
@dnkoutso dnkoutso changed the title Revamp dSYM integration Re-implement dSYM copying and stripping to avoid duplicate outputs. Feb 21, 2020
Copy link
Contributor

@nikolaykasyanov nikolaykasyanov left a comment

Working great 🎉

amorde
amorde approved these changes Feb 21, 2020
@dnkoutso dnkoutso force-pushed the dsym_revamp branch 3 times, most recently from 9662858 to 702cfe6 Compare Feb 21, 2020
@dnkoutso
Copy link
Contributor Author

@dnkoutso dnkoutso commented Feb 21, 2020

Re did it with a file instead but same functionality. Merging.

@dnkoutso dnkoutso merged commit 9817297 into CocoaPods:master Feb 21, 2020
4 checks passed
@dnkoutso dnkoutso deleted the dsym_revamp branch Feb 21, 2020
@migonin
Copy link

@migonin migonin commented Mar 4, 2020

@dnkoutso did it fix #7155 also?

@amorde
Copy link
Member

@amorde amorde commented Mar 4, 2020

@migonin no, this PR only changes the implementation details of how .dSYM symbols get included in archives.

Please keep comments about issues on that issue itself, it's difficult to jump around issues & PRs to follow conversation when its not in one place.

@umar1136
Copy link

@umar1136 umar1136 commented Jan 27, 2022

expect

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.

5 participants