Unverified Commit 41b87255 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

Add support for depfile dependency in assemble (#41530)

parent 60eeb60f
...@@ -121,16 +121,17 @@ abstract class Target { ...@@ -121,16 +121,17 @@ abstract class Target {
/// Create a [Node] with resolved inputs and outputs. /// Create a [Node] with resolved inputs and outputs.
Node _toNode(Environment environment) { Node _toNode(Environment environment) {
final List<File> inputs = resolveInputs(environment); final ResolvedFiles inputsFiles = resolveInputs(environment);
final List<File> outputs = resolveOutputs(environment); final ResolvedFiles outputFiles = resolveOutputs(environment);
return Node( return Node(
this, this,
inputs, inputsFiles.sources,
outputs, outputFiles.sources,
<Node>[ <Node>[
for (Target target in dependencies) target._toNode(environment), for (Target target in dependencies) target._toNode(environment),
], ],
environment, environment,
inputsFiles.containsNewDepfile,
); );
} }
...@@ -168,9 +169,7 @@ abstract class Target { ...@@ -168,9 +169,7 @@ abstract class Target {
/// Resolve the set of input patterns and functions into a concrete list of /// Resolve the set of input patterns and functions into a concrete list of
/// files. /// files.
List<File> resolveInputs( ResolvedFiles resolveInputs(Environment environment) {
Environment environment,
) {
return _resolveConfiguration(inputs, environment, implicit: true, inputs: true); return _resolveConfiguration(inputs, environment, implicit: true, inputs: true);
} }
...@@ -178,11 +177,8 @@ abstract class Target { ...@@ -178,11 +177,8 @@ abstract class Target {
/// ///
/// The [implicit] flag controls whether it is safe to evaluate [Source]s /// The [implicit] flag controls whether it is safe to evaluate [Source]s
/// which uses functions, behaviors, or patterns. /// which uses functions, behaviors, or patterns.
List<File> resolveOutputs( ResolvedFiles resolveOutputs(Environment environment) {
Environment environment, return _resolveConfiguration(outputs, environment, inputs: false);
) {
final List<File> outputEntities = _resolveConfiguration(outputs, environment, inputs: false);
return outputEntities;
} }
/// Performs a fold across this target and its dependencies. /// Performs a fold across this target and its dependencies.
...@@ -200,13 +196,15 @@ abstract class Target { ...@@ -200,13 +196,15 @@ abstract class Target {
Map<String, Object> toJson(Environment environment) { Map<String, Object> toJson(Environment environment) {
return <String, Object>{ return <String, Object>{
'name': name, 'name': name,
'dependencies': dependencies.map((Target target) => target.name).toList(), 'dependencies': <String>[
'inputs': resolveInputs(environment) for (Target target in dependencies) target.name
.map((File file) => file.path) ],
.toList(), 'inputs': <String>[
'outputs': resolveOutputs(environment) for (File file in resolveInputs(environment).sources) file.path,
.map((File file) => file.path) ],
.toList(), 'outputs': <String>[
for (File file in resolveOutputs(environment).sources) file.path,
],
'stamp': _findStampFile(environment).absolute.path, 'stamp': _findStampFile(environment).absolute.path,
}; };
} }
...@@ -217,13 +215,14 @@ abstract class Target { ...@@ -217,13 +215,14 @@ abstract class Target {
return environment.buildDir.childFile(fileName); return environment.buildDir.childFile(fileName);
} }
static List<File> _resolveConfiguration( static ResolvedFiles _resolveConfiguration(List<Source> config, Environment environment, {
List<Source> config, Environment environment, { bool implicit = true, bool inputs = true }) { bool implicit = true, bool inputs = true,
}) {
final SourceVisitor collector = SourceVisitor(environment, inputs); final SourceVisitor collector = SourceVisitor(environment, inputs);
for (Source source in config) { for (Source source in config) {
source.accept(collector); source.accept(collector);
} }
return collector.sources; return collector;
} }
} }
...@@ -481,49 +480,70 @@ class _BuildInstance { ...@@ -481,49 +480,70 @@ class _BuildInstance {
final Stopwatch stopwatch = Stopwatch()..start(); final Stopwatch stopwatch = Stopwatch()..start();
bool passed = true; bool passed = true;
bool skipped = false; bool skipped = false;
try {
final bool canSkip = await node.computeChanges(environment, fileCache); // The build system produces a list of aggregate input and output
// files for the overall build. This list is provided to a hosting build
// system, such as Xcode, to configure logic for when to skip the
// rule/phase which contains the flutter build.
//
// When looking at the inputs and outputs for the individual rules, we need
// to be careful to remove inputs that were actually output from previous
// build steps. This indicates that the file is an intermediary. If
// these files are included as both inputs and outputs then it isn't
// possible to construct a DAG describing the build.
void updateGraph() {
for (File output in node.outputs) {
outputFiles[output.path] = output;
}
for (File input in node.inputs) { for (File input in node.inputs) {
// The build system should produce a list of aggregate input and output
// files for the overall build. The goal is to provide this to a hosting
// build system, such as Xcode, to configure logic for when to skip the
// rule/phase which contains the flutter build. When looking at the
// inputs and outputs for the individual rules, we need to be careful to
// remove inputs that were actually output from previous build steps.
// This indicates that the file is actual an output or intermediary. If
// these files are included as both inputs and outputs then it isn't
// possible to construct a DAG describing the build.
final String resolvedPath = input.resolveSymbolicLinksSync(); final String resolvedPath = input.resolveSymbolicLinksSync();
if (outputFiles.containsKey(resolvedPath)) { if (outputFiles.containsKey(resolvedPath)) {
continue; continue;
} }
inputFiles[resolvedPath] = input; inputFiles[resolvedPath] = input;
} }
}
try {
// If we're missing a depfile, wait until after evaluating the target to
// compute changes.
final bool canSkip = !node.missingDepfile &&
await node.computeChanges(environment, fileCache);
if (canSkip) { if (canSkip) {
skipped = true; skipped = true;
printTrace('Skipping target: ${node.target.name}'); printTrace('Skipping target: ${node.target.name}');
for (File output in node.outputs) { updateGraph();
outputFiles[output.path] = output; return passed;
} }
} else { printTrace('${node.target.name}: Starting due to ${node.invalidatedReasons}');
printTrace('${node.target.name}: Starting due to ${node.invalidatedReasons}'); await node.target.build(environment);
await node.target.build(environment); printTrace('${node.target.name}: Complete');
printTrace('${node.target.name}: Complete');
// If we were missing the depfile, resolve files after executing the
// Update hashes for output files. // target so that all file hashes are up to date on the next run.
await fileCache.hashFiles(node.outputs); if (node.missingDepfile) {
node.target._writeStamp(node.inputs, node.outputs, environment); node.inputs.clear();
for (File output in node.outputs) { node.outputs.clear();
outputFiles[output.path] = output; node.inputs.addAll(node.target.resolveInputs(environment).sources);
node.outputs.addAll(node.target.resolveOutputs(environment).sources);
await fileCache.hashFiles(node.inputs);
}
// Update hashes for output files.
await fileCache.hashFiles(node.outputs);
node.target._writeStamp(node.inputs, node.outputs, environment);
updateGraph();
// Delete outputs from previous stages that are no longer a part of the
// build.
for (String previousOutput in node.previousOutputs) {
if (outputFiles.containsKey(previousOutput)) {
continue;
} }
// Delete outputs from previous stages that are no longer a part of the build. final File previousFile = fs.file(previousOutput);
for (String previousOutput in node.previousOutputs) { if (previousFile.existsSync()) {
if (!outputFiles.containsKey(previousOutput)) { previousFile.deleteSync();
final File previousFile = fs.file(previousOutput);
if (previousFile.existsSync()) {
previousFile.deleteSync();
}
}
} }
} }
} catch (exception, stackTrace) { } catch (exception, stackTrace) {
...@@ -604,7 +624,7 @@ void verifyOutputDirectories(List<File> outputs, Environment environment, Target ...@@ -604,7 +624,7 @@ void verifyOutputDirectories(List<File> outputs, Environment environment, Target
/// A node in the build graph. /// A node in the build graph.
class Node { class Node {
Node(this.target, this.inputs, this.outputs, this.dependencies, Node(this.target, this.inputs, this.outputs, this.dependencies,
Environment environment) { Environment environment, this.missingDepfile) {
final File stamp = target._findStampFile(environment); final File stamp = target._findStampFile(environment);
// If the stamp file doesn't exist, we haven't run this step before and // If the stamp file doesn't exist, we haven't run this step before and
...@@ -651,6 +671,12 @@ class Node { ...@@ -651,6 +671,12 @@ class Node {
/// These files may not yet exist if the target hasn't run yet. /// These files may not yet exist if the target hasn't run yet.
final List<File> outputs; final List<File> outputs;
/// Whether this node is missing a depfile.
///
/// This requires an additional pass of source resolution after the target
/// has been executed.
final bool missingDepfile;
/// The target definition which contains the build action to invoke. /// The target definition which contains the build action to invoke.
final Target target; final Target target;
......
...@@ -13,8 +13,20 @@ import 'exceptions.dart'; ...@@ -13,8 +13,20 @@ import 'exceptions.dart';
/// [Environment]. /// [Environment].
typedef InputFunction = List<File> Function(Environment environment); typedef InputFunction = List<File> Function(Environment environment);
/// A set of source files.
abstract class ResolvedFiles {
/// Whether any of the sources we evaluated contained a missing depfile.
///
/// If so, the build system needs to rerun the visitor after executing the
/// build to ensure all hashes are up to date.
bool get containsNewDepfile;
/// The resolved source files.
List<File> get sources;
}
/// Collects sources for a [Target] into a single list of [FileSystemEntities]. /// Collects sources for a [Target] into a single list of [FileSystemEntities].
class SourceVisitor { class SourceVisitor implements ResolvedFiles {
/// Create a new [SourceVisitor] from an [Environment]. /// Create a new [SourceVisitor] from an [Environment].
SourceVisitor(this.environment, [this.inputs = true]); SourceVisitor(this.environment, [this.inputs = true]);
...@@ -26,9 +38,13 @@ class SourceVisitor { ...@@ -26,9 +38,13 @@ class SourceVisitor {
/// Defaults to `true`. /// Defaults to `true`.
final bool inputs; final bool inputs;
/// The entities are populated after visiting each source. @override
final List<File> sources = <File>[]; final List<File> sources = <File>[];
@override
bool get containsNewDepfile => _containsNewDepfile;
bool _containsNewDepfile = false;
/// Visit a [Source] which contains a function. /// Visit a [Source] which contains a function.
/// ///
/// The function is expected to produce a list of [FileSystemEntities]s. /// The function is expected to produce a list of [FileSystemEntities]s.
...@@ -36,6 +52,46 @@ class SourceVisitor { ...@@ -36,6 +52,46 @@ class SourceVisitor {
sources.addAll(function(environment)); sources.addAll(function(environment));
} }
/// Visit a depfile which contains both input and output files.
///
/// If the file is missing, this visitor is marked as [containsNewDepfile].
/// This is used by the [Node] class to tell the [BuildSystem] to
/// defer hash computation until after executing the target.
// depfile logic adopted from https://github.com/flutter/flutter/blob/7065e4330624a5a216c8ffbace0a462617dc1bf5/dev/devicelab/lib/framework/apk_utils.dart#L390
void visitDepfile(String name) {
final File depfile = environment.buildDir.childFile(name);
if (!depfile.existsSync()) {
_containsNewDepfile = true;
return;
}
final String contents = depfile.readAsStringSync();
final List<String> colonSeparated = contents.split(': ');
if (colonSeparated.length != 2) {
printError('Invalid depfile: ${depfile.path}');
return;
}
if (inputs) {
sources.addAll(_processList(colonSeparated[1].trim()));
} else {
sources.addAll(_processList(colonSeparated[0].trim()));
}
}
final RegExp _separatorExpr = RegExp(r'([^\\]) ');
final RegExp _escapeExpr = RegExp(r'\\(.)');
Iterable<File> _processList(String rawText) {
return rawText
// Put every file on right-hand side on the separate line
.replaceAllMapped(_separatorExpr, (Match match) => '${match.group(1)}\n')
.split('\n')
// Expand escape sequences, so that '\ ', for example,ß becomes ' '
.map<String>((String path) => path.replaceAllMapped(_escapeExpr, (Match match) => match.group(1)).trim())
.where((String path) => path.isNotEmpty)
.toSet()
.map((String path) => fs.file(path));
}
/// Visit a [Source] which contains a file uri. /// Visit a [Source] which contains a file uri.
/// ///
/// The uri may include constants defined in an [Environment]. If /// The uri may include constants defined in an [Environment]. If
...@@ -162,6 +218,16 @@ abstract class Source { ...@@ -162,6 +218,16 @@ abstract class Source {
const factory Source.artifact(Artifact artifact, {TargetPlatform platform, const factory Source.artifact(Artifact artifact, {TargetPlatform platform,
BuildMode mode}) = _ArtifactSource; BuildMode mode}) = _ArtifactSource;
/// The source is provided by a depfile generated at runtime.
///
/// The `name` is of the file, and is expected to be output relative to the
/// build directory.
///
/// Before the first build, the depfile is expected to be missing. Its
/// absence is interpreted as the build needing to run. Afterwards, both
/// input and output file hashes are updated.
const factory Source.depfile(String name) = _DepfileSource;
/// Visit the particular source type. /// Visit the particular source type.
void accept(SourceVisitor visitor); void accept(SourceVisitor visitor);
...@@ -237,3 +303,15 @@ class _ArtifactSource implements Source { ...@@ -237,3 +303,15 @@ class _ArtifactSource implements Source {
@override @override
bool get implicit => false; bool get implicit => false;
} }
class _DepfileSource implements Source {
const _DepfileSource(this.name);
final String name;
@override
void accept(SourceVisitor visitor) => visitor.visitDepfile(name);
@override
bool get implicit => false;
}
\ No newline at end of file
...@@ -176,15 +176,15 @@ class KernelSnapshot extends Target { ...@@ -176,15 +176,15 @@ class KernelSnapshot extends Target {
List<Source> get inputs => const <Source>[ List<Source> get inputs => const <Source>[
Source.pattern('{PROJECT_DIR}/.packages'), Source.pattern('{PROJECT_DIR}/.packages'),
Source.pattern('{FLUTTER_ROOT}/packages/flutter_tools/lib/src/build_system/targets/dart.dart'), Source.pattern('{FLUTTER_ROOT}/packages/flutter_tools/lib/src/build_system/targets/dart.dart'),
Source.function(listDartSources), // <- every dart file under {PROJECT_DIR}/lib and in .packages
Source.artifact(Artifact.platformKernelDill), Source.artifact(Artifact.platformKernelDill),
Source.artifact(Artifact.engineDartBinary), Source.artifact(Artifact.engineDartBinary),
Source.artifact(Artifact.frontendServerSnapshotForEngineDartSdk), Source.artifact(Artifact.frontendServerSnapshotForEngineDartSdk),
Source.depfile('kernel_snapshot.d'),
]; ];
@override @override
List<Source> get outputs => const <Source>[ List<Source> get outputs => const <Source>[
Source.pattern('{BUILD_DIR}/app.dill'), Source.depfile('kernel_snapshot.d'),
]; ];
@override @override
...@@ -213,6 +213,7 @@ class KernelSnapshot extends Target { ...@@ -213,6 +213,7 @@ class KernelSnapshot extends Target {
packagesPath: packagesPath, packagesPath: packagesPath,
linkPlatformKernelIn: buildMode == BuildMode.release, linkPlatformKernelIn: buildMode == BuildMode.release,
mainPath: targetFileAbsolute, mainPath: targetFileAbsolute,
depFilePath: environment.buildDir.childFile('kernel_snapshot.d').path,
); );
if (output == null || output.errorCount != 0) { if (output == null || output.errorCount != 0) {
throw Exception('Errors during snapshot creation: $output'); throw Exception('Errors during snapshot creation: $output');
......
...@@ -312,6 +312,28 @@ void main() { ...@@ -312,6 +312,28 @@ void main() {
expect(() => checkCycles(barTarget), throwsA(isInstanceOf<CycleException>())); expect(() => checkCycles(barTarget), throwsA(isInstanceOf<CycleException>()));
}); });
test('Target with depfile dependency will not run twice without invalidation', () => testbed.run(() async {
int called = 0;
final TestTarget target = TestTarget((Environment environment) async {
environment.buildDir.childFile('example.d')
.writeAsStringSync('a.txt: b.txt');
fs.file('a.txt').writeAsStringSync('a');
called += 1;
})
..inputs = const <Source>[Source.depfile('example.d')]
..outputs = const <Source>[Source.depfile('example.d')];
fs.file('b.txt').writeAsStringSync('b');
await buildSystem.build(target, environment);
expect(fs.file('a.txt').existsSync(), true);
expect(called, 1);
// Second build is up to date due to depfil parse.
await buildSystem.build(target, environment);
expect(called, 1);
}));
} }
class MockPlatform extends Mock implements Platform {} class MockPlatform extends Mock implements Platform {}
......
...@@ -4,11 +4,13 @@ ...@@ -4,11 +4,13 @@
import 'package:flutter_tools/src/artifacts.dart'; import 'package:flutter_tools/src/artifacts.dart';
import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/build_system/build_system.dart'; import 'package:flutter_tools/src/build_system/build_system.dart';
import 'package:flutter_tools/src/build_system/exceptions.dart'; import 'package:flutter_tools/src/build_system/exceptions.dart';
import 'package:flutter_tools/src/build_system/source.dart'; import 'package:flutter_tools/src/build_system/source.dart';
import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/cache.dart';
import 'package:mockito/mockito.dart';
import '../../src/common.dart'; import '../../src/common.dart';
import '../../src/testbed.dart'; import '../../src/testbed.dart';
...@@ -17,8 +19,11 @@ void main() { ...@@ -17,8 +19,11 @@ void main() {
Testbed testbed; Testbed testbed;
SourceVisitor visitor; SourceVisitor visitor;
Environment environment; Environment environment;
MockPlatform mockPlatform;
setUp(() { setUp(() {
mockPlatform = MockPlatform();
when(mockPlatform.isWindows).thenReturn(true);
testbed = Testbed(setup: () { testbed = Testbed(setup: () {
fs.directory('cache').createSync(); fs.directory('cache').createSync();
final Directory outputs = fs.directory('outputs') final Directory outputs = fs.directory('outputs')
...@@ -155,6 +160,62 @@ void main() { ...@@ -155,6 +160,62 @@ void main() {
missingSource.accept(visitor); missingSource.accept(visitor);
expect(visitor.sources, isEmpty); expect(visitor.sources, isEmpty);
})); }));
test('can resolve a missing depfile', () => testbed.run(() {
const Source depfile = Source.depfile('foo.d');
depfile.accept(visitor);
expect(visitor.sources, isEmpty);
expect(visitor.containsNewDepfile, true);
}));
test('can resolve a populated depfile', () => testbed.run(() {
const Source depfile = Source.depfile('foo.d');
environment.buildDir.childFile('foo.d')
.writeAsStringSync('a.dart : c.dart');
depfile.accept(visitor);
expect(visitor.sources.single.path, 'c.dart');
expect(visitor.containsNewDepfile, false);
final SourceVisitor outputVisitor = SourceVisitor(environment, false);
depfile.accept(outputVisitor);
expect(outputVisitor.sources.single.path, 'a.dart');
expect(outputVisitor.containsNewDepfile, false);
}));
test('does not crash on completely invalid depfile', () => testbed.run(() {
const Source depfile = Source.depfile('foo.d');
environment.buildDir.childFile('foo.d')
.writeAsStringSync('hello, world');
depfile.accept(visitor);
expect(visitor.sources, isEmpty);
expect(visitor.containsNewDepfile, false);
}));
test('can parse depfile with windows paths', () => testbed.run(() {
const Source depfile = Source.depfile('foo.d');
environment.buildDir.childFile('foo.d')
.writeAsStringSync(r'a.dart: C:\\foo\\bar.txt');
depfile.accept(visitor);
expect(visitor.sources.single.path, r'C:\foo\bar.txt');
expect(visitor.containsNewDepfile, false);
}, overrides: <Type, Generator>{
Platform: () => mockPlatform,
}));
test('can parse depfile with spaces in paths', () => testbed.run(() {
const Source depfile = Source.depfile('foo.d');
environment.buildDir.childFile('foo.d')
.writeAsStringSync(r'a.dart: foo\ bar.txt');
depfile.accept(visitor);
expect(visitor.sources.single.path, r'foo bar.txt');
expect(visitor.containsNewDepfile, false);
}));
} }
class TestBehavior extends SourceBehavior { class TestBehavior extends SourceBehavior {
...@@ -168,3 +229,6 @@ class TestBehavior extends SourceBehavior { ...@@ -168,3 +229,6 @@ class TestBehavior extends SourceBehavior {
return null; return null;
} }
} }
class MockPlatform extends Mock implements Platform {}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment