Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

iOS: Add dSYM binaries to without_entitlements.txt #54576

Merged
merged 5 commits into from
Aug 16, 2024

Conversation

cbracken
Copy link
Member

@cbracken cbracken commented Aug 16, 2024

In #54414, we added dSYM files for physical and simulator binaries in both regular and extension-safe framework builds, but did not add the dSYMs to the without_entitlements.txt list.

This passed all engine pre/post-submit tests, as well as framework tests, but failed during release code signing in Cocoon in a test here:
https://github.com/flutter/cocoon/blob/d849b14bab90e0f90e2f7667e37c9f9a5696b918/cipd_packages/codesign/lib/src/file_codesign_visitor.dart#L305-L313

This adds the missing files to without_entitlements.txt, which fixes a code-signing error as seen in this build log:
https://ci.chromium.org/ui/p/dart-internal/builders/flutter/Mac%20Production%20Engine%20Drone/13590/overview

A corresponding change was landed on the flutter-3.24-candidate.1 branch:
#54573

The build associated with that patch correctly completed code signing in this build:
https://ci.chromium.org/ui/p/dart-internal/builders/flutter/Mac%20engine_release_builder/688/overview

And more specifically, this sub-build:
https://ci.chromium.org/ui/p/dart-internal/builders/flutter/Mac%20Production%20Engine%20Drone/13650/overview

And even more specifically, this build step:
https://logs.chromium.org/logs/dart-internal/buildbucket/cr-buildbucket/8739493904842446705/+/u/Global_generators/Codesign__Volumes_Work_s_w_ir_cache_builder_src_out_release_unsigned_artifacts.zip/codesign_Apple_engine_binaries/stdout

Additionally, this patch adds sky_utils.assert_valid_codesign_config() which fails the script (and thus the build) with an error message if any file in scope for code signing (i.e. Mach-O binaries) is not listed in the code-signing config (entitlements.txt, without_entitlements.txt), or any listed file is not found on disk.

Issue: flutter/flutter#116493
Issue: flutter/flutter#153532

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@cbracken cbracken requested review from jmagman and zanderso August 16, 2024 01:35
@cbracken cbracken force-pushed the dSYM-entitlements-main branch from 8a5b7bc to 152e4b0 Compare August 16, 2024 01:37
In flutter#54414, we added dSYM files for physical and simulator
binaries in both regular and extension-safe framework builds, but did
not add the dSYMs to the without_entitlements.txt list.

This passed all engine pre/post-submit tests, as well as framework
tests, but failed during release codesigning in Cocoon in a test here:
https://github.com/flutter/cocoon/blob/d849b14bab90e0f90e2f7667e37c9f9a5696b918/cipd_packages/codesign/lib/src/file_codesign_visitor.dart#L305-L313

This adds the missing files to without_entitlements.txt, which fixes a
codesigning error as seen in this build log:
https://ci.chromium.org/ui/p/dart-internal/builders/flutter/Mac%20Production%20Engine%20Drone/13590/overview

A corresponding change was landed on the flutter-3.24-candidate.1 branch:
flutter#54573

The build associated with that patch correctly completed code signing in
this build:
https://ci.chromium.org/ui/p/dart-internal/builders/flutter/Mac%20engine_release_builder/688/overview

And more specifically, this sub-build:
https://ci.chromium.org/ui/p/dart-internal/builders/flutter/Mac%20Production%20Engine%20Drone/13650/overview

And even more specifically, this build step:
https://logs.chromium.org/logs/dart-internal/buildbucket/cr-buildbucket/8739493904842446705/+/u/Global_generators/Codesign__Volumes_Work_s_w_ir_cache_builder_src_out_release_unsigned_artifacts.zip/codesign_Apple_engine_binaries/stdout

Additionally, this patch adds `sky_utils.assert_valid_codesign_config()`
which fails the script (and thus the build) with an error message if any
file in scope for code signing (i.e. Mach-O binaries) is not listed in
the code-signing config (entitlements.txt, without_entitlements.txt), or
any listed file is not found on disk.

Issue: flutter/flutter#116493
@cbracken cbracken force-pushed the dSYM-entitlements-main branch from 152e4b0 to ec52964 Compare August 16, 2024 01:44
@jmagman
Copy link
Member

jmagman commented Aug 16, 2024

Binaries found in files to be zipped:
    Flutter.xcframework/ios-arm64/Flutter.framework/Flutter
    Flutter.xcframework/ios-arm64_x86_64-simulator/Flutter.framework/Flutter
    extension_safe/Flutter.xcframework/ios-arm64/Flutter.framework/Flutter
    extension_safe/Flutter.xcframework/ios-arm64_x86_64-simulator/Flutter.framework/Flutter
    gen_snapshot_arm64
Binaries listed in entitlements.txt/without_entitlements.txt but NOT FOUND:
    Flutter.xcframework/ios-arm64/dSYMs/Flutter.framework.dSYM/Contents/Resources/DWARF/Flutter
    Flutter.xcframework/ios-arm64_x86_64-simulator/dSYMs/Flutter.framework.dSYM/Contents/Resources/DWARF/Flutter
    extension_safe/Flutter.xcframework/ios-arm64/dSYMs/Flutter.framework.dSYM/Contents/Resources/DWARF/Flutter
    extension_safe/Flutter.xcframework/ios-arm64_x86_64-simulator/dSYMs/Flutter.framework.dSYM/Contents/Resources/DWARF/Flutter

Eliminate duplicate value found in without_entitlements.txt
@cbracken
Copy link
Member Author

cbracken commented Aug 16, 2024

Binaries listed in entitlements.txt/without_entitlements.txt but NOT FOUND:

Ha look at that, checks already catching issues! We only produce dSYMs in release builds, in others the dSYM flag isn't passed. This is actually harmless since the only builds we sign are ones with dSYMs, but we should still generate these correctly.

It's painful enough just testing the seven builds required to do release build testing locally, so hadn't been testing debug/profile cases locally on top of it. One more conditional and we should be good.

@cbracken
Copy link
Member Author

Alrighty, here we go again.

@jmagman
Copy link
Member

jmagman commented Aug 16, 2024

I may be misremembering but aren't dSYM usually only generated for release mode? I don't think it bakes the symbols into the debug binary (for linker speed) but there's some other mechanism for how it works? I don't exactly recall though.
I would think if debug crashes aren't currently symbolicated we would need the dSYMs.

@cbracken
Copy link
Member Author

cbracken commented Aug 16, 2024

I may be misremembering but aren't dSYM usually only generated for release mode?

Yep that's exactly what I was saying here, though perhaps badly. We now (as of this commit) only add them if the --dsym flag is passed. And that flag is only passed in release mode.

I would think if debug crashes aren't currently symbolicated we would need the dSYMs.

Debug/Profile builds aren't stripped, so they're properly symbolicated as-is. The release builds are the only ones we strip (here):

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

Tests! Tests! Tests! Tests! Tests! Tests!

@cbracken cbracken added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 16, 2024
@auto-submit auto-submit bot merged commit b66893e into flutter:main Aug 16, 2024
27 checks passed
@cbracken cbracken deleted the dSYM-entitlements-main branch August 16, 2024 16:27
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 16, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 16, 2024
…153582)

flutter/engine@d5bf3af...b66893e

2024-08-16 [email protected] iOS: Add dSYM binaries to without_entitlements.txt (flutter/engine#54576)
2024-08-16 [email protected] Manual roll Dart SDK from 44635f897535 to 8c02ad43e01a (3 revisions)  (flutter/engine#54581)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
…lutter#153582)

flutter/engine@d5bf3af...b66893e

2024-08-16 [email protected] iOS: Add dSYM binaries to without_entitlements.txt (flutter/engine#54576)
2024-08-16 [email protected] Manual roll Dart SDK from 44635f897535 to 8c02ad43e01a (3 revisions)  (flutter/engine#54581)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
cbracken added a commit to cbracken/flutter that referenced this pull request Sep 3, 2024
In flutter/engine#54414, we added dSYM files for physical and simulator binaries in both regular and extension-safe framework builds. In flutter/engine#54576 we added the dSYMs to the without_entitlements.txt list.

The `dev/bots/suite_runners/run_verify_binaries_codesigned_tests.dart` test was updated to reflect these changes in:
* flutter#153787
* flutter#154027

This cherrypicks these two changes to the release branch.

Issue: flutter#116493
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Sep 3, 2024
In flutter/engine#54414, we added dSYM files for physical and simulator binaries in both regular and extension-safe framework builds. In flutter/engine#54576 we added the dSYMs to the without_entitlements.txt list.

The `dev/bots/suite_runners/run_verify_binaries_codesigned_tests.dart` test was updated to reflect these changes in:
* #153787
* #154027

This cherrypicks these two changes to the release branch.

Issue: #116493
Gustl22 pushed a commit to Gustl22/flutter that referenced this pull request Nov 13, 2024
In flutter/engine#54414, we added dSYM files for physical and simulator binaries in both regular and extension-safe framework builds. In flutter/engine#54576 we added the dSYMs to the without_entitlements.txt list.

The `dev/bots/suite_runners/run_verify_binaries_codesigned_tests.dart` test was updated to reflect these changes in:
* flutter#153787
* flutter#154027

This cherrypicks these two changes to the release branch.

Issue: flutter#116493
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants