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

macOS: Extract framework creation to sky_utils #54586

Merged
merged 6 commits into from
Aug 17, 2024
Merged

macOS: Extract framework creation to sky_utils #54586

merged 6 commits into from
Aug 17, 2024

Conversation

@cbracken
Copy link
Member

@cbracken cbracken commented Aug 16, 2024 •
edited
Loading

This refactors create_mac_framework.py to extract framework creation to sky_utils.py. The only other changes are minor variable renaming and extraction of functions to make the code more readable/easier to work with.

The resulting zip archives have been verified to be identical before and after.

This is refactoring to support embedding dSYM debug information in a follow-up patch.

Related issue: flutter/flutter#153879

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 a review from zanderso August 16, 2024 19:57
@zanderso
Copy link
Member

zanderso commented Aug 16, 2024

Can we run the framework presubs on this too =)

@cbracken
Copy link
Member Author

cbracken commented Aug 16, 2024

Yep, waiting for it to build first.

cbracken added a commit to cbracken/flutter that referenced this pull request Aug 16, 2024
@cbracken
DO NOT SUBMIT: TEST ENGINE ROLL
fe96a21 
Test roll of flutter/engine#54586 "macOS: Extract framework creation to sky_utils" to framework.
cbracken added a commit to cbracken/flutter that referenced this pull request Aug 16, 2024
@cbracken
DO NOT SUBMIT: TEST ENGINE ROLL
720b439 
Test roll of flutter/engine#54586 "macOS: Extract framework creation to sky_utils" to framework.
@cbracken cbracken mentioned this pull request Aug 16, 2024
Closed
9 tasks
@cbracken
Copy link
Member Author

cbracken commented Aug 16, 2024

Test framework roll is: flutter/flutter#153598

@cbracken
Copy link
Member Author

cbracken commented Aug 16, 2024 •
edited
Loading

Almost all tests are failing, but if you manually pick through the failures, you can see that the symlinks are correct here:
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8739412731644140417/+/u/run_test.dart_for_build_tests_shard_and_subshard_3_4/stdout

@zanderso
Copy link
Member

zanderso commented Aug 16, 2024

Test framework roll is: flutter/flutter#153598

The framework web presubs are failing because the runIf clause in the engine .ci.yaml excluded the web build running as a presubmit check on this PR.

@cbracken
macOS: Extract framework creation to sky_utils.py
b40371d 
This refactors create_mac_framework.py to extract framework creation to
sky_utils.py. The only other changes are minor variable renaming and
extraction of functions to make the code more readable/easier to work
with.

The resulting zip archives have been verified to be identical before and
after.

This is refactoring to support embedding dSYM debug information in a
follow-up patch.
@cbracken
Rename fat_framework_bundle to fat_framework
e28e232
@cbracken
Extract create_fat_macos_framework
04ce399
@cbracken
Extract framework creation funcs to sky_utils
ee876db
@cbracken
Formatting
f10ffdd
@cbracken
Remove unused import
e25ac32
@cbracken
Copy link
Member Author

cbracken commented Aug 16, 2024

Updated the docs to mention this in #54593

cbracken added a commit to cbracken/flutter that referenced this pull request Aug 16, 2024
@cbracken
DO NOT SUBMIT: TEST ENGINE ROLL
21dbdd4 
Test roll of flutter/engine#54586 "macOS: Extract framework creation to sky_utils" to framework.
cbracken added a commit to cbracken/flutter that referenced this pull request Aug 16, 2024
@cbracken
DO NOT SUBMIT: TEST ENGINE ROLL
fe9ced7 
Test roll of flutter/engine#54586 "macOS: Extract framework creation to sky_utils" to framework.
@cbracken
Copy link
Member Author

cbracken commented Aug 16, 2024

All macOS tests are passing on the framework patch. flutter/flutter#153598

@cbracken cbracken requested a review from jmagman August 16, 2024 23:35
@cbracken
Copy link
Member Author

cbracken commented Aug 16, 2024 •
edited
Loading

I've removed the .ci.yaml commit (e519ba6) so we don't inadvertently land it. Engine presubmits running again now that we've verified this is fine.

@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 7682457 into flutter:main Aug 17, 2024
@cbracken cbracken deleted the reland-macos-refactor branch August 17, 2024 00:18
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 17, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 17, 2024
@engine-flutter-autoroll
7682457d1 macOS: Extract framework creation to sky_utils (flutter/eng…
9408184 
…ine#54586)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 17, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 17, 2024
@engine-flutter-autoroll
7682457d1 macOS: Extract framework creation to sky_utils (flutter/eng…
9978696 
…ine#54586)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 17, 2024
@engine-flutter-autoroll
Roll Flutter Engine from c94651c33b0e to c9fb47927fea (5 revisions) (#…
28c4dfa 
…153618)

flutter/engine@c94651c...c9fb479

2024-08-17 [email protected] Roll Fuchsia Linux SDK from Z5hq3ZkPNCpZWRQnl... to a3zdjZKduabZSBN0B... (flutter/engine#54596)
2024-08-17 [email protected] Roll Skia from 219bd1032761 to 570b18e1afda (1 revision) (flutter/engine#54595)
2024-08-17 [email protected] macOS: Extract framework creation to sky_utils (flutter/engine#54586)
2024-08-16 [email protected] Roll Skia from fc8769175d35 to 219bd1032761 (4 revisions) (flutter/engine#54592)
2024-08-16 [email protected] [docs] Add missing steps to Testing Presubmit Engine PRs (flutter/engine#54593)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from Z5hq3ZkPNCpZ to a3zdjZKduabZ

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
@auto-submit auto-submit bot mentioned this pull request Aug 17, 2024
Merged
auto-submit bot added a commit to flutter/flutter that referenced this pull request Aug 17, 2024
@auto-submit
Reverts "Roll Flutter Engine from c94651c33b0e to c9fb47927fea (5 rev…
5d70d19 
…isions) (#153618)" (#153627)

Reverts: #153618
Initiated by: zanderso
Reason for reverting: The tree is red.
Original PR Author: engine-flutter-autoroll

Reviewed By: {fluttergithubbot}

This change reverts the following previous change:

flutter/engine@c94651c...c9fb479

2024-08-17 [email protected] Roll Fuchsia Linux SDK from Z5hq3ZkPNCpZWRQnl... to a3zdjZKduabZSBN0B... (flutter/engine#54596)
2024-08-17 [email protected] Roll Skia from 219bd1032761 to 570b18e1afda (1 revision) (flutter/engine#54595)
2024-08-17 [email protected] macOS: Extract framework creation to sky_utils (flutter/engine#54586)
2024-08-16 [email protected] Roll Skia from fc8769175d35 to 219bd1032761 (4 revisions) (flutter/engine#54592)
2024-08-16 [email protected] [docs] Add missing steps to Testing Presubmit Engine PRs (flutter/engine#54593)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from Z5hq3ZkPNCpZ to a3zdjZKduabZ

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
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 17, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 17, 2024
@engine-flutter-autoroll
7682457d1 macOS: Extract framework creation to sky_utils (flutter/eng…
51c6303 
…ine#54586)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 17, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 17, 2024
@engine-flutter-autoroll
7682457d1 macOS: Extract framework creation to sky_utils (flutter/eng…
ae0bef8 
…ine#54586)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 17, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 17, 2024
@engine-flutter-autoroll
7682457d1 macOS: Extract framework creation to sky_utils (flutter/eng…
86e6d44 
…ine#54586)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 18, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 18, 2024
@engine-flutter-autoroll
7682457d1 macOS: Extract framework creation to sky_utils (flutter/eng…
215e4a9 
…ine#54586)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 18, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 18, 2024
@engine-flutter-autoroll
7682457d1 macOS: Extract framework creation to sky_utils (flutter/eng…
b0e5961 
…ine#54586)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 18, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 18, 2024
@engine-flutter-autoroll
7682457d1 macOS: Extract framework creation to sky_utils (flutter/eng…
fca4768 
…ine#54586)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 18, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 18, 2024
@engine-flutter-autoroll
7682457d1 macOS: Extract framework creation to sky_utils (flutter/eng…
73902be 
…ine#54586)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 18, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 18, 2024
@engine-flutter-autoroll
7682457d1 macOS: Extract framework creation to sky_utils (flutter/eng…
e47d357 
…ine#54586)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 18, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 18, 2024
@engine-flutter-autoroll
7682457d1 macOS: Extract framework creation to sky_utils (flutter/eng…
efa3497 
…ine#54586)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 18, 2024
@engine-flutter-autoroll
Manual roll Flutter Engine from c94651c33b0e to d0d7de5c333d (9 revis…
7e9d9e0 
…ions) (#153659)

Manual roll requested by [email protected]

flutter/engine@c94651c...d0d7de5

2024-08-18 [email protected] Roll Skia from ac7149e315ec to 7025ec4bff25 (1 revision) (flutter/engine#54605)
2024-08-18 [email protected] Roll Fuchsia Linux SDK from a3zdjZKduabZSBN0B... to yKkNB9F8Hwnjq2AMW... (flutter/engine#54602)
2024-08-17 [email protected] Roll Skia from 570b18e1afda to ac7149e315ec (1 revision) (flutter/engine#54600)
2024-08-17 [email protected] Shift linux_fuchsia_tests from staging to prod (flutter/engine#54597)
2024-08-17 [email protected] Roll Fuchsia Linux SDK from Z5hq3ZkPNCpZWRQnl... to a3zdjZKduabZSBN0B... (flutter/engine#54596)
2024-08-17 [email protected] Roll Skia from 219bd1032761 to 570b18e1afda (1 revision) (flutter/engine#54595)
2024-08-17 [email protected] macOS: Extract framework creation to sky_utils (flutter/engine#54586)
2024-08-16 [email protected] Roll Skia from fc8769175d35 to 219bd1032761 (4 revisions) (flutter/engine#54592)
2024-08-16 [email protected] [docs] Add missing steps to Testing Presubmit Engine PRs (flutter/engine#54593)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from Z5hq3ZkPNCpZ to yKkNB9F8Hwnj

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 Aug 20, 2024
@cbracken
DO NOT SUBMIT: TEST ENGINE ROLL
c02ddca 
Test roll of flutter/engine#54586 "macOS: Extract framework creation to sky_utils" to framework.
cbracken added a commit to cbracken/flutter that referenced this pull request Aug 20, 2024
@cbracken
DO NOT SUBMIT: TEST ENGINE ROLL
1f1adfa 
Test roll of flutter/engine#54586 "macOS: Extract framework creation to sky_utils" to framework.
cbracken added a commit to cbracken/flutter that referenced this pull request Aug 20, 2024
@cbracken
DO NOT SUBMIT: TEST ENGINE ROLL
0ec467d 
Test roll of flutter/engine#54586 "macOS: Extract framework creation to sky_utils" to framework.
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
@engine-flutter-autoroll @Buchimi
Roll Flutter Engine from c94651c33b0e to c9fb47927fea (5 revisions) (f…
94200a7 
…lutter#153618)

flutter/engine@c94651c...c9fb479

2024-08-17 [email protected] Roll Fuchsia Linux SDK from Z5hq3ZkPNCpZWRQnl... to a3zdjZKduabZSBN0B... (flutter/engine#54596)
2024-08-17 [email protected] Roll Skia from 219bd1032761 to 570b18e1afda (1 revision) (flutter/engine#54595)
2024-08-17 [email protected] macOS: Extract framework creation to sky_utils (flutter/engine#54586)
2024-08-16 [email protected] Roll Skia from fc8769175d35 to 219bd1032761 (4 revisions) (flutter/engine#54592)
2024-08-16 [email protected] [docs] Add missing steps to Testing Presubmit Engine PRs (flutter/engine#54593)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from Z5hq3ZkPNCpZ to a3zdjZKduabZ

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
@auto-submit @Buchimi
Reverts "Roll Flutter Engine from c94651c33b0e to c9fb47927fea (5 rev…
512a44a 
…isions) (flutter#153618)" (flutter#153627)

Reverts: flutter#153618
Initiated by: zanderso
Reason for reverting: The tree is red.
Original PR Author: engine-flutter-autoroll

Reviewed By: {fluttergithubbot}

This change reverts the following previous change:

flutter/engine@c94651c...c9fb479

2024-08-17 [email protected] Roll Fuchsia Linux SDK from Z5hq3ZkPNCpZWRQnl... to a3zdjZKduabZSBN0B... (flutter/engine#54596)
2024-08-17 [email protected] Roll Skia from 219bd1032761 to 570b18e1afda (1 revision) (flutter/engine#54595)
2024-08-17 [email protected] macOS: Extract framework creation to sky_utils (flutter/engine#54586)
2024-08-16 [email protected] Roll Skia from fc8769175d35 to 219bd1032761 (4 revisions) (flutter/engine#54592)
2024-08-16 [email protected] [docs] Add missing steps to Testing Presubmit Engine PRs (flutter/engine#54593)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from Z5hq3ZkPNCpZ to a3zdjZKduabZ

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
@engine-flutter-autoroll @Buchimi
Manual roll Flutter Engine from c94651c33b0e to d0d7de5c333d (9 revis…
3100cc5 
…ions) (flutter#153659)

Manual roll requested by [email protected]

flutter/engine@c94651c...d0d7de5

2024-08-18 [email protected] Roll Skia from ac7149e315ec to 7025ec4bff25 (1 revision) (flutter/engine#54605)
2024-08-18 [email protected] Roll Fuchsia Linux SDK from a3zdjZKduabZSBN0B... to yKkNB9F8Hwnjq2AMW... (flutter/engine#54602)
2024-08-17 [email protected] Roll Skia from 570b18e1afda to ac7149e315ec (1 revision) (flutter/engine#54600)
2024-08-17 [email protected] Shift linux_fuchsia_tests from staging to prod (flutter/engine#54597)
2024-08-17 [email protected] Roll Fuchsia Linux SDK from Z5hq3ZkPNCpZWRQnl... to a3zdjZKduabZSBN0B... (flutter/engine#54596)
2024-08-17 [email protected] Roll Skia from 219bd1032761 to 570b18e1afda (1 revision) (flutter/engine#54595)
2024-08-17 [email protected] macOS: Extract framework creation to sky_utils (flutter/engine#54586)
2024-08-16 [email protected] Roll Skia from fc8769175d35 to 219bd1032761 (4 revisions) (flutter/engine#54592)
2024-08-16 [email protected] [docs] Add missing steps to Testing Presubmit Engine PRs (flutter/engine#54593)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from Z5hq3ZkPNCpZ to yKkNB9F8Hwnj

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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jmagman jmagman Awaiting requested review from jmagman

1 more reviewer

@zanderso zanderso zanderso approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cbracken @zanderso