From 0321ef5587d59e0ad361937cedada5184a515a86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Valentin=20H=C4=83loiu?= Date: Wed, 9 Apr 2025 17:22:33 +0100 Subject: [PATCH] Add `buildMode`, `icuDataPath` and `engineRevision` interpolations for custom devices (#164455) Make 3 additional string interpolated values available to the `postBuild` and `runDebug` custom device commands: - `${icuDataPath}` - `${engineRevision}` - `${buildMode}` #164454 provides some additional context and the main motivation for this change. Fixes #164454. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --- .../lib/src/custom_devices/custom_device.dart | 24 ++++- .../static/custom-devices.schema.json | 4 +- .../custom_devices/custom_device_test.dart | 95 +++++++++++++++++++ 3 files changed, 118 insertions(+), 5 deletions(-) diff --git a/packages/flutter_tools/lib/src/custom_devices/custom_device.dart b/packages/flutter_tools/lib/src/custom_devices/custom_device.dart index 66aa535189b..f362f7aface 100644 --- a/packages/flutter_tools/lib/src/custom_devices/custom_device.dart +++ b/packages/flutter_tools/lib/src/custom_devices/custom_device.dart @@ -8,6 +8,7 @@ import 'package:meta/meta.dart'; import 'package:process/process.dart'; import '../application_package.dart'; +import '../artifacts.dart'; import '../base/common.dart'; import '../base/file_system.dart'; import '../base/io.dart'; @@ -21,6 +22,7 @@ import '../convert.dart'; import '../device.dart'; import '../device_port_forwarder.dart'; import '../features.dart'; +import '../globals.dart' as globals; import '../project.dart'; import '../protocol_discovery.dart'; import '../vmservice.dart'; @@ -341,6 +343,7 @@ class CustomDeviceAppSession { Map platformArgs = const {}, bool prebuiltApplication = false, String? userIdentifier, + Map additionalReplacementValues = const {}, }) async { final bool traceStartup = platformArgs['trace-startup'] as bool? ?? false; final String? packageName = _appPackage.name; @@ -352,7 +355,7 @@ class CustomDeviceAppSession { 'remotePath': '/tmp/', 'appName': packageName, 'engineOptions': _getEngineOptionsForCmdline(debuggingOptions, traceStartup, route), - }); + }, additionalReplacementValues: additionalReplacementValues); final Process process = await _processUtils.start(interpolated); assert(_process == null); @@ -700,6 +703,16 @@ class CustomDevice extends Device { String? userIdentifier, BundleBuilder? bundleBuilder, }) async { + final TargetPlatform platform = await targetPlatform; + final Artifacts artifacts = globals.artifacts!; + + final Map additionalReplacementValues = { + 'buildMode': debuggingOptions.buildInfo.modeName, + 'icuDataPath': artifacts.getArtifactPath(Artifact.icuData, platform: platform), + 'engineRevision': + artifacts.usesLocalArtifacts ? 'local' : globals.flutterVersion.engineRevision, + }; + if (!prebuiltApplication) { final String assetBundleDir = getAssetBuildDirectory(); @@ -707,7 +720,7 @@ class CustomDevice extends Device { // this just builds the asset bundle, it's the same as `flutter build bundle` await bundleBuilder.build( - platform: await targetPlatform, + platform: platform, buildInfo: debuggingOptions.buildInfo, mainPath: mainPath, depfilePath: defaultDepfilePath, @@ -720,7 +733,11 @@ class CustomDevice extends Device { if (packageName == null) { throwToolExit('Could not start app, name for $package is unknown.'); } - await _tryPostBuild(appName: packageName, localPath: assetBundleDir); + await _tryPostBuild( + appName: packageName, + localPath: assetBundleDir, + additionalReplacementValues: additionalReplacementValues, + ); } } @@ -736,6 +753,7 @@ class CustomDevice extends Device { platformArgs: platformArgs, prebuiltApplication: prebuiltApplication, userIdentifier: userIdentifier, + additionalReplacementValues: additionalReplacementValues, ); } diff --git a/packages/flutter_tools/static/custom-devices.schema.json b/packages/flutter_tools/static/custom-devices.schema.json index da0a78400d9..9c5b894b828 100644 --- a/packages/flutter_tools/static/custom-devices.schema.json +++ b/packages/flutter_tools/static/custom-devices.schema.json @@ -59,7 +59,7 @@ "required": false }, "postBuild": { - "description": "The command to be invoked after the build process is done, to do any additional packaging for example.", + "description": "The command to be invoked after the build process is done, to do any additional packaging for example. The following variables are available via string interpolation:\n- ${appName}\n- ${localPath}\n- ${buildMode}\n- ${icuDataPath}\n- ${engineRevision}", "type": ["array", "null"], "items": { "type": "string" @@ -91,7 +91,7 @@ ] }, "runDebug": { - "description": "The command to be invoked to run the app in debug mode. The name of the app to be started is available via the ${appName} string interpolation. Make sure the flutter cmdline output is available via this commands stdout/stderr since the SDK needs the \"VM Service is now listening on ...\" message to function. If the forwardPort command is not specified, the VM Service URL will be connected to as-is, without any port forwarding. In that case you need to make sure it is reachable from your host device, possibly via the \"--vm-service-host=\" engine flag.", + "description": "The command to be invoked to run the app in debug mode. Make sure the flutter cmdline output is available via this commands stdout/stderr since the SDK needs the \"VM Service is now listening on ...\" message to function. If the forwardPort command is not specified, the VM Service URL will be connected to as-is, without any port forwarding. In that case you need to make sure it is reachable from your host device, possibly via the \"--vm-service-host=\" engine flag. The following variables are available via string interpolation:\n- ${appName}\n- ${engineOptions}\n- ${buildMode}\n- ${icuDataPath}\n- ${engineRevision}", "type": "array", "items": { "type": "string" diff --git a/packages/flutter_tools/test/general.shard/custom_devices/custom_device_test.dart b/packages/flutter_tools/test/general.shard/custom_devices/custom_device_test.dart index bf0f9f2ddac..3250d11bd5c 100644 --- a/packages/flutter_tools/test/general.shard/custom_devices/custom_device_test.dart +++ b/packages/flutter_tools/test/general.shard/custom_devices/custom_device_test.dart @@ -7,6 +7,7 @@ import 'dart:async'; import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:file_testing/file_testing.dart'; +import 'package:flutter_tools/src/artifacts.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/build_system/build_system.dart'; @@ -530,6 +531,100 @@ void main() { }, ); + testUsingContext( + 'custom device command string interpolation end-to-end test', + () async { + final Completer runDebugCompleter = Completer(); + + final CustomDeviceConfig config = testConfig.copyWith( + platform: TargetPlatform.linux_arm64, + postBuildCommand: const [ + 'testpostbuild', + r'--buildMode=${buildMode}', + r'--icuDataPath=${icuDataPath}', + r'--engineRevision=${engineRevision}', + ], + runDebugCommand: const [ + 'testrundebug', + r'--buildMode=${buildMode}', + r'--icuDataPath=${icuDataPath}', + r'--engineRevision=${engineRevision}', + ], + ); + + final List commandArgumentsPattern = [ + RegExp(r'--buildMode=.*'), + RegExp(r'--icuDataPath=.*'), + RegExp(r'--engineRevision=.*'), + ]; + + final String expectedIcuDataPath = globals.artifacts!.getArtifactPath( + Artifact.icuData, + platform: config.platform, + ); + final String expectedEngineRevision = globals.flutterVersion.engineRevision; + + final List expectedCommandArguments = [ + '--buildMode=debug', + '--icuDataPath=$expectedIcuDataPath', + '--engineRevision=$expectedEngineRevision', + ]; + + final List expectedRunDebugCommand = [ + 'testrundebug', + ...expectedCommandArguments, + ]; + final List expectedPostBuildCommand = [ + 'testpostbuild', + ...expectedCommandArguments, + ]; + + final FakeProcessManager processManager = FakeProcessManager.list([ + FakeCommand( + command: ['testpostbuild', ...commandArgumentsPattern], + onRun: (List command) => expect(command, expectedPostBuildCommand), + ), + FakeCommand(command: config.uninstallCommand), + FakeCommand(command: config.installCommand), + FakeCommand( + command: ['testrundebug', ...commandArgumentsPattern], + completer: runDebugCompleter, + onRun: (List command) => expect(command, expectedRunDebugCommand), + stdout: 'The Dart VM service is listening on http://127.0.0.1:12345/abcd/\n', + ), + FakeCommand( + command: config.forwardPortCommand!, + stdout: testConfigForwardPortSuccessOutput, + ), + ]); + + // CustomDevice.startApp doesn't care whether we pass a prebuilt app or + // buildable app as long as we pass prebuiltApplication as false + final PrebuiltLinuxApp app = PrebuiltLinuxApp(executable: 'testexecutable'); + + // finally start actually testing things + final CustomDevice device = CustomDevice( + config: config, + logger: BufferLogger.test(), + processManager: processManager, + ); + + await device.startApp( + app, + debuggingOptions: DebuggingOptions.enabled(BuildInfo.debug), + bundleBuilder: FakeBundleBuilder(), + ); + expect(runDebugCompleter.isCompleted, false); + + expect(await device.stopApp(app), true); + expect(runDebugCompleter.isCompleted, true); + }, + overrides: { + FileSystem: () => MemoryFileSystem.test(), + ProcessManager: () => FakeProcessManager.any(), + }, + ); + testWithoutContext('CustomDevice screenshotting', () async { bool screenshotCommandWasExecuted = false;