Default new project to the ios-signing-cert development team #90021

jmagman · 2021-09-14T02:15:36Z

Set DEVELOPMENT_TEAM Xcode build setting if the Developer Team is already saved in flutter config.

packages/flutter_tools/lib/src/ios/code_signing.dart

christopherfujino · 2021-09-21T18:05:14Z

packages/flutter_tools/lib/src/ios/code_signing.dart

+  required Terminal terminal,
+  bool shouldExitOnNoCerts = false,
+}) async {
+  if (!platform.isMacOS) {


should this be an exception?

Or not...

[plugin_dependencies_test] [STDOUT] Executing: /b/s/w/ir/x/w/recipe_cleanup/tmpbKETFw/flutter sdk/bin/flutter create --org io.flutter.devicelab.plugin_a --template=plugin --platforms=android,ios /b/s/w/ir/x/t/flutter_plugin_dependencies.FXIOVA/plugin_a in /b/s/w/ir/x/t/flutter_plugin_dependencies.FXIOVA with environment {BOT: true, LANG: en_US.UTF-8} [plugin_dependencies_test] [STDOUT] stderr: [plugin_dependencies_test] [STDOUT] stderr: Exception: Codesigning identity lookup is only supported on macOS.

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8834084345731287777/+/u/run_plugin_dependencies_test/test_stdout

Let's keep this null, flutter create follows this code path on non-macOS as well, you should be able to create an iOS app on any platform (and presumably check it in and later test on macOS).

packages/flutter_tools/test/commands.shard/permeable/build_apk_test.dart

cyanglaz

LGTM % some nits

cyanglaz · 2021-09-21T21:46:21Z

packages/flutter_tools/lib/src/commands/create.dart

@@ -231,6 +232,17 @@ class CreateCommand extends CreateBase {
    }

    final String dartSdk = globals.cache.dartSdkBuild;
+    final bool includeIos = featureFlags.isIOSEnabled && platforms.contains('ios');
+    String developmentTeam;


How about init it with '', so later, we only need to check if (iosDevelopmentTeam.isNotEmpty), it would also fit better with NNBD, and make the migration easier.

And nits: maybe iosDevelopmentTeam would be a better name as it was only used for iOS.

I'd prefer if we left it as uninitialized, so we can differentiate between explicitly set as an empty string and never set.

Is there a specific meaning of development team explicitly set as empty string? @jmagman Might be able to answer that. I was looking at the line below 'iosDevelopmentTeam': iosDevelopmentTeam ?? '', thinking that they should be the same.

In terms of build settings, '' overrides the project setting and sets it to nothing. Null should inherit from the project setting.

And generally, null means uninitialized, empty string means set to something, and that thing is the empty string. We shouldn't have the empty string mean "not set" in some places and not others.

cyanglaz · 2021-09-21T21:49:24Z

packages/flutter_tools/lib/src/commands/create_base.dart

@@ -375,6 +376,8 @@ abstract class CreateBase extends FlutterCommand {
      'withPluginHook': withPluginHook,
      'androidLanguage': androidLanguage,
      'iosLanguage': iosLanguage,
+      'hasIosDevelopmentTeam': iosDevelopmentTeam != null && iosDevelopmentTeam.isNotEmpty,


Can just be 'hasIosDevelopmentTeam': iosDevelopmentTeam.isNotEmpty if taking my suggestion above.

cyanglaz · 2021-09-21T21:49:45Z

packages/flutter_tools/lib/src/commands/create_base.dart

@@ -375,6 +376,8 @@ abstract class CreateBase extends FlutterCommand {
      'withPluginHook': withPluginHook,
      'androidLanguage': androidLanguage,
      'iosLanguage': iosLanguage,
+      'hasIosDevelopmentTeam': iosDevelopmentTeam != null && iosDevelopmentTeam.isNotEmpty,
+      'iosDevelopmentTeam': iosDevelopmentTeam ?? '',


Can just be 'iosDevelopmentTeam': iosDevelopmentTeam, if taking my suggestion above.

packages/flutter_tools/lib/src/ios/code_signing.dart

cyanglaz · 2021-09-21T21:57:45Z

...ter_tools/templates/module/ios/host_app_ephemeral/Runner.xcodeproj.tmpl/project.pbxproj.tmpl

@@ -437,6 +443,9 @@
 			buildSettings = {
 				ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon;
 				CURRENT_PROJECT_VERSION = "$(FLUTTER_BUILD_NUMBER)";
+{{#hasIosDevelopmentTeam}}
+				DEVELOPMENT_TEAM = {{iosDevelopmentTeam}};


Just curious, what happens for DEVELOPMENT_TEAM = "";, would it be cleaner that DEVELOPMENT_TEAM is set to an empty string if the developer didn't set it?

If DEVELOPMENT_TEAM = ""; then the user could set it at the project level (say they have multiple executable targets, like notification extensions, etc) but it would be set to nothing at the target level. If we don't have something good to put here we should leave it off.

cyanglaz · 2021-10-06T20:56:48Z

@jmagman Do you think you will land this soon? If so I can start the project-name work after you land this so I can directly use the new flags etc.

jmagman · 2021-10-06T21:37:30Z

@jmagman Do you think you will land this soon? If so I can start the project-name work after you land this so I can directly use the new flags etc.

Sorry, this fell down the list a bit. I will work on this today.

cyanglaz · 2021-10-06T21:42:21Z

@jmagman Do you think you will land this soon? If so I can start the project-name work after you land this so I can directly use the new flags etc.

Sorry, this fell down the list a bit. I will work on this today.

Awesome, then I'll hold off coding the project-name thing until we land this one so I can directly use your code :)

christopherfujino

LGTM

…#90021)

jmagman added platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. t: xcode "xcodebuild" on iOS and general Xcode project management labels Sep 14, 2021

jmagman self-assigned this Sep 14, 2021

google-cla bot added the cla: yes label Sep 14, 2021

jmagman marked this pull request as draft September 14, 2021 22:51

jmagman force-pushed the developer-team branch 2 times, most recently from 8ae5e5c to 8966162 Compare September 15, 2021 02:15

jmagman commented Sep 15, 2021

View reviewed changes

packages/flutter_tools/lib/src/ios/code_signing.dart Show resolved Hide resolved

jmagman force-pushed the developer-team branch 2 times, most recently from f15c3f1 to c54464b Compare September 16, 2021 00:47

jmagman requested review from christopherfujino and cyanglaz September 16, 2021 01:29

jmagman marked this pull request as ready for review September 16, 2021 01:29

christopherfujino reviewed Sep 21, 2021

View reviewed changes

packages/flutter_tools/lib/src/ios/code_signing.dart Show resolved Hide resolved

christopherfujino reviewed Sep 21, 2021

View reviewed changes

packages/flutter_tools/test/commands.shard/permeable/build_apk_test.dart Show resolved Hide resolved

cyanglaz approved these changes Sep 21, 2021

View reviewed changes

Default new project to the ios-signing-cert development team

e8b9654

const build setting key

51f449d

jmagman force-pushed the developer-team branch from c54464b to 51f449d Compare October 6, 2021 21:51

jmagman added 2 commits October 6, 2021 15:00

Throw non-macOS exception

3f12812

Revert exception

e0220d2

christopherfujino approved these changes Oct 7, 2021

View reviewed changes

jmagman added the waiting for tree to go green label Oct 7, 2021

fluttergithubbot merged commit 91d1e3e into flutter:master Oct 7, 2021

jmagman deleted the developer-team branch October 7, 2021 17:24

jmagman mentioned this pull request Oct 29, 2021

Running "flutter pub get" in a module app prints "Found saved certificate choice" and Apple cert info twice #92701

Open

clocksmith pushed a commit to clocksmith/flutter that referenced this pull request Oct 29, 2021

Default new project to the ios-signing-cert development team (flutter…

5b143c8

…#90021)

jmagman mentioned this pull request May 3, 2022

Running "flutter create -t module" prints "Found saved certificate choice" and Apple cert info twice #102950

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Default new project to the ios-signing-cert development team #90021

Default new project to the ios-signing-cert development team #90021

jmagman commented Sep 14, 2021

christopherfujino Sep 21, 2021

jmagman Oct 6, 2021

jmagman Oct 6, 2021

cyanglaz left a comment

cyanglaz Sep 21, 2021

christopherfujino Sep 22, 2021

cyanglaz Sep 22, 2021

jmagman Oct 6, 2021

jmagman Oct 6, 2021

cyanglaz Sep 21, 2021

cyanglaz Sep 21, 2021

cyanglaz Sep 21, 2021

jmagman Oct 6, 2021

cyanglaz commented Oct 6, 2021

jmagman commented Oct 6, 2021

cyanglaz commented Oct 6, 2021

christopherfujino left a comment

Default new project to the ios-signing-cert development team #90021

Default new project to the ios-signing-cert development team #90021

Conversation

jmagman commented Sep 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyanglaz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyanglaz commented Oct 6, 2021

jmagman commented Oct 6, 2021

cyanglaz commented Oct 6, 2021

christopherfujino left a comment

Choose a reason for hiding this comment