[flutter_tools] surgically remove outputs from shared directory (#53773)

packages/flutter_tools/lib/src/build_system/build_system.dart

@@ -394,8 +394,11 @@ class Environment {
 
   /// 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.
@@ -519,6 +522,12 @@ class BuildSystem {
                      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,
@@ -529,6 +538,61 @@ class BuildSystem {
           ..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();
+        }
+      }
+    }
+  }
 }
 
 

packages/flutter_tools/lib/src/build_system/targets/ios.dart

@@ -276,13 +276,6 @@ abstract class IosAssetBundle extends Target {
     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.

packages/flutter_tools/lib/src/build_system/targets/macos.dart

@@ -56,12 +56,7 @@ abstract class UnpackMacOS extends Target {
     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.
-    if (targetDirectory.existsSync()) {
-      targetDirectory.deleteSync(recursive: true);
-    }
+    targetDirectory.createSync(recursive: true);
     final List<File> inputs = globals.fs.directory(basePath)
       .listSync(recursive: true)
       .whereType<File>()
@@ -291,12 +286,6 @@ abstract class MacOSBundleFlutterAssets extends Target {
     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(

packages/flutter_tools/test/general.shard/build_system/build_system_test.dart

@@ -409,6 +409,109 @@ void main() {
     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) {

packages/flutter_tools/test/general.shard/build_system/targets/macos_test.dart

@@ -101,10 +101,6 @@ void main() {
       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 @@ void main() {
     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);