Unverified Commit 3cfe3720 authored by Victoria Ashworth's avatar Victoria Ashworth Committed by GitHub

Wait for CONFIGURATION_BUILD_DIR to update when debugging with Xcode (#135444)

So there appears to be a race situation between the flutter CLI and Xcode. In the CLI, we update the `CONFIGURATION_BUILD_DIR` in the Xcode build settings and then tell Xcode to install, launch, and debug the app. When Xcode installs the app, it should use the `CONFIGURATION_BUILD_DIR` to find the bundle. However, it appears that sometimes Xcode hasn't processed the change to the build settings before the install happens, which causes it to not be able to find the bundle.

Fixes https://github.com/flutter/flutter/issues/135442

--- 

Since it's a timing issue, there's not really a consistent way to test it.

I was able to confirm that it works, though, by using the following steps:
1. Create a flutter project
2. Open the project in Xcode
3. `flutter clean`
4. `flutter run --profile -v`

If I saw a print line `stderr: CONFIGURATION_BUILD_DIR: build/Debug-iphoneos`, that means it first found the old and incorrect `CONFIGURATION_BUILD_DIR` before updating to the the new, so I was able to confirm that it would wait until it updated.
parent 936763f5
......@@ -61,6 +61,11 @@ class CommandArguments {
this.xcodePath = this.validatedStringArgument('--xcode-path', parsedArguments['--xcode-path']);
this.projectPath = this.validatedStringArgument('--project-path', parsedArguments['--project-path']);
this.projectName = this.validatedStringArgument('--project-name', parsedArguments['--project-name']);
this.expectedConfigurationBuildDir = this.validatedStringArgument(
'--expected-configuration-build-dir',
parsedArguments['--expected-configuration-build-dir'],
);
this.workspacePath = this.validatedStringArgument('--workspace-path', parsedArguments['--workspace-path']);
this.targetDestinationId = this.validatedStringArgument('--device-id', parsedArguments['--device-id']);
this.targetSchemeName = this.validatedStringArgument('--scheme', parsedArguments['--scheme']);
......@@ -92,42 +97,76 @@ class CommandArguments {
}
/**
* Validates the flag is allowed for the current command.
* Returns map of commands to map of allowed arguments. For each command, if
* an argument flag is a key, than that flag is allowed for that command. If
* the value for the key is true, then it is required for the command.
*
* @param {!string} flag
* @param {?string} value
* @returns {!bool}
* @throws Will throw an error if the flag is not allowed for the current
* command and the value is not null, undefined, or empty.
* @returns {!string} Map of commands to allowed and optionally required
* arguments.
*/
isArgumentAllowed(flag, value) {
const allowedArguments = {
'common': {
argumentSettings() {
return {
'check-workspace-opened': {
'--xcode-path': true,
'--project-path': true,
'--workspace-path': true,
'--verbose': true,
'--verbose': false,
},
'check-workspace-opened': {},
'debug': {
'--xcode-path': true,
'--project-path': true,
'--workspace-path': true,
'--project-name': true,
'--expected-configuration-build-dir': false,
'--device-id': true,
'--scheme': true,
'--skip-building': true,
'--launch-args': true,
'--verbose': false,
},
'stop': {
'--xcode-path': true,
'--project-path': true,
'--workspace-path': true,
'--close-window': true,
'--prompt-to-save': true,
'--verbose': false,
},
};
}
const isAllowed = allowedArguments['common'][flag] === true || allowedArguments[this.command][flag] === true;
/**
* Validates the flag is allowed for the current command.
*
* @param {!string} flag
* @param {?string} value
* @returns {!bool}
* @throws Will throw an error if the flag is not allowed for the current
* command and the value is not null, undefined, or empty.
*/
isArgumentAllowed(flag, value) {
const isAllowed = this.argumentSettings()[this.command].hasOwnProperty(flag);
if (isAllowed === false && (value != null && value !== '')) {
throw `The flag ${flag} is not allowed for the command ${this.command}.`;
}
return isAllowed;
}
/**
* Validates required flag has a value.
*
* @param {!string} flag
* @param {?string} value
* @throws Will throw an error if the flag is required for the current
* command and the value is not null, undefined, or empty.
*/
validateRequiredArgument(flag, value) {
const isRequired = this.argumentSettings()[this.command][flag] === true;
if (isRequired === true && (value == null || value === '')) {
throw `Missing value for ${flag}`;
}
}
/**
* Parses the command line arguments into an object.
*
......@@ -182,9 +221,7 @@ class CommandArguments {
if (this.isArgumentAllowed(flag, value) === false) {
return null;
}
if (value == null || value === '') {
throw `Missing value for ${flag}`;
}
this.validateRequiredArgument(flag, value);
return value;
}
......@@ -227,9 +264,7 @@ class CommandArguments {
if (this.isArgumentAllowed(flag, value) === false) {
return null;
}
if (value == null || value === '') {
throw `Missing value for ${flag}`;
}
this.validateRequiredArgument(flag, value);
try {
return JSON.parse(value);
} catch (e) {
......@@ -348,6 +383,15 @@ function debugApp(xcode, args) {
return new FunctionResult(null, destinationResult.error)
}
// If expectedConfigurationBuildDir is available, ensure that it matches the
// build settings.
if (args.expectedConfigurationBuildDir != null && args.expectedConfigurationBuildDir !== '') {
const updateResult = waitForConfigurationBuildDirToUpdate(targetWorkspace, args);
if (updateResult.error != null) {
return new FunctionResult(null, updateResult.error);
}
}
try {
// Documentation from the Xcode Script Editor dictionary indicates that the
// `debug` function has a parameter called `runDestinationSpecifier` which
......@@ -529,3 +573,92 @@ function stopApp(xcode, args) {
}
return new FunctionResult(null, null);
}
/**
* Gets resolved build setting for CONFIGURATION_BUILD_DIR and waits until its
* value matches the `--expected-configuration-build-dir` argument. Waits up to
* 2 minutes.
*
* @param {!WorkspaceDocument} targetWorkspace A `WorkspaceDocument` (Xcode Mac
* Scripting class).
* @param {!CommandArguments} args
* @returns {!FunctionResult} Always returns null as the `result`.
*/
function waitForConfigurationBuildDirToUpdate(targetWorkspace, args) {
// Get the project
let project;
try {
project = targetWorkspace.projects().find(x => x.name() == args.projectName);
} catch (e) {
return new FunctionResult(null, `Failed to find project ${args.projectName}: ${e}`);
}
if (project == null) {
return new FunctionResult(null, `Failed to find project ${args.projectName}.`);
}
// Get the target
let target;
try {
// The target is probably named the same as the project, but if not, just use the first.
const targets = project.targets();
target = targets.find(x => x.name() == args.projectName);
if (target == null && targets.length > 0) {
target = targets[0];
if (args.verbose) {
console.log(`Failed to find target named ${args.projectName}, picking first target: ${target.name()}.`);
}
}
} catch (e) {
return new FunctionResult(null, `Failed to find target: ${e}`);
}
if (target == null) {
return new FunctionResult(null, `Failed to find target.`);
}
try {
// Use the first build configuration (Debug). Any should do since they all
// include Generated.xcconfig.
const buildConfig = target.buildConfigurations()[0];
const buildSettings = buildConfig.resolvedBuildSettings().reverse();
// CONFIGURATION_BUILD_DIR is often at (reverse) index 225 for Xcode
// projects, so check there first. If it's not there, search the build
// settings (which can be a little slow).
const defaultIndex = 225;
let configurationBuildDirSettings;
if (buildSettings[defaultIndex] != null && buildSettings[defaultIndex].name() === 'CONFIGURATION_BUILD_DIR') {
configurationBuildDirSettings = buildSettings[defaultIndex];
} else {
configurationBuildDirSettings = buildSettings.find(x => x.name() === 'CONFIGURATION_BUILD_DIR');
}
if (configurationBuildDirSettings == null) {
// This should not happen, even if it's not set by Flutter, there should
// always be a resolved build setting for CONFIGURATION_BUILD_DIR.
return new FunctionResult(null, `Unable to find CONFIGURATION_BUILD_DIR.`);
}
// Wait up to 2 minutes for the CONFIGURATION_BUILD_DIR to update to the
// expected value.
const checkFrequencyInSeconds = 0.5;
const maxWaitInSeconds = 2 * 60; // 2 minutes
const verboseLogInterval = 10 * (1 / checkFrequencyInSeconds);
const iterations = maxWaitInSeconds * (1 / checkFrequencyInSeconds);
for (let i = 0; i < iterations; i++) {
const verbose = args.verbose && i % verboseLogInterval === 0;
const configurationBuildDir = configurationBuildDirSettings.value();
if (configurationBuildDir === args.expectedConfigurationBuildDir) {
console.log(`CONFIGURATION_BUILD_DIR: ${configurationBuildDir}`);
return new FunctionResult(null, null);
}
if (verbose) {
console.log(`Current CONFIGURATION_BUILD_DIR: ${configurationBuildDir} while expecting ${args.expectedConfigurationBuildDir}`);
}
delay(checkFrequencyInSeconds);
}
return new FunctionResult(null, 'Timed out waiting for CONFIGURATION_BUILD_DIR to update.');
} catch (e) {
return new FunctionResult(null, `Failed to get CONFIGURATION_BUILD_DIR: ${e}`);
}
}
......@@ -723,6 +723,18 @@ class IOSDevice extends Device {
return LaunchResult.failed();
} finally {
startAppStatus.stop();
if ((isCoreDevice || forceXcodeDebugWorkflow) && debuggingOptions.debuggingEnabled && package is BuildableIOSApp) {
// When debugging via Xcode, after the app launches, reset the Generated
// settings to not include the custom configuration build directory.
// This is to prevent confusion if the project is later ran via Xcode
// rather than the Flutter CLI.
await updateGeneratedXcodeProperties(
project: FlutterProject.current(),
buildInfo: debuggingOptions.buildInfo,
targetOverride: mainPath,
);
}
}
}
......@@ -868,6 +880,8 @@ class IOSDevice extends Device {
scheme: scheme,
xcodeProject: project.xcodeProject,
xcodeWorkspace: project.xcodeWorkspace!,
hostAppProjectName: project.hostAppProjectName,
expectedConfigurationBuildDir: bundle.parent.absolute.path,
verboseLogging: _logger.isVerbose,
);
} else {
......@@ -889,18 +903,6 @@ class IOSDevice extends Device {
shutdownHooks.addShutdownHook(() => _xcodeDebug.exit(force: true));
}
if (package is BuildableIOSApp) {
// After automating Xcode, reset the Generated settings to not include
// the custom configuration build directory. This is to prevent
// confusion if the project is later ran via Xcode rather than the
// Flutter CLI.
await updateGeneratedXcodeProperties(
project: flutterProject,
buildInfo: debuggingOptions.buildInfo,
targetOverride: mainPath,
);
}
return debugSuccess;
}
}
......
......@@ -85,6 +85,13 @@ class XcodeDebug {
project.xcodeProject.path,
'--workspace-path',
project.xcodeWorkspace.path,
'--project-name',
project.hostAppProjectName,
if (project.expectedConfigurationBuildDir != null)
...<String>[
'--expected-configuration-build-dir',
project.expectedConfigurationBuildDir!,
],
'--device-id',
deviceId,
'--scheme',
......@@ -310,6 +317,7 @@ class XcodeDebug {
_xcode.xcodeAppPath,
'-g', // Do not bring the application to the foreground.
'-j', // Launches the app hidden.
'-F', // Open "fresh", without restoring windows.
xcodeWorkspace.path
],
throwOnError: true,
......@@ -396,6 +404,7 @@ class XcodeDebug {
return XcodeDebugProject(
scheme: 'Runner',
hostAppProjectName: 'Runner',
xcodeProject: tempXcodeProject.childDirectory('Runner.xcodeproj'),
xcodeWorkspace: tempXcodeProject.childDirectory('Runner.xcworkspace'),
isTemporaryProject: true,
......@@ -470,6 +479,8 @@ class XcodeDebugProject {
required this.scheme,
required this.xcodeWorkspace,
required this.xcodeProject,
required this.hostAppProjectName,
this.expectedConfigurationBuildDir,
this.isTemporaryProject = false,
this.verboseLogging = false,
});
......@@ -477,6 +488,8 @@ class XcodeDebugProject {
final String scheme;
final Directory xcodeWorkspace;
final Directory xcodeProject;
final String hostAppProjectName;
final String? expectedConfigurationBuildDir;
final bool isTemporaryProject;
/// When [verboseLogging] is true, the xcode_debug.js script will log
......
......@@ -472,6 +472,7 @@ void main() {
scheme: 'Runner',
xcodeWorkspace: fileSystem.directory('/ios/Runner.xcworkspace'),
xcodeProject: fileSystem.directory('/ios/Runner.xcodeproj'),
hostAppProjectName: 'Runner',
),
expectedDeviceId: '123',
expectedLaunchArguments: <String>['--enable-dart-profiling'],
......@@ -534,6 +535,8 @@ void main() {
scheme: 'Runner',
xcodeWorkspace: fileSystem.directory('/ios/Runner.xcworkspace'),
xcodeProject: fileSystem.directory('/ios/Runner.xcodeproj'),
hostAppProjectName: 'Runner',
expectedConfigurationBuildDir: '/build/ios/iphoneos',
),
expectedDeviceId: '123',
expectedLaunchArguments: <String>['--enable-dart-profiling'],
......
......@@ -713,6 +713,7 @@ void main() {
scheme: 'Runner',
xcodeWorkspace: temporaryXcodeProjectDirectory.childDirectory('Runner.xcworkspace'),
xcodeProject: temporaryXcodeProjectDirectory.childDirectory('Runner.xcodeproj'),
hostAppProjectName: 'Runner',
),
expectedDeviceId: '123',
expectedLaunchArguments: <String>['--enable-dart-profiling'],
......@@ -757,6 +758,7 @@ void main() {
scheme: 'Runner',
xcodeWorkspace: temporaryXcodeProjectDirectory.childDirectory('Runner.xcworkspace'),
xcodeProject: temporaryXcodeProjectDirectory.childDirectory('Runner.xcodeproj'),
hostAppProjectName: 'Runner',
),
expectedDeviceId: '123',
expectedLaunchArguments: <String>['--enable-dart-profiling'],
......@@ -817,6 +819,7 @@ void main() {
scheme: 'Runner',
xcodeWorkspace: temporaryXcodeProjectDirectory.childDirectory('Runner.xcworkspace'),
xcodeProject: temporaryXcodeProjectDirectory.childDirectory('Runner.xcodeproj'),
hostAppProjectName: 'Runner',
),
expectedDeviceId: '123',
expectedLaunchArguments: <String>['--enable-dart-profiling'],
......@@ -869,6 +872,7 @@ void main() {
scheme: 'Runner',
xcodeWorkspace: temporaryXcodeProjectDirectory.childDirectory('Runner.xcworkspace'),
xcodeProject: temporaryXcodeProjectDirectory.childDirectory('Runner.xcodeproj'),
hostAppProjectName: 'Runner',
),
expectedDeviceId: '123',
expectedLaunchArguments: <String>['--enable-dart-profiling'],
......
......@@ -56,10 +56,11 @@ void main() {
scheme: 'Runner',
xcodeProject: xcodeproj,
xcodeWorkspace: xcworkspace,
hostAppProjectName: 'Runner',
);
});
testWithoutContext('succeeds in opening and debugging with launch options and verbose logging', () async {
testWithoutContext('succeeds in opening and debugging with launch options, expectedConfigurationBuildDir, and verbose logging', () async {
fakeProcessManager.addCommands(<FakeCommand>[
FakeCommand(
command: <String>[
......@@ -88,6 +89,7 @@ void main() {
pathToXcodeApp,
'-g',
'-j',
'-F',
xcworkspace.path
],
),
......@@ -105,6 +107,10 @@ void main() {
project.xcodeProject.path,
'--workspace-path',
project.xcodeWorkspace.path,
'--project-name',
project.hostAppProjectName,
'--expected-configuration-build-dir',
'/build/ios/iphoneos',
'--device-id',
deviceId,
'--scheme',
......@@ -131,6 +137,8 @@ void main() {
scheme: 'Runner',
xcodeProject: xcodeproj,
xcodeWorkspace: xcworkspace,
hostAppProjectName: 'Runner',
expectedConfigurationBuildDir: '/build/ios/iphoneos',
verboseLogging: true,
);
......@@ -150,7 +158,7 @@ void main() {
expect(status, true);
});
testWithoutContext('succeeds in opening and debugging without launch options and verbose logging', () async {
testWithoutContext('succeeds in opening and debugging without launch options, expectedConfigurationBuildDir, and verbose logging', () async {
fakeProcessManager.addCommands(<FakeCommand>[
FakeCommand(
command: <String>[
......@@ -178,6 +186,7 @@ void main() {
pathToXcodeApp,
'-g',
'-j',
'-F',
xcworkspace.path
],
),
......@@ -195,6 +204,8 @@ void main() {
project.xcodeProject.path,
'--workspace-path',
project.xcodeWorkspace.path,
'--project-name',
project.hostAppProjectName,
'--device-id',
deviceId,
'--scheme',
......@@ -257,6 +268,7 @@ void main() {
pathToXcodeApp,
'-g',
'-j',
'-F',
xcworkspace.path
],
exception: ProcessException(
......@@ -266,6 +278,7 @@ void main() {
'/non_existant_path',
'-g',
'-j',
'-F',
xcworkspace.path,
],
'The application /non_existant_path cannot be opened for an unexpected reason',
......@@ -332,6 +345,8 @@ void main() {
project.xcodeProject.path,
'--workspace-path',
project.xcodeWorkspace.path,
'--project-name',
project.hostAppProjectName,
'--device-id',
deviceId,
'--scheme',
......@@ -401,6 +416,8 @@ void main() {
project.xcodeProject.path,
'--workspace-path',
project.xcodeWorkspace.path,
'--project-name',
project.hostAppProjectName,
'--device-id',
deviceId,
'--scheme',
......@@ -474,6 +491,8 @@ void main() {
project.xcodeProject.path,
'--workspace-path',
project.xcodeWorkspace.path,
'--project-name',
project.hostAppProjectName,
'--device-id',
deviceId,
'--scheme',
......@@ -547,6 +566,8 @@ void main() {
project.xcodeProject.path,
'--workspace-path',
project.xcodeWorkspace.path,
'--project-name',
project.hostAppProjectName,
'--device-id',
deviceId,
'--scheme',
......@@ -674,6 +695,7 @@ void main() {
scheme: 'Runner',
xcodeProject: xcodeproj,
xcodeWorkspace: xcworkspace,
hostAppProjectName: 'Runner',
);
final XcodeDebug xcodeDebug = XcodeDebug(
logger: logger,
......@@ -731,6 +753,7 @@ void main() {
scheme: 'Runner',
xcodeProject: xcodeproj,
xcodeWorkspace: xcworkspace,
hostAppProjectName: 'Runner',
isTemporaryProject: true,
);
......@@ -794,6 +817,7 @@ void main() {
scheme: 'Runner',
xcodeProject: xcodeproj,
xcodeWorkspace: xcworkspace,
hostAppProjectName: 'Runner',
isTemporaryProject: true,
);
final XcodeDebug xcodeDebug = XcodeDebug(
......@@ -857,6 +881,7 @@ void main() {
scheme: 'Runner',
xcodeProject: xcodeproj,
xcodeWorkspace: xcworkspace,
hostAppProjectName: 'Runner',
);
final XcodeDebug xcodeDebug = XcodeDebug(
logger: logger,
......@@ -899,6 +924,7 @@ void main() {
scheme: 'Runner',
xcodeProject: xcodeproj,
xcodeWorkspace: xcworkspace,
hostAppProjectName: 'Runner',
isTemporaryProject: true,
);
final XcodeDebug xcodeDebug = XcodeDebug(
......@@ -950,6 +976,7 @@ void main() {
scheme: 'Runner',
xcodeProject: xcodeproj,
xcodeWorkspace: xcworkspace,
hostAppProjectName: 'Runner',
);
});
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment