Skip to content

Commit

Permalink
[dartdev] Add --depfile option to dart compile
Browse files Browse the repository at this point in the history
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
[email protected]

Change-Id: Id674f7353342c8275a8a0c4a70e3f5eaeb7f05d5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/367023
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Slava Egorov <[email protected]>
  • Loading branch information
mraleph authored and Commit Queue committed May 23, 2024
1 parent 5cda2a8 commit 9451535
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 12 deletions.
2 changes: 2 additions & 0 deletions pkg/dart2native/lib/dart2native.dart
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ Future<ProcessResult> generateKernelHelper({
List<String> extraGenKernelOptions = const [],
String? nativeAssets,
String? resourcesFile,
String? depFile,
bool enableAsserts = false,
bool fromDill = false,
bool aot = false,
Expand All @@ -138,6 +139,7 @@ Future<ProcessResult> 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,
Expand Down
43 changes: 35 additions & 8 deletions pkg/dart2native/lib/generate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<SnapshotGenerator> generate({String? resourcesFile}) =>
_generator.generateKernel(resourcesFile: resourcesFile);
Future<SnapshotGenerator> generate({
String? resourcesFile,
List<String>? extraOptions,
}) =>
_generator.generateKernel(
resourcesFile: resourcesFile,
extraOptions: extraOptions,
);
}

/// Second step of generating a snapshot is generating the snapshot itself.
Expand Down Expand Up @@ -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<String> defines,
Expand All @@ -150,6 +161,7 @@ class _Generator {
String? debugFile,
String? packages,
String? targetOS,
String? depFile,
required String enableExperiment,
required bool enableAsserts,
required bool verbose,
Expand All @@ -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) {
Expand All @@ -178,7 +191,10 @@ class _Generator {
}
}

Future<SnapshotGenerator> generateKernel({String? resourcesFile}) async {
Future<SnapshotGenerator> generateKernel({
String? resourcesFile,
List<String>? extraOptions,
}) async {
if (_verbose) {
if (_targetOS != null) {
print('Specializing Platform getters for target OS $_targetOS.');
Expand All @@ -192,13 +208,16 @@ class _Generator {
kernelFile: _programKernelFile,
packages: _packages,
defines: _defines,
depFile: _depFile,
fromDill: await isKernelFile(_sourcePath),
enableAsserts: _enableAsserts,
enableExperiment: _enableExperiment,
targetOS: _targetOS,
extraGenKernelOptions: [
'--invocation-modes=compile',
'--verbosity=$_verbosity',
if (_depFile != null) '--depfile-target=$_outputPath',
...?extraOptions,
],
resourcesFile: resourcesFile,
aot: true,
Expand All @@ -218,17 +237,21 @@ class _Generator {
await _generateSnapshot(extraOptions, kernelFile);
}

Future<void> _generateSnapshot(
List<String> 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<void> _generateSnapshot(
List<String> extraOptions,
String kernelFile,
) async {
final outputPath = _outputPath;
final debugPath = _normalize(_debugFile);

if (_verbose) {
Expand Down Expand Up @@ -360,6 +383,8 @@ Future<void> generateKernel({
bool verbose = false,
String? nativeAssets,
String? resourcesFile,
String? depFile,
List<String>? extraOptions,
}) async {
final sourcePath = _normalize(sourceFile)!;
final outputPath = _normalize(outputFile)!;
Expand All @@ -375,9 +400,11 @@ Future<void> generateKernel({
embedSources: embedSources,
fromDill: await isKernelFile(sourcePath),
enableExperiment: enableExperiment,
depFile: depFile,
extraGenKernelOptions: [
'--invocation-modes=compile',
'--verbosity=$verbosity',
...?extraOptions,
],
nativeAssets: nativeAssets,
resourcesFile: resourcesFile,
Expand Down
38 changes: 35 additions & 3 deletions pkg/dartdev/lib/src/commands/compile.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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')!,
Expand Down Expand Up @@ -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.
<path> can be relative or absolute.''')
<path> 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)
Expand Down Expand Up @@ -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'),
);
Expand Down
46 changes: 46 additions & 0 deletions pkg/dartdev/test/commands/compile_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1461,4 +1461,50 @@ void main() {
await basicCompileTest();
}, skip: isRunningOnIA32);
}

// Tests for --depfile for compiling to AOT snapshots, executables and
// kernel.
group('depfiles', () {
Future<void> 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);
});
}
12 changes: 11 additions & 1 deletion pkg/vm/lib/kernel_front_end.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -209,6 +214,7 @@ Future<int> 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<String>? fileSystemRoots = options['filesystem-root'];
final String? targetOS = options['target-os'];
Expand Down Expand Up @@ -373,7 +379,11 @@ Future<int> runCompiler(ArgResults options, String usage) async {

if (depfile != null) {
await writeDepfile(
fileSystem, results.compiledSources!, outputFileName, depfile);
fileSystem,
results.compiledSources!,
depfileTarget ?? outputFileName,
depfile,
);
}

if (splitOutputByPackages) {
Expand Down

0 comments on commit 9451535

Please sign in to comment.