From 945153584c4b9dcede7db1562ae98b6e3f30b225 Mon Sep 17 00:00:00 2001 From: Vyacheslav Egorov Date: Thu, 23 May 2024 12:58:29 +0000 Subject: [PATCH] [dartdev] Add --depfile option to `dart compile` I would like to add an option to our BUILD files to precompile gen_kernel and compile_platform to speedup iteration cycles when working on core library changes. To make this robust I would like to use depfile to track dependencies and know when compile_platform should be recompiled. TEST=pkg/dartdev/test/commands/compile_test.dart R=kustermann@google.com Change-Id: Id674f7353342c8275a8a0c4a70e3f5eaeb7f05d5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/367023 Reviewed-by: Martin Kustermann Commit-Queue: Slava Egorov --- pkg/dart2native/lib/dart2native.dart | 2 + pkg/dart2native/lib/generate.dart | 43 +++++++++++++++---- pkg/dartdev/lib/src/commands/compile.dart | 38 +++++++++++++++-- pkg/dartdev/test/commands/compile_test.dart | 46 +++++++++++++++++++++ pkg/vm/lib/kernel_front_end.dart | 12 +++++- 5 files changed, 129 insertions(+), 12 deletions(-) diff --git a/pkg/dart2native/lib/dart2native.dart b/pkg/dart2native/lib/dart2native.dart index 621911c7e1fc..5293fe9168dc 100644 --- a/pkg/dart2native/lib/dart2native.dart +++ b/pkg/dart2native/lib/dart2native.dart @@ -116,6 +116,7 @@ Future generateKernelHelper({ List extraGenKernelOptions = const [], String? nativeAssets, String? resourcesFile, + String? depFile, bool enableAsserts = false, bool fromDill = false, bool aot = false, @@ -138,6 +139,7 @@ Future generateKernelHelper({ if (packages != null) '--packages=$packages', if (nativeAssets != null) '--native-assets=$nativeAssets', if (resourcesFile != null) '--resources-file=$resourcesFile', + if (depFile != null) '--depfile=$depFile', '--output=$kernelFile', ...extraGenKernelOptions, if (sourceFile != null) sourceFile, diff --git a/pkg/dart2native/lib/generate.dart b/pkg/dart2native/lib/generate.dart index 7f1f6d1b5f19..8bf6ec2d36ff 100644 --- a/pkg/dart2native/lib/generate.dart +++ b/pkg/dart2native/lib/generate.dart @@ -41,6 +41,7 @@ extension type KernelGenerator._(_Generator _generator) { String? debugFile, String? packages, String? targetOS, + String? depFile, String enableExperiment = '', bool enableAsserts = false, bool verbose = false, @@ -59,14 +60,21 @@ extension type KernelGenerator._(_Generator _generator) { targetOS: targetOS, verbose: verbose, verbosity: verbosity, + depFile: depFile, ); /// Generate a kernel file, /// /// [resourcesFile] is the path to `resources.json`, where the tree-shaking /// information collected during kernel compilation is stored. - Future generate({String? resourcesFile}) => - _generator.generateKernel(resourcesFile: resourcesFile); + Future generate({ + String? resourcesFile, + List? extraOptions, + }) => + _generator.generateKernel( + resourcesFile: resourcesFile, + extraOptions: extraOptions, + ); } /// Second step of generating a snapshot is generating the snapshot itself. @@ -142,6 +150,9 @@ class _Generator { /// The path to the `.dart_tool/package_config.json`. final String? _packages; + /// The path to the [depfile](https://ninja-build.org/manual.html#_depfile). + final String? _depFile; + _Generator({ required String sourceFile, required List defines, @@ -150,6 +161,7 @@ class _Generator { String? debugFile, String? packages, String? targetOS, + String? depFile, required String enableExperiment, required bool enableAsserts, required bool verbose, @@ -165,6 +177,7 @@ class _Generator { _debugFile = debugFile, _outputFile = outputFile, _defines = defines, + _depFile = depFile, _programKernelFile = path.join(tempDir.path, 'program.dill'), _sourcePath = _normalize(sourceFile)!, _packages = _normalize(packages) { @@ -178,7 +191,10 @@ class _Generator { } } - Future generateKernel({String? resourcesFile}) async { + Future generateKernel({ + String? resourcesFile, + List? extraOptions, + }) async { if (_verbose) { if (_targetOS != null) { print('Specializing Platform getters for target OS $_targetOS.'); @@ -192,6 +208,7 @@ class _Generator { kernelFile: _programKernelFile, packages: _packages, defines: _defines, + depFile: _depFile, fromDill: await isKernelFile(_sourcePath), enableAsserts: _enableAsserts, enableExperiment: _enableExperiment, @@ -199,6 +216,8 @@ class _Generator { extraGenKernelOptions: [ '--invocation-modes=compile', '--verbosity=$_verbosity', + if (_depFile != null) '--depfile-target=$_outputPath', + ...?extraOptions, ], resourcesFile: resourcesFile, aot: true, @@ -218,17 +237,21 @@ class _Generator { await _generateSnapshot(extraOptions, kernelFile); } - Future _generateSnapshot( - List extraOptions, - String kernelFile, - ) async { + String get _outputPath { final sourceWithoutDartOrDill = _sourcePath.replaceFirst( RegExp(r'\.(dart|dill)$'), '', ); - final outputPath = _normalize( + return _normalize( _outputFile ?? _kind.appendFileExtension(sourceWithoutDartOrDill), )!; + } + + Future _generateSnapshot( + List extraOptions, + String kernelFile, + ) async { + final outputPath = _outputPath; final debugPath = _normalize(_debugFile); if (_verbose) { @@ -360,6 +383,8 @@ Future generateKernel({ bool verbose = false, String? nativeAssets, String? resourcesFile, + String? depFile, + List? extraOptions, }) async { final sourcePath = _normalize(sourceFile)!; final outputPath = _normalize(outputFile)!; @@ -375,9 +400,11 @@ Future generateKernel({ embedSources: embedSources, fromDill: await isKernelFile(sourcePath), enableExperiment: enableExperiment, + depFile: depFile, extraGenKernelOptions: [ '--invocation-modes=compile', '--verbosity=$verbosity', + ...?extraOptions, ], nativeAssets: nativeAssets, resourcesFile: resourcesFile, diff --git a/pkg/dartdev/lib/src/commands/compile.dart b/pkg/dartdev/lib/src/commands/compile.dart index cd8b0ea1b837..34772dea0e48 100644 --- a/pkg/dartdev/lib/src/commands/compile.dart +++ b/pkg/dartdev/lib/src/commands/compile.dart @@ -184,6 +184,17 @@ class CompileKernelSnapshotCommand extends CompileSubcommandCommand { defaultsTo: soundNullSafetyOption.flagDefaultsTo, hide: true, ) + ..addOption( + 'depfile', + valueHelp: 'path', + help: 'Path to output Ninja depfile', + ) + ..addMultiOption( + 'extra-gen-kernel-options', + help: 'Pass additional options to gen_kernel.', + hide: true, + valueHelp: 'opt1,opt2,...', + ) ..addExperimentalFlags(verbose: verbose); } @@ -233,6 +244,8 @@ class CompileKernelSnapshotCommand extends CompileSubcommandCommand { packages: args.option('packages'), enableExperiment: args.enabledExperiments.join(','), linkPlatform: args.flag('link-platform'), + depFile: args.option('depfile'), + extraOptions: args.multiOption('extra-gen-kernel-options'), embedSources: args.flag('embed-sources'), verbose: verbose, verbosity: args.option('verbosity')!, @@ -425,15 +438,31 @@ class CompileNativeCommand extends CompileSubcommandCommand { help: soundNullSafetyOption.help, defaultsTo: soundNullSafetyOption.flagDefaultsTo, hide: true) - ..addOption('save-debugging-info', abbr: 'S', valueHelp: 'path', help: ''' + ..addOption( + 'save-debugging-info', + abbr: 'S', + valueHelp: 'path', + help: ''' Remove debugging information from the output and save it separately to the specified file. - can be relative or absolute.''') + can be relative or absolute.''', + ) + ..addOption( + 'depfile', + valueHelp: 'path', + help: 'Path to output Ninja depfile', + ) ..addMultiOption( 'extra-gen-snapshot-options', help: 'Pass additional options to gen_snapshot.', hide: true, valueHelp: 'opt1,opt2,...', ) + ..addMultiOption( + 'extra-gen-kernel-options', + help: 'Pass additional options to gen_kernel.', + hide: true, + valueHelp: 'opt1,opt2,...', + ) ..addOption('target-os', help: 'Compile to a specific target operating system.', allowed: TargetOS.names) @@ -521,8 +550,11 @@ Remove debugging information from the output and save it separately to the speci verbosity: args.option('verbosity')!, targetOS: targetOS, tempDir: tempDir, + depFile: args.option('depfile'), + ); + final snapshotGenerator = await kernelGenerator.generate( + extraOptions: args.multiOption('extra-gen-kernel-options'), ); - final snapshotGenerator = await kernelGenerator.generate(); await snapshotGenerator.generate( extraOptions: args.multiOption('extra-gen-snapshot-options'), ); diff --git a/pkg/dartdev/test/commands/compile_test.dart b/pkg/dartdev/test/commands/compile_test.dart index c76a15b18fcd..4fc81487b077 100644 --- a/pkg/dartdev/test/commands/compile_test.dart +++ b/pkg/dartdev/test/commands/compile_test.dart @@ -1461,4 +1461,50 @@ void main() { await basicCompileTest(); }, skip: isRunningOnIA32); } + + // Tests for --depfile for compiling to AOT snapshots, executables and + // kernel. + group('depfiles', () { + Future testDepFileGeneration(String subcommand) async { + final p = project(mainSrc: '''void main() {}'''); + final inFile = + path.canonicalize(path.join(p.dirPath, p.relativeFilePath)); + final outFile = + path.canonicalize(path.join(p.dirPath, 'output.$subcommand')); + final depFile = + path.canonicalize(path.join(p.dirPath, 'output.$subcommand.d')); + + final result = await p.run( + [ + 'compile', + subcommand, + '--depfile', + depFile, + '-o', + outFile, + inFile, + ], + ); + + expect(result.stderr, isEmpty); + expect(result.exitCode, 0); + + expect(File(depFile).existsSync(), isTrue); + + final depFileContent = File(depFile).readAsStringSync(); + + String escapePath(String path) => + path.replaceAll('\\', '\\\\').replaceAll(' ', '\\ '); + + expect(depFileContent, startsWith('${escapePath(outFile)}: ')); + expect(depFileContent, contains(escapePath(inFile))); + } + + test('compile aot-snapshot', () => testDepFileGeneration('aot-snapshot'), + skip: isRunningOnIA32); + test('compile exe', () => testDepFileGeneration('exe'), + skip: isRunningOnIA32); + test('compile kernel', () => testDepFileGeneration('kernel'), + skip: isRunningOnIA32); + }); } diff --git a/pkg/vm/lib/kernel_front_end.dart b/pkg/vm/lib/kernel_front_end.dart index 460876fb6d7e..1db9eaa0a126 100644 --- a/pkg/vm/lib/kernel_front_end.dart +++ b/pkg/vm/lib/kernel_front_end.dart @@ -84,6 +84,11 @@ void declareCompilerOptions(ArgParser args) { defaultsTo: null); args.addFlag('compact-async', help: 'Obsolete, ignored.', hide: true); args.addOption('depfile', help: 'Path to output Ninja depfile'); + args.addOption( + 'depfile-target', + help: 'Override the target in the generated depfile', + hide: true, + ); args.addOption('from-dill', help: 'Read existing dill file instead of compiling from sources', defaultsTo: null); @@ -209,6 +214,7 @@ Future runCompiler(ArgResults options, String usage) async { final String targetName = options['target']; final String? fileSystemScheme = options['filesystem-scheme']; final String? depfile = options['depfile']; + final String? depfileTarget = options['depfile-target']; final String? fromDillFile = options['from-dill']; final List? fileSystemRoots = options['filesystem-root']; final String? targetOS = options['target-os']; @@ -373,7 +379,11 @@ Future runCompiler(ArgResults options, String usage) async { if (depfile != null) { await writeDepfile( - fileSystem, results.compiledSources!, outputFileName, depfile); + fileSystem, + results.compiledSources!, + depfileTarget ?? outputFileName, + depfile, + ); } if (splitOutputByPackages) {