Skip to content

Disable SwiftPM for xcode-analyze #9666

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 19 additions & 24 deletions script/tool/lib/src/build_examples_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -118,20 +118,14 @@ class BuildExamplesCommand extends PackageLoopingCommand {
'arguments.';

/// Returns whether the Swift Package Manager feature should be enabled,
/// disabled, or left to the release channel's default value.
/// disabled, or left to the default value.
bool? get _swiftPackageManagerFeatureConfig {
final List<String> platformFlags = _platforms.keys.toList();
if (!platformFlags.contains(platformIOS) &&
!platformFlags.contains(platformMacOS)) {
return null;
}

// TODO(loic-sharma): Allow enabling on stable once Swift Package Manager
// feature is available on stable.
if (platform.environment['CHANNEL'] != 'master') {
return null;
}

return getNullableBoolArg(_swiftPackageManagerFlag);
}

Expand All @@ -150,23 +144,6 @@ class BuildExamplesCommand extends PackageLoopingCommand {
'were specified. At least one platform must be provided.');
throw ToolExit(_exitNoPlatformFlags);
}

switch (_swiftPackageManagerFeatureConfig) {
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 change from the flag to the pubspec config does mean that we are delaying stable testing of SwiftPM in this repo longer, since the pubspec config won't do anything on current stable, but since we were already accidentally not testing it on stable anyway even though we could have been, I don't think delaying a bit longer is a big deal. And unlike the old code, this version will not require us to remember to turn it on for stable later, it'll just start working as soon as stable gets the config feature.

case true:
await processRunner.runAndStream(
flutterCommand,
<String>['config', '--enable-swift-package-manager'],
exitOnError: true,
);
case false:
await processRunner.runAndStream(
flutterCommand,
<String>['config', '--no-enable-swift-package-manager'],
exitOnError: true,
);
case null:
break;
}
}

@override
Expand Down Expand Up @@ -212,8 +189,20 @@ class BuildExamplesCommand extends PackageLoopingCommand {
}
print('');

final bool? swiftPackageManagerOverride =
isPlugin ? _swiftPackageManagerFeatureConfig : null;

bool builtSomething = false;
for (final RepositoryPackage example in package.getExamples()) {
// Rather than changing global config state, enable SwiftPM via a
// temporary package-level override.
if (swiftPackageManagerOverride != null) {
print('Overriding enable-swift-package-manager to '
'$swiftPackageManagerOverride');
setSwiftPackageManagerState(example,
enabled: swiftPackageManagerOverride);
}

final String packageName =
getRelativePosixPath(example.directory, from: packagesDir);

Expand All @@ -240,6 +229,12 @@ class BuildExamplesCommand extends PackageLoopingCommand {
errors.add('$packageName (${platform.label})');
}
}

// If an override was added, remove it.
if (swiftPackageManagerOverride != null) {
print('Removing enable-swift-package-manager override');
setSwiftPackageManagerState(example, enabled: null);
}
}

if (!builtSomething) {
Expand Down
52 changes: 52 additions & 0 deletions script/tool/lib/src/common/plugin_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

import 'package:yaml/yaml.dart';
import 'package:yaml_edit/yaml_edit.dart';

import 'core.dart';
import 'repository_package.dart';
Expand Down Expand Up @@ -83,6 +84,57 @@ bool pluginHasNativeCodeForPlatform(String platform, RepositoryPackage plugin) {
return pluginClass != null && pluginClass != 'none';
}

/// Adds or removes a package-level Swift Package Manager override to the given
/// package.
///
/// A null enabled state clears the package-local override, defaulting to whatever the
/// global state is.
void setSwiftPackageManagerState(RepositoryPackage package,
{required bool? enabled}) {
const String swiftPMFlag = 'enable-swift-package-manager';
const String flutterKey = 'flutter';
const List<String> flutterPath = <String>[flutterKey];
const List<String> configPath = <String>[flutterKey, 'config'];

final YamlEditor editablePubspec =
YamlEditor(package.pubspecFile.readAsStringSync());
final YamlMap configMap =
editablePubspec.parseAt(configPath, orElse: () => YamlMap()) as YamlMap;
if (enabled == null) {
if (!configMap.containsKey(swiftPMFlag)) {
// Nothing to do.
return;
} else if (configMap.length == 1) {
// The config section only exists for this override, so remove the whole
// section.
editablePubspec.remove(configPath);
// The entire flutter: section may also only have been added for the
// config, in which case it should be removed as well.
final YamlMap flutterMap = editablePubspec.parseAt(flutterPath,
orElse: () => YamlMap()) as YamlMap;
if (flutterMap.isEmpty) {
editablePubspec.remove(flutterPath);
}
} else {
// Remove the specific entry, leaving the rest of the config section.
editablePubspec.remove(<String>[...configPath, swiftPMFlag]);
}
} else {
// Ensure that the section exists.
if (configMap.isEmpty) {
final YamlMap root = editablePubspec.parseAt(<String>[]) as YamlMap;
if (!root.containsKey(flutterKey)) {
editablePubspec.update(flutterPath, YamlMap());
}
editablePubspec.update(configPath, YamlMap());
}
// Then add the flag.
editablePubspec.update(<String>[...configPath, swiftPMFlag], enabled);
}

package.pubspecFile.writeAsStringSync(editablePubspec.toString());
}

/// Returns the
/// flutter:
/// plugin:
Expand Down
11 changes: 10 additions & 1 deletion script/tool/lib/src/xcode_analyze_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,14 @@ class XcodeAnalyzeCommand extends PackageLoopingCommand {
targetPlatform == FlutterPlatform.ios ? 'iOS' : 'macOS';
bool passing = true;
for (final RepositoryPackage example in plugin.getExamples()) {
// See https://github.com/flutter/flutter/issues/172427 for discussion of
// why this is currently necessary.
print('Disabling Swift Package Manager...');
setSwiftPackageManagerState(example, enabled: false);

// Unconditionally re-run build with --debug --config-only, to ensure that
// the project is in a debug state even if it was previously configured.
// the project is in a debug state even if it was previously configured,
// and that SwiftPM is disabled.
print('Running flutter build --config-only...');
final bool buildSuccess = await runConfigOnlyBuild(
example,
Expand Down Expand Up @@ -162,6 +168,9 @@ class XcodeAnalyzeCommand extends PackageLoopingCommand {
printError('$examplePath ($platformString) failed analysis.');
passing = false;
}

print('Removing Swift Package Manager override...');
setSwiftPackageManagerState(example, enabled: null);
}
return passing;
}
Expand Down
Loading