Skip to content
This repository has been archived by the owner on Jun 13, 2024. It is now read-only.

Use new macOS build system #397

Merged
merged 11 commits into from
Dec 24, 2020
Merged

Use new macOS build system #397

merged 11 commits into from
Dec 24, 2020

Conversation

guidezpl
Copy link
Member

@guidezpl guidezpl commented Dec 22, 2020

All generated from flutter create . and a run, except for the window size, app icons, and PRODUCT_NAME.

Also remove extra web favicon

@@ -15,7 +15,7 @@
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "33CC10EC2044A3C60003C045"
BuildableName = "Flutter Gallery.app"
BuildableName = "gallery.app"
Copy link
Member Author

@guidezpl guidezpl Dec 22, 2020

Choose a reason for hiding this comment

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

When built, the app is named Flutter Gallery.app, per macos/Flutter/ephemeral/.app_filename and macos/Runner/Configs/AppInfo.xcconfig's PRODUCT_NAME. Should this also be?

Copy link
Member

@jmagman jmagman Dec 22, 2020

Choose a reason for hiding this comment

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

Do you know where the Flutter Gallery -> gallery change came from? When I flutter build macos it updated the project file in the same way, but doesn't touch the buildable name, which is set

PRODUCT_NAME = Flutter Gallery

Did you regenerate this file? It shouldn't have changed...

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 tried to reproduce and it fixed itself, my favorite kind of issue. Changing the PRODUCT_NAME correctly propagated to Runner.xcheme.

@@ -204,7 +193,7 @@
);
name = Runner;
productName = Runner;
productReference = 33CC10ED2044A3C60003C045 /* Flutter Gallery.app */;
productReference = 33CC10ED2044A3C60003C045 /* gallery.app */;
Copy link
Member Author

@guidezpl guidezpl Dec 22, 2020

Choose a reason for hiding this comment

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

Should comments use macos/Flutter/ephemeral/.app_filename/macos/Runner/Configs/AppInfo.xcconfig's PRODUCT_NAME?

Copy link
Member

Choose a reason for hiding this comment

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

No, I think in the comments Xcode actually hydrates the build settings, it thinks the name of the product is gallery.app.

@@ -13,7 +13,7 @@
</customObject>
<customObject id="-1" userLabel="First Responder" customClass="FirstResponder"/>
<customObject id="-3" userLabel="Application" customClass="NSObject"/>
<customObject id="Voe-Tx-rLC" customClass="AppDelegate" customModule="Flutter_Gallery" customModuleProvider="target">
<customObject id="Voe-Tx-rLC" customClass="AppDelegate" customModule="Runner" customModuleProvider="target">
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean to change this file? I don't know why the customModule changed from Flutter_Gallery to Runner or why the fullSizeContentView window style mask was removed.

Did you regenerate it, or did Xcode decide to touch the file when you opened it?

Copy link
Member Author

@guidezpl guidezpl Dec 23, 2020

Choose a reason for hiding this comment

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

This was a regeneration. I think we modified both at some point in the past so this seems fine

macos/Runner.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
end

require File.expand_path(File.join('packages', 'flutter_tools', 'bin', 'podhelper'), flutter_root)
Copy link
Member

Choose a reason for hiding this comment

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

What version of Flutter does the gallery need to run against? This will fail on anything less than flutter/flutter#72020 which is only in master.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, it runs against master

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.

LGTM, thanks for making these changes!

@guidezpl guidezpl merged commit b931265 into master Dec 24, 2020
@guidezpl guidezpl deleted the refactor branch December 24, 2020 10:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants