From 69b28c5e1929b6c9bff71db689b159574a7e6563 Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Tue, 30 Jul 2019 14:31:55 -0700 Subject: [PATCH] Add a check for empty kernel files Towards #2362 We don't know where the zero byte files are being introduced to the system - if they are coming from the kernel worker try to catch them earlier by checking for when we copy a file with no content. This won't solve the issue but if errors surface through this they should be more clear. - Add a `requireContent` argument to `copyOutput`. In the future we might consider making the default either with or without an output - we don't know of any use cases where we want to allow copying a zero byte file. - Use the new argument from the kernel builder to check earlier whether a kernel file is empty. --- build_modules/CHANGELOG.md | 4 ++++ build_modules/lib/src/kernel_builder.dart | 2 +- build_modules/pubspec.yaml | 8 ++++++-- scratch_space/CHANGELOG.md | 5 +++++ scratch_space/lib/scratch_space.dart | 3 ++- scratch_space/lib/src/scratch_space.dart | 14 +++++++++++++- scratch_space/pubspec.yaml | 2 +- 7 files changed, 32 insertions(+), 6 deletions(-) diff --git a/build_modules/CHANGELOG.md b/build_modules/CHANGELOG.md index 882e5b6f7..79e397097 100644 --- a/build_modules/CHANGELOG.md +++ b/build_modules/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.4.1 + +- Require non-empty output from kernel build steps. + ## 2.4.0 - Allow overriding the target name passed to the kernel worker. diff --git a/build_modules/lib/src/kernel_builder.dart b/build_modules/lib/src/kernel_builder.dart index de433f2e4..b1d626b5b 100644 --- a/build_modules/lib/src/kernel_builder.dart +++ b/build_modules/lib/src/kernel_builder.dart @@ -186,7 +186,7 @@ Future _createKernel( } // Copy the output back using the buildStep. - await scratchSpace.copyOutput(outputId, buildStep); + await scratchSpace.copyOutput(outputId, buildStep, requireContent: true); } finally { await packagesFile.parent.delete(recursive: true); } diff --git a/build_modules/pubspec.yaml b/build_modules/pubspec.yaml index f58fdb8c5..9b3d0c311 100644 --- a/build_modules/pubspec.yaml +++ b/build_modules/pubspec.yaml @@ -1,5 +1,5 @@ name: build_modules -version: 2.4.0 +version: 2.4.1 description: Builders for Dart modules author: Dart Team homepage: https://github.com/dart-lang/build/tree/master/build_modules @@ -22,7 +22,7 @@ dependencies: meta: ^1.1.0 path: ^1.4.2 pedantic: ^1.0.0 - scratch_space: ^0.0.3 + scratch_space: ^0.0.4 dev_dependencies: build_runner: ^1.0.0 @@ -34,3 +34,7 @@ dev_dependencies: path: test/fixtures/a b: path: test/fixtures/b + +dependency_overrides: + scratch_space: + path: ../scratch_space diff --git a/scratch_space/CHANGELOG.md b/scratch_space/CHANGELOG.md index e19235712..c315f9579 100644 --- a/scratch_space/CHANGELOG.md +++ b/scratch_space/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.0.4 + +- Add `requireContent` argument to `copyOutput` to allow asserting that a file + produced in a scratch space is not empty. + ## 0.0.3+2 - Declare support for `package:build` version `1.x.x`. diff --git a/scratch_space/lib/scratch_space.dart b/scratch_space/lib/scratch_space.dart index 9241b5518..95c4c8e50 100644 --- a/scratch_space/lib/scratch_space.dart +++ b/scratch_space/lib/scratch_space.dart @@ -2,4 +2,5 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -export 'src/scratch_space.dart' show ScratchSpace, canonicalUriFor; +export 'src/scratch_space.dart' + show ScratchSpace, canonicalUriFor, EmptyOutputException; diff --git a/scratch_space/lib/src/scratch_space.dart b/scratch_space/lib/src/scratch_space.dart index bb55a2c6b..97449911a 100644 --- a/scratch_space/lib/src/scratch_space.dart +++ b/scratch_space/lib/src/scratch_space.dart @@ -51,9 +51,14 @@ class ScratchSpace { /// This must be called for all outputs which you want to be included as a /// part of the actual build (any other outputs will be deleted with the /// tmp dir and won't be available to other [Builder]s). - Future copyOutput(AssetId id, AssetWriter writer) async { + /// + /// If [requireContent] is true and the file is empty an + /// [EmptyOutputException] is thrown. + Future copyOutput(AssetId id, AssetWriter writer, + {bool requireContent = false}) async { var file = fileFor(id); var bytes = await _descriptorPool.withResource(file.readAsBytes); + if (requireContent && bytes.isEmpty) throw EmptyOutputException(id); await writer.writeAsBytes(id, bytes); } @@ -137,3 +142,10 @@ String canonicalUriFor(AssetId id) { /// The path relative to the root of the environment for a given [id]. String _relativePathFor(AssetId id) => canonicalUriFor(id).replaceFirst('package:', 'packages/'); + +/// An indication that an output file which was expect to be non-empty had no +/// content. +class EmptyOutputException implements Exception { + final AssetId id; + EmptyOutputException(this.id); +} diff --git a/scratch_space/pubspec.yaml b/scratch_space/pubspec.yaml index 14e6497e7..5496d8abd 100644 --- a/scratch_space/pubspec.yaml +++ b/scratch_space/pubspec.yaml @@ -1,5 +1,5 @@ name: scratch_space -version: 0.0.3+2 +version: 0.0.4 description: A tool to manage running external executables within package:build author: Dart Team homepage: https://github.com/dart-lang/build/tree/master/scratch_space