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

Fix DWARF binary not copied for AppleDsymBundleInfo #2396

Conversation

luispadron
Copy link
Contributor

@luispadron luispadron commented Feb 8, 2024

From what I can tell the AppleDebugInfo provider correctly gives us the binary paths, the only difference is that AppleDsymBundleInfo seems to add a /dSYMS/ directory path:

AppleDsymBundleInfo: bazel-out/<sha>/bin/Code/Apps/CashAppWidgets/WidgetExtension/dSYMs/WidgetExtension_dsyms/Widget Extension.appex.dSYM
AppleDebugInfo:      bazel-out/<sha>/bin/Code/Apps/CashAppWidgets/WidgetExtension/WidgetExtension_dsyms/Widget Extension.appex.dSYM

It looks like thats intentional given:

# Put the tree artifact dSYMs in a subdirectory to avoid conflicts with the legacy dSYMs
# provided through existing APIs such as --output_groups=+dsyms.
dsym_bundle_dir = actions.declare_directory("dSYMs/" + dsym_bundle_name)

But then the rule copies it without the /dSYMS:

output_binary = actions.declare_file(
"%s/Contents/Resources/DWARF/%s" % (
dsym_bundle_name,
debug_output_filename,
),
)


Before:

bazel-bin/Code/Apps/CashApp/Cash.xcarchive/dSYMS
├── Cash.app.dSYM
│   └── Contents
│       ├── Info.plist
│       └── Resources
│           └── DWARF
├── Siri UI.appex.dSYM
│   └── Contents
│       ├── Info.plist
│       └── Resources
│           └── DWARF
├── Siri.appex.dSYM
│   └── Contents
│       ├── Info.plist
│       └── Resources
│           └── DWARF
└── Widget Extension.appex.dSYM
    └── Contents
        ├── Info.plist
        └── Resources
            └── DWARF

After:

bazel-bin/Code/Apps/CashApp/Cash.xcarchive/dSYMS
├── Cash.app.dSYM
│   └── Contents
│       ├── Info.plist
│       └── Resources
│           └── DWARF
│               └── Cash
├── Siri UI.appex.dSYM
│   └── Contents
│       ├── Info.plist
│       └── Resources
│           └── DWARF
│               └── Siri UI
├── Siri.appex.dSYM
│   └── Contents
│       ├── Info.plist
│       └── Resources
│           └── DWARF
│               └── Siri
└── Widget Extension.appex.dSYM
    └── Contents
        ├── Info.plist
        └── Resources
            └── DWARF
                └── Widget Extension

@luispadron luispadron force-pushed the luis/fix-dsym-dwarf-binary-not-copied-to-appledsyminfoprovider branch from 1fb68e3 to 97880f8 Compare February 8, 2024 20:06
@luispadron luispadron force-pushed the luis/fix-dsym-dwarf-binary-not-copied-to-appledsyminfoprovider branch 2 times, most recently from 8105457 to dbed722 Compare February 16, 2024 16:51
@luispadron luispadron marked this pull request as ready for review February 16, 2024 16:52
@luispadron luispadron force-pushed the luis/fix-dsym-dwarf-binary-not-copied-to-appledsyminfoprovider branch 3 times, most recently from 7a56973 to 52f159f Compare February 23, 2024 01:49
Copy link
Contributor

@mattrobmattrob mattrobmattrob left a comment

Choose a reason for hiding this comment

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

Is it possible to add a test for this?

apple/internal/partials/debug_symbols.bzl Outdated Show resolved Hide resolved
@luispadron luispadron force-pushed the luis/fix-dsym-dwarf-binary-not-copied-to-appledsyminfoprovider branch from daad6f0 to 574ebdd Compare February 23, 2024 15:40
@luispadron
Copy link
Contributor Author

@mattrobmattrob updated and added tests for the xcarchive rule to ensure these are now getting copied (these tests were previously failing without this change)

@luispadron luispadron force-pushed the luis/fix-dsym-dwarf-binary-not-copied-to-appledsyminfoprovider branch from 574ebdd to 04e5f2f Compare February 23, 2024 15:50
@luispadron luispadron enabled auto-merge (squash) February 23, 2024 15:50
@luispadron luispadron force-pushed the luis/fix-dsym-dwarf-binary-not-copied-to-appledsyminfoprovider branch from 04e5f2f to 17c1b80 Compare February 23, 2024 22:23
Co-authored-by: Matt Robinson <mattrob@hey.com>
@luispadron luispadron force-pushed the luis/fix-dsym-dwarf-binary-not-copied-to-appledsyminfoprovider branch from 17c1b80 to e1ce241 Compare February 24, 2024 00:18
@luispadron luispadron merged commit 5db6e74 into master Feb 24, 2024
10 checks passed
@luispadron luispadron deleted the luis/fix-dsym-dwarf-binary-not-copied-to-appledsyminfoprovider branch February 24, 2024 00:35
acecilia pushed a commit to revolut-mobile/rules_apple that referenced this pull request Apr 12, 2024
From what I can tell the `AppleDebugInfo` provider correctly gives us the binary paths, the only difference is that `AppleDsymBundleInfo` seems to add a `/dSYMS/` directory path. This PR updates to copy to the dwarf binaries the correct `/dSYMS/` as well.

Co-authored-by: Matt Robinson <mattrob@hey.com>
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.

2 participants