Unverified Commit 9e72bf56 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter_tools] don't compute hashes of well known artifacts (#53848)

parent d21ab518
...@@ -81,6 +81,9 @@ class AotBuilder { ...@@ -81,6 +81,9 @@ class AotBuilder {
buildDir: FlutterProject.current().dartTool.childDirectory('flutter_build'), buildDir: FlutterProject.current().dartTool.childDirectory('flutter_build'),
cacheDir: null, cacheDir: null,
flutterRootDir: globals.fs.directory(Cache.flutterRoot), flutterRootDir: globals.fs.directory(Cache.flutterRoot),
engineVersion: globals.artifacts.isLocalEngine
? null
: globals.flutterVersion.engineRevision,
defines: <String, String>{ defines: <String, String>{
kTargetFile: mainDartFile ?? globals.fs.path.join('lib', 'main.dart'), kTargetFile: mainDartFile ?? globals.fs.path.join('lib', 'main.dart'),
kBuildMode: getNameForBuildMode(buildInfo.mode), kBuildMode: getNameForBuildMode(buildInfo.mode),
......
...@@ -169,6 +169,9 @@ abstract class Artifacts { ...@@ -169,6 +169,9 @@ abstract class Artifacts {
// Returns which set of engine artifacts is currently used for the [platform] // Returns which set of engine artifacts is currently used for the [platform]
// and [mode] combination. // and [mode] combination.
String getEngineType(TargetPlatform platform, [ BuildMode mode ]); String getEngineType(TargetPlatform platform, [ BuildMode mode ]);
/// Whether these artifacts correspond to a non-versioned local engine.
bool get isLocalEngine;
} }
...@@ -405,6 +408,9 @@ class CachedArtifacts extends Artifacts { ...@@ -405,6 +408,9 @@ class CachedArtifacts extends Artifacts {
assert(false, 'Invalid platform $platform.'); assert(false, 'Invalid platform $platform.');
return null; return null;
} }
@override
bool get isLocalEngine => false;
} }
TargetPlatform _currentHostPlatform(Platform platform) { TargetPlatform _currentHostPlatform(Platform platform) {
...@@ -576,6 +582,9 @@ class LocalEngineArtifacts extends Artifacts { ...@@ -576,6 +582,9 @@ class LocalEngineArtifacts extends Artifacts {
} }
throw Exception('Unsupported platform $platform.'); throw Exception('Unsupported platform $platform.');
} }
@override
bool get isLocalEngine => true;
} }
/// An implementation of [Artifacts] that provides individual overrides. /// An implementation of [Artifacts] that provides individual overrides.
...@@ -618,4 +627,7 @@ class OverrideArtifacts implements Artifacts { ...@@ -618,4 +627,7 @@ class OverrideArtifacts implements Artifacts {
@override @override
String getEngineType(TargetPlatform platform, [ BuildMode mode ]) => parent.getEngineType(platform, mode); String getEngineType(TargetPlatform platform, [ BuildMode mode ]) => parent.getEngineType(platform, mode);
@override
bool get isLocalEngine => parent.isLocalEngine;
} }
...@@ -284,6 +284,8 @@ abstract class Target { ...@@ -284,6 +284,8 @@ abstract class Target {
/// } /// }
class Environment { class Environment {
/// Create a new [Environment] object. /// Create a new [Environment] object.
///
/// [engineVersion] should be set to null for local engine builds.
factory Environment({ factory Environment({
@required Directory projectDir, @required Directory projectDir,
@required Directory outputDir, @required Directory outputDir,
...@@ -293,6 +295,7 @@ class Environment { ...@@ -293,6 +295,7 @@ class Environment {
@required Logger logger, @required Logger logger,
@required Artifacts artifacts, @required Artifacts artifacts,
@required ProcessManager processManager, @required ProcessManager processManager,
@required String engineVersion,
Directory buildDir, Directory buildDir,
Map<String, String> defines = const <String, String>{}, Map<String, String> defines = const <String, String>{},
Map<String, String> inputs = const <String, String>{}, Map<String, String> inputs = const <String, String>{},
...@@ -303,6 +306,10 @@ class Environment { ...@@ -303,6 +306,10 @@ class Environment {
String buildPrefix; String buildPrefix;
final List<String> keys = defines.keys.toList()..sort(); final List<String> keys = defines.keys.toList()..sort();
final StringBuffer buffer = StringBuffer(); final StringBuffer buffer = StringBuffer();
// The engine revision is `null` for local or custom engines.
if (engineVersion != null) {
buffer.write(engineVersion);
}
for (final String key in keys) { for (final String key in keys) {
buffer.write(key); buffer.write(key);
buffer.write(defines[key]); buffer.write(defines[key]);
...@@ -326,6 +333,7 @@ class Environment { ...@@ -326,6 +333,7 @@ class Environment {
logger: logger, logger: logger,
artifacts: artifacts, artifacts: artifacts,
processManager: processManager, processManager: processManager,
engineVersion: engineVersion,
inputs: inputs, inputs: inputs,
); );
} }
...@@ -333,6 +341,7 @@ class Environment { ...@@ -333,6 +341,7 @@ class Environment {
/// Create a new [Environment] object for unit testing. /// Create a new [Environment] object for unit testing.
/// ///
/// Any directories not provided will fallback to a [testDirectory] /// Any directories not provided will fallback to a [testDirectory]
@visibleForTesting
factory Environment.test(Directory testDirectory, { factory Environment.test(Directory testDirectory, {
Directory projectDir, Directory projectDir,
Directory outputDir, Directory outputDir,
...@@ -341,6 +350,7 @@ class Environment { ...@@ -341,6 +350,7 @@ class Environment {
Directory buildDir, Directory buildDir,
Map<String, String> defines = const <String, String>{}, Map<String, String> defines = const <String, String>{},
Map<String, String> inputs = const <String, String>{}, Map<String, String> inputs = const <String, String>{},
String engineVersion,
@required FileSystem fileSystem, @required FileSystem fileSystem,
@required Logger logger, @required Logger logger,
@required Artifacts artifacts, @required Artifacts artifacts,
...@@ -358,6 +368,7 @@ class Environment { ...@@ -358,6 +368,7 @@ class Environment {
logger: logger, logger: logger,
artifacts: artifacts, artifacts: artifacts,
processManager: processManager, processManager: processManager,
engineVersion: engineVersion,
); );
} }
...@@ -373,6 +384,7 @@ class Environment { ...@@ -373,6 +384,7 @@ class Environment {
@required this.logger, @required this.logger,
@required this.fileSystem, @required this.fileSystem,
@required this.artifacts, @required this.artifacts,
@required this.engineVersion,
@required this.inputs, @required this.inputs,
}); });
...@@ -448,6 +460,9 @@ class Environment { ...@@ -448,6 +460,9 @@ class Environment {
final Artifacts artifacts; final Artifacts artifacts;
final FileSystem fileSystem; final FileSystem fileSystem;
/// The version of the current engine, or `null` if built with a local engine.
final String engineVersion;
} }
/// The result information from the build system. /// The result information from the build system.
......
...@@ -166,7 +166,19 @@ class SourceVisitor implements ResolvedFiles { ...@@ -166,7 +166,19 @@ class SourceVisitor implements ResolvedFiles {
/// Visit a [Source] which is defined by an [Artifact] from the flutter cache. /// Visit a [Source] which is defined by an [Artifact] from the flutter cache.
/// ///
/// If the [Artifact] points to a directory then all child files are included. /// If the [Artifact] points to a directory then all child files are included.
/// To increase the performance of builds that use a known revision of Flutter,
/// these are updated to point towards the engine.version file instead of
/// the artifact itself.
void visitArtifact(Artifact artifact, TargetPlatform platform, BuildMode mode) { void visitArtifact(Artifact artifact, TargetPlatform platform, BuildMode mode) {
// This is not a local engine.
if (environment.engineVersion != null) {
sources.add(environment.flutterRootDir
.childDirectory('bin')
.childDirectory('internal')
.childFile('engine.version'),
);
return;
}
final String path = environment.artifacts final String path = environment.artifacts
.getArtifactPath(artifact, platform: platform, mode: mode); .getArtifactPath(artifact, platform: platform, mode: mode);
if (environment.fileSystem.isDirectorySync(path)) { if (environment.fileSystem.isDirectorySync(path)) {
...@@ -175,9 +187,9 @@ class SourceVisitor implements ResolvedFiles { ...@@ -175,9 +187,9 @@ class SourceVisitor implements ResolvedFiles {
if (entity is File) if (entity is File)
entity, entity,
]); ]);
} else { return;
sources.add(environment.fileSystem.file(path));
} }
sources.add(environment.fileSystem.file(path));
} }
} }
......
...@@ -122,6 +122,9 @@ Future<void> buildWithAssemble({ ...@@ -122,6 +122,9 @@ Future<void> buildWithAssemble({
buildDir: flutterProject.dartTool.childDirectory('flutter_build'), buildDir: flutterProject.dartTool.childDirectory('flutter_build'),
cacheDir: globals.cache.getRoot(), cacheDir: globals.cache.getRoot(),
flutterRootDir: globals.fs.directory(Cache.flutterRoot), flutterRootDir: globals.fs.directory(Cache.flutterRoot),
engineVersion: globals.artifacts.isLocalEngine
? null
: globals.flutterVersion.engineRevision,
defines: <String, String>{ defines: <String, String>{
kTargetFile: mainPath, kTargetFile: mainPath,
kBuildMode: getNameForBuildMode(buildMode), kBuildMode: getNameForBuildMode(buildMode),
......
...@@ -164,6 +164,9 @@ class AssembleCommand extends FlutterCommand { ...@@ -164,6 +164,9 @@ class AssembleCommand extends FlutterCommand {
fileSystem: globals.fs, fileSystem: globals.fs,
logger: globals.logger, logger: globals.logger,
processManager: globals.processManager, processManager: globals.processManager,
engineVersion: globals.artifacts.isLocalEngine
? null
: globals.flutterVersion.engineRevision
); );
return result; return result;
} }
......
...@@ -462,6 +462,9 @@ end ...@@ -462,6 +462,9 @@ end
fileSystem: globals.fs, fileSystem: globals.fs,
logger: globals.logger, logger: globals.logger,
processManager: globals.processManager, processManager: globals.processManager,
engineVersion: globals.artifacts.isLocalEngine
? null
: globals.flutterVersion.engineRevision,
); );
final BuildResult result = await buildSystem.build(target, environment); final BuildResult result = await buildSystem.build(target, environment);
if (!result.success) { if (!result.success) {
......
...@@ -13,6 +13,7 @@ import '../build_system/build_system.dart'; ...@@ -13,6 +13,7 @@ import '../build_system/build_system.dart';
import '../build_system/targets/dart.dart'; import '../build_system/targets/dart.dart';
import '../build_system/targets/icon_tree_shaker.dart'; import '../build_system/targets/icon_tree_shaker.dart';
import '../build_system/targets/web.dart'; import '../build_system/targets/web.dart';
import '../cache.dart';
import '../globals.dart' as globals; import '../globals.dart' as globals;
import '../platform_plugins.dart'; import '../platform_plugins.dart';
import '../plugins.dart'; import '../plugins.dart';
...@@ -37,8 +38,8 @@ Future<void> buildWeb( ...@@ -37,8 +38,8 @@ Future<void> buildWeb(
final Status status = globals.logger.startProgress('Compiling $target for the Web...', timeout: null); final Status status = globals.logger.startProgress('Compiling $target for the Web...', timeout: null);
final Stopwatch sw = Stopwatch()..start(); final Stopwatch sw = Stopwatch()..start();
try { try {
final BuildResult result = await globals.buildSystem.build(const WebServiceWorker(), Environment.test( final BuildResult result = await globals.buildSystem.build(const WebServiceWorker(), Environment(
globals.fs.currentDirectory, projectDir: globals.fs.currentDirectory,
outputDir: globals.fs.directory(getWebBuildDirectory()), outputDir: globals.fs.directory(getWebBuildDirectory()),
buildDir: flutterProject.directory buildDir: flutterProject.directory
.childDirectory('.dart_tool') .childDirectory('.dart_tool')
...@@ -56,6 +57,11 @@ Future<void> buildWeb( ...@@ -56,6 +57,11 @@ Future<void> buildWeb(
fileSystem: globals.fs, fileSystem: globals.fs,
logger: globals.logger, logger: globals.logger,
processManager: globals.processManager, processManager: globals.processManager,
cacheDir: globals.cache.getRoot(),
engineVersion: globals.artifacts.isLocalEngine
? null
: globals.flutterVersion.engineRevision,
flutterRootDir: globals.fs.directory(Cache.flutterRoot),
)); ));
if (!result.success) { if (!result.success) {
for (final ExceptionMeasurement measurement in result.exceptions.values) { for (final ExceptionMeasurement measurement in result.exceptions.values) {
......
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
import 'package:args/command_runner.dart'; import 'package:args/command_runner.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/build_system/build_system.dart'; import 'package:flutter_tools/src/build_system/build_system.dart';
import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/cache.dart';
...@@ -96,6 +97,22 @@ void main() { ...@@ -96,6 +97,22 @@ void main() {
expect(testLogger.errorText, isNot(contains(testStackTrace.toString()))); expect(testLogger.errorText, isNot(contains(testStackTrace.toString())));
}); });
testbed.test('flutter assemble does not inject engine revision with local-engine', () async {
Environment environment;
when(globals.artifacts.isLocalEngine).thenReturn(true);
when(globals.buildSystem.build(any, any, buildSystemConfig: anyNamed('buildSystemConfig')))
.thenAnswer((Invocation invocation) async {
environment = invocation.positionalArguments[1] as Environment;
return BuildResult(success: true);
});
final CommandRunner<void> commandRunner = createTestCommandRunner(AssembleCommand());
await commandRunner.run(<String>['assemble', '-o Output', 'debug_macos_bundle_flutter_assets']);
expect(environment.engineVersion, isNull);
}, overrides: <Type, Generator>{
Artifacts: () => MockLocalEngineArtifacts()
});
testbed.test('flutter assemble 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'))) when(globals.buildSystem.build(any, any, buildSystemConfig: anyNamed('buildSystemConfig')))
.thenAnswer((Invocation invocation) async { .thenAnswer((Invocation invocation) async {
...@@ -156,3 +173,4 @@ void main() { ...@@ -156,3 +173,4 @@ void main() {
} }
class MockBuildSystem extends Mock implements BuildSystem {} class MockBuildSystem extends Mock implements BuildSystem {}
class MockLocalEngineArtifacts extends Mock implements LocalEngineArtifacts {}
...@@ -84,9 +84,11 @@ void main() { ...@@ -84,9 +84,11 @@ void main() {
..inputs = const <Source>[ ..inputs = const <Source>[
Source.pattern('{PROJECT_DIR}/foo.dart'), Source.pattern('{PROJECT_DIR}/foo.dart'),
]; ];
final MockArtifacts artifacts = MockArtifacts();
when(artifacts.isLocalEngine).thenReturn(false);
environment = Environment.test( environment = Environment.test(
fileSystem.currentDirectory, fileSystem.currentDirectory,
artifacts: MockArtifacts(), artifacts: artifacts,
processManager: FakeProcessManager.any(), processManager: FakeProcessManager.any(),
fileSystem: fileSystem, fileSystem: fileSystem,
logger: BufferLogger.test(), logger: BufferLogger.test(),
......
...@@ -17,15 +17,16 @@ import '../../src/common.dart'; ...@@ -17,15 +17,16 @@ import '../../src/common.dart';
import '../../src/context.dart'; import '../../src/context.dart';
import '../../src/testbed.dart'; import '../../src/testbed.dart';
final Platform windowsPlatform = FakePlatform(
operatingSystem: 'windows',
);
void main() { 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: () {
globals.fs.directory('cache').createSync(); globals.fs.directory('cache').createSync();
final Directory outputs = globals.fs.directory('outputs') final Directory outputs = globals.fs.directory('outputs')
...@@ -37,6 +38,7 @@ void main() { ...@@ -37,6 +38,7 @@ void main() {
processManager: FakeProcessManager.any(), processManager: FakeProcessManager.any(),
fileSystem: globals.fs, fileSystem: globals.fs,
logger: globals.logger, logger: globals.logger,
engineVersion: null, // simulate a local engine.
); );
visitor = SourceVisitor(environment); visitor = SourceVisitor(environment);
environment.buildDir.createSync(recursive: true); environment.buildDir.createSync(recursive: true);
...@@ -203,7 +205,7 @@ void main() { ...@@ -203,7 +205,7 @@ void main() {
expect(visitor.sources.single.path, r'C:\foo\bar.txt'); expect(visitor.sources.single.path, r'C:\foo\bar.txt');
expect(visitor.containsNewDepfile, false); expect(visitor.containsNewDepfile, false);
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
Platform: () => mockPlatform, Platform: () => windowsPlatform,
})); }));
test('can parse depfile with spaces in paths', () => testbed.run(() { test('can parse depfile with spaces in paths', () => testbed.run(() {
...@@ -214,7 +216,26 @@ void main() { ...@@ -214,7 +216,26 @@ void main() {
expect(visitor.sources.single.path, r'foo bar.txt'); expect(visitor.sources.single.path, r'foo bar.txt');
expect(visitor.containsNewDepfile, false); expect(visitor.containsNewDepfile, false);
})); }));
test('Non-local engine builds use the engine.version file as an Artifact dependency', () => testbed.run(() {
final MockArtifacts artifacts = MockArtifacts();
final Environment environment = Environment.test(
globals.fs.currentDirectory,
artifacts: artifacts, // using real artifacts
processManager: FakeProcessManager.any(),
fileSystem: globals.fs,
logger: globals.logger,
engineVersion: 'abcdefghijklmon' // Use a versioned engine.
);
visitor = SourceVisitor(environment);
const Source fizzSource = Source.artifact(Artifact.windowsDesktopPath, platform: TargetPlatform.windows_x64);
fizzSource.accept(visitor);
expect(visitor.sources.single.path, contains('engine.version'));
verifyNever(artifacts.getArtifactPath(
any, platform: anyNamed('platform'), mode: anyNamed('mode')));
}));
} }
class MockPlatform extends Mock implements Platform {}
class MockArtifacts extends Mock implements Artifacts {} class MockArtifacts extends Mock implements Artifacts {}
...@@ -144,6 +144,8 @@ void main() { ...@@ -144,6 +144,8 @@ void main() {
any, any,
mode: anyNamed('mode') mode: anyNamed('mode')
)).thenReturn(artifactPath); )).thenReturn(artifactPath);
when(mockArtifacts.isLocalEngine)
.thenReturn(false);
when(mockBuildSystem.build( when(mockBuildSystem.build(
any, any,
......
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