More details from getExecutableForCommand (#3186)
diff --git a/lib/pub.dart b/lib/pub.dart index b2fed29..5b3054f 100644 --- a/lib/pub.dart +++ b/lib/pub.dart
@@ -6,7 +6,11 @@ import 'src/command_runner.dart'; import 'src/pub_embeddable_command.dart'; export 'src/executable.dart' - show getExecutableForCommand, CommandResolutionFailedException; + show + getExecutableForCommand, + CommandResolutionFailedException, + CommandResolutionIssue, + DartExecutableWithPackageConfig; export 'src/pub_embeddable_command.dart' show PubAnalytics; /// Returns a [Command] for pub functionality that can be used by an embedding
diff --git a/lib/src/executable.dart b/lib/src/executable.dart index 8c7be58..2112acc 100644 --- a/lib/src/executable.dart +++ b/lib/src/executable.dart
@@ -7,6 +7,7 @@ import 'dart:isolate'; import 'package:args/args.dart'; +import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; import 'entrypoint.dart'; @@ -202,7 +203,23 @@ } } -/// Returns the path to dart program/snapshot to invoke for running [descriptor] +/// The result of a `getExecutableForCommand` command resolution. +@sealed +class DartExecutableWithPackageConfig { + /// Can be a .dart file or a incremental snapshot. + final String executable; + + /// The package_config.json to run [executable] with. Or <null> if the VM + /// should find it according to the standard rules. + final String? packageConfig; + + DartExecutableWithPackageConfig({ + required this.executable, + required this.packageConfig, + }); +} + +/// Returns the dart program/snapshot to invoke for running [descriptor] /// resolved according to the package configuration of the package at [root] /// (defaulting to the current working directory). Using the pub-cache at /// [pubCacheDir] (defaulting to the default pub cache). @@ -237,7 +254,7 @@ /// * `` and `:` both resolves to `<current>:bin/<current>.dart` or /// `bin/<current>:main.dart`. /// -/// If that doesn't resolve as an existing file throw an exception. +/// If that doesn't resolve as an existing file, throw an exception. /// /// ## Snapshotting /// @@ -250,7 +267,7 @@ /// /// Throws an [CommandResolutionFailedException] if the command is not found or /// if the entrypoint is not up to date (requires `pub get`) and a `pub get`. -Future<String> getExecutableForCommand( +Future<DartExecutableWithPackageConfig> getExecutableForCommand( String descriptor, { bool allowSnapshot = true, String? root, @@ -270,71 +287,127 @@ } final asDirectFile = p.join(root, asPath); - if (fileExists(asDirectFile)) return p.relative(asDirectFile, from: root); - if (!fileExists(p.join(root, 'pubspec.yaml'))) { - throw CommandResolutionFailedException('Could not find file `$descriptor`'); + if (fileExists(asDirectFile)) { + return DartExecutableWithPackageConfig( + executable: p.relative(asDirectFile, from: root), + packageConfig: null, + ); } + if (!fileExists(p.join(root, 'pubspec.yaml'))) { + throw CommandResolutionFailedException._( + 'Could not find file `$descriptor`', + CommandResolutionIssue.fileNotFound); + } + final entrypoint = Entrypoint(root, SystemCache(rootDir: pubCacheDir)); try { - final entrypoint = Entrypoint(root, SystemCache(rootDir: pubCacheDir)); + // TODO(sigurdm): it would be nicer with a 'isUpToDate' function. + entrypoint.assertUpToDate(); + } on DataException { try { - // TODO(sigurdm): it would be nicer with a 'isUpToDate' function. - entrypoint.assertUpToDate(); - } on DataException { await warningsOnlyUnlessTerminal( () => entrypoint.acquireDependencies( SolveType.GET, analytics: analytics, ), ); + } on ApplicationException catch (e) { + throw CommandResolutionFailedException._( + e.toString(), CommandResolutionIssue.pubGetFailed); } + } - String command; - String package; - if (descriptor.contains(':')) { - final parts = descriptor.split(':'); - if (parts.length > 2) { - throw CommandResolutionFailedException( - '[<package>[:command]] cannot contain multiple ":"'); - } - package = parts[0]; - if (package.isEmpty) package = entrypoint.root.name; - command = parts[1]; - } else { - package = descriptor; - if (package.isEmpty) package = entrypoint.root.name; - command = package; + late final String command; + String package; + if (descriptor.contains(':')) { + final parts = descriptor.split(':'); + if (parts.length > 2) { + throw CommandResolutionFailedException._( + '[<package>[:command]] cannot contain multiple ":"', + CommandResolutionIssue.parseError, + ); } + package = parts[0]; + if (package.isEmpty) package = entrypoint.root.name; + command = parts[1]; + } else { + package = descriptor; + if (package.isEmpty) package = entrypoint.root.name; + command = package; + } - final executable = Executable(package, p.join('bin', '$command.dart')); - if (!entrypoint.packageGraph.packages.containsKey(package)) { - throw CommandResolutionFailedException( - 'Could not find package `$package` or file `$descriptor`'); - } - final path = entrypoint.resolveExecutable(executable); - if (!fileExists(path)) { - throw CommandResolutionFailedException( - 'Could not find `bin${p.separator}$command.dart` in package `$package`.'); - } - if (!allowSnapshot) { - return p.relative(path, from: root); - } else { - final snapshotPath = entrypoint.pathOfExecutable(executable); - if (!fileExists(snapshotPath) || - entrypoint.packageGraph.isPackageMutable(package)) { + if (!entrypoint.packageGraph.packages.containsKey(package)) { + throw CommandResolutionFailedException._( + 'Could not find package `$package` or file `$descriptor`', + CommandResolutionIssue.packageNotFound, + ); + } + final executable = Executable(package, p.join('bin', '$command.dart')); + final packageConfig = p.join('.dart_tool', 'package_config.json'); + + final path = entrypoint.resolveExecutable(executable); + if (!fileExists(path)) { + throw CommandResolutionFailedException._( + 'Could not find `bin${p.separator}$command.dart` in package `$package`.', + CommandResolutionIssue.noBinaryFound, + ); + } + if (!allowSnapshot) { + return DartExecutableWithPackageConfig( + executable: p.relative(path, from: root), + packageConfig: packageConfig, + ); + } else { + final snapshotPath = entrypoint.pathOfExecutable(executable); + if (!fileExists(snapshotPath) || + entrypoint.packageGraph.isPackageMutable(package)) { + try { await warningsOnlyUnlessTerminal( () => entrypoint.precompileExecutable(executable), ); + } on ApplicationException catch (e) { + throw CommandResolutionFailedException._( + e.toString(), + CommandResolutionIssue.compilationFailed, + ); } - return p.relative(snapshotPath, from: root); } - } on ApplicationException catch (e) { - throw CommandResolutionFailedException(e.toString()); + return DartExecutableWithPackageConfig( + executable: p.relative(snapshotPath, from: root), + packageConfig: packageConfig, + ); } } +/// Information on why no executable is returned. +enum CommandResolutionIssue { + /// The command string looked like a file (contained '.' '/' or '\\'), but no + /// such file exists. + fileNotFound, + + /// The command-string was '<package>:<binary>' or '<package>', and <package> + /// was not in dependencies. + packageNotFound, + + /// The command string was '<package>:<binary>' or ':<binary>' and <binary> + /// was not found. + noBinaryFound, + + /// Failed retrieving dependencies (pub get). + pubGetFailed, + + /// Pre-compilation of the binary failed. + compilationFailed, + + /// The command string did not have a valid form (eg. more than one ':'). + parseError, +} + +/// Indicates that a command string did not resolve to an executable. +@sealed class CommandResolutionFailedException implements Exception { final String message; - CommandResolutionFailedException(this.message); + final CommandResolutionIssue issue; + CommandResolutionFailedException._(this.message, this.issue); @override String toString() {
diff --git a/test/embedding/get_executable_for_command_test.dart b/test/embedding/get_executable_for_command_test.dart index e174f3a..7667e12 100644 --- a/test/embedding/get_executable_for_command_test.dart +++ b/test/embedding/get_executable_for_command_test.dart
@@ -9,15 +9,26 @@ import 'package:path/path.dart' show separator; import 'package:path/path.dart' as p; import 'package:pub/pub.dart'; +import 'package:pub/src/log.dart' as log; + import 'package:test/test.dart'; import '../descriptor.dart' as d; import '../test_pub.dart'; -Future<void> testGetExecutable(String command, String root, - {allowSnapshot = true, result, errorMessage}) async { +Future<void> testGetExecutable( + String command, + String root, { + allowSnapshot = true, + executable, + packageConfig, + errorMessage, + CommandResolutionIssue issue, +}) async { final _cachePath = getPubTestEnvironment()['PUB_CACHE']; - if (result == null) { + final oldVerbosity = log.verbosity; + log.verbosity = log.Verbosity.NONE; + if (executable == null) { expect( () => getExecutableForCommand( command, @@ -27,18 +38,25 @@ ), throwsA( isA<CommandResolutionFailedException>() - .having((e) => e.message, 'message', errorMessage), + .having((e) => e.message, 'message', errorMessage) + .having((e) => e.issue, 'issue', issue), ), ); } else { - final path = await getExecutableForCommand( + final e = await getExecutableForCommand( command, root: root, pubCacheDir: _cachePath, allowSnapshot: allowSnapshot, ); - expect(path, result); - expect(File(p.join(root, path)).existsSync(), true); + expect( + e, + isA<DartExecutableWithPackageConfig>() + .having((e) => e.executable, 'executable', executable) + .having((e) => e.packageConfig, 'packageConfig', packageConfig), + ); + expect(File(p.join(root, e.executable)).existsSync(), true); + log.verbosity = oldVerbosity; } } @@ -50,13 +68,13 @@ final dir = d.path('foo'); await testGetExecutable('bar/bar.dart', dir, - result: p.join('bar', 'bar.dart')); + executable: p.join('bar', 'bar.dart')); await testGetExecutable(p.join('bar', 'bar.dart'), dir, - result: p.join('bar', 'bar.dart')); + executable: p.join('bar', 'bar.dart')); await testGetExecutable('${p.toUri(dir)}/bar/bar.dart', dir, - result: p.join('bar', 'bar.dart')); + executable: p.join('bar', 'bar.dart')); }); test('Looks for file when no pubspec.yaml', () async { @@ -66,9 +84,11 @@ final dir = d.path('foo'); await testGetExecutable('bar/m.dart', dir, - errorMessage: contains('Could not find file `bar/m.dart`')); + errorMessage: contains('Could not find file `bar/m.dart`'), + issue: CommandResolutionIssue.fileNotFound); await testGetExecutable(p.join('bar', 'm.dart'), dir, - errorMessage: contains('Could not find file `bar${separator}m.dart`')); + errorMessage: contains('Could not find file `bar${separator}m.dart`'), + issue: CommandResolutionIssue.fileNotFound); }); test('Error message when pubspec is broken', () async { @@ -95,13 +115,15 @@ contains( 'Error on line 1, column 9 of ${d.sandbox}${p.separator}foo${p.separator}pubspec.yaml: "name" field must be a valid Dart identifier.'), contains( - '{"name":"broken name","environment":{"sdk":">=0.1.2 <1.0.0"}}'))); + '{"name":"broken name","environment":{"sdk":">=0.1.2 <1.0.0"}}')), + issue: CommandResolutionIssue.pubGetFailed); }); test('Does `pub get` if there is a pubspec.yaml', () async { await d.dir(appPath, [ d.pubspec({ 'name': 'myapp', + 'environment': {'sdk': '>=$_currentVersion <3.0.0'}, 'dependencies': {'foo': '^1.0.0'} }), d.dir('bin', [ @@ -112,8 +134,49 @@ await serveNoPackages(); // The solver uses word-wrapping in its error message, so we use \s to // accomodate. - await testGetExecutable('bar/m.dart', d.path(appPath), - errorMessage: matches(r'version\s+solving\s+failed')); + await testGetExecutable( + 'bar/m.dart', + d.path(appPath), + errorMessage: matches(r'version\s+solving\s+failed'), + issue: CommandResolutionIssue.pubGetFailed, + ); + }); + + test('Reports parse failure', () async { + await d.dir(appPath, [ + d.pubspec({ + 'name': 'myapp', + 'environment': {'sdk': '>=$_currentVersion <3.0.0'}, + }), + ]).create(); + await testGetExecutable( + '::', + d.path(appPath), + errorMessage: contains(r'cannot contain multiple ":"'), + issue: CommandResolutionIssue.parseError, + ); + }); + + test('Reports compilation failure', () async { + await d.dir(appPath, [ + d.pubspec({ + 'name': 'myapp', + 'environment': {'sdk': '>=$_currentVersion <3.0.0'}, + }), + d.dir('bin', [ + d.file('foo.dart', 'main() {'), + ]) + ]).create(); + + await serveNoPackages(); + // The solver uses word-wrapping in its error message, so we use \s to + // accomodate. + await testGetExecutable( + ':foo', + d.path(appPath), + errorMessage: matches(r'foo.dart:1:8:'), + issue: CommandResolutionIssue.compilationFailed, + ); }); test('Finds files', () async { @@ -148,46 +211,81 @@ ]).create(); final dir = d.path(appPath); - await testGetExecutable('myapp', dir, - result: p.join('.dart_tool', 'pub', 'bin', 'myapp', - 'myapp.dart-$_currentVersion.snapshot')); - await testGetExecutable('myapp:myapp', dir, - result: p.join('.dart_tool', 'pub', 'bin', 'myapp', - 'myapp.dart-$_currentVersion.snapshot')); - await testGetExecutable(':myapp', dir, - result: p.join('.dart_tool', 'pub', 'bin', 'myapp', - 'myapp.dart-$_currentVersion.snapshot')); - await testGetExecutable(':tool', dir, - result: p.join('.dart_tool', 'pub', 'bin', 'myapp', - 'tool.dart-$_currentVersion.snapshot')); - await testGetExecutable('foo', dir, - allowSnapshot: false, - result: endsWith('foo-1.0.0${separator}bin${separator}foo.dart')); - await testGetExecutable('foo', dir, - result: - '.dart_tool${separator}pub${separator}bin${separator}foo${separator}foo.dart-$_currentVersion.snapshot'); - await testGetExecutable('foo:tool', dir, - allowSnapshot: false, - result: endsWith('foo-1.0.0${separator}bin${separator}tool.dart')); - await testGetExecutable('foo:tool', dir, - result: - '.dart_tool${separator}pub${separator}bin${separator}foo${separator}tool.dart-$_currentVersion.snapshot'); + await testGetExecutable( + 'myapp', + dir, + executable: p.join('.dart_tool', 'pub', 'bin', 'myapp', + 'myapp.dart-$_currentVersion.snapshot'), + packageConfig: p.join('.dart_tool', 'package_config.json'), + ); + await testGetExecutable( + 'myapp:myapp', + dir, + executable: p.join('.dart_tool', 'pub', 'bin', 'myapp', + 'myapp.dart-$_currentVersion.snapshot'), + packageConfig: p.join('.dart_tool', 'package_config.json'), + ); + await testGetExecutable( + ':myapp', + dir, + executable: p.join('.dart_tool', 'pub', 'bin', 'myapp', + 'myapp.dart-$_currentVersion.snapshot'), + packageConfig: p.join('.dart_tool', 'package_config.json'), + ); + await testGetExecutable( + ':tool', + dir, + executable: p.join('.dart_tool', 'pub', 'bin', 'myapp', + 'tool.dart-$_currentVersion.snapshot'), + packageConfig: p.join('.dart_tool', 'package_config.json'), + ); + await testGetExecutable( + 'foo', + dir, + allowSnapshot: false, + executable: endsWith('foo-1.0.0${separator}bin${separator}foo.dart'), + packageConfig: p.join('.dart_tool', 'package_config.json'), + ); + await testGetExecutable( + 'foo', + dir, + executable: + '.dart_tool${separator}pub${separator}bin${separator}foo${separator}foo.dart-$_currentVersion.snapshot', + packageConfig: p.join('.dart_tool', 'package_config.json'), + ); + await testGetExecutable( + 'foo:tool', + dir, + allowSnapshot: false, + executable: endsWith('foo-1.0.0${separator}bin${separator}tool.dart'), + packageConfig: p.join('.dart_tool', 'package_config.json'), + ); + await testGetExecutable( + 'foo:tool', + dir, + executable: + '.dart_tool${separator}pub${separator}bin${separator}foo${separator}tool.dart-$_currentVersion.snapshot', + packageConfig: p.join('.dart_tool', 'package_config.json'), + ); await testGetExecutable( 'unknown:tool', dir, errorMessage: 'Could not find package `unknown` or file `unknown:tool`', + issue: CommandResolutionIssue.packageNotFound, ); await testGetExecutable( 'foo:unknown', dir, errorMessage: 'Could not find `bin${separator}unknown.dart` in package `foo`.', + issue: CommandResolutionIssue.noBinaryFound, ); await testGetExecutable( 'unknownTool', dir, errorMessage: 'Could not find package `unknownTool` or file `unknownTool`', + issue: CommandResolutionIssue.packageNotFound, ); }); }