[flutter_tools] surgically remove outputs from shared directory (#53962)
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 0c61b85..a8696a8 100644 --- a/packages/flutter_tools/lib/src/build_system/build_system.dart +++ b/packages/flutter_tools/lib/src/build_system/build_system.dart
@@ -399,8 +399,11 @@ /// The `BUILD_DIR` environment variable. /// - /// Defaults to `{PROJECT_ROOT}/build`. The root of the output directory where - /// build step intermediates and outputs are written. + /// The root of the output directory where build step intermediates and + /// outputs are written. Current usages of assemble configure ths to be + /// a unique directory under `.dart_tool/flutter_build`, though it can + /// be placed anywhere. The uniqueness is only enforced by callers, and + /// is currently done by hashing the build configuration. final Directory buildDir; /// The `CACHE_DIR` environment variable. @@ -534,6 +537,12 @@ path.contains('.dart_tool'); }); } + trackSharedBuildDirectory( + environment, _fileSystem, buildInstance.outputFiles, + ); + environment.buildDir.childFile('outputs.json') + .writeAsStringSync(json.encode(buildInstance.outputFiles.keys.toList())); + return BuildResult( success: passed, exceptions: buildInstance.exceptionMeasurements, @@ -544,6 +553,61 @@ ..sort((File a, File b) => a.path.compareTo(b.path)), ); } + + /// Write the identifier of the last build into the output directory and + /// remove the previous build's output. + /// + /// The build identifier is the basename of the build directory where + /// outputs and intermediaries are written, under `.dart_tool/flutter_build`. + /// This is computed from a hash of the build's configuration. + /// + /// This identifier is used to perform a targeted cleanup of the last output + /// files, if these were not already covered by the built-in cleanup. This + /// cleanup is only necessary when multiple different build configurations + /// output to the same directory. + @visibleForTesting + static void trackSharedBuildDirectory( + Environment environment, + FileSystem fileSystem, + Map<String, File> currentOutputs, + ) { + final String currentBuildId = fileSystem.path.basename(environment.buildDir.path); + final File lastBuildIdFile = environment.outputDir.childFile('.last_build_id'); + if (!lastBuildIdFile.existsSync()) { + lastBuildIdFile.writeAsStringSync(currentBuildId); + // No config file, either output was cleaned or this is the first build. + return; + } + final String lastBuildId = lastBuildIdFile.readAsStringSync().trim(); + if (lastBuildId == currentBuildId) { + // The last build was the same configuration as the current build + return; + } + // Update the output dir with the latest config. + lastBuildIdFile + ..createSync() + ..writeAsStringSync(currentBuildId); + final File outputsFile = environment.buildDir + .parent + .childDirectory(lastBuildId) + .childFile('outputs.json'); + + if (!outputsFile.existsSync()) { + // There is no output list. This could happen if the user manually + // edited .last_config or deleted .dart_tool. + return; + } + final List<String> lastOutputs = (json.decode(outputsFile.readAsStringSync()) as List<Object>) + .cast<String>(); + for (final String lastOutput in lastOutputs) { + if (!currentOutputs.containsKey(lastOutput)) { + final File lastOutputFile = fileSystem.file(lastOutput); + if (lastOutputFile.existsSync()) { + lastOutputFile.deleteSync(); + } + } + } + } }
diff --git a/packages/flutter_tools/lib/src/build_system/targets/ios.dart b/packages/flutter_tools/lib/src/build_system/targets/ios.dart index bcb5803..8d1943d 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/ios.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/ios.dart
@@ -276,13 +276,6 @@ final Directory frameworkDirectory = environment.outputDir.childDirectory('App.framework'); final Directory assetDirectory = frameworkDirectory.childDirectory('flutter_assets'); frameworkDirectory.createSync(recursive: true); - - // This is necessary because multiple different build configurations will - // output different files here. Build cleaning only works when the files - // change within a build configuration. - if (assetDirectory.existsSync()) { - assetDirectory.deleteSync(recursive: true); - } assetDirectory.createSync(); // Only copy the prebuilt runtimes and kernel blob in debug mode.
diff --git a/packages/flutter_tools/lib/src/build_system/targets/macos.dart b/packages/flutter_tools/lib/src/build_system/targets/macos.dart index 220e821..447212c 100644 --- a/packages/flutter_tools/lib/src/build_system/targets/macos.dart +++ b/packages/flutter_tools/lib/src/build_system/targets/macos.dart
@@ -56,9 +56,8 @@ final Directory targetDirectory = environment .outputDir .childDirectory('FlutterMacOS.framework'); - // This is necessary because multiple different build configurations will - // output different files here. Build cleaning only works when the files - // change within a build configuration. + // Deleting this directory is required or else the FlutterMacOS module + // cannot be found. if (targetDirectory.existsSync()) { targetDirectory.deleteSync(recursive: true); } @@ -291,12 +290,6 @@ final Directory assetDirectory = outputDirectory .childDirectory('Resources') .childDirectory('flutter_assets'); - // This is necessary because multiple different build configurations will - // output different files here. Build cleaning only works when the files - // change within a build configuration. - if (assetDirectory.existsSync()) { - assetDirectory.deleteSync(recursive: true); - } assetDirectory.createSync(recursive: true); final Depfile depfile = await copyAssets(environment, assetDirectory); final DepfileService depfileService = DepfileService(
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 9dc0b8c..90daf36 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
@@ -434,6 +434,109 @@ expect(fileSystem.file('c.txt'), isNot(exists)); expect(called, 2); }); + + testWithoutContext('trackSharedBuildDirectory handles a missing .last_build_id', () { + BuildSystem.trackSharedBuildDirectory(environment, fileSystem, <String, File>{}); + + expect(environment.outputDir.childFile('.last_build_id'), exists); + expect(environment.outputDir.childFile('.last_build_id').readAsStringSync(), + '6666cd76f96956469e7be39d750cc7d9'); + }); + + testWithoutContext('trackSharedBuildDirectory does not modify .last_build_id when config is identical', () { + environment.outputDir.childFile('.last_build_id') + ..writeAsStringSync('6666cd76f96956469e7be39d750cc7d9') + ..setLastModifiedSync(DateTime(1991, 8, 23)); + BuildSystem.trackSharedBuildDirectory(environment, fileSystem, <String, File>{}); + + expect(environment.outputDir.childFile('.last_build_id').lastModifiedSync(), + DateTime(1991, 8, 23)); + }); + + testWithoutContext('trackSharedBuildDirectory does not delete files when outputs.json is missing', () { + environment.outputDir + .childFile('.last_build_id') + .writeAsStringSync('foo'); + environment.buildDir.parent + .childDirectory('foo') + .createSync(recursive: true); + environment.outputDir + .childFile('stale') + .createSync(); + BuildSystem.trackSharedBuildDirectory(environment, fileSystem, <String, File>{}); + + expect(environment.outputDir.childFile('.last_build_id').readAsStringSync(), + '6666cd76f96956469e7be39d750cc7d9'); + expect(environment.outputDir.childFile('stale'), exists); + }); + + testWithoutContext('trackSharedBuildDirectory deletes files in outputs.json but not in current outputs', () { + environment.outputDir + .childFile('.last_build_id') + .writeAsStringSync('foo'); + final Directory otherBuildDir = environment.buildDir.parent + .childDirectory('foo') + ..createSync(recursive: true); + final File staleFile = environment.outputDir + .childFile('stale') + ..createSync(); + otherBuildDir.childFile('outputs.json') + .writeAsStringSync(json.encode(<String>[staleFile.absolute.path])); + BuildSystem.trackSharedBuildDirectory(environment, fileSystem, <String, File>{}); + + expect(environment.outputDir.childFile('.last_build_id').readAsStringSync(), + '6666cd76f96956469e7be39d750cc7d9'); + expect(environment.outputDir.childFile('stale'), isNot(exists)); + }); + + testWithoutContext('multiple builds to the same output directory do no leave stale artifacts', () async { + final BuildSystem buildSystem = setUpBuildSystem(fileSystem); + final Environment testEnvironmentDebug = Environment.test( + fileSystem.currentDirectory, + outputDir: fileSystem.directory('output'), + defines: <String, String>{ + 'config': 'debug', + }, + artifacts: MockArtifacts(), + processManager: FakeProcessManager.any(), + logger: BufferLogger.test(), + fileSystem: fileSystem, + ); + final Environment testEnvironmentProfle = Environment.test( + fileSystem.currentDirectory, + outputDir: fileSystem.directory('output'), + defines: <String, String>{ + 'config': 'profile', + }, + artifacts: MockArtifacts(), + processManager: FakeProcessManager.any(), + logger: BufferLogger.test(), + fileSystem: fileSystem, + ); + + final TestTarget debugTarget = TestTarget((Environment environment) async { + environment.outputDir.childFile('debug').createSync(); + })..outputs = const <Source>[Source.pattern('{OUTPUT_DIR}/debug')]; + final TestTarget releaseTarget = TestTarget((Environment environment) async { + environment.outputDir.childFile('release').createSync(); + })..outputs = const <Source>[Source.pattern('{OUTPUT_DIR}/release')]; + + await buildSystem.build(debugTarget, testEnvironmentDebug); + + // Verify debug output was created + expect(fileSystem.file('output/debug'), exists); + + await buildSystem.build(releaseTarget, testEnvironmentProfle); + + // Last build config is updated properly + expect(testEnvironmentProfle.outputDir.childFile('.last_build_id'), exists); + expect(testEnvironmentProfle.outputDir.childFile('.last_build_id').readAsStringSync(), + 'c20b3747fb2aa148cc4fd39bfbbd894f'); + + // Verify debug output removeds + expect(fileSystem.file('output/debug'), isNot(exists)); + expect(fileSystem.file('output/release'), exists); + }); } BuildSystem setUpBuildSystem(FileSystem fileSystem) {
diff --git a/packages/flutter_tools/test/general.shard/build_system/targets/macos_test.dart b/packages/flutter_tools/test/general.shard/build_system/targets/macos_test.dart index d360506..1ac2a7f 100644 --- a/packages/flutter_tools/test/general.shard/build_system/targets/macos_test.dart +++ b/packages/flutter_tools/test/general.shard/build_system/targets/macos_test.dart
@@ -101,10 +101,6 @@ final Directory source = globals.fs.directory(sourcePath); final Directory target = globals.fs.directory(targetPath); - // verify directory was deleted by command. - expect(target.existsSync(), false); - target.createSync(recursive: true); - for (final FileSystemEntity entity in source.listSync(recursive: true)) { if (entity is File) { final String relative = globals.fs.path.relative(entity.path, from: source.path); @@ -178,54 +174,6 @@ expect(globals.fs.file(precompiledIsolate), isNot(exists)); })); - test('release/profile macOS application has no blob or precompiled runtime when ' - 'run ontop of different configuration', () => testbed.run(() async { - globals.fs.file(globals.fs.path.join('bin', 'cache', 'artifacts', 'engine', 'darwin-x64', - 'vm_isolate_snapshot.bin')).createSync(recursive: true); - globals.fs.file(globals.fs.path.join('bin', 'cache', 'artifacts', 'engine', 'darwin-x64', - 'isolate_snapshot.bin')).createSync(recursive: true); - globals.fs.file(globals.fs.path.join(environment.buildDir.path, 'App.framework', 'App')) - .createSync(recursive: true); - - final String inputKernel = globals.fs.path.join(environment.buildDir.path, 'app.dill'); - final String outputKernel = globals.fs.path.join('App.framework', 'Versions', 'A', 'Resources', - 'flutter_assets', 'kernel_blob.bin'); - globals.fs.file(inputKernel) - ..createSync(recursive: true) - ..writeAsStringSync('testing'); - - await const DebugMacOSBundleFlutterAssets().build(environment); - - globals.fs.file(globals.fs.path.join('bin', 'cache', 'artifacts', 'engine', 'darwin-x64', - 'vm_isolate_snapshot.bin')).createSync(recursive: true); - globals.fs.file(globals.fs.path.join('bin', 'cache', 'artifacts', 'engine', 'darwin-x64', - 'isolate_snapshot.bin')).createSync(recursive: true); - - final Environment testEnvironment = Environment.test( - globals.fs.currentDirectory, - defines: <String, String>{ - kBuildMode: 'profile', - kTargetPlatform: 'darwin-x64', - }, - artifacts: MockArtifacts(), - processManager: FakeProcessManager.any(), - logger: globals.logger, - fileSystem: globals.fs, - ); - testEnvironment.buildDir.createSync(recursive: true); - globals.fs.file(globals.fs.path.join(testEnvironment.buildDir.path, 'App.framework', 'App')) - .createSync(recursive: true); - final String precompiledVm = globals.fs.path.join('App.framework', 'Resources', - 'flutter_assets', 'vm_snapshot_data'); - final String precompiledIsolate = globals.fs.path.join('App.framework', 'Resources', - 'flutter_assets', 'isolate_snapshot_data'); - await const ProfileMacOSBundleFlutterAssets().build(testEnvironment); - - expect(globals.fs.file(outputKernel), isNot(exists)); - expect(globals.fs.file(precompiledVm), isNot(exists)); - expect(globals.fs.file(precompiledIsolate), isNot(exists)); - })); - test('release/profile macOS application updates when App.framework updates', () => testbed.run(() async { globals.fs.file(globals.fs.path.join('bin', 'cache', 'artifacts', 'engine', 'darwin-x64', 'vm_isolate_snapshot.bin')).createSync(recursive: true);