Allow invoking `getExecutableForCommand` everywhere in workspace (#4257)
diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index ad14962..4a99513 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart
@@ -740,22 +740,29 @@ /// /// If [onlyOutputWhenTerminal] is `true` (the default) there will be no /// output if no terminal is attached. - static Future<PackageConfig> ensureUpToDate( + /// + /// When succesfull returns the found/created `PackageConfig` and the + /// directory containing it. + static Future<({PackageConfig packageConfig, String rootDir})> ensureUpToDate( String dir, { required SystemCache cache, bool summaryOnly = true, bool onlyOutputWhenTerminal = true, }) async { - final lockFilePath = p.normalize(p.join(dir, 'pubspec.lock')); - final packageConfigPath = - p.normalize(p.join(dir, '.dart_tool', 'package_config.json')); + late final wasRelative = p.isRelative(dir); + String relativeIfNeeded(String path) => + wasRelative ? p.relative(path) : path; /// Whether the lockfile is out of date with respect to the dependencies' /// pubspecs. /// /// If any mutable pubspec contains dependencies that are not in the lockfile /// or that don't match what's in there, this will return `false`. - bool isLockFileUpToDate(LockFile lockFile, Package root) { + bool isLockFileUpToDate( + LockFile lockFile, + Package root, { + required String lockFilePath, + }) { /// Returns whether the locked version of [dep] matches the dependency. bool isDependencyUpToDate(PackageRange dep) { if (dep.name == root.name) return true; @@ -827,16 +834,20 @@ bool isPackageConfigUpToDate( PackageConfig packageConfig, LockFile lockFile, - Package root, - ) { + Package root, { + required String packageConfigPath, + required String lockFilePath, + }) { /// Determines if [lockFile] agrees with the given [packagePathsMapping]. /// /// The [packagePathsMapping] is a mapping from package names to paths where /// the packages are located. (The library is located under /// `lib/` relative to the path given). bool isPackagePathsMappingUpToDateWithLockfile( - Map<String, String> packagePathsMapping, - ) { + Map<String, String> packagePathsMapping, { + required String lockFilePath, + required String packageConfigPath, + }) { // Check that [packagePathsMapping] does not contain more packages than what // is required. This could lead to import statements working, when they are // not supposed to work. @@ -899,7 +910,11 @@ packagePathsMapping[pkg.name] = root.path('.dart_tool', p.fromUri(pkg.rootUri)); } - if (!isPackagePathsMappingUpToDateWithLockfile(packagePathsMapping)) { + if (!isPackagePathsMappingUpToDateWithLockfile( + packagePathsMapping, + packageConfigPath: packageConfigPath, + lockFilePath: lockFilePath, + )) { log.fine('The $lockFilePath file has changed since the ' '$packageConfigPath file ' 'was generated, please run "$topLevelProgram pub get" again.'); @@ -950,8 +965,9 @@ } /// The [PackageConfig] object representing `.dart_tool/package_config.json` - /// if it and `pubspec.lock` exist and are up to date with respect to - /// pubspec.yaml and its dependencies. Or `null` if it is outdate + /// along with the dir where it resides, if it and `pubspec.lock` exist and + /// are up to date with respect to pubspec.yaml and its dependencies. Or + /// `null` if it is outdated. /// /// Always returns `null` if `.dart_tool/package_config.json` was generated /// with a different PUB_CACHE location, a different $FLUTTER_ROOT or a @@ -969,7 +985,8 @@ /// where version control or other processes mess up the timestamp order. /// /// If the resolution is still valid, the timestamps are updated and this - /// returns `true`. Otherwise this returns `false`. + /// returns the package configuration and the root dir. Otherwise this + /// returns `null`. /// /// This check is on the fast-path of `dart run` and should do as little /// work as possible. Specifically we avoid parsing any yaml when the @@ -984,11 +1001,100 @@ /// `touch pubspec.lock; touch .dart_tool/package_config.json`) - that is /// hard to avoid, but also unlikely to happen by accident because /// `.dart_tool/package_config.json` is not checked into version control. - PackageConfig? isResolutionUpToDate() { + (PackageConfig, String)? isResolutionUpToDate() { + FileStat? packageConfigStat; + late final String packageConfigPath; + late final String rootDir; + for (final parent in parentDirs(dir)) { + final potentialPackageConfigPath = + p.normalize(p.join(parent, '.dart_tool', 'package_config.json')); + packageConfigStat = tryStatFile(potentialPackageConfigPath); + + if (packageConfigStat != null) { + packageConfigPath = potentialPackageConfigPath; + rootDir = parent; + break; + } + final potentialPubspacPath = p.join(parent, 'pubspec.yaml'); + if (tryStatFile(potentialPubspacPath) == null) { + // No package at [parent] continue to next dir. + continue; + } + + final potentialWorkspaceRefPath = p.normalize( + p.join(parent, '.dart_tool', 'pub', 'workspace_ref.json'), + ); + + final workspaceRefText = tryReadTextFile(potentialWorkspaceRefPath); + if (workspaceRefText == null) { + log.fine( + '`$potentialPubspacPath` exists without corresponding `$potentialPubspacPath` or `$potentialWorkspaceRefPath`.', + ); + return null; + } else { + try { + if (jsonDecode(workspaceRefText) + case {'workspaceRoot': final String path}) { + final potentialPackageConfigPath2 = relativeIfNeeded( + p.normalize( + p.absolute( + p.join( + p.dirname(potentialWorkspaceRefPath), + workspaceRefText, + path, + '.dart_tool', + 'package_config.json', + ), + ), + ), + ); + packageConfigStat = tryStatFile(potentialPackageConfigPath2); + if (packageConfigStat == null) { + log.fine( + '`$potentialWorkspaceRefPath` points to non-existing `$potentialPackageConfigPath2`', + ); + return null; + } else { + packageConfigPath = potentialPackageConfigPath2; + rootDir = relativeIfNeeded( + p.normalize( + p.absolute( + p.join( + p.dirname(potentialWorkspaceRefPath), + workspaceRefText, + path, + ), + ), + ), + ); + + break; + } + } else { + log.fine( + '`$potentialWorkspaceRefPath` is missing "workspaceRoot" property', + ); + return null; + } + } on FormatException catch (e) { + log.fine( + '`$potentialWorkspaceRefPath` not valid json: $e.', + ); + return null; + } + } + } + if (packageConfigStat == null) { + log.fine( + 'Found no .dart_tool/package_config.json - no existing resolution.', + ); + return null; + } + final lockFilePath = p.normalize(p.join(rootDir, 'pubspec.lock')); late final packageConfig = _loadPackageConfig(packageConfigPath); if (p.isWithin(cache.rootDir, packageConfigPath)) { // We always consider a global package (inside the cache) up-to-date. - return packageConfig; + return (packageConfig, rootDir); } /// Whether or not the `.dart_tool/package_config.json` file is was @@ -1004,11 +1110,6 @@ return true; } - final packageConfigStat = tryStatFile(packageConfigPath); - if (packageConfigStat == null) { - log.fine('No $packageConfigPath file found".\n'); - return null; - } final flutter = FlutterSdk(); // If Flutter has moved since last invocation, we want to have new // sdk-packages, and therefore do a new resolution. @@ -1095,7 +1196,7 @@ ); if (!lockfileNewerThanPubspecs) { - if (isLockFileUpToDate(lockFile, root)) { + if (isLockFileUpToDate(lockFile, root, lockFilePath: lockFilePath)) { touch(lockFilePath); touchedLockFile = true; } else { @@ -1106,40 +1207,52 @@ if (touchedLockFile || lockFileModified.isAfter(packageConfigStat.modified)) { log.fine('`$lockFilePath` is newer than `$packageConfigPath`'); - if (isPackageConfigUpToDate(packageConfig, lockFile, root)) { + if (isPackageConfigUpToDate( + packageConfig, + lockFile, + root, + packageConfigPath: packageConfigPath, + lockFilePath: lockFilePath, + )) { touch(packageConfigPath); } else { return null; } } - return packageConfig; + return (packageConfig, rootDir); } - switch (isResolutionUpToDate()) { - case null: - final entrypoint = Entrypoint( - dir, cache, - // [ensureUpToDate] is also used for entries in 'global_packages/' - checkInCache: false, - ); - if (onlyOutputWhenTerminal) { - await log.errorsOnlyUnlessTerminal(() async { - await entrypoint.acquireDependencies( - SolveType.get, - summaryOnly: summaryOnly, - ); - }); - } else { - await entrypoint.acquireDependencies( - SolveType.get, - summaryOnly: summaryOnly, - ); - } - return entrypoint.packageConfig; - case final PackageConfig packageConfig: - log.fine('Package Config up to date.'); - return packageConfig; + if (isResolutionUpToDate() + case (final PackageConfig packageConfig, final String rootDir)) { + log.fine('Package Config up to date.'); + return (packageConfig: packageConfig, rootDir: rootDir); } + final entrypoint = Entrypoint( + dir, cache, + // [ensureUpToDate] is also used for entries in 'global_packages/' + checkInCache: false, + ); + if (onlyOutputWhenTerminal) { + await log.errorsOnlyUnlessTerminal(() async { + await entrypoint.acquireDependencies( + SolveType.get, + summaryOnly: summaryOnly, + ); + }); + } else { + await entrypoint.acquireDependencies( + SolveType.get, + summaryOnly: summaryOnly, + ); + } + return ( + packageConfig: entrypoint.packageConfig, + rootDir: relativeIfNeeded( + p.normalize( + p.absolute(entrypoint.workspaceRoot.dir), + ), + ) + ); } /// We require an SDK constraint lower-bound as of Dart 2.12.0
diff --git a/lib/src/executable.dart b/lib/src/executable.dart index 1fb7180..518398e 100644 --- a/lib/src/executable.dart +++ b/lib/src/executable.dart
@@ -240,19 +240,20 @@ /// /// [descriptor] is resolved as follows: /// * If `<descriptor>` is an existing file (resolved relative to root, either -/// as a path or a file uri): -/// return that (without snapshotting). +/// as a path or a file uri): return that file with a `null` packageConfig. /// -/// * Otherwise if [root] contains no `pubspec.yaml`, throws a -/// [CommandResolutionFailedException]. +/// * Otherwise if it looks like a file name (ends with '.dart' or contains a +/// '/' or a r'\') throw a [CommandResolutionFailedException]. (This is for +/// more clear error messages). /// -/// * Otherwise if the current package resolution is outdated do an implicit -/// `pub get`, if that fails, throw a [CommandResolutionFailedException]. +/// * Otherwise call [Entrypoint.ensureUpToDate] in the current directory to +/// obtain a package config. If that fails, return a +/// [CommandResolutionFailedException]. /// -/// * Otherwise let `<current>` be the name of the package at [root], and -/// interpret [descriptor] as `[<package>][:<command>]`. +/// * Otherwise let `<current>` be the name of the innermost package containing +/// [root], and interpret [descriptor] as `[<package>][:<command>]`. /// -/// * If `<package>` is empty: default to the package at [root]. +/// * If `<package>` is empty: default to the package at [current]. /// * If `<command>` is empty, resolve it as `bin/<package>.dart` or /// `bin/main.dart` to the first that exists. /// @@ -270,8 +271,8 @@ /// the package is an immutable (non-path) dependency of [root]. /// /// If returning the path to a snapshot that doesn't already exist, the script -/// Will be built. And a message will be printed only if a terminal is -/// attached to stdout. +/// Will be built. And a message will be printed only if a terminal is attached +/// to stdout. /// /// 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`. @@ -280,8 +281,8 @@ /// additional source files into compilation even if they are not referenced /// from the main library that [descriptor] resolves to. /// -/// The [nativeAssets], if provided, instructs the compiler to include -/// the native-assets mapping for @Native external functions. +/// The [nativeAssets], if provided, instructs the compiler to include the +/// native-assets mapping for @Native external functions. Future<DartExecutableWithPackageConfig> getExecutableForCommand( String descriptor, { bool allowSnapshot = true, @@ -305,42 +306,53 @@ final asDirectFile = p.join(rootOrCurrent, asPath); if (fileExists(asDirectFile)) { return DartExecutableWithPackageConfig( - executable: p.relative(asDirectFile, from: rootOrCurrent), + executable: p.normalize(p.relative(asDirectFile, from: rootOrCurrent)), packageConfig: null, ); - } - if (!fileExists(p.join(rootOrCurrent, 'pubspec.yaml'))) { + } else if (_looksLikeFile(asPath)) { throw CommandResolutionFailedException._( 'Could not find file `$descriptor`', CommandResolutionIssue.fileNotFound, ); } final PackageConfig packageConfig; + final String workspaceRootDir; try { - packageConfig = await Entrypoint.ensureUpToDate( + final String workspaceRootRelativeToCwd; + (packageConfig: packageConfig, rootDir: workspaceRootRelativeToCwd) = + await Entrypoint.ensureUpToDate( rootOrCurrent, cache: SystemCache(rootDir: pubCacheDir), ); + workspaceRootDir = p.absolute(workspaceRootRelativeToCwd); } on ApplicationException catch (e) { throw CommandResolutionFailedException._( e.toString(), CommandResolutionIssue.pubGetFailed, ); } - // TODO(https://github.com/dart-lang/pub/issues/4127): for workspaces: close - // the nearest enclosing package. That is the "current package" the one to - // default to. - late final rootPackageName = packageConfig.packages - .firstWhereOrNull( - (package) => p.equals( - p.join(rootOrCurrent, '.dart_tool', p.fromUri(package.rootUri)), - rootOrCurrent, - ), - ) - ?.name; + // Find the first directory from [rootOrCurrent] to [workspaceRootDir] (both + // inclusive) that contains a package from the package config. + final packageConfigDir = + p.join(workspaceRootDir, '.dart_tool', 'package_config.json'); + + final rootPackageName = maxBy<(String, String), int>( + packageConfig.packages.map((package) { + final packageRootDir = + p.canonicalize(package.resolvedRootDir(packageConfigDir)); + if (p.equals(packageRootDir, rootOrCurrent) || + p.isWithin(packageRootDir, rootOrCurrent)) { + return (package.name, packageRootDir); + } else { + return null; + } + }).whereNotNull(), + (tuple) => tuple.$2.length, + )?.$1; + if (rootPackageName == null) { throw CommandResolutionFailedException._( - '.dart_tool/package_config did not contain the root package', + '${p.join(workspaceRootDir, '.dart_tool', 'package_config.json')} did not contain its own root package', CommandResolutionIssue.fileNotFound, ); } @@ -370,10 +382,13 @@ ); } final executable = Executable(package, p.join('bin', '$command.dart')); - - final packageConfigPath = p.relative( - p.join(rootOrCurrent, '.dart_tool', 'package_config.json'), - from: rootOrCurrent, + final packageConfigPath = p.normalize( + p.join( + rootOrCurrent, + workspaceRootDir, + '.dart_tool', + 'package_config.json', + ), ); final path = executable.resolve(packageConfig, packageConfigPath); if (!fileExists(p.join(rootOrCurrent, path))) { @@ -384,8 +399,8 @@ } if (!allowSnapshot) { return DartExecutableWithPackageConfig( - executable: p.relative(path, from: rootOrCurrent), - packageConfig: packageConfigPath, + executable: p.normalize(path), + packageConfig: p.relative(packageConfigPath, from: rootOrCurrent), ); } else { // TODO(sigurdm): attempt to decide on package mutability without looking at @@ -419,12 +434,19 @@ } } return DartExecutableWithPackageConfig( - executable: p.relative(snapshotPath, from: rootOrCurrent), - packageConfig: packageConfigPath, + executable: p.normalize(p.relative(snapshotPath, from: rootOrCurrent)), + packageConfig: p.relative(packageConfigPath, from: rootOrCurrent), ); } } +bool _looksLikeFile(String candidate) { + return candidate.contains('/') || + (Platform.isWindows && candidate.contains(r'\')) || + candidate.endsWith('.dart') || + candidate.endsWith('.snapshot'); +} + /// Information on why no executable is returned. enum CommandResolutionIssue { /// The command string looked like a file (contained '.' '/' or '\\'), but no
diff --git a/lib/src/io.dart b/lib/src/io.dart index ffaa5fd..ec92718 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart
@@ -197,8 +197,19 @@ return link; } -/// Reads the contents of the text file [file]. -String readTextFile(String file) => File(file).readAsStringSync(); +/// Reads the contents of the text file at [path]. +String readTextFile(String path) => File(path).readAsStringSync(); + +/// Reads the contents of the text file at [path]. +/// Returns `null` if the operation fails. +String? tryReadTextFile(String path) { + try { + return readTextFile(path); + } on FileSystemException { + // TODO: Consider handlind file-not-found differently from other exceptions. + return null; + } +} /// Reads the contents of the text file [file]. Future<String> readTextFileAsync(String file) {
diff --git a/test/descriptor.dart b/test/descriptor.dart index 7984f40..cee394d 100644 --- a/test/descriptor.dart +++ b/test/descriptor.dart
@@ -117,7 +117,7 @@ }) { final map = packageMap(name, version, deps, devDeps); if (resolutionWorkspace && sdk == null) { - sdk = '3.5.0'; + sdk = '^3.5.0-0'; } if (sdk != null) { map['environment'] = {'sdk': sdk};
diff --git a/test/embedding/ensure_pubspec_resolved.dart b/test/embedding/ensure_pubspec_resolved.dart index 75d4d2d..e98273b 100644 --- a/test/embedding/ensure_pubspec_resolved.dart +++ b/test/embedding/ensure_pubspec_resolved.dart
@@ -80,7 +80,7 @@ .deleteSync(); await _implicitPubGet( - 'No .dart_tool/package_config.json file found', + '`./pubspec.yaml` exists without corresponding `./pubspec.yaml` or `.dart_tool/pub/workspace_ref.json`.', ); });
diff --git a/test/embedding/get_executable_for_command_test.dart b/test/embedding/get_executable_for_command_test.dart index fe49de8..5b07871 100644 --- a/test/embedding/get_executable_for_command_test.dart +++ b/test/embedding/get_executable_for_command_test.dart
@@ -138,7 +138,7 @@ ); }); - test('Does `pub get` if there is a pubspec.yaml', () async { + test('Reports file not found if the path looks like a file', () async { await d.dir(appPath, [ d.pubspec({ 'name': 'myapp', @@ -156,8 +156,8 @@ await testGetExecutable( 'bar/m.dart', d.path(appPath), - errorMessage: matches(r'version\s+solving\s+failed'), - issue: CommandResolutionIssue.pubGetFailed, + errorMessage: matches(r'Could not find file `bar/m.dart`'), + issue: CommandResolutionIssue.fileNotFound, ); }); @@ -206,6 +206,9 @@ pubspec: { 'environment': {'sdk': '^$_currentVersion'}, }, + deps: { + 'transitive': {'hosted': globalServer.url}, + }, contents: [ d.dir('bin', [ d.file('foo.dart', 'main() {print(42);}'), @@ -214,16 +217,24 @@ ], ); + server.serve( + 'transitive', + '1.0.0', + pubspec: { + 'environment': {'sdk': '^$_currentVersion'}, + }, + contents: [ + d.dir('bin', [d.file('transitive.dart', 'main() {print(42);}')]), + ], + ); + await d.dir(appPath, [ d.pubspec({ 'name': 'myapp', 'environment': {'sdk': '^$_currentVersion'}, 'dependencies': { 'foo': { - 'hosted': { - 'name': 'foo', - 'url': globalServer.url, - }, + 'hosted': globalServer.url, 'version': '^1.0.0', }, }, @@ -331,6 +342,152 @@ 'Could not find package `unknownTool` or file `unknownTool`', issue: CommandResolutionIssue.packageNotFound, ); + await testGetExecutable( + 'transitive', + dir, + executable: p.join( + d.sandbox, + d.hostedCachePath(port: globalServer.port), + 'transitive-1.0.0', + 'bin', + 'transitive.dart', + ), + allowSnapshot: false, + packageConfig: p.join('.dart_tool', 'package_config.json'), + ); + }); + + test('works with workspace', () async { + final server = await servePackages(); + server.serve( + 'foo', + '1.0.0', + contents: [ + d.dir('bin', [ + d.file('foo.dart', 'main() {print(42);}'), + d.file('tool.dart', 'main() {print(42);}'), + ]), + ], + ); + + await d.dir(appPath, [ + d.libPubspec( + 'myapp', + '1.2.3', + deps: { + 'a': 'any', + 'foo': { + 'hosted': globalServer.url, + 'version': '^1.0.0', + }, + }, + extras: { + 'workspace': ['pkgs/a', 'pkgs/b'], + }, + sdk: '^3.5.0-0', + ), + d.dir('bin', [ + d.file('myapp.dart', 'main() {print(42);}'), + d.file('tool.dart', 'main() {print(42);}'), + ]), + d.dir('pkgs', [ + d.dir('a', [ + d.libPubspec( + 'a', + '1.0.0', + resolutionWorkspace: true, + ), + d.dir('bin', [ + d.file('a.dart', 'main() {print(42);}'), + d.file('tool.dart', 'main() {print(42);}'), + ]), + ]), + d.dir('b', [ + d.libPubspec( + 'b', + '1.0.0', + resolutionWorkspace: true, + ), + d.dir('bin', [ + d.file('b.dart', 'main() {print(42);}'), + d.file('tool.dart', 'main() {print(42);}'), + ]), + ]), + ]), + ]).create(); + await pubGet( + environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'}, + ); + + await testGetExecutable( + 'myapp', + p.join(d.sandbox, appPath, 'pkgs', 'a'), + executable: p.join( + '..', + '..', + '.dart_tool', + 'pub', + 'bin', + 'myapp', + 'myapp.dart-$_currentVersion.snapshot', + ), + packageConfig: p.join('..', '..', '.dart_tool', 'package_config.json'), + ); + await testGetExecutable( + 'a', + p.join(d.sandbox, appPath, 'pkgs'), + executable: p.join( + d.sandbox, + appPath, + 'pkgs', + 'a', + 'bin', + 'a.dart', + ), + allowSnapshot: false, + packageConfig: p.join('..', '.dart_tool', 'package_config.json'), + ); + await testGetExecutable( + 'b:tool', + p.join(d.sandbox, appPath), + allowSnapshot: false, + executable: p.join( + d.sandbox, + appPath, + 'pkgs', + 'b', + 'bin', + 'tool.dart', + ), + packageConfig: p.join('.dart_tool', 'package_config.json'), + ); + await testGetExecutable( + 'foo', + p.join(d.sandbox, appPath), + allowSnapshot: false, + executable: p.join( + d.sandbox, + d.hostedCachePath(), + 'foo-1.0.0', + 'bin', + 'foo.dart', + ), + packageConfig: p.join('.dart_tool', 'package_config.json'), + ); + await testGetExecutable( + ':tool', + p.join(d.sandbox, appPath, 'pkgs', 'a'), + allowSnapshot: false, + executable: p.join( + d.sandbox, + appPath, + 'pkgs', + 'a', + 'bin', + 'tool.dart', + ), + packageConfig: p.join('..', '..', '.dart_tool', 'package_config.json'), + ); }); }