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

MacOS framework was simplified to remove an internal zip. #35563

Merged
auto-submit merged 2 commits into from
Aug 20, 2022
Merged

MacOS framework was simplified to remove an internal zip. #35563

auto-submit merged 2 commits into from
Aug 20, 2022

Conversation

@godofredoc
Copy link
Contributor

@godofredoc godofredoc commented Aug 20, 2022

This PR is removing the nested zip file and removing all the boilerplate
GN used to generate the zip within the zip.

Bug: flutter/flutter#81855

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 Hixie said 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.

@godofredoc
MacOS framework was simplified to remove an internal zip.
8f1bd90 
This PR is removing the nested zip file and removing all the boilerplate
GN used to generate the zip within the zip.

Bug: flutter/flutter#81855
@godofredoc godofredoc requested review from jmagman and zanderso August 20, 2022 01:59
@godofredoc godofredoc added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 20, 2022
@auto-submit auto-submit bot merged commit 835ba6b into flutter:main Aug 20, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 20, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 20, 2022
@engine-flutter-autoroll
835ba6b MacOS framework was simplified to remove an internal zip. (fl…
9d3a0df 
…utter/engine#35563)
@jmagman
Copy link
Member

jmagman commented Aug 22, 2022 •
edited
Loading

This isn't correct and will break
https://github.com/flutter/flutter/blob/7f1a8f79489412ff992b8ba0aaecc386fe628557/packages/flutter_tools/lib/src/commands/build_macos_framework.dart#L167 and
https://github.com/flutter/flutter/blob/4b361ccea3d1d47f3ecc1decc8112766bd89821a/packages/flutter_tools/lib/src/cache.dart#L917

@godofredoc there are framework tests for this that I would have expected to fail on the roll, is the recipe still zipping the zip?

@jmagman jmagman mentioned this pull request Aug 22, 2022
Closed
@jmagman
Copy link
Member

jmagman commented Aug 22, 2022

Filed flutter/flutter#110034 to track the tool, recipe, and roll script work.

@zanderso
Copy link
Member

zanderso commented Aug 22, 2022

@godofredoc My understanding was that this change only affects the engine_v2 artifacts, and all of the engine_v2 artifacts will require some tooling migration. Is that correct, or does this affect our current artifacts, as well?

@jmagman
Copy link
Member

jmagman commented Aug 22, 2022 •
edited
Loading

@godofredoc confirmed this only changes the engine_v2. We can either ship both zipped and unzipped for incremental adoption as proposed in flutter/flutter#110034, or I can prep the tool work needed to be used when engine_v2 goes into effect, and the rollers can make the script changes when the engine_v2 roll in.

@jmagman jmagman mentioned this pull request Apr 18, 2023
Merged
8 tasks
@godofredoc godofredoc deleted the simplify_macos_archive branch January 12, 2024 19:48
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 platform-macos

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@godofredoc @jmagman @zanderso