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

Move embedding and linking macOS Flutter frameworks into the tool #71764

Merged
merged 2 commits into from Dec 8, 2020

Conversation

jmagman
Copy link
Member

@jmagman jmagman commented Dec 5, 2020

Description

macOS version of #51453. See details and discussion there.

Push linker and embedding logic into the tool so future changes (like distributing as an XCFramework, renaming FlutterMacOS.framework, etc) do not require changes in the user's project.

At some point FlutterMacOS.framework will ship with x86 and ARM slices. With this change, we will be able to introduce thinning (as iOS does) in the script without requiring a user project migration.

This change would also set us up to stop copying FlutterMacOS.framework to Flutter/ephemeral. On iOS, for example, the framework is copied directly from the artifacts directory to the BUILT_PRODUCTS_DIR so there's never a mismatch between a Release build and a Debug version of Flutter.

Changes

Future Documentation

Related Issues

Fixes #56581
Part of #70413
Will make #60113 easier.

Tests

macos_project_migration_test

@jmagman jmagman self-assigned this Dec 5, 2020
@flutter-dashboard flutter-dashboard bot added team Infra upgrades, team productivity, code health, technical debt. See also team: labels. tool Affects the "flutter" command-line tool. See also t: labels. labels Dec 5, 2020
@google-cla google-cla bot added the cla: yes label Dec 5, 2020
if [[ -n "$VERBOSE_SCRIPT_LOGGING" ]]; then
verbose_flag="--verbose"
fi
BuildApp() {
Copy link
Member Author

Choose a reason for hiding this comment

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

This logic is unchanged other than putting it in a function (so whitespace indentation) and adding locals.


# Adds the App.framework as an embedded binary and the flutter_assets as
# resources.
EmbedFrameworks() {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is new.

fi
}

# Main entry point.
Copy link
Member Author

Choose a reason for hiding this comment

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

Also new.

@@ -0,0 +1,102 @@
// Copyright 2014 The Flutter Authors. All rights reserved.
Copy link
Member Author

Choose a reason for hiding this comment

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

This file is almost the same as https://github.com/flutter/flutter/blob/e1598c96c072a6323e8dbf2a1724a896533a6c27/packages/flutter_tools/lib/src/ios/migrations/remove_framework_link_and_embedding_migration.dart with different identifiers, doesn't do anything with add-to-app, and leaves off any Xcode version checks.

label: 'failure', flutterUsage: _usage)
.send();
throwToolExit(
'Your Xcode project requires migration.');
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll write a doc like https://flutter.dev/docs/development/ios-project-migration and link it here in a future PR.

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM with nit. The Dart rework part is optional; if you go that route I can re-review that piece.

Thanks for tackling this; it's great to reduce delta relative to iOS, and this sets us up for some great improvements!

dev/manual_tests/macos/Runner.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
"embed")
EmbedFrameworks ;;
esac
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

This is non-trivial new bash code; any chance we could use this as the impetus to finally switch macOS over to tool_backend.* so that this could all be Dart code instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm converging macOS and iOS scripts, and I'd like to do https://github.com/flutter/flutter/pull/70224/files#diff-cf570f69a008f028e95cfad7690ee32b3acbbc88379d340ea08ef67c0a3aff69R297 for macOS next (copy the FlutterMacOS framework directly to BUILT_PRODUCTS_DIR instead of ephemeral).

I'd rather get them to be the same bash script first since I know the iOS variant works, then swap them both over to the same dart code. That way each PR is a good improvement that isn't doing too much and is easy to review, and if it causes issues won't be a big painful revert.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, a macOS version of #59044 needs to happen first because the BUILT_PRODUCTS_DIR update requires a Podfile change, but you get the idea.

@jmagman
Copy link
Member Author

jmagman commented Dec 8, 2020

Google testing failing because #71757 hasn't rolled yet.

@jmagman jmagman merged commit 70f8fde into flutter:master Dec 8, 2020
@jmagman jmagman deleted the macos-link-tool branch December 8, 2020 19:59
jmagman added a commit that referenced this pull request Dec 8, 2020
jmagman added a commit that referenced this pull request Dec 8, 2020
jmagman added a commit to jmagman/flutter that referenced this pull request Dec 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team Infra upgrades, team productivity, code health, technical debt. See also team: labels. tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

macOS xcode project should move Framework linking to tool
2 participants