From 9808b1f916fa913619d0f11cb0e16a33e29ebe73 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 23 Jul 2025 11:04:40 -0400 Subject: [PATCH] Disable SwiftPM for `xcode-analyze` Until https://github.com/flutter/flutter/issues/172427 is resolved, `xcode-analyze` doesn't work as desired with SwiftPM enabled (it analyzes only the test code, not the plugin code). To avoid losing analysis coverage in the meantime, this disabled SwiftPM temporarily while running analysis. This PR also updates `build-examples` to use the newer pubspec-based config option to set the SwiftPM flag state instead of setting global state, to avoid future issues where we are unintentionally bleeding flag changes across different tests, and to make local runs not impact developer machine state. To unit test this functionality, this adds a new feature to the existing process mock system that allows running an arbitrary test callback at the ponit where a process is being run, which in this case allows reading the temporarily-modified pubspec contents at the right point in the command execution. Fixes https://github.com/flutter/flutter/issues/171442 --- .../tool/lib/src/build_examples_command.dart | 43 ++- script/tool/lib/src/common/plugin_utils.dart | 52 ++++ .../tool/lib/src/xcode_analyze_command.dart | 11 +- .../test/build_examples_command_test.dart | 282 +++++------------- script/tool/test/util.dart | 19 +- .../tool/test/xcode_analyze_command_test.dart | 30 ++ 6 files changed, 207 insertions(+), 230 deletions(-) diff --git a/script/tool/lib/src/build_examples_command.dart b/script/tool/lib/src/build_examples_command.dart index 932e3090a39..ac548f9c32f 100644 --- a/script/tool/lib/src/build_examples_command.dart +++ b/script/tool/lib/src/build_examples_command.dart @@ -118,7 +118,7 @@ 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 platformFlags = _platforms.keys.toList(); if (!platformFlags.contains(platformIOS) && @@ -126,12 +126,6 @@ class BuildExamplesCommand extends PackageLoopingCommand { 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); } @@ -150,23 +144,6 @@ class BuildExamplesCommand extends PackageLoopingCommand { 'were specified. At least one platform must be provided.'); throw ToolExit(_exitNoPlatformFlags); } - - switch (_swiftPackageManagerFeatureConfig) { - case true: - await processRunner.runAndStream( - flutterCommand, - ['config', '--enable-swift-package-manager'], - exitOnError: true, - ); - case false: - await processRunner.runAndStream( - flutterCommand, - ['config', '--no-enable-swift-package-manager'], - exitOnError: true, - ); - case null: - break; - } } @override @@ -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); @@ -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) { diff --git a/script/tool/lib/src/common/plugin_utils.dart b/script/tool/lib/src/common/plugin_utils.dart index 94677fe7e5a..218ec9d2789 100644 --- a/script/tool/lib/src/common/plugin_utils.dart +++ b/script/tool/lib/src/common/plugin_utils.dart @@ -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'; @@ -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 flutterPath = [flutterKey]; + const List configPath = [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([...configPath, swiftPMFlag]); + } + } else { + // Ensure that the section exists. + if (configMap.isEmpty) { + final YamlMap root = editablePubspec.parseAt([]) as YamlMap; + if (!root.containsKey(flutterKey)) { + editablePubspec.update(flutterPath, YamlMap()); + } + editablePubspec.update(configPath, YamlMap()); + } + // Then add the flag. + editablePubspec.update([...configPath, swiftPMFlag], enabled); + } + + package.pubspecFile.writeAsStringSync(editablePubspec.toString()); +} + /// Returns the /// flutter: /// plugin: diff --git a/script/tool/lib/src/xcode_analyze_command.dart b/script/tool/lib/src/xcode_analyze_command.dart index 8494550bc77..c6c908a3f58 100644 --- a/script/tool/lib/src/xcode_analyze_command.dart +++ b/script/tool/lib/src/xcode_analyze_command.dart @@ -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, @@ -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; } diff --git a/script/tool/test/build_examples_command_test.dart b/script/tool/test/build_examples_command_test.dart index 98ebe28ffa5..fd01e73b4d2 100644 --- a/script/tool/test/build_examples_command_test.dart +++ b/script/tool/test/build_examples_command_test.dart @@ -165,16 +165,25 @@ void main() { ])); }); - test('building for iOS with CocoaPods on master channel', () async { + test('building for iOS with CocoaPods', () async { mockPlatform.isMacOS = true; - mockPlatform.environment['CHANNEL'] = 'master'; final RepositoryPackage plugin = createFakePlugin('plugin', packagesDir, platformSupport: { platformIOS: const PlatformDetails(PlatformSupport.inline), }); - final Directory pluginExampleDirectory = getExampleDir(plugin); + final RepositoryPackage example = plugin.getExamples().first; + final String originalPubspecContents = + example.pubspecFile.readAsStringSync(); + String? buildTimePubspecContents; + processRunner + .mockProcessesForExecutable[getFlutterCommand(mockPlatform)] = + [ + FakeProcessInfo(MockProcess(), ['build'], () { + buildTimePubspecContents = example.pubspecFile.readAsStringSync(); + }) + ]; final List output = await runCapturingPrint(runner, [ 'build-examples', @@ -190,102 +199,13 @@ void main() { ]), ); - expect( - processRunner.recordedCalls, - orderedEquals([ - ProcessCall( - getFlutterCommand(mockPlatform), - const ['config', '--no-enable-swift-package-manager'], - null, - ), - ProcessCall( - getFlutterCommand(mockPlatform), - const [ - 'build', - 'ios', - '--no-codesign', - '--enable-experiment=exp1' - ], - pluginExampleDirectory.path, - ), - ]), - ); - }); - - test('building for iOS with Swift Package Manager on master channel', - () async { - mockPlatform.isMacOS = true; - mockPlatform.environment['CHANNEL'] = 'master'; - - final RepositoryPackage plugin = createFakePlugin('plugin', packagesDir, - platformSupport: { - platformIOS: const PlatformDetails(PlatformSupport.inline), - }); - - final Directory pluginExampleDirectory = getExampleDir(plugin); - - final List output = await runCapturingPrint(runner, [ - 'build-examples', - '--ios', - '--enable-experiment=exp1', - '--swift-package-manager', - ]); - - expect( - output, - containsAllInOrder([ - '\nBUILDING plugin/example for iOS', - ]), - ); - - expect( - processRunner.recordedCalls, - orderedEquals([ - ProcessCall( - getFlutterCommand(mockPlatform), - const ['config', '--enable-swift-package-manager'], - null, - ), - ProcessCall( - getFlutterCommand(mockPlatform), - const [ - 'build', - 'ios', - '--no-codesign', - '--enable-experiment=exp1' - ], - pluginExampleDirectory.path, - ), - ]), - ); - }); - - test( - 'building for iOS with CocoaPods on stable channel does not disable SPM', - () async { - mockPlatform.isMacOS = true; - mockPlatform.environment['CHANNEL'] = 'stable'; - - final RepositoryPackage plugin = createFakePlugin('plugin', packagesDir, - platformSupport: { - platformIOS: const PlatformDetails(PlatformSupport.inline), - }); - - final Directory pluginExampleDirectory = getExampleDir(plugin); - - final List output = await runCapturingPrint(runner, [ - 'build-examples', - '--ios', - '--enable-experiment=exp1', - '--no-swift-package-manager', - ]); - - expect( - output, - containsAllInOrder([ - '\nBUILDING plugin/example for iOS', - ]), - ); + // Ensure that SwiftPM was disabled for the package. + expect(originalPubspecContents, + isNot(contains('enable-swift-package-manager: false'))); + expect(buildTimePubspecContents, + contains('enable-swift-package-manager: false')); + // And that it was undone after. + expect(example.pubspecFile.readAsStringSync(), originalPubspecContents); expect( processRunner.recordedCalls, @@ -298,24 +218,31 @@ void main() { '--no-codesign', '--enable-experiment=exp1' ], - pluginExampleDirectory.path, + example.path, ), ]), ); }); - test( - 'building for iOS with Swift Package Manager on stable channel does not enable SPM', - () async { + test('building for iOS with Swift Package Manager', () async { mockPlatform.isMacOS = true; - mockPlatform.environment['CHANNEL'] = 'stable'; final RepositoryPackage plugin = createFakePlugin('plugin', packagesDir, platformSupport: { platformIOS: const PlatformDetails(PlatformSupport.inline), }); - final Directory pluginExampleDirectory = getExampleDir(plugin); + final RepositoryPackage example = plugin.getExamples().first; + final String originalPubspecContents = + example.pubspecFile.readAsStringSync(); + String? buildTimePubspecContents; + processRunner + .mockProcessesForExecutable[getFlutterCommand(mockPlatform)] = + [ + FakeProcessInfo(MockProcess(), ['build'], () { + buildTimePubspecContents = example.pubspecFile.readAsStringSync(); + }) + ]; final List output = await runCapturingPrint(runner, [ 'build-examples', @@ -331,6 +258,14 @@ void main() { ]), ); + // Ensure that SwiftPM was enabled for the package. + expect(originalPubspecContents, + isNot(contains('enable-swift-package-manager: true'))); + expect(buildTimePubspecContents, + contains('enable-swift-package-manager: true')); + // And that it was undone after. + expect(example.pubspecFile.readAsStringSync(), originalPubspecContents); + expect( processRunner.recordedCalls, orderedEquals([ @@ -342,7 +277,7 @@ void main() { '--no-codesign', '--enable-experiment=exp1' ], - pluginExampleDirectory.path, + example.path, ), ]), ); @@ -445,16 +380,25 @@ void main() { ])); }); - test('building for macOS with CocoaPods on master channel', () async { + test('building for macOS with CocoaPods', () async { mockPlatform.isMacOS = true; - mockPlatform.environment['CHANNEL'] = 'master'; final RepositoryPackage plugin = createFakePlugin('plugin', packagesDir, platformSupport: { platformMacOS: const PlatformDetails(PlatformSupport.inline), }); - final Directory pluginExampleDirectory = getExampleDir(plugin); + final RepositoryPackage example = plugin.getExamples().first; + final String originalPubspecContents = + example.pubspecFile.readAsStringSync(); + String? buildTimePubspecContents; + processRunner + .mockProcessesForExecutable[getFlutterCommand(mockPlatform)] = + [ + FakeProcessInfo(MockProcess(), ['build'], () { + buildTimePubspecContents = example.pubspecFile.readAsStringSync(); + }) + ]; final List output = await runCapturingPrint(runner, ['build-examples', '--macos', '--no-swift-package-manager']); @@ -466,90 +410,13 @@ void main() { ]), ); - expect( - processRunner.recordedCalls, - orderedEquals([ - ProcessCall( - getFlutterCommand(mockPlatform), - const ['config', '--no-enable-swift-package-manager'], - null, - ), - ProcessCall( - getFlutterCommand(mockPlatform), - const [ - 'build', - 'macos', - ], - pluginExampleDirectory.path, - ), - ]), - ); - }); - - test('building for macOS with Swift Package Manager on master channel', - () async { - mockPlatform.isMacOS = true; - mockPlatform.environment['CHANNEL'] = 'master'; - - final RepositoryPackage plugin = createFakePlugin('plugin', packagesDir, - platformSupport: { - platformMacOS: const PlatformDetails(PlatformSupport.inline), - }); - - final Directory pluginExampleDirectory = getExampleDir(plugin); - - final List output = await runCapturingPrint(runner, - ['build-examples', '--macos', '--swift-package-manager']); - - expect( - output, - containsAllInOrder([ - '\nBUILDING plugin/example for macOS', - ]), - ); - - expect( - processRunner.recordedCalls, - orderedEquals([ - ProcessCall( - getFlutterCommand(mockPlatform), - const ['config', '--enable-swift-package-manager'], - null, - ), - ProcessCall( - getFlutterCommand(mockPlatform), - const [ - 'build', - 'macos', - ], - pluginExampleDirectory.path, - ), - ]), - ); - }); - - test( - 'building for macOS with CocoaPods on stable channel does not disable SPM', - () async { - mockPlatform.isMacOS = true; - mockPlatform.environment['CHANNEL'] = 'stable'; - - final RepositoryPackage plugin = createFakePlugin('plugin', packagesDir, - platformSupport: { - platformMacOS: const PlatformDetails(PlatformSupport.inline), - }); - - final Directory pluginExampleDirectory = getExampleDir(plugin); - - final List output = await runCapturingPrint(runner, - ['build-examples', '--macos', '--no-swift-package-manager']); - - expect( - output, - containsAllInOrder([ - '\nBUILDING plugin/example for macOS', - ]), - ); + // Ensure that SwiftPM was enabled for the package. + expect(originalPubspecContents, + isNot(contains('enable-swift-package-manager: false'))); + expect(buildTimePubspecContents, + contains('enable-swift-package-manager: false')); + // And that it was undone after. + expect(example.pubspecFile.readAsStringSync(), originalPubspecContents); expect( processRunner.recordedCalls, @@ -560,24 +427,31 @@ void main() { 'build', 'macos', ], - pluginExampleDirectory.path, + example.path, ), ]), ); }); - test( - 'building for macOS with Swift Package Manager on stable channel does not enable SPM', - () async { + test('building for macOS with Swift Package Manager', () async { mockPlatform.isMacOS = true; - mockPlatform.environment['CHANNEL'] = 'stable'; final RepositoryPackage plugin = createFakePlugin('plugin', packagesDir, platformSupport: { platformMacOS: const PlatformDetails(PlatformSupport.inline), }); - final Directory pluginExampleDirectory = getExampleDir(plugin); + final RepositoryPackage example = plugin.getExamples().first; + final String originalPubspecContents = + example.pubspecFile.readAsStringSync(); + String? buildTimePubspecContents; + processRunner + .mockProcessesForExecutable[getFlutterCommand(mockPlatform)] = + [ + FakeProcessInfo(MockProcess(), ['build'], () { + buildTimePubspecContents = example.pubspecFile.readAsStringSync(); + }) + ]; final List output = await runCapturingPrint(runner, ['build-examples', '--macos', '--swift-package-manager']); @@ -589,6 +463,14 @@ void main() { ]), ); + // Ensure that SwiftPM was enabled for the package. + expect(originalPubspecContents, + isNot(contains('enable-swift-package-manager: true'))); + expect(buildTimePubspecContents, + contains('enable-swift-package-manager: true')); + // And that it was undone after. + expect(example.pubspecFile.readAsStringSync(), originalPubspecContents); + expect( processRunner.recordedCalls, orderedEquals([ @@ -598,7 +480,7 @@ void main() { 'build', 'macos', ], - pluginExampleDirectory.path, + example.path, ), ]), ); diff --git a/script/tool/test/util.dart b/script/tool/test/util.dart index a608c69339b..4923cb26a1e 100644 --- a/script/tool/test/util.dart +++ b/script/tool/test/util.dart @@ -350,7 +350,7 @@ Future> runCapturingPrint( /// Information about a process to return from [RecordingProcessRunner]. class FakeProcessInfo { const FakeProcessInfo(this.process, - [this.expectedInitialArgs = const []]); + [this.expectedInitialArgs = const [], this.runCallback]); /// The process to return. final io.Process process; @@ -360,6 +360,12 @@ class FakeProcessInfo { /// This does not have to be a full list of arguments, only enough of the /// start to ensure that the call is as expected. final List expectedInitialArgs; + + /// If present, a function to call when the process would be run. + /// + /// This can be used to validate state at specific points in a command run, + /// such as temporary file modifications. + final void Function()? runCallback; } /// A mock [ProcessRunner] which records process calls. @@ -388,7 +394,7 @@ class RecordingProcessRunner extends ProcessRunner { bool exitOnError = false, }) async { recordedCalls.add(ProcessCall(executable, args, workingDir?.path)); - final io.Process? processToReturn = _getProcessToReturn(executable, args); + final io.Process? processToReturn = _runFakeProcess(executable, args); final int exitCode = processToReturn == null ? 0 : await processToReturn.exitCode; if (exitOnError && (exitCode != 0)) { @@ -411,7 +417,7 @@ class RecordingProcessRunner extends ProcessRunner { }) async { recordedCalls.add(ProcessCall(executable, args, workingDir?.path)); - final io.Process? process = _getProcessToReturn(executable, args); + final io.Process? process = _runFakeProcess(executable, args); final List? processStdout = await process?.stdout.transform(stdoutEncoding.decoder).toList(); final String stdout = processStdout?.join() ?? ''; @@ -435,10 +441,12 @@ class RecordingProcessRunner extends ProcessRunner { {Directory? workingDirectory}) async { recordedCalls.add(ProcessCall(executable, args, workingDirectory?.path)); return Future.value( - _getProcessToReturn(executable, args) ?? MockProcess()); + _runFakeProcess(executable, args) ?? MockProcess()); } - io.Process? _getProcessToReturn(String executable, List args) { + /// Returns the fake process for the given executable and args after running + /// any callback it provides. + io.Process? _runFakeProcess(String executable, List args) { final List fakes = mockProcessesForExecutable[executable] ?? []; if (fakes.isNotEmpty) { @@ -450,6 +458,7 @@ class RecordingProcessRunner extends ProcessRunner { '[${fake.expectedInitialArgs.join(', ')}] but was called with ' 'arguments [${args.join(', ')}]'); } + fake.runCallback?.call(); return fake.process; } return null; diff --git a/script/tool/test/xcode_analyze_command_test.dart b/script/tool/test/xcode_analyze_command_test.dart index 0ec408c6ba1..2e168c2fbbb 100644 --- a/script/tool/test/xcode_analyze_command_test.dart +++ b/script/tool/test/xcode_analyze_command_test.dart @@ -56,6 +56,36 @@ void main() { ); }); + test('temporarily disables Swift Package Manager', () async { + final RepositoryPackage plugin = createFakePlugin('plugin', packagesDir, + platformSupport: { + platformIOS: const PlatformDetails(PlatformSupport.inline), + }); + + final RepositoryPackage example = plugin.getExamples().first; + final String originalPubspecContents = + example.pubspecFile.readAsStringSync(); + String? buildTimePubspecContents; + processRunner.mockProcessesForExecutable['xcrun'] = [ + FakeProcessInfo(MockProcess(), [], () { + buildTimePubspecContents = example.pubspecFile.readAsStringSync(); + }) + ]; + + await runCapturingPrint(runner, [ + 'xcode-analyze', + '--ios', + ]); + + // Ensure that SwiftPM was disabled for the package. + expect(originalPubspecContents, + isNot(contains('enable-swift-package-manager: false'))); + expect(buildTimePubspecContents, + contains('enable-swift-package-manager: false')); + // And that it was undone after. + expect(example.pubspecFile.readAsStringSync(), originalPubspecContents); + }); + group('iOS', () { test('skip if iOS is not supported', () async { createFakePlugin('plugin', packagesDir,