这是indexloc提供的服务,不要输入任何密码
Skip to content

In xcode_backend.sh, build flutter assemble arguments as an array of quoted strings #86534

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

Merged
merged 1 commit into from
Jul 17, 2021
Merged

In xcode_backend.sh, build flutter assemble arguments as an array of quoted strings #86534

merged 1 commit into from
Jul 17, 2021

Conversation

zanderso
Copy link
Member

@zanderso zanderso commented Jul 16, 2021

Prior to this PR, some arguments such as --sksl-bundle-path are passed to flutter assemble without quotes such that a path with a space in it would be interpreted as two separate command line arguments, leading to various errors.

This PR assembles the argument array as a bash array of quoted strings, which makes it easier to conditionally add some arguments while ensuring that they are properly quoted.

This will help the benchmarks added in #86460 get closer to running.

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 16, 2021
@google-cla google-cla bot added the cla: yes label Jul 16, 2021
@zanderso zanderso changed the title Quote arguments to flutter assemble in xcode_backend.sh In xcode_backend.sh, build flutter assemble arguments as an array of quoted strings Jul 16, 2021
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.

Does this fail on master and pass on this PR if you change this:

to:

 '--split-debug-info=foo with space/',

Adding a --dart-define= with spaces would also be a way to test this (and expand coverage).

macOS has the same issue:
https://github.com/flutter/flutter/blob/master/packages/flutter_tools/bin/macos_assemble.sh
tested in:

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.

Huh I'm surprised nothing complained about checking in those app_icon_*.png binaries.

@zanderso
Copy link
Member Author

Huh I'm surprised nothing complained about checking in those app_icon_*.png binaries.

They're there in the ios/ subdir as well.

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.

Huh I'm surprised nothing complained about checking in those app_icon_*.png binaries.

They're there in the ios/ subdir as well.

I noticed that, I was surprised by that too!

The build test on the new macos project passed:

▌16:45:03▐ RUNNING: cd examples/hello_world; ../../bin/flutter build macos --debug -v

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket.appspot.com/8841508241899792080/+/u/run_test.dart_for_build_tests_shard_and_subshard_3_4/stdout

expect(result.exitCode, 0);
}, skip: true); // Extremely flaky due to https://github.com/flutter/flutter/issues/68144
Copy link
Member

Choose a reason for hiding this comment

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

👍

@zanderso zanderso merged commit 8642a9c into flutter:master Jul 17, 2021
@zanderso zanderso deleted the xcode-backend-spaces branch July 17, 2021 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

2 participants