[flutter_tools] Port xcode backend to dart #86753

christopherfujino · 2021-07-21T01:29:08Z

fixes #84382

Manually ran flavors_test_ios and it passes.

…ended on

packages/flutter_tools/bin/xcode_backend.dart

packages/flutter_tools/test/general.shard/xcode_backend_test.dart

packages/flutter_tools/bin/xcode_backend.dart

jmagman · 2021-07-22T20:28:14Z

The done from the pipe isn't being suppressed anymore:

Running Xcode build...                                                  
done └─Compiling, linking and signing...                     5.3s

Should be:

Running Xcode build...                                                  
 └─Compiling, linking and signing...                         3.2s

jmagman · 2021-07-22T20:35:55Z

There seems to be additional newlines in Xcode's log:
Before:

This PR:

packages/flutter_tools/bin/xcode_backend.dart

christopherfujino · 2021-07-23T00:20:23Z

The done from the pipe isn't being suppressed anymore:

Running Xcode build...                                                  
done └─Compiling, linking and signing...                     5.3s

Should be:

Running Xcode build...                                                  
 └─Compiling, linking and signing...                         3.2s

fixed with unit test coverage

christopherfujino · 2021-07-23T18:54:08Z

There seems to be additional newlines in Xcode's log:
Before:

This PR:

I fixed this by changing print() -> stdio.write() in the echo() method. However, I'm pretty sure echoError() is supposed to append a newline char(?)

christopherfujino · 2021-07-23T19:04:44Z

@jmagman I addressed all your comments, and ran flavors_test_ios locally and it passed. please take another look.

christopherfujino · 2021-07-23T19:40:17Z

packages/flutter_tools/bin/xcode_backend.dart

+    );
+
+    if (result.exitCode != 0) {
+      echo(result.stdout as String);


As far as I can tell, the only behavioral difference after this change are these additional echoes before exiting if flutter assemble failed.

Does this satisfy:

Would be really awesome also to get the logging piped from assemble back into the Xcode logs--right now it just gets eaten and you get an exit code in Xcode and you don't know why it failed.

#84382 (comment)

It's outputting a bunch of newlines when assemble fails, which is unexpected:

To reproduce I changed

flutter/packages/flutter_tools/lib/src/build_system/targets/ios.dart

Lines 224 to 226 in 0de6bd4

if (environment.defines[kSdkRoot] == null) {

throw MissingDefineException(kSdkRoot, name);

}

to instead throw:

if (environment.defines[kSdkRoot] != null) { throw MissingDefineException(kSdkRoot, name); }

Would be really awesome also to get the logging piped from assemble back into the Xcode logs

I would hope to see "Target debug_universal_framework required define SdkRoot but it was not provided" in the Xcode logs or flutter build ios output. But I think feature parity is good enough for this PR.

I removed the extraneous newlines. Not seeing assemble failures (except in verbose mode) will have to be fixed in the assemble command.

christopherfujino · 2021-07-26T22:46:18Z

friendly ping @jmagman

jmagman

LGTM, thank you so much for tackling this!

christopherfujino added 2 commits July 20, 2021 13:02

wip

dfc1d12

wip more refactor

5e26674

flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 21, 2021

google-cla bot added the cla: yes label Jul 21, 2021

christopherfujino added 16 commits July 20, 2021 21:34

fix bugs

c87eb74

cleanup

5618c8e

fix no SOURCE_ROOT env var causing failure

31a7bb6

fix test failing from stray newline

a2c3b47

fix typo

c8b821e

fix wrong comparison on exitcode of plutil

7f178ea

fix wrong bool comparison, plus add in echo that integration test dep…

69ec48c

…ended on

add xcode_backend.dart to allowlist of forbidden_imports_test

e114a45

remove extra quotes

a7460a3

remove unused code

e44b1f6

add missing call to addObservatoryBonjourService()

9ff8a3c

cleanup

c83c460

remove commented out bash

e304ad9

add wip unit tests

5dc63e1

finish test

11b8e33

clean up comments

afc7e4b

christopherfujino marked this pull request as ready for review July 22, 2021 01:23

christopherfujino requested a review from jmagman July 22, 2021 01:30

christopherfujino changed the title ~~Refactor xcode backend~~ [flutter_tools] Port xcode backend to dart Jul 22, 2021

jmagman reviewed Jul 22, 2021

View reviewed changes

packages/flutter_tools/bin/xcode_backend.dart Outdated Show resolved Hide resolved

christopherfujino added 3 commits July 22, 2021 16:33

fix typo in dCodesignIdentity flag

11bb018

update test

7c8f603

fix stray "done" print

2760a44

christopherfujino added 4 commits July 22, 2021 17:23

update test to have space in arches var

a718c5e

fix helpful message not printed to stderr when no build mode set

222b935

prefer writeln over write + \n

61fd441

fix xcode echo output

5c51c37

code review update

f570ae1

log output of flutter assemble if it fails

ed22bd0

christopherfujino commented Jul 23, 2021

View reviewed changes

christopherfujino added 2 commits July 23, 2021 17:04

clean up stray newlines

a4a4f6c

remove obsolete test

197070c

jmagman approved these changes Jul 27, 2021

View reviewed changes

christopherfujino added the waiting for tree to go green label Jul 27, 2021

fluttergithubbot merged commit 2d07436 into flutter:master Jul 27, 2021

christopherfujino deleted the refactor-xcode-backend branch July 29, 2021 00:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flutter_tools] Port xcode backend to dart #86753

[flutter_tools] Port xcode backend to dart #86753

Uh oh!

christopherfujino commented Jul 21, 2021 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jmagman commented Jul 22, 2021 •

edited

Loading

Uh oh!

jmagman commented Jul 22, 2021

Uh oh!

Uh oh!

christopherfujino commented Jul 23, 2021

Uh oh!

christopherfujino commented Jul 23, 2021

Uh oh!

christopherfujino commented Jul 23, 2021

Uh oh!

christopherfujino Jul 23, 2021 •

edited

Loading

Uh oh!

jmagman Jul 23, 2021

Uh oh!

christopherfujino Jul 24, 2021

Uh oh!

christopherfujino commented Jul 26, 2021

Uh oh!

jmagman left a comment

Uh oh!

Uh oh!

	if (environment.defines[kSdkRoot] == null) {
	throw MissingDefineException(kSdkRoot, name);
	}

[flutter_tools] Port xcode backend to dart #86753

[flutter_tools] Port xcode backend to dart #86753

Uh oh!

Conversation

christopherfujino commented Jul 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jmagman commented Jul 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmagman commented Jul 22, 2021

Uh oh!

Uh oh!

christopherfujino commented Jul 23, 2021

Uh oh!

christopherfujino commented Jul 23, 2021

Uh oh!

christopherfujino commented Jul 23, 2021

Uh oh!

christopherfujino Jul 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmagman Jul 23, 2021

Choose a reason for hiding this comment

Uh oh!

christopherfujino Jul 24, 2021

Choose a reason for hiding this comment

Uh oh!

christopherfujino commented Jul 26, 2021

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

christopherfujino commented Jul 21, 2021 •

edited

Loading

jmagman commented Jul 22, 2021 •

edited

Loading

christopherfujino Jul 23, 2021 •

edited

Loading