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

macOS: Refactor create_macos_framework.py #54557

Merged
merged 1 commit into from
Aug 14, 2024
Merged

macOS: Refactor create_macos_framework.py #54557

merged 1 commit into from
Aug 14, 2024

Conversation

cbracken
Copy link
Member

This is a refactoring with no semantic changes.

This refactors the macOS framework creation code to be more readable, and extracts it to sky_utils.py.

While I was pulling this out, also generalised the code to not hardcode FlutterMacOS.framework in case we one day manage to generate the iOS and macOS frameworks with the same name.

This is a reland of #54546 (reverted in #54549), the original was reverted in order to revert #54543 (reverted in #54550), which was reverted because it failed to preserve symlinks while zipping the macOS framework. That patch has been relanded with a fix in #54555. This patch has been rebased to tip-of-tree for attempt two.

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
macOS: Refactor create_macos_framework.py
ee05bd1 
This is a refactoring with no semantic changes.

This refactors the macOS framework creation code to be more readable, and extracts it to `sky_utils.py`.

While I was pulling this out, also generalised the code to not hardcode `FlutterMacOS.framework` in case we one day manage to generate the iOS and macOS frameworks with the same name.

This is a reland of #54546 (reverted in #54549), the original was reverted in order to revert #54543 (reverted in #54550), which was reverted because it failed to preserve symlinks while zipping the macOS framework. That patch has been relanded with a fix in #54555. This patch has been rebased to tip-of-tree for attempt two.
@cbracken cbracken requested review from zanderso and jmagman August 14, 2024 20:16
@cbracken cbracken added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 14, 2024
@auto-submit auto-submit bot merged commit 7949ab3 into flutter:main Aug 14, 2024
27 checks passed
@cbracken cbracken deleted the reland-even-more-macos-refactor branch August 14, 2024 22:16
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 14, 2024
@engine-flutter-autoroll
7949ab379 macOS: Refactor create_macos_framework.py (flutter/engine#5…
c94c395 
…4557)
This was referenced Aug 14, 2024
Closed
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 15, 2024
@engine-flutter-autoroll
7949ab379 macOS: Refactor create_macos_framework.py (flutter/engine#5…
6eacfee 
…4557)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 15, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 15, 2024
@engine-flutter-autoroll
7949ab379 macOS: Refactor create_macos_framework.py (flutter/engine#5…
63fd5a1 
…4557)
@zanderso
Copy link
Member

Reason for revert: Failing on framework tests on the roll https://ci.chromium.org/ui/p/flutter/builders/try/Mac_arm64%20run_debug_test_macos/9841/overview

@zanderso zanderso added the revert Label used to revert changes in a closed and merged pull request. label Aug 15, 2024
auto-submit bot pushed a commit that referenced this pull request Aug 15, 2024
Revert "macOS: Refactor create_macos_framework.py (#54557)"
222436c 
This reverts commit 7949ab3.
@auto-submit auto-submit bot mentioned this pull request Aug 15, 2024
Merged
8 tasks
@auto-submit auto-submit bot removed the revert Label used to revert changes in a closed and merged pull request. label Aug 15, 2024
auto-submit bot added a commit that referenced this pull request Aug 15, 2024
@auto-submit
Reverts "macOS: Refactor create_macos_framework.py (#54557)" (#54566)
971ddd9 
Reverts: #54557
Initiated by: zanderso
Reason for reverting: Failing on framework tests on the roll https://ci.chromium.org/ui/p/flutter/builders/try/Mac_arm64%20run_debug_test_macos/9841/overview
Original PR Author: cbracken

Reviewed By: {jmagman}

This change reverts the following previous change:
This is a refactoring with no semantic changes.

This refactors the macOS framework creation code to be more readable, and extracts it to `sky_utils.py`.

While I was pulling this out, also generalised the code to not hardcode `FlutterMacOS.framework` in case we one day manage to generate the iOS and macOS frameworks with the same name.

This is a reland of #54546 (reverted in #54549), the original was reverted in order to revert #54543 (reverted in #54550), which was reverted because it failed to preserve symlinks while zipping the macOS framework. That patch has been relanded with a fix in #54555. This patch has been rebased to tip-of-tree for attempt two.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
@cbracken
Copy link
Member Author

Thanks. Ugh. These are incredibly painful to test.

@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 15, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 15, 2024
@engine-flutter-autoroll
7949ab379 macOS: Refactor create_macos_framework.py (flutter/engine#5…
23b902a 
…4557)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 15, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 15, 2024
@engine-flutter-autoroll
7949ab379 macOS: Refactor create_macos_framework.py (flutter/engine#5…
65b917d 
…4557)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 15, 2024
@engine-flutter-autoroll
Roll Flutter Engine from 76a1c64f4e63 to 971ddd9fe1bf (6 revisions) (#…
99f00a1 
…153487)

flutter/engine@76a1c64...971ddd9

2024-08-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "macOS: Refactor create_macos_framework.py (#54557)" (flutter/engine#54566)
2024-08-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] migrate blur tests to display list. (#54519)" (flutter/engine#54564)
2024-08-15 jonahwilliams@google.com [Impeller] migrate blur tests to display list. (flutter/engine#54519)
2024-08-14 skia-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from J1EXQ2ozMIQEa6F7H... to -XDS8eBA0Le-zCEfW... (flutter/engine#54561)
2024-08-14 chris@bracken.jp macOS: Refactor create_macos_framework.py (flutter/engine#54557)
2024-08-14 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from MeV5i7xXXFPHF5sBK... to qpfC-GAVZvOwAQU5f... (flutter/engine#54559)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from MeV5i7xXXFPH to qpfC-GAVZvOw

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 rmistry@google.com,zra@google.com 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
DBowen33 pushed a commit to DBowen33/flutter that referenced this pull request Aug 16, 2024
@engine-flutter-autoroll @DBowen33
Roll Flutter Engine from 76a1c64f4e63 to 971ddd9fe1bf (6 revisions) (f…
144a713 
…lutter#153487)

flutter/engine@76a1c64...971ddd9

2024-08-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "macOS: Refactor create_macos_framework.py (flutter#54557)" (flutter/engine#54566)
2024-08-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] migrate blur tests to display list. (flutter#54519)" (flutter/engine#54564)
2024-08-15 jonahwilliams@google.com [Impeller] migrate blur tests to display list. (flutter/engine#54519)
2024-08-14 skia-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from J1EXQ2ozMIQEa6F7H... to -XDS8eBA0Le-zCEfW... (flutter/engine#54561)
2024-08-14 chris@bracken.jp macOS: Refactor create_macos_framework.py (flutter/engine#54557)
2024-08-14 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from MeV5i7xXXFPHF5sBK... to qpfC-GAVZvOwAQU5f... (flutter/engine#54559)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from MeV5i7xXXFPH to qpfC-GAVZvOw

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 rmistry@google.com,zra@google.com 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
Roll Flutter Engine from 76a1c64f4e63 to 971ddd9fe1bf (6 revisions) (f…
dae0fe0 
…lutter#153487)

flutter/engine@76a1c64...971ddd9

2024-08-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "macOS: Refactor create_macos_framework.py (flutter#54557)" (flutter/engine#54566)
2024-08-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] migrate blur tests to display list. (flutter#54519)" (flutter/engine#54564)
2024-08-15 jonahwilliams@google.com [Impeller] migrate blur tests to display list. (flutter/engine#54519)
2024-08-14 skia-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from J1EXQ2ozMIQEa6F7H... to -XDS8eBA0Le-zCEfW... (flutter/engine#54561)
2024-08-14 chris@bracken.jp macOS: Refactor create_macos_framework.py (flutter/engine#54557)
2024-08-14 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from MeV5i7xXXFPHF5sBK... to qpfC-GAVZvOwAQU5f... (flutter/engine#54559)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from MeV5i7xXXFPH to qpfC-GAVZvOw

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 rmistry@google.com,zra@google.com 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 jmagman approved these changes

@zanderso zanderso Awaiting requested review from zanderso

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.
3 participants
@cbracken @zanderso @jmagman