From fa35698ee37e47b6c243e14853a3565571914803 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 8 Apr 2020 09:04:32 -0700 Subject: [PATCH] [flutter_tools] allow passing non-config inputs (#54228) --- .../lib/src/build_system/build_system.dart | 15 ++++++++++ .../lib/src/commands/assemble.dart | 30 +++++++++++++------ .../hermetic/assemble_test.dart | 24 +++++++++++---- .../build_system/build_system_test.dart | 25 ++++++++++++++++ 4 files changed, 79 insertions(+), 15 deletions(-) diff --git a/packages/flutter_tools/lib/src/build_system/build_system.dart b/packages/flutter_tools/lib/src/build_system/build_system.dart index 621702cb958..0c61b854abe 100644 --- a/packages/flutter_tools/lib/src/build_system/build_system.dart +++ b/packages/flutter_tools/lib/src/build_system/build_system.dart @@ -295,6 +295,7 @@ class Environment { @required ProcessManager processManager, Directory buildDir, Map defines = const {}, + Map inputs = const {}, }) { // Compute a unique hash of this build's particular environment. // Sort the keys by key so that the result is stable. We always @@ -325,6 +326,7 @@ class Environment { logger: logger, artifacts: artifacts, processManager: processManager, + inputs: inputs, ); } @@ -338,6 +340,7 @@ class Environment { Directory flutterRootDir, Directory buildDir, Map defines = const {}, + Map inputs = const {}, @required FileSystem fileSystem, @required Logger logger, @required Artifacts artifacts, @@ -350,6 +353,7 @@ class Environment { flutterRootDir: flutterRootDir ?? testDirectory, buildDir: buildDir, defines: defines, + inputs: inputs, fileSystem: fileSystem, logger: logger, artifacts: artifacts, @@ -369,6 +373,7 @@ class Environment { @required this.logger, @required this.fileSystem, @required this.artifacts, + @required this.inputs, }); /// The [Source] value which is substituted with the path to [projectDir]. @@ -420,6 +425,16 @@ class Environment { /// which prevents the config from leaking into different builds. final Map defines; + /// Additional input files passed to the build targets. + /// + /// Unlike [defines], values set here do not force a new build configuration. + /// This is useful for passing file inputs that may have changing paths + /// without running builds from scratch. + /// + /// It is the responsibility of the [Target] to declare that an input was + /// used in an output depfile. + final Map inputs; + /// The root build directory shared by all builds. final Directory rootBuildDir; diff --git a/packages/flutter_tools/lib/src/commands/assemble.dart b/packages/flutter_tools/lib/src/commands/assemble.dart index bf1d65a8310..6c015c96c16 100644 --- a/packages/flutter_tools/lib/src/commands/assemble.dart +++ b/packages/flutter_tools/lib/src/commands/assemble.dart @@ -64,6 +64,13 @@ class AssembleCommand extends FlutterCommand { abbr: 'd', help: 'Allows passing configuration to a target with --define=target=key=value.', ); + argParser.addMultiOption( + 'input', + abbr: 'i', + help: 'Allows passing additional inputs with --input=key=value. Unlike ' + 'defines, additional inputs do not generate a new configuration, instead ' + 'they are treated as dependencies of the targets that use them.' + ); argParser.addOption('depfile', help: 'A file path where a depfile will be written. ' 'This contains all build inputs and outputs in a make style syntax' ); @@ -99,7 +106,7 @@ class AssembleCommand extends FlutterCommand { return const {}; } try { - final Environment localEnvironment = environment; + final Environment localEnvironment = createEnvironment(); return { CustomDimensions.commandBuildBundleTargetPlatform: localEnvironment.defines['TargetPlatform'], CustomDimensions.commandBuildBundleIsModule: '${futterProject.isModule}', @@ -111,7 +118,7 @@ class AssembleCommand extends FlutterCommand { } /// The target(s) we are building. - List get targets { + List createTargets() { if (argResults.rest.isEmpty) { throwToolExit('missing target name for flutter assemble.'); } @@ -132,7 +139,7 @@ class AssembleCommand extends FlutterCommand { } /// The environmental configuration for a build invocation. - Environment get environment { + Environment createEnvironment() { final FlutterProject flutterProject = FlutterProject.current(); String output = stringArg('output'); if (output == null) { @@ -149,6 +156,7 @@ class AssembleCommand extends FlutterCommand { .childDirectory('flutter_build'), projectDir: flutterProject.directory, defines: _parseDefines(stringsArg('define')), + inputs: _parseDefines(stringsArg('input')), cacheDir: globals.cache.getRoot(), flutterRootDir: globals.fs.directory(Cache.flutterRoot), artifacts: globals.artifacts, @@ -179,13 +187,17 @@ class AssembleCommand extends FlutterCommand { @override Future runCommand() async { - final List targets = this.targets; + final List targets = createTargets(); final Target target = targets.length == 1 ? targets.single : _CompositeTarget(targets); - final BuildResult result = await globals.buildSystem.build(target, environment, buildSystemConfig: BuildSystemConfig( - resourcePoolSize: argResults.wasParsed('resource-pool-size') - ? int.tryParse(stringArg('resource-pool-size')) - : null, - )); + final BuildResult result = await globals.buildSystem.build( + target, + createEnvironment(), + buildSystemConfig: BuildSystemConfig( + resourcePoolSize: argResults.wasParsed('resource-pool-size') + ? int.tryParse(stringArg('resource-pool-size')) + : null, + ), + ); if (!result.success) { for (final ExceptionMeasurement measurement in result.exceptions.values) { globals.printError('Target ${measurement.target} failed: ${measurement.exception}', diff --git a/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart index b689a797c34..85f0550cf8a 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart @@ -23,7 +23,7 @@ void main() { Cache: () => FakeCache(), }); - testbed.test('Can run a build', () async { + testbed.test('flutter assemble can run a build', () async { when(globals.buildSystem.build(any, any, buildSystemConfig: anyNamed('buildSystemConfig'))) .thenAnswer((Invocation invocation) async { return BuildResult(success: true); @@ -34,7 +34,7 @@ void main() { expect(testLogger.traceText, contains('build succeeded.')); }); - testbed.test('Can parse defines whose values contain =', () async { + testbed.test('flutter assemble can parse defines whose values contain =', () async { when(globals.buildSystem.build(any, any, buildSystemConfig: anyNamed('buildSystemConfig'))) .thenAnswer((Invocation invocation) async { expect((invocation.positionalArguments[1] as Environment).defines, containsPair('FooBar', 'fizz=2')); @@ -46,7 +46,19 @@ void main() { expect(testLogger.traceText, contains('build succeeded.')); }); - testbed.test('Throws ToolExit if not provided with output', () async { + testbed.test('flutter assemble can parse inputs', () async { + when(globals.buildSystem.build(any, any, buildSystemConfig: anyNamed('buildSystemConfig'))) + .thenAnswer((Invocation invocation) async { + expect((invocation.positionalArguments[1] as Environment).inputs, containsPair('Foo', 'Bar.txt')); + return BuildResult(success: true); + }); + final CommandRunner commandRunner = createTestCommandRunner(AssembleCommand()); + await commandRunner.run(['assemble', '-o Output', '-iFoo=Bar.txt', 'debug_macos_bundle_flutter_assets']); + + expect(testLogger.traceText, contains('build succeeded.')); + }); + + testbed.test('flutter assemble throws ToolExit if not provided with output', () async { when(globals.buildSystem.build(any, any, buildSystemConfig: anyNamed('buildSystemConfig'))) .thenAnswer((Invocation invocation) async { return BuildResult(success: true); @@ -57,7 +69,7 @@ void main() { throwsToolExit()); }); - testbed.test('Throws ToolExit if called with non-existent rule', () async { + testbed.test('flutter assemble throws ToolExit if called with non-existent rule', () async { when(globals.buildSystem.build(any, any, buildSystemConfig: anyNamed('buildSystemConfig'))) .thenAnswer((Invocation invocation) async { return BuildResult(success: true); @@ -68,7 +80,7 @@ void main() { throwsToolExit()); }); - testbed.test('Does not log stack traces during build failure', () async { + testbed.test('flutter assemble does not log stack traces during build failure', () async { final StackTrace testStackTrace = StackTrace.current; when(globals.buildSystem.build(any, any, buildSystemConfig: anyNamed('buildSystemConfig'))) .thenAnswer((Invocation invocation) async { @@ -84,7 +96,7 @@ void main() { expect(testLogger.errorText, isNot(contains(testStackTrace.toString()))); }); - testbed.test('Only writes input and output files when the values change', () async { + testbed.test('flutter assemble only writes input and output files when the values change', () async { when(globals.buildSystem.build(any, any, buildSystemConfig: anyNamed('buildSystemConfig'))) .thenAnswer((Invocation invocation) async { return BuildResult( diff --git a/packages/flutter_tools/test/general.shard/build_system/build_system_test.dart b/packages/flutter_tools/test/general.shard/build_system/build_system_test.dart index 5948bda228d..9dc0b8cf9df 100644 --- a/packages/flutter_tools/test/general.shard/build_system/build_system_test.dart +++ b/packages/flutter_tools/test/general.shard/build_system/build_system_test.dart @@ -375,6 +375,31 @@ void main() { expect(environmentA.buildDir.path, isNot(environmentB.buildDir.path)); }); + testWithoutContext('Additional inputs do not change the build configuration', () async { + final Environment environmentA = Environment.test( + fileSystem.currentDirectory, + artifacts: MockArtifacts(), + processManager: FakeProcessManager.any(), + fileSystem: fileSystem, + logger: BufferLogger.test(), + inputs: { + 'C': 'D', + } + ); + final Environment environmentB = Environment.test( + fileSystem.currentDirectory, + artifacts: MockArtifacts(), + processManager: FakeProcessManager.any(), + fileSystem: fileSystem, + logger: BufferLogger.test(), + inputs: { + 'A': 'B', + } + ); + + expect(environmentA.buildDir.path, equals(environmentB.buildDir.path)); + }); + testWithoutContext('A target with depfile dependencies can delete stale outputs on the first run', () async { final BuildSystem buildSystem = setUpBuildSystem(fileSystem); int called = 0;