Skip to content

Conversation

@cyanglaz
Copy link
Contributor

@cyanglaz cyanglaz commented Mar 25, 2022

Always log XCResult first so it won't get lost if there are other issues.

Also updated the order issues are detected.
Some sample error messages after the change are:

  1. no provisioning profile:
Error (Xcode): "Runner" requires a provisioning profile. Select a provisioning profile in the Signing & Capabilities editor.


════════════════════════════════════════════════════════════════════════════════
No Provisioning Profile was found for your project's Bundle Identifier or your 
device. You can create a new Provisioning Profile for your project in Xcode for 
your team by:
  1- Open the Flutter project's Xcode target with
       open ios/Runner.xcworkspace
  2- Select the 'Runner' project in the navigator then the 'Runner' target
     in the project settings
  3- Make sure a 'Development Team' is selected under Signing & Capabilities > Team. 
     You may need to:
         - Log in with your Apple ID in Xcode first
         - Ensure you have a valid unique Bundle ID
         - Register your device with your Apple Developer Account
         - Let Xcode automatically provision a profile for your app
  4- Build or run your project again

It's also possible that a previously installed app with the same Bundle 
Identifier was signed with a different certificate.

For more information, please visit:
  https://flutter.dev/docs/get-started/install/macos#deploy-to-ios-devices

Or run on an iOS simulator without code signing
════════════════════════════════════════════════════════════════════════════════
  1. no development team
Error (Xcode): Signing for "Runner" requires a development team. Select a development team in the Signing & Capabilities editor.


════════════════════════════════════════════════════════════════════════════════
Building a deployable iOS app requires a selected Development Team with a 
Provisioning Profile. Please ensure that a Development Team is selected by:
  1- Open the Flutter project's Xcode target with
       open ios/Runner.xcworkspace
  2- Select the 'Runner' project in the navigator then the 'Runner' target
     in the project settings
  3- Make sure a 'Development Team' is selected under Signing & Capabilities > Team. 
     You may need to:
         - Log in with your Apple ID in Xcode first
         - Ensure you have a valid unique Bundle ID
         - Register your device with your Apple Developer Account
         - Let Xcode automatically provision a profile for your app
  4- Build or run your project again

For more information, please visit:
  https://flutter.dev/docs/get-started/install/macos#deploy-to-ios-devices

Or run on an iOS simulator without code signing
════════════════════════════════════════════════════════════════════════════════

It appears that your application still contains the default signing identifier.
Try replacing 'com.example' with your signing id in Xcode:
  open ios/Runner.xcworkspace
Encountered error while building for device.

Fixes #100723

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Mar 25, 2022
@cyanglaz cyanglaz requested a review from jmagman March 25, 2022 18:55
Copy link
Member

Choose a reason for hiding this comment

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

What happens when there's provisioning profile in the error? Looks like _handleXCResultIssue and the noProvisioningProfileInstruction check below both output?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean? The _handleXCResultIssue has a special case to handles general provisioning profile issues and have some special logs for it. It doesn't print the noProvisioningProfileInstruction. The later check (result.stdout?.contains("Xcode couldn't find a provisioning profile matching") would print an instruction for user if they don't have a provisioning profile in the project.

Did you mean we can add the special "no provisioning profile" case check in _handleXCResultIssue?

Copy link
Member

Choose a reason for hiding this comment

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

Before your change, if there was a provisioning profile issue it would print this and return:

if (xcodeBuildExecution != null
&& xcodeBuildExecution.environmentType == EnvironmentType.physical
&& (result.stdout?.contains('BCEROR') ?? false)
// May need updating if Xcode changes its outputs.
&& (result.stdout?.contains("Xcode couldn't find a provisioning profile matching") ?? false)) {
logger.printError(noProvisioningProfileInstruction, emphasis: true);
return;
}

Now, it prints
if (issue.message != null && issue.message!.toLowerCase().contains('provisioning profile')) {
logger.printError('');
logger.printError('It appears that there was a problem signing your application prior to installation on the device.');
logger.printError('');
logger.printError('Verify that the Bundle Identifier in your project is your signing id in Xcode');
logger.printError(' open ios/Runner.xcworkspace');
logger.printError('');
logger.printError("Also try selecting 'Product > Build' to fix the problem:");
}

and then the above. Unless I'm missing something.

What does that text together look like? Probably needs to be adjusted, or the one only printed if the xcresult wasn't created?

Copy link
Contributor Author

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

@jmagman Updated per our offline discussion. PTAL.
I will update the PR description to reflect all the changes when the general idea of the PR is agreed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The noDevelopmentTeamInstruction is documented as a fallback message for signing issues. I do think it is a more proper user message + instructions. Let me know if you think otherwise. @jmagman

@jmagman
Copy link
Member

jmagman commented Mar 29, 2022

Looks like there are some test failures

00:36 +1153 -1: test/general.shard/ios/mac_test.dart: Diagnose Xcode build failure No development team shows message [E]                                                                               
  Expected: contains 'Building a deployable iOS app requires a selected Development Team with a \n'
              'Provisioning Profile.'
    Actual: ''
  
  package:test_api                            expect
  test/general.shard/ios/mac_test.dart 317:7  main.<fn>.<fn>

Copy link
Member

Choose a reason for hiding this comment

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

This seems like a better situation to use noDevelopmentTeamInstruction instead of message.toLowerCase().contains('provisioning profile') since it actually checks if you have DEVELOPMENT_TEAM or PROVISIONING_PROFILE set. Can we pipe the xcodeBuildExecution into the xcresult handler to do similar checking?

Copy link
Contributor Author

@cyanglaz cyanglaz Mar 30, 2022

Choose a reason for hiding this comment

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

This is technically not part of handing xcresult. XCResult does detect this error by showing a message: Showing Recent Messages Signing for "Runner" requires a development team. Select a development team in the Signing & Capabilities editor.

How about move this part where we directly check the flags all the way above xcresult handling?

So the flow becomes:

  1. Check DEVELOPMENT_TEAM and PROVISIONING_PROFILE flags.
  2. Handle xcresults
  3. Handle other issues with stdout.

Copy link
Member

@jmagman jmagman Mar 30, 2022

Choose a reason for hiding this comment

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

This is technically not part of handing xcresult

I know, I meant, maybe it should be, the xcresult can print noDevelopmentTeamInstruction if, instead of just message.toLowerCase().contains('provisioning profile') it also checks the build settings?

  1. Check DEVELOPMENT_TEAM and PROVISIONING_PROFILE flags.
  2. Handle xcresults
  3. Handle other issues with stdout.

I'm not following this, why would this one example be a special case where it's handled before the message is parsed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not following this, why would this one example be a special case where it's handled before the message is parsed?

So the "Check DEVELOPMENT_TEAM and PROVISIONING_PROFILE flag" can be done regardless if xcresult's parsing is successful or not. If this check is part of xcresult handling, it will not be run when xcresult is failed to parse. Or we do the same thing in xcresult handling as well as the fallback error handling when xcresult is failed to parse?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how you want to organize it 🙂 We should only print out the same error once, and include useful recovery suggestions where possible. There's currently fallback to string parsing if the result bundle fails to build, but maybe that never happens (or we should add Usage data about it to see).

@jmagman
Copy link
Member

jmagman commented Mar 30, 2022

00:23 +142 -1: test/commands.shard/hermetic/build_ipa_test.dart: Extra error message for provision profile issue in xcresulb bundle. [E]                                                               
  Expected: contains 'It appears that there was a problem signing your application prior to installation on the device.'
    Actual: 'Action Required: You must set a build name and number in the pubspec.yaml file version field before submitting to the App Store.\n'
              'Semantic Issue (Xcode): Some Provisioning profile issue.\n'
              '\n'
              '════════════════════════════════════════════════════════════════════════════════\n'
              'Building a deployable iOS app requires a selected Development Team with a \n'
              'Provisioning Profile. Please ensure that a Development Team is selected by:\n'
              '  1- Open the Flutter project\'s Xcode target with\n'
              '       open ios/Runner.xcworkspace\n'
              '  2- Select the \'Runner\' project in the navigator then the \'Runner\' target\n'
              '     in the project settings\n'
              '  3- Make sure a \'Development Team\' is selected under Signing & Capabilities > Team. \n'
              '     You may need to:\n'
              '         - Log in with your Apple ID in Xcode first\n'
              '         - Ensure you have a valid unique Bundle ID\n'
              '         - Register your device with your Apple Developer Account\n'
              '         - Let Xcode automatically provision a profile for your app\n'
              '  4- Build or run your project again\n'
              '\n'
              'For more information, please visit:\n'
              '  [https://flutter.dev/docs/get-started/install/macos#deploy-to-ios-devices\n](https://flutter.dev/docs/get-started/install/macos#deploy-to-ios-devices%5cn)'
              '\n'
              'Or run on an iOS simulator without code signing\n'
              '════════════════════════════════════════════════════════════════════════════════\n'
              ''

@cyanglaz cyanglaz force-pushed the xcresult_print_early branch from 492cc2d to a70903c Compare April 7, 2022 00:26
@cyanglaz cyanglaz requested a review from jmagman April 7, 2022 00:54
@jmagman
Copy link
Member

jmagman commented Apr 7, 2022

  1. no development team
    It appears that your application still contains the default signing identifier.
    Try replacing 'com.example' with your signing id in Xcode:
    open ios/Runner.xcworkspace
    Encountered error while building for device.

It appears that your application still contains the default signing identifier.
Try replacing 'com.example' with your signing id in Xcode:
open ios/Runner.xcworkspace
Encountered error while building for device.

This is twice in the description. Is that a typo or did it actually print twice?

@cyanglaz
Copy link
Contributor Author

cyanglaz commented Apr 7, 2022

  1. no development team
    It appears that your application still contains the default signing identifier.
    Try replacing 'com.example' with your signing id in Xcode:
    open ios/Runner.xcworkspace
    Encountered error while building for device.

It appears that your application still contains the default signing identifier.
Try replacing 'com.example' with your signing id in Xcode:
open ios/Runner.xcworkspace
Encountered error while building for device.

This is twice in the description. Is that a typo or did it actually print twice?

It was a typo

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 w/ minor nit

Comment on lines 711 to 716
Copy link
Member

Choose a reason for hiding this comment

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

If you make the local before the null check you won't need the !

Suggested change
if (issue.message == null) {
return _XCResultIssueHandlingResult(requiresProvisioningProfile: false, hasProvisioningProfileIssue: false);
}
// Add more error messages for flutter users for some special errors.
final String message = issue.message!;
// Add more error messages for flutter users for some special errors.
final String? message = issue.message;
if (issue.message == null) {
return _XCResultIssueHandlingResult(requiresProvisioningProfile: false, hasProvisioningProfileIssue: false);
}

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Linux tool_tests_commands has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux web_long_running_tests_4_5 has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Mac build_tests_4_4 has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Mac tool_tests_commands has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Windows tool_tests_commands has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite tool_tests-commands-linux has failed. Please fix the issues identified (or deflake) before re-applying this label.

cleanup

cleanup

re-arrange issues

fix old tests

some review fixes

re-arrange messages

tests

remove unnecessary imort

fix warnigns

fix nits
@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Mac module_test_ios has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

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.

Xcode build failures are not printed when codesigning is off

3 participants