Skip to content
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

Wait for CONFIGURATION_BUILD_DIR to update when debugging with Xcode #135444

Merged
merged 6 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}`);
christopherfujino marked this conversation as resolved.
Show resolved Hide resolved
}
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();
Copy link
Member

Choose a reason for hiding this comment

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

what does this do? Is this calling an Xcode API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it calls an Xcode API that gets the value of the setting

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 @@ -723,6 +723,18 @@ class IOSDevice extends Device {
return LaunchResult.failed();
} finally {
startAppStatus.stop();

if ((isCoreDevice || forceXcodeDebugWorkflow) && debuggingOptions.debuggingEnabled && package is BuildableIOSApp) {
christopherfujino marked this conversation as resolved.
Show resolved Hide resolved
// 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,
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just moved so that it resets the settings after the Dart VM url has been found, rather than directly after starting the Xcode debug process.

There wasn't any reason to indicate that this was the cause of the issue, but it doesn't hurt to do it a little later, so just to be safe I moved it.

}
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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;
}
}
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 @@ -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'],
Expand Down Expand Up @@ -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'],
Expand Down Expand Up @@ -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'],
Expand Down Expand Up @@ -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'],
Expand Down