From 8ffba76a0bdfa32da5fefe17257cb3d2d64c2eab Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Mon, 6 May 2024 09:40:13 +0000 Subject: [PATCH 1/7] Allow invoking `getExecutableForCommand` everywhere in workspace --- lib/src/entrypoint.dart | 137 +++++++++++++---- lib/src/executable.dart | 91 +++++++----- lib/src/io.dart | 14 +- test/descriptor.dart | 2 +- test/embedding/ensure_pubspec_resolved.dart | 2 +- .../get_executable_for_command_test.dart | 138 +++++++++++++++++- 6 files changed, 311 insertions(+), 73 deletions(-) diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index 96c9fe087..b2038c747 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -740,22 +740,25 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without /// /// If [onlyOutputWhenTerminal] is `true` (the default) there will be no /// output if no terminal is attached. - static Future 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')); - /// 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 +830,20 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without 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 packagePathsMapping, - ) { + Map 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. @@ -901,7 +908,11 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without 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.'); @@ -952,8 +963,9 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without } /// 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 @@ -986,11 +998,80 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without /// `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 { + switch (jsonDecode(workspaceRefText)) { + case {'workspaceRoot': final String path}: + final potentialPackageConfigPath2 = p.normalize( + 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; + } + case _: + log.fine( + '`$potentialWorkspaceRefPath` is missing "workspaceRoot" property', + ); + } + } 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 @@ -1006,11 +1087,6 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without 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. @@ -1097,7 +1173,7 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without ); if (!lockfileNewerThanPubspecs) { - if (isLockFileUpToDate(lockFile, root)) { + if (isLockFileUpToDate(lockFile, root, lockFilePath: lockFilePath)) { touch(lockFilePath); touchedLockFile = true; } else { @@ -1108,13 +1184,19 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without 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()) { @@ -1137,10 +1219,13 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without summaryOnly: summaryOnly, ); } - return entrypoint.packageConfig; - case final PackageConfig packageConfig: + return ( + packageConfig: entrypoint.packageConfig, + rootDir: entrypoint.workspaceRoot.dir + ); + case (final PackageConfig packageConfig, final String rootDir): log.fine('Package Config up to date.'); - return packageConfig; + return (packageConfig: packageConfig, rootDir: rootDir); } } diff --git a/lib/src/executable.dart b/lib/src/executable.dart index 1fb718058..a726a20d3 100644 --- a/lib/src/executable.dart +++ b/lib/src/executable.dart @@ -240,19 +240,20 @@ final class DartExecutableWithPackageConfig { /// /// [descriptor] is resolved as follows: /// * If `` 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 `` be the name of the package at [root], and -/// interpret [descriptor] as `[][:]`. +/// * Otherwise let `` be the name of the innermost package containing +/// [root], and interpret [descriptor] as `[][:]`. /// -/// * If `` is empty: default to the package at [root]. +/// * If `` is empty: default to the package at [current]. /// * If `` is empty, resolve it as `bin/.dart` or /// `bin/main.dart` to the first that exists. /// @@ -270,8 +271,8 @@ final class DartExecutableWithPackageConfig { /// 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 @@ final class DartExecutableWithPackageConfig { /// 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 getExecutableForCommand( String descriptor, { bool allowSnapshot = true, @@ -305,19 +306,20 @@ Future getExecutableForCommand( 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( + (packageConfig: packageConfig, rootDir: workspaceRootDir) = + await Entrypoint.ensureUpToDate( rootOrCurrent, cache: SystemCache(rootDir: pubCacheDir), ); @@ -327,20 +329,27 @@ Future getExecutableForCommand( 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. + String? rootPackageName; + for (final parent in parentDirs(rootOrCurrent)) { + final rootPackage = packageConfig.packages.firstWhereOrNull( + (package) => p.equals( + p.join(workspaceRootDir, '.dart_tool', p.fromUri(package.rootUri)), + parent, + ), + ); + if (rootPackage != null) { + rootPackageName = rootPackage.name; + break; + } + if (p.equals(parent, workspaceRootDir)) { + break; + } + } 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, ); } @@ -371,9 +380,13 @@ Future getExecutableForCommand( } 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 +397,8 @@ Future getExecutableForCommand( } if (!allowSnapshot) { return DartExecutableWithPackageConfig( - executable: p.relative(path, from: rootOrCurrent), - packageConfig: packageConfigPath, + executable: p.normalize(p.relative(path, from: rootOrCurrent)), + packageConfig: p.relative(packageConfigPath, from: rootOrCurrent), ); } else { // TODO(sigurdm): attempt to decide on package mutability without looking at @@ -419,12 +432,18 @@ Future getExecutableForCommand( } } 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('/') || + 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 ffaa5fd68..c5c119fc9 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart @@ -197,8 +197,18 @@ String _resolveLink(String link) { 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 { + return null; + } +} /// Reads the contents of the text file [file]. Future readTextFileAsync(String file) { diff --git a/test/descriptor.dart b/test/descriptor.dart index 7984f40c5..cee394d4f 100644 --- a/test/descriptor.dart +++ b/test/descriptor.dart @@ -117,7 +117,7 @@ FileDescriptor libPubspec( }) { 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 75d4d2dc3..1f0e11439 100644 --- a/test/embedding/ensure_pubspec_resolved.dart +++ b/test/embedding/ensure_pubspec_resolved.dart @@ -80,7 +80,7 @@ void testEnsurePubspecResolved() { .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 fe49de861..1f49fc716 100644 --- a/test/embedding/get_executable_for_command_test.dart +++ b/test/embedding/get_executable_for_command_test.dart @@ -138,7 +138,7 @@ Future main() async { ); }); - 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 @@ Future main() async { 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 @@ Future main() async { pubspec: { 'environment': {'sdk': '^$_currentVersion'}, }, + deps: { + 'transitive': {'hosted': globalServer.url}, + }, contents: [ d.dir('bin', [ d.file('foo.dart', 'main() {print(42);}'), @@ -214,16 +217,24 @@ Future main() async { ], ); + 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,119 @@ Future main() async { 'Could not find package `unknownTool` or file `unknownTool`', issue: CommandResolutionIssue.packageNotFound, ); + await testGetExecutable( + 'transitive', + dir, + executable: p.relative( + p.join( + d.sandbox, + d.hostedCachePath(port: globalServer.port), + 'transitive-1.0.0', + 'bin', + 'transitive.dart', + ), + from: dir, + ), + 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', + }, + 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( + '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( + 'pkgs', + 'b', + 'bin', + 'tool.dart', + ), + packageConfig: p.join('.dart_tool', 'package_config.json'), + ); }); } From 98a4e80627be5f0a93f08699ec40454a0ef1a1bc Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Mon, 6 May 2024 10:52:46 +0000 Subject: [PATCH 2/7] Fix expectation --- test/embedding/ensure_pubspec_resolved.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/embedding/ensure_pubspec_resolved.dart b/test/embedding/ensure_pubspec_resolved.dart index 1f0e11439..e98273bc4 100644 --- a/test/embedding/ensure_pubspec_resolved.dart +++ b/test/embedding/ensure_pubspec_resolved.dart @@ -80,7 +80,7 @@ void testEnsurePubspecResolved() { .deleteSync(); await _implicitPubGet( - './pubspec.yaml` exists without corresponding `./pubspec.yaml` or `.dart_tool/pub/workspace_ref.json`.', + '`./pubspec.yaml` exists without corresponding `./pubspec.yaml` or `.dart_tool/pub/workspace_ref.json`.', ); }); From ec1918dc398fdf0ea026e163cc4273503d0c100b Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Mon, 13 May 2024 11:17:21 +0000 Subject: [PATCH 3/7] Improvements and fixes --- lib/src/entrypoint.dart | 104 ++++++++++-------- lib/src/executable.dart | 48 +++++--- lib/src/io.dart | 1 + .../get_executable_for_command_test.dart | 51 +++++++-- 4 files changed, 133 insertions(+), 71 deletions(-) diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index b2038c747..734dcd465 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -983,7 +983,8 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without /// 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 @@ -1001,8 +1002,9 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without (PackageConfig, String)? isResolutionUpToDate() { FileStat? packageConfigStat; late final String packageConfigPath; - late final String rootDir; - for (final parent in parentDirs(dir)) { + late String rootDir; + final wasRelative = p.isRelative(dir); + for (final parent in parentDirs(p.absolute(dir))) { final potentialPackageConfigPath = p.normalize(p.join(parent, '.dart_tool', 'package_config.json')); packageConfigStat = tryStatFile(potentialPackageConfigPath); @@ -1030,28 +1032,43 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without return null; } else { try { - switch (jsonDecode(workspaceRefText)) { - case {'workspaceRoot': final String path}: - final potentialPackageConfigPath2 = p.normalize( + if (jsonDecode(workspaceRefText) + case {'workspaceRoot': final String path}) { + final potentialPackageConfigPath2 = p.normalize( + 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; + final rootDirAbsolute = 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; - } - case _: - log.fine( - '`$potentialWorkspaceRefPath` is missing "workspaceRoot" property', + + rootDir = p.normalize( + wasRelative ? p.relative(rootDirAbsolute) : rootDirAbsolute, ); + break; + } + } else { + log.fine( + '`$potentialWorkspaceRefPath` is missing "workspaceRoot" property', + ); + return null; } } on FormatException catch (e) { log.fine( @@ -1199,34 +1216,33 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without 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 ( - packageConfig: entrypoint.packageConfig, - rootDir: entrypoint.workspaceRoot.dir + 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, ); - case (final PackageConfig packageConfig, final String rootDir): - log.fine('Package Config up to date.'); - return (packageConfig: packageConfig, rootDir: rootDir); + }); + } else { + await entrypoint.acquireDependencies( + SolveType.get, + summaryOnly: summaryOnly, + ); } + return ( + packageConfig: entrypoint.packageConfig, + rootDir: 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 a726a20d3..c68128644 100644 --- a/lib/src/executable.dart +++ b/lib/src/executable.dart @@ -331,22 +331,23 @@ Future getExecutableForCommand( } // Find the first directory from [rootOrCurrent] to [workspaceRootDir] (both // inclusive) that contains a package from the package config. - String? rootPackageName; - for (final parent in parentDirs(rootOrCurrent)) { - final rootPackage = packageConfig.packages.firstWhereOrNull( - (package) => p.equals( - p.join(workspaceRootDir, '.dart_tool', p.fromUri(package.rootUri)), - parent, - ), - ); - if (rootPackage != null) { - rootPackageName = rootPackage.name; - break; - } - if (p.equals(parent, workspaceRootDir)) { - break; - } - } + 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._( '${p.join(workspaceRootDir, '.dart_tool', 'package_config.json')} did not contain its own root package', @@ -379,7 +380,7 @@ Future getExecutableForCommand( ); } final executable = Executable(package, p.join('bin', '$command.dart')); - + print('wor $workspaceRootDir'); final packageConfigPath = p.normalize( p.join( rootOrCurrent, @@ -397,7 +398,7 @@ Future getExecutableForCommand( } if (!allowSnapshot) { return DartExecutableWithPackageConfig( - executable: p.normalize(p.relative(path, from: rootOrCurrent)), + executable: p.normalize(path), packageConfig: p.relative(packageConfigPath, from: rootOrCurrent), ); } else { @@ -440,6 +441,7 @@ Future getExecutableForCommand( bool _looksLikeFile(String candidate) { return candidate.contains('/') || + (Platform.isWindows && candidate.contains(r'\')) || candidate.endsWith('.dart') || candidate.endsWith('.snapshot'); } @@ -508,6 +510,16 @@ class Executable { /// The path to this executable given [packageConfig] Relative package dirs /// are resolved relative to `dirname(packageConfigPath)`. String resolve(PackageConfig packageConfig, String packageConfigPath) { + print( + ( + p.dirname(packageConfigPath), + p.fromUri( + packageConfig.packages.firstWhere((p) => p.name == package).rootUri, + ), + relativePath, + ), + ); + return p.normalize( p.join( p.dirname(packageConfigPath), diff --git a/lib/src/io.dart b/lib/src/io.dart index c5c119fc9..ec927184f 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart @@ -206,6 +206,7 @@ String? tryReadTextFile(String path) { try { return readTextFile(path); } on FileSystemException { + // TODO: Consider handlind file-not-found differently from other exceptions. return null; } } diff --git a/test/embedding/get_executable_for_command_test.dart b/test/embedding/get_executable_for_command_test.dart index 1f49fc716..5b07871fc 100644 --- a/test/embedding/get_executable_for_command_test.dart +++ b/test/embedding/get_executable_for_command_test.dart @@ -345,15 +345,12 @@ Future main() async { await testGetExecutable( 'transitive', dir, - executable: p.relative( - p.join( - d.sandbox, - d.hostedCachePath(port: globalServer.port), - 'transitive-1.0.0', - 'bin', - 'transitive.dart', - ), - from: 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'), @@ -379,6 +376,10 @@ Future main() async { '1.2.3', deps: { 'a': 'any', + 'foo': { + 'hosted': globalServer.url, + 'version': '^1.0.0', + }, }, extras: { 'workspace': ['pkgs/a', 'pkgs/b'], @@ -436,6 +437,9 @@ Future main() async { 'a', p.join(d.sandbox, appPath, 'pkgs'), executable: p.join( + d.sandbox, + appPath, + 'pkgs', 'a', 'bin', 'a.dart', @@ -448,6 +452,8 @@ Future main() async { p.join(d.sandbox, appPath), allowSnapshot: false, executable: p.join( + d.sandbox, + appPath, 'pkgs', 'b', 'bin', @@ -455,6 +461,33 @@ Future main() async { ), 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'), + ); }); } From 306bd9de7823073e025f17ad14163d83f600c26e Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Mon, 13 May 2024 11:25:12 +0000 Subject: [PATCH 4/7] Remove debug prints --- lib/src/executable.dart | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/src/executable.dart b/lib/src/executable.dart index c68128644..4adc35b26 100644 --- a/lib/src/executable.dart +++ b/lib/src/executable.dart @@ -380,7 +380,6 @@ Future getExecutableForCommand( ); } final executable = Executable(package, p.join('bin', '$command.dart')); - print('wor $workspaceRootDir'); final packageConfigPath = p.normalize( p.join( rootOrCurrent, @@ -510,16 +509,6 @@ class Executable { /// The path to this executable given [packageConfig] Relative package dirs /// are resolved relative to `dirname(packageConfigPath)`. String resolve(PackageConfig packageConfig, String packageConfigPath) { - print( - ( - p.dirname(packageConfigPath), - p.fromUri( - packageConfig.packages.firstWhere((p) => p.name == package).rootUri, - ), - relativePath, - ), - ); - return p.normalize( p.join( p.dirname(packageConfigPath), From ad2d44b1665c37b18a9f67f953663df06c4c117d Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Mon, 13 May 2024 11:27:04 +0000 Subject: [PATCH 5/7] make var final --- lib/src/entrypoint.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index 734dcd465..e662c3970 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -1002,7 +1002,7 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without (PackageConfig, String)? isResolutionUpToDate() { FileStat? packageConfigStat; late final String packageConfigPath; - late String rootDir; + late final String rootDir; final wasRelative = p.isRelative(dir); for (final parent in parentDirs(p.absolute(dir))) { final potentialPackageConfigPath = From 5ed3040b6ac6daf5c562f4cc0e9985716607ddf8 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Mon, 13 May 2024 12:30:37 +0000 Subject: [PATCH 6/7] Fix relative paths --- lib/src/entrypoint.dart | 42 ++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index e662c3970..502074373 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -749,6 +749,10 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without bool summaryOnly = true, bool onlyOutputWhenTerminal = true, }) async { + 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. /// @@ -1003,8 +1007,7 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without FileStat? packageConfigStat; late final String packageConfigPath; late final String rootDir; - final wasRelative = p.isRelative(dir); - for (final parent in parentDirs(p.absolute(dir))) { + for (final parent in parentDirs(dir)) { final potentialPackageConfigPath = p.normalize(p.join(parent, '.dart_tool', 'package_config.json')); packageConfigStat = tryStatFile(potentialPackageConfigPath); @@ -1034,13 +1037,17 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without try { if (jsonDecode(workspaceRefText) case {'workspaceRoot': final String path}) { - final potentialPackageConfigPath2 = p.normalize( - p.join( - p.dirname(potentialWorkspaceRefPath), - workspaceRefText, - path, - '.dart_tool', - 'package_config.json', + final potentialPackageConfigPath2 = relativeIfNeeded( + p.normalize( + p.absolute( + p.join( + p.dirname(potentialWorkspaceRefPath), + workspaceRefText, + path, + '.dart_tool', + 'package_config.json', + ), + ), ), ); packageConfigStat = tryStatFile(potentialPackageConfigPath2); @@ -1051,17 +1058,18 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without return null; } else { packageConfigPath = potentialPackageConfigPath2; - final rootDirAbsolute = p.absolute( - p.join( - p.dirname(potentialWorkspaceRefPath), - workspaceRefText, - path, + rootDir = relativeIfNeeded( + p.normalize( + p.absolute( + p.join( + p.dirname(potentialWorkspaceRefPath), + workspaceRefText, + path, + ), + ), ), ); - rootDir = p.normalize( - wasRelative ? p.relative(rootDirAbsolute) : rootDirAbsolute, - ); break; } } else { From 003ac3e7295aa39b9dde88869cdca6c388724efb Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Mon, 13 May 2024 13:50:19 +0000 Subject: [PATCH 7/7] Fix relative paths2 --- lib/src/entrypoint.dart | 6 +++++- lib/src/executable.dart | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index 502074373..b9e18631f 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -1249,7 +1249,11 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without } return ( packageConfig: entrypoint.packageConfig, - rootDir: entrypoint.workspaceRoot.dir + rootDir: relativeIfNeeded( + p.normalize( + p.absolute(entrypoint.workspaceRoot.dir), + ), + ) ); } diff --git a/lib/src/executable.dart b/lib/src/executable.dart index 4adc35b26..518398e1b 100644 --- a/lib/src/executable.dart +++ b/lib/src/executable.dart @@ -318,11 +318,13 @@ Future getExecutableForCommand( final PackageConfig packageConfig; final String workspaceRootDir; try { - (packageConfig: packageConfig, rootDir: workspaceRootDir) = + 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(),