Skip to content

Case insensitive check flavor names against Xcode schemes #61140

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.

Sign up for GitHub

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

Jump to bottom
Merged
jmagman merged 1 commit into from
Jul 9, 2020
Merged

Case insensitive check flavor names against Xcode schemes #61140

jmagman merged 1 commit into from
Jul 9, 2020

Conversation

@jmagman
Copy link
Member

@jmagman jmagman commented Jul 9, 2020 •
edited
Loading

Description

Check --flavor parameter against Xcode schemes case insensitively. Exit earlier if the scheme names don't match.
Remove dead "make editable" code while we're here.

Related Issues

Fixes #59029.

Tests

  • Updated flavor integration test to have different cased flavor/scheme names.
  • project_tests
  • xcodeproj_tests

Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

  • No, no existing tests failed, so this is not a breaking change.
  • Yes, this is a breaking change. If not, delete the remainder of this section.

@jmagman jmagman added platform-ios iOS applications specifically t: xcode "xcodebuild" on iOS and general Xcode project management labels Jul 9, 2020
@jmagman jmagman self-assigned this Jul 9, 2020
@fluttergithubbot fluttergithubbot added c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. work in progress; do not review labels Jul 9, 2020
jmagman
jmagman commented Jul 9, 2020
View reviewed changes
dev/integration_tests/flavors/ios/Runner.xcodeproj/xcshareddata/xcschemes/free.xcscheme Outdated
Copy link
Member Author

@jmagman jmagman Jul 9, 2020

Choose a reason for hiding this comment

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

Xcode 12 didn't like this and removed it.

Copy link
Contributor

@jonahwilliams jonahwilliams Jul 9, 2020

Choose a reason for hiding this comment

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

rip

jmagman
jmagman commented Jul 9, 2020
View reviewed changes
packages/flutter_tools/lib/src/project.dart
}
}

Future<void> makeHostAppEditable() async {
Copy link
Member Author

@jmagman jmagman Jul 9, 2020

Choose a reason for hiding this comment

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

☠️ ded

Copy link
Contributor

@jonahwilliams jonahwilliams Jul 9, 2020

Choose a reason for hiding this comment

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

rip

jmagman
jmagman commented Jul 9, 2020
View reviewed changes
packages/flutter_tools/lib/src/project.dart
) || globals.cache.isOlderThanToolsStamp(ephemeralDirectory);
}

Future<void> makeHostAppEditable() async {
Copy link
Member Author

@jmagman jmagman Jul 9, 2020

Choose a reason for hiding this comment

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

moar ded

Copy link
Contributor

@jonahwilliams jonahwilliams Jul 9, 2020

Choose a reason for hiding this comment

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

rip

jmagman
jmagman commented Jul 9, 2020
View reviewed changes
packages/flutter_tools/test/general.shard/project_test.dart
});
});

group('editable Android host app', () {
Copy link
Member Author

@jmagman jmagman Jul 9, 2020

Choose a reason for hiding this comment

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

Remove dead code tests

Copy link
Contributor

@jonahwilliams jonahwilliams Jul 9, 2020

Choose a reason for hiding this comment

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

rip

@jmagman jmagman marked this pull request as ready for review July 9, 2020 23:25
jmagman
jmagman commented Jul 9, 2020
View reviewed changes
packages/flutter_tools/lib/src/project.dart
Comment on lines +109 to +118
if (ios.existsSync())
await ios.productBundleIdentifier(null),
if (android.existsSync()) ...<String>[
android.applicationId,
android.group,
],
if (example.android.existsSync())
example.android.applicationId,
if (example.ios.existsSync())
await example.ios.productBundleIdentifier(null),
Copy link
Member Author

@jmagman jmagman Jul 9, 2020

Choose a reason for hiding this comment

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

This wasted time shelling out to tooling in nonexistent directories when creating a new project.

jonahwilliams
jonahwilliams reviewed Jul 9, 2020
View reviewed changes
packages/flutter_tools/test/general.shard/project_test.dart
expect(await project.ios.productBundleIdentifier(null), 'io.flutter.someProject.suffix');
});

testWithMocks('fails with no flavor and defined schemes', () async {
Copy link
Contributor

@jonahwilliams jonahwilliams Jul 9, 2020

Choose a reason for hiding this comment

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

TIL about testWithMocks

Copy link
Contributor

@jonahwilliams jonahwilliams Jul 9, 2020

Choose a reason for hiding this comment

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

Will add this to my deprecate/remove list :)

jonahwilliams
jonahwilliams reviewed Jul 9, 2020
View reviewed changes
packages/flutter_tools/test/general.shard/ios/xcodeproj_test.dart
);

expect(
() => defaultInfo.reportFlavorNotFoundAndExit(),
Copy link
Contributor

@jonahwilliams jonahwilliams Jul 9, 2020

Choose a reason for hiding this comment

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

technically, you could pass in defaultInfo.reportFlavorNotFoundAndExit as a tear-off, since the throws matchers just require a zero-arity function. Just an FYI

Copy link
Member Author

@jmagman jmagman Jul 9, 2020

Choose a reason for hiding this comment

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

Audited, got a PR coming your way.

jonahwilliams
jonahwilliams approved these changes Jul 9, 2020
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

@jmagman jmagman merged commit e110ca7 into flutter:master Jul 9, 2020
@jmagman jmagman deleted the flavor-name branch July 9, 2020 23:56
This was referenced Jul 9, 2020
Merged
Closed
pcsosinski pushed a commit to pcsosinski/flutter that referenced this pull request Aug 12, 2020
@jmagman
Case insensitive check flavor names against Xcode schemes (flutter#61140
b17a215 
)
pcsosinski pushed a commit that referenced this pull request Aug 13, 2020
@Piinks @jmagman
@rami-a @darrenaustin
1.20.2 Cherrypicks (#63591)
bbfbf17 
* Fix SliverList scrollOffsetCorrection 0 case (#62615)

* Case insensitive check flavor names against Xcode schemes (#61140)

* Address misc time picker design issues (#62803)

* Update to the latest localizations (#63026)

* Removed the inputFormatters from the text input fields used by the Date Pickers (#63461)

* Fix App.framework path in Podfile (#63412)

* Update engine hash to 1.20.2

Co-authored-by: Kate Lovett <[email protected]>
Co-authored-by: Jenn Magder <[email protected]>
Co-authored-by: Rami <[email protected]>
Co-authored-by: Shi-Hao Hong <[email protected]>
Co-authored-by: Darren Austin <[email protected]>
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@jmagman @mingwandroid
Case insensitive check flavor names against Xcode schemes (flutter#61140
685a0d2 
)
@jmagman jmagman mentioned this pull request Nov 14, 2020
Merged
@jmagman jmagman mentioned this pull request Jun 9, 2021
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@christopherfujino christopherfujino Awaiting requested review from christopherfujino

1 more reviewer

@jonahwilliams jonahwilliams jonahwilliams approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jmagman jmagman

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. platform-ios iOS applications specifically t: xcode "xcodebuild" on iOS and general Xcode project management tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[tool_crash] build ios release - ArgumentError: Invalid argument(s)

4 participants

@jmagman @jonahwilliams @googlebot @fluttergithubbot