Skip to content

Commit

Permalink
Eliminate snapshot/depfile options to build bundle (flutter#21507)
Browse files Browse the repository at this point in the history
The --snapshot argument was only necessary in Dart 1. The --depfile
argument was only used in Dart 2 mode to pass to the kernel compiler,
but was inconsistent with the 'build aot' command, where the depfile was
always set to build/kernel_compile.d.

This patch updates 'build bundle' to emit the depfile to a location
consistent with the 'build aot' command; since it's not intended to be
user-configurable and flutter.gradle hardcodes the location to
build/kernel_compile.d either way, this patch also eliminates the
ability to configure the filename altogether.
  • Loading branch information
cbracken authored Sep 7, 2018
1 parent a2acc6a commit 43a106e
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 47 deletions.
2 changes: 0 additions & 2 deletions packages/flutter_tools/bin/xcode_backend.sh
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,7 @@ BuildApp() {
build bundle \
--target-platform=ios \
--target="${target_path}" \
--snapshot="${build_dir}/snapshot_blob.bin" \
--${build_mode} \
--depfile="${build_dir}/snapshot_blob.bin.d" \
--asset-dir="${derived_dir}/flutter_assets" \
${precompilation_flag} \
${local_engine_flag} \
Expand Down
7 changes: 0 additions & 7 deletions packages/flutter_tools/gradle/flutter.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -409,11 +409,6 @@ abstract class BaseFlutterTask extends DefaultTask {
// first stage of AOT build in this mode, and it includes all the Dart
// sources.
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
}

Expand Down Expand Up @@ -495,8 +490,6 @@ abstract class BaseFlutterTask extends DefaultTask {
}
if (buildMode == "release" || buildMode == "profile") {
args "--precompiled"
} else {
args "--depfile", "${intermediateDir}/snapshot_blob.bin.d"
}
args "--asset-dir", "${intermediateDir}/flutter_assets"
if (buildMode == "debug") {
Expand Down
2 changes: 0 additions & 2 deletions packages/flutter_tools/lib/src/base/build.dart
Original file line number Diff line number Diff line change
Expand Up @@ -311,12 +311,10 @@ class AOTSnapshotter {
if ((extraFrontEndOptions != null) && extraFrontEndOptions.isNotEmpty)
printTrace('Extra front-end options: $extraFrontEndOptions');

final String depfilePath = fs.path.join(outputPath, 'kernel_compile.d');
final CompilerOutput compilerOutput = await kernelCompiler.compile(
sdkRoot: artifacts.getArtifactPath(Artifact.flutterPatchedSdkPath),
mainPath: mainPath,
outputFilePath: fs.path.join(outputPath, 'app.dill'),
depFilePath: depfilePath,
extraFrontEndOptions: extraFrontEndOptions,
linkPlatformKernelIn: true,
aot: true,
Expand Down
7 changes: 0 additions & 7 deletions packages/flutter_tools/lib/src/bundle.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ import 'globals.dart';
const String defaultMainPath = 'lib/main.dart';
const String defaultAssetBasePath = '.';
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');
const String defaultPrivateKeyPath = 'privatekey.der';

Expand All @@ -36,9 +34,7 @@ Future<void> build({
BuildMode buildMode,
String mainPath = defaultMainPath,
String manifestPath = defaultManifestPath,
String snapshotPath,
String applicationKernelFilePath,
String depfilePath,
String privateKeyPath = defaultPrivateKeyPath,
String assetDirPath,
String packagesPath,
Expand All @@ -51,8 +47,6 @@ Future<void> build({
List<String> fileSystemRoots,
String fileSystemScheme,
}) async {
snapshotPath ??= defaultSnapshotPath;
depfilePath ??= defaultDepfilePath;
assetDirPath ??= getAssetBuildDirectory();
packagesPath ??= fs.path.absolute(PackageMap.globalPackagesPath);
applicationKernelFilePath ??= defaultApplicationKernelPath;
Expand All @@ -68,7 +62,6 @@ Future<void> build({
fs.path.absolute(getIncrementalCompilerByteStoreDirectory()),
mainPath: fs.file(mainPath).absolute.path,
outputFilePath: applicationKernelFilePath,
depFilePath: depfilePath,
trackWidgetCreation: trackWidgetCreation,
extraFrontEndOptions: extraFrontEndOptions,
fileSystemRoots: fileSystemRoots,
Expand Down
4 changes: 0 additions & 4 deletions packages/flutter_tools/lib/src/commands/build_bundle.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ class BuildBundleCommand extends BuildSubCommand {
..addOption('asset-base', help: 'Ignored. Will be removed.', hide: !verboseHelp)
..addOption('manifest', defaultsTo: defaultManifestPath)
..addOption('private-key', defaultsTo: defaultPrivateKeyPath)
..addOption('snapshot', defaultsTo: defaultSnapshotPath)
..addOption('depfile', defaultsTo: defaultDepfilePath)
..addOption('kernel-file', defaultsTo: defaultApplicationKernelPath)
..addOption('target-platform',
defaultsTo: 'android-arm',
Expand Down Expand Up @@ -85,9 +83,7 @@ class BuildBundleCommand extends BuildSubCommand {
buildMode: buildMode,
mainPath: targetFile,
manifestPath: argResults['manifest'],
snapshotPath: argResults['snapshot'],
applicationKernelFilePath: argResults['kernel-file'],
depfilePath: argResults['depfile'],
privateKeyPath: argResults['private-key'],
assetDirPath: argResults['asset-dir'],
precompiledSnapshot: argResults['precompiled'],
Expand Down
45 changes: 21 additions & 24 deletions packages/flutter_tools/lib/src/compile.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ import 'package:usage/uuid/uuid.dart';
import 'artifacts.dart';
import 'base/common.dart';
import 'base/context.dart';
import 'base/file_system.dart';
import 'base/fingerprint.dart';
import 'base/io.dart';
import 'base/process_manager.dart';
import 'build_info.dart';
import 'globals.dart';

KernelCompiler get kernelCompiler => context[KernelCompiler];
Expand Down Expand Up @@ -74,7 +76,6 @@ class KernelCompiler {
String sdkRoot,
String mainPath,
String outputFilePath,
String depFilePath,
bool linkPlatformKernelIn = false,
bool aot = false,
List<String> entryPointsJsonFiles,
Expand All @@ -93,24 +94,22 @@ class KernelCompiler {
// TODO(cbracken): eliminate pathFilter.
// Currently the compiler emits buildbot paths for the core libs in the
// depfile. None of these are available on the local host.
Fingerprinter fingerprinter;
if (depFilePath != null) {
fingerprinter = new Fingerprinter(
fingerprintPath: '$depFilePath.fingerprint',
paths: <String>[mainPath],
properties: <String, String>{
'entryPoint': mainPath,
'trackWidgetCreation': trackWidgetCreation.toString(),
'linkPlatformKernelIn': linkPlatformKernelIn.toString(),
},
depfilePaths: <String>[depFilePath],
pathFilter: (String path) => !path.startsWith('/b/build/slave/'),
);

if (await fingerprinter.doesFingerprintMatch()) {
printTrace('Skipping kernel compilation. Fingerprint match.');
return new CompilerOutput(outputFilePath, 0);
}
final String depfilePath = fs.path.join(getBuildDirectory(), 'kernel_compile.d');
final Fingerprinter fingerprinter = new Fingerprinter(
fingerprintPath: '$depfilePath.fingerprint',
paths: <String>[mainPath],
properties: <String, String>{
'entryPoint': mainPath,
'trackWidgetCreation': trackWidgetCreation.toString(),
'linkPlatformKernelIn': linkPlatformKernelIn.toString(),
},
depfilePaths: <String>[depfilePath],
pathFilter: (String path) => !path.startsWith('/b/build/slave/'),
);

if (await fingerprinter.doesFingerprintMatch()) {
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.
Expand Down Expand Up @@ -153,8 +152,8 @@ class KernelCompiler {
if (outputFilePath != null) {
command.addAll(<String>['--output-dill', outputFilePath]);
}
if (depFilePath != null && (fileSystemRoots == null || fileSystemRoots.isEmpty)) {
command.addAll(<String>['--depfile', depFilePath]);
if (fileSystemRoots == null || fileSystemRoots.isEmpty) {
command.addAll(<String>['--depfile', depfilePath]);
}
if (fileSystemRoots != null) {
for (String root in fileSystemRoots) {
Expand Down Expand Up @@ -186,9 +185,7 @@ class KernelCompiler {
.listen(_stdoutHandler.handler);
final int exitCode = await server.exitCode;
if (exitCode == 0) {
if (fingerprinter != null) {
await fingerprinter.writeFingerprint();
}
await fingerprinter.writeFingerprint();
return _stdoutHandler.compilerOutput.future;
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ Hello!
incrementalCompilerByteStorePath: anyNamed('incrementalCompilerByteStorePath'),
mainPath: anyNamed('mainPath'),
outputFilePath: anyNamed('outputFilePath'),
depFilePath: anyNamed('depFilePath'),
trackWidgetCreation: anyNamed('trackWidgetCreation'),
extraFrontEndOptions: anyNamed('extraFrontEndOptions'),
fileSystemRoots: anyNamed('fileSystemRoots'),
Expand Down

0 comments on commit 43a106e

Please sign in to comment.