Unverified Commit 5ab9e707 authored by Chris Bracken's avatar Chris Bracken Committed by GitHub

Revert "Eliminate snapshot/depfile options to build bundle (#21507)" (#21563)

This tickled a bug in KernelCompiler.compile() where the fingerprinter
doesn't include the outputFilePath in its list of dependencies. As such,
if the output .dill file is missing or corrupted, the fingerprint still
matches and re-compile is skipped, even though it shouldn't be. I'll fix
that in a followup, then look at how this triggered that issue. My
hypothesis is that that it's due to the aot kernel compile and bundle
kernel compile have separate output directories for the .dill files
(build/ vs build/aot) but the same output directory for the associated
depfiles (due to this patch).

This reverts commit 43a106e9.
parent 11943ce7
...@@ -176,7 +176,9 @@ BuildApp() { ...@@ -176,7 +176,9 @@ BuildApp() {
build bundle \ build bundle \
--target-platform=ios \ --target-platform=ios \
--target="${target_path}" \ --target="${target_path}" \
--snapshot="${build_dir}/snapshot_blob.bin" \
--${build_mode} \ --${build_mode} \
--depfile="${build_dir}/snapshot_blob.bin.d" \
--asset-dir="${derived_dir}/flutter_assets" \ --asset-dir="${derived_dir}/flutter_assets" \
${precompilation_flag} \ ${precompilation_flag} \
${local_engine_flag} \ ${local_engine_flag} \
......
...@@ -409,6 +409,11 @@ abstract class BaseFlutterTask extends DefaultTask { ...@@ -409,6 +409,11 @@ abstract class BaseFlutterTask extends DefaultTask {
// first stage of AOT build in this mode, and it includes all the Dart // first stage of AOT build in this mode, and it includes all the Dart
// sources. // sources.
depfiles += project.files("${intermediateDir}/kernel_compile.d") depfiles += project.files("${intermediateDir}/kernel_compile.d")
// Include Core JIT kernel compiler depfile, since kernel compile is
// the first stage of JIT builds in this mode, and it includes all the
// Dart sources.
depfiles += project.files("${intermediateDir}/snapshot_blob.bin.d")
return depfiles return depfiles
} }
...@@ -490,6 +495,8 @@ abstract class BaseFlutterTask extends DefaultTask { ...@@ -490,6 +495,8 @@ abstract class BaseFlutterTask extends DefaultTask {
} }
if (buildMode == "release" || buildMode == "profile") { if (buildMode == "release" || buildMode == "profile") {
args "--precompiled" args "--precompiled"
} else {
args "--depfile", "${intermediateDir}/snapshot_blob.bin.d"
} }
args "--asset-dir", "${intermediateDir}/flutter_assets" args "--asset-dir", "${intermediateDir}/flutter_assets"
if (buildMode == "debug") { if (buildMode == "debug") {
......
...@@ -311,10 +311,12 @@ class AOTSnapshotter { ...@@ -311,10 +311,12 @@ class AOTSnapshotter {
if ((extraFrontEndOptions != null) && extraFrontEndOptions.isNotEmpty) if ((extraFrontEndOptions != null) && extraFrontEndOptions.isNotEmpty)
printTrace('Extra front-end options: $extraFrontEndOptions'); printTrace('Extra front-end options: $extraFrontEndOptions');
final String depfilePath = fs.path.join(outputPath, 'kernel_compile.d');
final CompilerOutput compilerOutput = await kernelCompiler.compile( final CompilerOutput compilerOutput = await kernelCompiler.compile(
sdkRoot: artifacts.getArtifactPath(Artifact.flutterPatchedSdkPath), sdkRoot: artifacts.getArtifactPath(Artifact.flutterPatchedSdkPath),
mainPath: mainPath, mainPath: mainPath,
outputFilePath: fs.path.join(outputPath, 'app.dill'), outputFilePath: fs.path.join(outputPath, 'app.dill'),
depFilePath: depfilePath,
extraFrontEndOptions: extraFrontEndOptions, extraFrontEndOptions: extraFrontEndOptions,
linkPlatformKernelIn: true, linkPlatformKernelIn: true,
aot: true, aot: true,
......
...@@ -18,6 +18,8 @@ import 'globals.dart'; ...@@ -18,6 +18,8 @@ import 'globals.dart';
const String defaultMainPath = 'lib/main.dart'; const String defaultMainPath = 'lib/main.dart';
const String defaultAssetBasePath = '.'; const String defaultAssetBasePath = '.';
const String defaultManifestPath = 'pubspec.yaml'; const String defaultManifestPath = 'pubspec.yaml';
String get defaultSnapshotPath => fs.path.join(getBuildDirectory(), 'snapshot_blob.bin');
String get defaultDepfilePath => fs.path.join(getBuildDirectory(), 'snapshot_blob.bin.d');
String get defaultApplicationKernelPath => fs.path.join(getBuildDirectory(), 'app.dill'); String get defaultApplicationKernelPath => fs.path.join(getBuildDirectory(), 'app.dill');
const String defaultPrivateKeyPath = 'privatekey.der'; const String defaultPrivateKeyPath = 'privatekey.der';
...@@ -34,7 +36,9 @@ Future<void> build({ ...@@ -34,7 +36,9 @@ Future<void> build({
BuildMode buildMode, BuildMode buildMode,
String mainPath = defaultMainPath, String mainPath = defaultMainPath,
String manifestPath = defaultManifestPath, String manifestPath = defaultManifestPath,
String snapshotPath,
String applicationKernelFilePath, String applicationKernelFilePath,
String depfilePath,
String privateKeyPath = defaultPrivateKeyPath, String privateKeyPath = defaultPrivateKeyPath,
String assetDirPath, String assetDirPath,
String packagesPath, String packagesPath,
...@@ -47,6 +51,8 @@ Future<void> build({ ...@@ -47,6 +51,8 @@ Future<void> build({
List<String> fileSystemRoots, List<String> fileSystemRoots,
String fileSystemScheme, String fileSystemScheme,
}) async { }) async {
snapshotPath ??= defaultSnapshotPath;
depfilePath ??= defaultDepfilePath;
assetDirPath ??= getAssetBuildDirectory(); assetDirPath ??= getAssetBuildDirectory();
packagesPath ??= fs.path.absolute(PackageMap.globalPackagesPath); packagesPath ??= fs.path.absolute(PackageMap.globalPackagesPath);
applicationKernelFilePath ??= defaultApplicationKernelPath; applicationKernelFilePath ??= defaultApplicationKernelPath;
...@@ -62,6 +68,7 @@ Future<void> build({ ...@@ -62,6 +68,7 @@ Future<void> build({
fs.path.absolute(getIncrementalCompilerByteStoreDirectory()), fs.path.absolute(getIncrementalCompilerByteStoreDirectory()),
mainPath: fs.file(mainPath).absolute.path, mainPath: fs.file(mainPath).absolute.path,
outputFilePath: applicationKernelFilePath, outputFilePath: applicationKernelFilePath,
depFilePath: depfilePath,
trackWidgetCreation: trackWidgetCreation, trackWidgetCreation: trackWidgetCreation,
extraFrontEndOptions: extraFrontEndOptions, extraFrontEndOptions: extraFrontEndOptions,
fileSystemRoots: fileSystemRoots, fileSystemRoots: fileSystemRoots,
......
...@@ -22,6 +22,8 @@ class BuildBundleCommand extends BuildSubCommand { ...@@ -22,6 +22,8 @@ class BuildBundleCommand extends BuildSubCommand {
..addOption('asset-base', help: 'Ignored. Will be removed.', hide: !verboseHelp) ..addOption('asset-base', help: 'Ignored. Will be removed.', hide: !verboseHelp)
..addOption('manifest', defaultsTo: defaultManifestPath) ..addOption('manifest', defaultsTo: defaultManifestPath)
..addOption('private-key', defaultsTo: defaultPrivateKeyPath) ..addOption('private-key', defaultsTo: defaultPrivateKeyPath)
..addOption('snapshot', defaultsTo: defaultSnapshotPath)
..addOption('depfile', defaultsTo: defaultDepfilePath)
..addOption('kernel-file', defaultsTo: defaultApplicationKernelPath) ..addOption('kernel-file', defaultsTo: defaultApplicationKernelPath)
..addOption('target-platform', ..addOption('target-platform',
defaultsTo: 'android-arm', defaultsTo: 'android-arm',
...@@ -83,7 +85,9 @@ class BuildBundleCommand extends BuildSubCommand { ...@@ -83,7 +85,9 @@ class BuildBundleCommand extends BuildSubCommand {
buildMode: buildMode, buildMode: buildMode,
mainPath: targetFile, mainPath: targetFile,
manifestPath: argResults['manifest'], manifestPath: argResults['manifest'],
snapshotPath: argResults['snapshot'],
applicationKernelFilePath: argResults['kernel-file'], applicationKernelFilePath: argResults['kernel-file'],
depfilePath: argResults['depfile'],
privateKeyPath: argResults['private-key'], privateKeyPath: argResults['private-key'],
assetDirPath: argResults['asset-dir'], assetDirPath: argResults['asset-dir'],
precompiledSnapshot: argResults['precompiled'], precompiledSnapshot: argResults['precompiled'],
......
...@@ -10,11 +10,9 @@ import 'package:usage/uuid/uuid.dart'; ...@@ -10,11 +10,9 @@ import 'package:usage/uuid/uuid.dart';
import 'artifacts.dart'; import 'artifacts.dart';
import 'base/common.dart'; import 'base/common.dart';
import 'base/context.dart'; import 'base/context.dart';
import 'base/file_system.dart';
import 'base/fingerprint.dart'; import 'base/fingerprint.dart';
import 'base/io.dart'; import 'base/io.dart';
import 'base/process_manager.dart'; import 'base/process_manager.dart';
import 'build_info.dart';
import 'globals.dart'; import 'globals.dart';
KernelCompiler get kernelCompiler => context[KernelCompiler]; KernelCompiler get kernelCompiler => context[KernelCompiler];
...@@ -76,6 +74,7 @@ class KernelCompiler { ...@@ -76,6 +74,7 @@ class KernelCompiler {
String sdkRoot, String sdkRoot,
String mainPath, String mainPath,
String outputFilePath, String outputFilePath,
String depFilePath,
bool linkPlatformKernelIn = false, bool linkPlatformKernelIn = false,
bool aot = false, bool aot = false,
List<String> entryPointsJsonFiles, List<String> entryPointsJsonFiles,
...@@ -94,22 +93,24 @@ class KernelCompiler { ...@@ -94,22 +93,24 @@ class KernelCompiler {
// TODO(cbracken): eliminate pathFilter. // TODO(cbracken): eliminate pathFilter.
// Currently the compiler emits buildbot paths for the core libs in the // Currently the compiler emits buildbot paths for the core libs in the
// depfile. None of these are available on the local host. // depfile. None of these are available on the local host.
final String depfilePath = fs.path.join(getBuildDirectory(), 'kernel_compile.d'); Fingerprinter fingerprinter;
final Fingerprinter fingerprinter = new Fingerprinter( if (depFilePath != null) {
fingerprintPath: '$depfilePath.fingerprint', fingerprinter = new Fingerprinter(
paths: <String>[mainPath], fingerprintPath: '$depFilePath.fingerprint',
properties: <String, String>{ paths: <String>[mainPath],
'entryPoint': mainPath, properties: <String, String>{
'trackWidgetCreation': trackWidgetCreation.toString(), 'entryPoint': mainPath,
'linkPlatformKernelIn': linkPlatformKernelIn.toString(), 'trackWidgetCreation': trackWidgetCreation.toString(),
}, 'linkPlatformKernelIn': linkPlatformKernelIn.toString(),
depfilePaths: <String>[depfilePath], },
pathFilter: (String path) => !path.startsWith('/b/build/slave/'), depfilePaths: <String>[depFilePath],
); pathFilter: (String path) => !path.startsWith('/b/build/slave/'),
);
if (await fingerprinter.doesFingerprintMatch()) {
printTrace('Skipping kernel compilation. Fingerprint match.'); if (await fingerprinter.doesFingerprintMatch()) {
return new CompilerOutput(outputFilePath, 0); printTrace('Skipping kernel compilation. Fingerprint match.');
return new CompilerOutput(outputFilePath, 0);
}
} }
// This is a URI, not a file path, so the forward slash is correct even on Windows. // This is a URI, not a file path, so the forward slash is correct even on Windows.
...@@ -152,8 +153,8 @@ class KernelCompiler { ...@@ -152,8 +153,8 @@ class KernelCompiler {
if (outputFilePath != null) { if (outputFilePath != null) {
command.addAll(<String>['--output-dill', outputFilePath]); command.addAll(<String>['--output-dill', outputFilePath]);
} }
if (fileSystemRoots == null || fileSystemRoots.isEmpty) { if (depFilePath != null && (fileSystemRoots == null || fileSystemRoots.isEmpty)) {
command.addAll(<String>['--depfile', depfilePath]); command.addAll(<String>['--depfile', depFilePath]);
} }
if (fileSystemRoots != null) { if (fileSystemRoots != null) {
for (String root in fileSystemRoots) { for (String root in fileSystemRoots) {
...@@ -185,7 +186,9 @@ class KernelCompiler { ...@@ -185,7 +186,9 @@ class KernelCompiler {
.listen(_stdoutHandler.handler); .listen(_stdoutHandler.handler);
final int exitCode = await server.exitCode; final int exitCode = await server.exitCode;
if (exitCode == 0) { if (exitCode == 0) {
await fingerprinter.writeFingerprint(); if (fingerprinter != null) {
await fingerprinter.writeFingerprint();
}
return _stdoutHandler.compilerOutput.future; return _stdoutHandler.compilerOutput.future;
} }
return null; return null;
......
...@@ -169,6 +169,7 @@ Hello! ...@@ -169,6 +169,7 @@ Hello!
incrementalCompilerByteStorePath: anyNamed('incrementalCompilerByteStorePath'), incrementalCompilerByteStorePath: anyNamed('incrementalCompilerByteStorePath'),
mainPath: anyNamed('mainPath'), mainPath: anyNamed('mainPath'),
outputFilePath: anyNamed('outputFilePath'), outputFilePath: anyNamed('outputFilePath'),
depFilePath: anyNamed('depFilePath'),
trackWidgetCreation: anyNamed('trackWidgetCreation'), trackWidgetCreation: anyNamed('trackWidgetCreation'),
extraFrontEndOptions: anyNamed('extraFrontEndOptions'), extraFrontEndOptions: anyNamed('extraFrontEndOptions'),
fileSystemRoots: anyNamed('fileSystemRoots'), fileSystemRoots: anyNamed('fileSystemRoots'),
......
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