diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c277c7e..581a040d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,18 @@ # Changelog +## 4.2.0 + +- Add a `clean` command by default that removes temporary files used by +dart_dev, like the compiled version of the run script. +- Use `dart compile exe` to compile the `.dart_tool/dart_dev/run.dart` script +for better startup performance on subsequent runs. This compilation step will be +cached until `tool/dart_dev/config.dart`, the installed packages, or the current +Dart SDK is changed. + +## 4.1.1 + +- Optimization pass to reduce filesystem iteration. + ## 4.1.0 - Raise maximum SDK to include Dart 3. diff --git a/lib/src/dart_dev_runner.dart b/lib/src/dart_dev_runner.dart index 7a4b352a..c23f6323 100644 --- a/lib/src/dart_dev_runner.dart +++ b/lib/src/dart_dev_runner.dart @@ -2,14 +2,27 @@ import 'dart:async'; import 'package:args/args.dart'; import 'package:args/command_runner.dart'; +import 'package:dart_dev/dart_dev.dart'; import 'dart_dev_tool.dart'; import 'events.dart' as events; +import 'tools/clean_tool.dart'; import 'utils/version.dart'; class DartDevRunner extends CommandRunner { DartDevRunner(Map commands) : super('dart_dev', 'Dart tool runner.') { + // For backwards-compatibility, only add the `clean` command if it doesn't + // conflict with any configured command. + if (!commands.containsKey('clean')) { + // Construct a new commands map here, to work around a runtime typecheck + // failure: + // `type 'CleanTool' is not a subtype of type 'FormatTool' of 'value'` + // As seen in this CI run: + // https://github.com/Workiva/dart_dev/actions/runs/8161855516/job/22311519665?pr=426#step:8:295 + commands = {...commands, 'clean': CleanTool()}; + } + commands.forEach((name, builder) { final command = builder.toCommand(name); if (command.name != name) { @@ -17,6 +30,7 @@ class DartDevRunner extends CommandRunner { } addCommand(command); }); + argParser ..addFlag('verbose', abbr: 'v', negatable: false, help: 'Enables verbose logging.') diff --git a/lib/src/executable.dart b/lib/src/executable.dart index 9b23ecd4..531fbfa3 100644 --- a/lib/src/executable.dart +++ b/lib/src/executable.dart @@ -1,13 +1,22 @@ import 'dart:async'; +import 'dart:convert'; import 'dart:io'; import 'package:analyzer/dart/analysis/utilities.dart'; import 'package:args/command_runner.dart'; +import 'package:collection/collection.dart'; +import 'package:crypto/crypto.dart'; import 'package:dart_dev/dart_dev.dart'; +import 'package:dart_dev/src/utils/parse_imports.dart'; +import 'package:dart_dev/src/utils/pubspec_lock.dart'; +import 'package:glob/glob.dart'; +import 'package:glob/list_local_fs.dart'; import 'package:io/ansi.dart'; import 'package:io/io.dart' show ExitCode; import 'package:logging/logging.dart'; import 'package:path/path.dart' as p; +import 'package:pubspec_parse/pubspec_parse.dart'; +import 'package:yaml/yaml.dart'; import 'dart_dev_runner.dart'; import 'tools/over_react_format_tool.dart'; @@ -16,7 +25,6 @@ import 'utils/cached_pubspec.dart'; import 'utils/dart_dev_paths.dart'; import 'utils/dart_tool_cache.dart'; import 'utils/ensure_process_exit.dart'; -import 'utils/executables.dart' as exe; import 'utils/format_tool_builder.dart'; import 'utils/get_dart_version_comment.dart'; import 'utils/logging.dart'; @@ -24,35 +32,37 @@ import 'utils/parse_flag_from_args.dart'; typedef _ConfigGetter = Map Function(); -final paths = DartDevPaths(); - -final _runScript = File(paths.runScript); +final _paths = DartDevPaths(); Future run(List args) async { attachLoggerToStdio(args); - final configExists = File(paths.config).existsSync(); - final oldDevDartExists = File(paths.legacyConfig).existsSync(); + final configExists = File(_paths.config).existsSync(); + final oldDevDartExists = File(_paths.legacyConfig).existsSync(); if (!configExists) { - log.fine('No custom `${paths.config}` file found; ' + log.fine('No custom `${_paths.config}` file found; ' 'using default config.'); } if (oldDevDartExists) { log.warning(yellow.wrap( - 'dart_dev v3 now expects configuration to be at `${paths.config}`,\n' - 'but `${paths.legacyConfig}` still exists. View the guide to see how to upgrade:\n' + 'dart_dev v3 now expects configuration to be at `${_paths.config}`,\n' + 'but `${_paths.legacyConfig}` still exists. View the guide to see how to upgrade:\n' 'https://github.com/Workiva/dart_dev/blob/master/doc/v3-upgrade-guide.md')); } if (args.contains('hackFastFormat') && !oldDevDartExists) { await handleFastFormat(args); - return; } - generateRunScript(); - final process = await Process.start(exe.dart, [paths.runScript, ...args], + final processArgs = generateRunScript(); + final process = await Process.start( + processArgs.first, + [ + if (processArgs.length > 1) ...processArgs.sublist(1), + ...args, + ], mode: ProcessStartMode.inheritStdio); ensureProcessExit(process); exitCode = await process.exitCode; @@ -62,7 +72,7 @@ Future handleFastFormat(List args) async { assertDirIsDartPackage(); DevTool? formatTool; - final configFile = File(paths.config); + final configFile = File(_paths.config); if (configFile.existsSync()) { final toolBuilder = FormatToolBuilder(); parseString(content: configFile.readAsStringSync()) @@ -94,25 +104,142 @@ Future handleFastFormat(List args) async { } } -void generateRunScript() { - if (shouldWriteRunScript) { +void _deleteRunExecutableAndDigest() => + [_paths.runExecutable, _paths.runExecutableDigest].forEach((p) { + final f = File(p); + if (f.existsSync()) f.deleteSync(); + }); + +/// Return true iff all the provided package names can be traced to 'hosted' +/// entries in pubspec.lock. +bool _allPackagesAreImportedImmutably(Iterable packageNames) { + File pubspecLockFile = File('pubspec.lock'); + if (!pubspecLockFile.existsSync()) return false; + final pubSpecLock = loadYamlDocument(pubspecLockFile.readAsStringSync()); + return getDependencySources(pubSpecLock, packageNames) + .values + .every((source) => source == 'hosted' || source == 'git'); +} + +/// Return null iff it is not possible to account for all +/// recompilation-necessitating factors in the digest. +String? _computeRunScriptDigest() { + final currentPackageName = + Pubspec.parse(File('pubspec.yaml').readAsStringSync()).name; + final configFile = File(_paths.config); + var configHasRelativeImports = false; + if (configFile.existsSync()) { + final configImports = parseImports(configFile.readAsStringSync()); + configHasRelativeImports = configImports.any((i) => !i.contains(':')); + final configHasSamePackageImports = + configImports.any((i) => i.startsWith('package:$currentPackageName')); + + if (configHasSamePackageImports) { + log.fine( + 'Skipping compilation because ${_paths.config} imports from its own package.'); + + // If the config imports from its own source files, we don't have a way of + // efficiently tracking changes that would require recompilation of this + // executable, so we skip the compilation altogether. + _deleteRunExecutableAndDigest(); + return null; + } + + final packageNames = computePackageNamesFromImports(configImports); + if (!_allPackagesAreImportedImmutably(packageNames)) { + log.fine( + 'Skipping compilation because ${_paths.config} imports non-hosted packages.'); + _deleteRunExecutableAndDigest(); + return null; + } + } + + // Include the packageConfig in the digest so that when dependencies change, + // we recompile. + final packageConfig = File(_paths.packageConfig); + + final digest = md5.convert([ + if (packageConfig.existsSync()) ...packageConfig.readAsBytesSync(), + if (configFile.existsSync()) ...configFile.readAsBytesSync(), + if (configHasRelativeImports) + for (final file in Glob('tool/**.dart', recursive: true) + .listSync() + .whereType() + .where((f) => p.canonicalize(f.path) != p.canonicalize(_paths.config)) + .sortedBy((f) => f.path)) + ...file.readAsBytesSync(), + ]); + return base64.encode(digest.bytes); +} + +List generateRunScript() { + // Generate the run script if it doesn't yet exist or regenerate it if the + // existing script is outdated. + final runScriptContents = buildDartDevRunScriptContents(); + final runScript = File(_paths.runScript); + final runExecutable = File(_paths.runExecutable); + final runExecutableDigest = File(_paths.runExecutableDigest); + if (!runScript.existsSync() || + runScript.readAsStringSync() != runScriptContents) { logTimedSync(log, 'Generating run script', () { createCacheDir(); - _runScript.writeAsStringSync(buildDartDevRunScriptContents()); + runScript.writeAsStringSync(runScriptContents); }, level: Level.INFO); } -} -bool get shouldWriteRunScript => - !_runScript.existsSync() || - _runScript.readAsStringSync() != buildDartDevRunScriptContents(); + // Generate a digest of inputs to the run script. We use this to determine + // whether we need to recompile the executable. + String? encodedDigest; + logTimedSync(log, 'Computing run script digest', + () => encodedDigest = _computeRunScriptDigest(), + level: Level.FINE); + + if (encodedDigest != null && + (!runExecutableDigest.existsSync() || + runExecutableDigest.readAsStringSync() != encodedDigest)) { + // Digest is missing or outdated, so we (re-)compile. + final logMessage = runExecutable.existsSync() + ? 'Recompiling run script (digest changed)' + : 'Compiling run script'; + logTimedSync(log, logMessage, () { + // Delete the previous executable and digest so that if we hit a failure + // trying to compile, we don't leave the outdated one in place. + _deleteRunExecutableAndDigest(); + final args = [ + 'compile', + 'exe', + _paths.runScript, + '-o', + _paths.runExecutable + ]; + final result = Process.runSync(Platform.executable, args); + if (result.exitCode == 0) { + // Compilation succeeded. Write the new digest, as well. + runExecutableDigest.writeAsStringSync(encodedDigest!); + } else { + // Compilation failed. Dump some logs for debugging, but note to the + // user that dart_dev should still work. + log.warning( + 'Could not compile run script; dart_dev will continue without precompilation.'); + log.fine('CMD: ${Platform.executable} ${args.join(" ")}'); + log.fine('STDOUT:\n${result.stdout}'); + log.fine('STDERR:\n${result.stderr}'); + } + }); + } + + return runExecutable.existsSync() + // Using the absolute path is necessary for Windows to find the executable. + ? [runExecutable.absolute.path] + : [Platform.executable, 'run', _paths.runScript]; +} String buildDartDevRunScriptContents() { - final hasCustomToolDevDart = File(paths.config).existsSync(); + final hasCustomToolDevDart = File(_paths.config).existsSync(); // If the config has a dart version comment (e.g., if it opts out of null safety), // copy it over to the entrypoint so the program is run in that language version. var dartVersionComment = hasCustomToolDevDart - ? getDartVersionComment(File(paths.config).readAsStringSync()) + ? getDartVersionComment(File(_paths.config).readAsStringSync()) : null; return ''' @@ -121,7 +248,7 @@ import 'dart:io'; import 'package:dart_dev/src/core_config.dart'; import 'package:dart_dev/src/executable.dart' as executable; -${hasCustomToolDevDart ? "import '${paths.configFromRunScriptForDart}' as custom_dev;" : ""} +${hasCustomToolDevDart ? "import '${_paths.configFromRunScriptForDart}' as custom_dev;" : ""} void main(List args) async { await executable.runWithConfig(args, @@ -149,7 +276,7 @@ Future runWithConfig( config = configGetter(); } catch (error) { stderr - ..writeln('Invalid "${paths.config}" in ${p.absolute(p.current)}') + ..writeln('Invalid "${_paths.config}" in ${p.absolute(p.current)}') ..writeln() ..writeln('It should provide a `Map config;` getter,' ' but it either does not exist or threw unexpectedly:') diff --git a/lib/src/tools/clean_tool.dart b/lib/src/tools/clean_tool.dart new file mode 100644 index 00000000..46a184b9 --- /dev/null +++ b/lib/src/tools/clean_tool.dart @@ -0,0 +1,25 @@ +import 'dart:async'; +import 'dart:io'; + +import 'package:dart_dev/src/utils/logging.dart'; +import 'package:io/io.dart'; + +import '../dart_dev_tool.dart'; +import '../utils/dart_dev_paths.dart' show DartDevPaths; + +class CleanTool extends DevTool { + @override + final String? description = 'Cleans up temporary files used by dart_dev.'; + + @override + FutureOr run([DevToolExecutionContext? context]) { + final cache = Directory(DartDevPaths().cache()); + if (cache.existsSync()) { + log.info('Deleting ${cache.path}'); + cache.deleteSync(recursive: true); + } else { + log.info('Nothing to do: no ${cache.path} found'); + } + return ExitCode.success.code; + } +} diff --git a/lib/src/utils/dart_dev_paths.dart b/lib/src/utils/dart_dev_paths.dart index 99ebfc42..6547cdd3 100644 --- a/lib/src/utils/dart_dev_paths.dart +++ b/lib/src/utils/dart_dev_paths.dart @@ -25,7 +25,14 @@ class DartDevPaths { from: p.url.absolute(_cacheForDart), ); + String get packageConfig => + _context.join('.dart_tool', 'package_config.json'); + String get legacyConfig => _context.join('tool', 'dev.dart'); String get runScript => cache('run.dart'); + + String get runExecutable => cache('run'); + + String get runExecutableDigest => cache('run.digest'); } diff --git a/lib/src/utils/parse_imports.dart b/lib/src/utils/parse_imports.dart new file mode 100644 index 00000000..e6618e1f --- /dev/null +++ b/lib/src/utils/parse_imports.dart @@ -0,0 +1,18 @@ +import 'package:collection/collection.dart'; + +/// Return the contents of the enquoted portion of the import statements in the +/// file. Not 100% accurate, since we use regular expressions instead of the +/// Dart AST to extract the imports. +Iterable parseImports(String fileContents) => + _importRegex.allMatches(fileContents).map((m) => m.group(1)).whereNotNull(); + +final _importRegex = + RegExp(r'''^import ['"]([^'"]+)['"];?$''', multiLine: true); + +/// Return a set of package names given a list of imports. +Set computePackageNamesFromImports(Iterable imports) => imports + .map((i) => _packageNameFromImportRegex.matchAsPrefix(i)?.group(1)) + .whereNotNull() + .toSet(); + +final _packageNameFromImportRegex = RegExp(r'package:([^/]+)/.+'); diff --git a/lib/src/utils/pubspec_lock.dart b/lib/src/utils/pubspec_lock.dart new file mode 100644 index 00000000..74d780d9 --- /dev/null +++ b/lib/src/utils/pubspec_lock.dart @@ -0,0 +1,27 @@ +import 'dart:collection'; + +import 'package:yaml/yaml.dart'; + +/// Index into the pubspecLock to locate the 'source' field for the given +/// package. +String? _getPubSpecLockPackageSource( + YamlDocument pubSpecLock, String packageName) { + final contents = pubSpecLock.contents; + if (contents is YamlMap) { + final packages = contents['packages']; + if (packages is YamlMap) { + final specificDependency = packages[packageName]; + if (specificDependency is YamlMap) return specificDependency['source']; + } + } + return null; +} + +/// Return a mapping of package name to dependency 'type', using the pubspec +/// lock document. If a package cannot be located in the pubspec lock document, +/// it will map to null. +HashMap getDependencySources( + YamlDocument pubspecLockDocument, Iterable packageNames) => + HashMap.fromIterable(packageNames, + value: (name) => + _getPubSpecLockPackageSource(pubspecLockDocument, name)); diff --git a/pubspec.yaml b/pubspec.yaml index 2ff3ca3d..23d7f240 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -14,6 +14,7 @@ dependencies: args: ^2.0.0 async: ^2.5.0 build_runner: ^2.0.0 + crypto: ^3.0.1 glob: ^2.0.0 io: ^1.0.0 logging: ^1.0.0 diff --git a/test/utils/parse_imports_test.dart b/test/utils/parse_imports_test.dart new file mode 100644 index 00000000..5717f0f8 --- /dev/null +++ b/test/utils/parse_imports_test.dart @@ -0,0 +1,61 @@ +import 'package:dart_dev/src/utils/parse_imports.dart'; +import 'package:test/test.dart'; + +void main() { + final expectedImportList = [ + 'dart:async', + 'dart:convert', + 'dart:io', + 'package:analyzer/dart/analysis/utilities.dart', + 'package:dart_dev/dart_dev.dart', + 'package:dart_dev/src/tools/over_react_format_tool.dart', + 'package:dart_dev/src/utils/format_tool_builder.dart', + 'package:test/test.dart', + 'utils/assert_dir_is_dart_package.dart', + 'utils/cached_pubspec.dart', + 'utils/dart_dev_paths.dart', + ]; + group( + 'parseImports', + () => test('correctly returns all imports', + () => expect(parseImports(sampleFile), equals(expectedImportList)))); + + group( + 'computePackageNamesFromImports', + () => test( + 'correctly computes package names from imports', + () => expect(computePackageNamesFromImports(expectedImportList), + equals(['analyzer', 'dart_dev', 'test'])))); +} + +const sampleFile = ''' +@TestOn('vm') +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:analyzer/dart/analysis/utilities.dart'; +import 'package:dart_dev/dart_dev.dart'; +import 'package:dart_dev/src/tools/over_react_format_tool.dart'; +import 'package:dart_dev/src/utils/format_tool_builder.dart'; +import 'package:test/test.dart'; + +import 'utils/assert_dir_is_dart_package.dart'; +import 'utils/cached_pubspec.dart'; +import 'utils/dart_dev_paths.dart'; + +void main() { + group('FormatToolBuilder', () { + group('detects an OverReactFormatTool correctly', () { + test('when the tool is a MethodInvocation', () { + final visitor = FormatToolBuilder(); + + parseString(content: orfNoCascadeSrc).unit.accept(visitor); + + expect(visitor.formatDevTool, isNotNull); + expect(visitor.formatDevTool, isA()); + }); + }); + }); +} +''';