Skip to content

Commit

Permalink
Wait for CONFIGURATION_BUILD_DIR to update when debugging with Xcode (f…
Browse files Browse the repository at this point in the history
…lutter#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 flutter#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.
  • Loading branch information
vashworth committed Sep 27, 2023
1 parent ead4559 commit ef7d6d6
Show file tree
Hide file tree
Showing 6 changed files with 215 additions and 33 deletions.
171 changes: 152 additions & 19 deletions packages/flutter_tools/bin/xcode_debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -226,9 +263,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) {
Expand Down Expand Up @@ -347,6 +382,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
Expand Down Expand Up @@ -528,3 +572,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}`);
}
}
26 changes: 14 additions & 12 deletions packages/flutter_tools/lib/src/ios/devices.dart
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,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,
);
}
}
}

Expand Down Expand Up @@ -818,6 +830,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 {
Expand All @@ -839,18 +853,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;
}
}
Expand Down
13 changes: 13 additions & 0 deletions packages/flutter_tools/lib/src/ios/xcode_debug.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -396,6 +404,7 @@ class XcodeDebug {

return XcodeDebugProject(
scheme: 'Runner',
hostAppProjectName: 'Runner',
xcodeProject: tempXcodeProject.childDirectory('Runner.xcodeproj'),
xcodeWorkspace: tempXcodeProject.childDirectory('Runner.xcworkspace'),
isTemporaryProject: true,
Expand Down Expand Up @@ -470,13 +479,17 @@ class XcodeDebugProject {
required this.scheme,
required this.xcodeWorkspace,
required this.xcodeProject,
required this.hostAppProjectName,
this.expectedConfigurationBuildDir,
this.isTemporaryProject = false,
this.verboseLogging = false,
});

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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down Expand Up @@ -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'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,7 @@ void main() {
scheme: 'Runner',
xcodeWorkspace: temporaryXcodeProjectDirectory.childDirectory('Runner.xcworkspace'),
xcodeProject: temporaryXcodeProjectDirectory.childDirectory('Runner.xcodeproj'),
hostAppProjectName: 'Runner',
),
expectedDeviceId: '123',
expectedLaunchArguments: <String>['--enable-dart-profiling'],
Expand Down Expand Up @@ -669,6 +670,7 @@ void main() {
scheme: 'Runner',
xcodeWorkspace: temporaryXcodeProjectDirectory.childDirectory('Runner.xcworkspace'),
xcodeProject: temporaryXcodeProjectDirectory.childDirectory('Runner.xcodeproj'),
hostAppProjectName: 'Runner',
),
expectedDeviceId: '123',
expectedLaunchArguments: <String>['--enable-dart-profiling'],
Expand Down Expand Up @@ -729,6 +731,7 @@ void main() {
scheme: 'Runner',
xcodeWorkspace: temporaryXcodeProjectDirectory.childDirectory('Runner.xcworkspace'),
xcodeProject: temporaryXcodeProjectDirectory.childDirectory('Runner.xcodeproj'),
hostAppProjectName: 'Runner',
),
expectedDeviceId: '123',
expectedLaunchArguments: <String>['--enable-dart-profiling'],
Expand Down Expand Up @@ -781,6 +784,7 @@ void main() {
scheme: 'Runner',
xcodeWorkspace: temporaryXcodeProjectDirectory.childDirectory('Runner.xcworkspace'),
xcodeProject: temporaryXcodeProjectDirectory.childDirectory('Runner.xcodeproj'),
hostAppProjectName: 'Runner',
),
expectedDeviceId: '123',
expectedLaunchArguments: <String>['--enable-dart-profiling'],
Expand Down

0 comments on commit ef7d6d6

Please sign in to comment.