From dceb212c1886c3f686aa93fc462a1065912eebda Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Mon, 18 Mar 2024 13:16:17 +0000 Subject: [PATCH 01/10] Resolve workspace root and workPackage when invoking pub from any sub-directory --- lib/src/command/add.dart | 5 +- lib/src/command/remove.dart | 2 +- lib/src/command/upgrade.dart | 2 +- lib/src/entrypoint.dart | 123 +++++++++++++----- lib/src/io.dart | 104 +++++++++------ lib/src/package.dart | 42 ++++-- lib/src/solver/report.dart | 2 +- lib/src/utils.dart | 2 + test/add/git/git_test.dart | 4 +- test/add/path/relative_path_test.dart | 4 +- test/descriptor.dart | 13 +- test/get/enforce_lockfile_test.dart | 6 +- test/get/gets_in_example_folder_test.dart | 8 +- test/get/path/relative_path_test.dart | 2 +- .../installs_dependencies_for_path_test.dart | 2 +- .../global/run/missing_path_package_test.dart | 6 +- test/pub_get_and_upgrade_test.dart | 5 +- ...taking a --directory~-C parameter work.txt | 78 +++++------ .../outdated/outdated_test/no pubspec.txt | 2 +- ... not update major versions in example~.txt | 8 +- test/unpack_test.dart | 4 +- test/workspace_test.dart | 68 +++++++++- 22 files changed, 339 insertions(+), 153 deletions(-) diff --git a/lib/src/command/add.dart b/lib/src/command/add.dart index 9d74d3e177..88f4bf5031 100644 --- a/lib/src/command/add.dart +++ b/lib/src/command/add.dart @@ -256,8 +256,7 @@ Specify multiple sdk packages with descriptors.'''); } String? overridesFileContents; - final overridesPath = - p.join(entrypoint.workspaceRoot.dir, Pubspec.pubspecOverridesFilename); + final overridesPath = entrypoint.workspaceRoot.pubspecOverridesPath; try { overridesFileContents = readTextFile(overridesPath); } on IOException { @@ -268,7 +267,7 @@ Specify multiple sdk packages with descriptors.'''); /// gets a report on the other packages that might change version due /// to this new dependency. await entrypoint - .withPubspec( + .withWorkPubspec( Pubspec.parse( newPubspecText, cache.sources, diff --git a/lib/src/command/remove.dart b/lib/src/command/remove.dart index 152c1df6bf..11eea6ba15 100644 --- a/lib/src/command/remove.dart +++ b/lib/src/command/remove.dart @@ -93,7 +93,7 @@ To remove a dependency override of a package prefix the package name with final rootPubspec = entrypoint.workspaceRoot.pubspec; final newPubspec = _removePackagesFromPubspec(rootPubspec, targets); - await entrypoint.withPubspec(newPubspec).acquireDependencies( + await entrypoint.withWorkPubspec(newPubspec).acquireDependencies( SolveType.get, precompile: !isDryRun && argResults.flag('precompile'), dryRun: isDryRun, diff --git a/lib/src/command/upgrade.dart b/lib/src/command/upgrade.dart index 91b0e26850..1832187f64 100644 --- a/lib/src/command/upgrade.dart +++ b/lib/src/command/upgrade.dart @@ -364,7 +364,7 @@ be direct 'dependencies' or 'dev_dependencies', following packages are not: } await entrypoint - .withPubspec(_updatedPubspec(newPubspecText, entrypoint)) + .withWorkPubspec(_updatedPubspec(newPubspecText, entrypoint)) .acquireDependencies( solveType, dryRun: _dryRun, diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index e20ef20a88..cd20235392 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -33,6 +33,7 @@ import 'solver.dart'; import 'solver/report.dart'; import 'solver/solve_suggestions.dart'; import 'source/cached.dart'; +import 'source/root.dart'; import 'source/unknown.dart'; import 'system_cache.dart'; import 'utils.dart'; @@ -62,21 +63,78 @@ class Entrypoint { // true. final String workingDir; - Package? _workspaceRoot; + static ({Package root, Package work}) _loadWorkspace( + String workingDir, + SystemCache cache, + ) { + if (!dirExists(workingDir)) { + fail('The directory `$workingDir` does not exist.'); + } + // Keep track of all the pubspecs met when walking up the file system. + // The first of these is the workingPackage. + final pubspecsMet = {}; + for (final dir in parentDirs(workingDir)) { + final Pubspec pubspec; + + try { + pubspec = Pubspec.load( + dir, + cache.sources, + containingDescription: RootDescription(dir), + allowOverridesFile: true, + ); + } on FileException { + continue; + } + pubspecsMet[p.canonicalize(dir)] = pubspec; + final Package root; + if (pubspec.resolution == Resolution.none) { + root = Package.load( + null, + dir, + cache.sources, + alreadyLoadedPubspecs: pubspecsMet, + withPubspecOverrides: true, + ); + for (final package in root.transitiveWorkspace) { + pubspecsMet.entries.first; + if (identical(pubspecsMet.entries.first.value, package.pubspec)) { + return (root: root, work: package); + } + } + assert(false); + } + } + if (pubspecsMet.isEmpty) { + throw FileException( + 'Found no `pubspec.yaml` file in `${p.canonicalize(workingDir)}` or parent directories', + p.join(workingDir, 'pubspec.yaml'), + ); + } else { + final firstEntry = pubspecsMet.entries.first; + throw FileException( + ''' +Found a pubspec.yaml at ${firstEntry.key}. But it has resolution `${firstEntry.value.resolution.name}`. +But found no workspace root including it in parent directories. + +See $workspacesDocUrl for more information.''', + p.join(workingDir, 'pubspec.yaml'), + ); + } + } + + ({Package root, Package work})? _packages; + ({Package root, Package work}) get _getPackages => + _packages ??= _loadWorkspace(workingDir, cache); /// The root package this entrypoint is associated with. /// /// For a global package, this is the activated package. - Package get workspaceRoot => _workspaceRoot ??= Package.load( - null, - workingDir, - cache.sources, - withPubspecOverrides: true, - ); + Package get workspaceRoot => _getPackages.root; bool get canFindWorkspaceRoot { try { - workspaceRoot; + _getPackages; return true; } on FileException { return false; @@ -98,7 +156,7 @@ class Entrypoint { /// /// Running `pub add` in `foo` will have foo as workPackage, and add /// dependencies to `foo/pubspec.yaml`. - Package get workPackage => workspaceRoot; + Package get workPackage => _getPackages.work; /// The system-wide cache which caches packages that need to be fetched over /// the network. @@ -229,46 +287,50 @@ class Entrypoint { this._example, this._packageGraph, this.cache, - this._workspaceRoot, + this._packages, this.isCachedGlobal, ); - /// An entrypoint representing a package at [rootDir]. + /// An entrypoint for the workspace containing [workingDir]/ /// /// If [checkInCache] is `true` (the default) an error will be thrown if /// [rootDir] is located inside [cache.rootDir]. + Entrypoint( this.workingDir, this.cache, { - ({Pubspec pubspec, List workspacePackages})? preloaded, bool checkInCache = true, - }) : _workspaceRoot = preloaded == null - ? null - : Package( - preloaded.pubspec, - workingDir, - preloaded.workspacePackages, - ), - isCachedGlobal = false { + }) : isCachedGlobal = false { if (checkInCache && p.isWithin(cache.rootDir, workingDir)) { fail('Cannot operate on packages inside the cache.'); } } /// Creates an entrypoint at the same location, that will use [pubspec] for - /// resolution. - Entrypoint withPubspec(Pubspec pubspec) { + /// resolution of the [workPackage]. + Entrypoint withWorkPubspec(Pubspec pubspec) { + final existingPubspecs = {}; + // First extract all pubspecs from the workspace. + for (final package in workspaceRoot.transitiveWorkspace) { + existingPubspecs[package.dir] = package.pubspec; + } + // Then override the one of the workPackage. + existingPubspecs[p.canonicalize(workPackage.dir)] = pubspec; + final newWorkspaceRoot = Package.load( + null, + workspaceRoot.dir, + cache.sources, + alreadyLoadedPubspecs: existingPubspecs, + ); + final newWorkPackage = newWorkspaceRoot.transitiveWorkspace + .firstWhere((package) => package.dir == workPackage.dir); return Entrypoint._( workingDir, _lockFile, _example, _packageGraph, cache, - Package( - pubspec, - workingDir, - workspaceRoot.workspaceChildren, - ), + (root: newWorkspaceRoot, work: newWorkPackage), isCachedGlobal, ); } @@ -276,11 +338,12 @@ class Entrypoint { /// Creates an entrypoint given package and lockfile objects. /// If a SolveResult is already created it can be passed as an optimization. Entrypoint.global( - Package this._workspaceRoot, + Package package, this._lockFile, this.cache, { SolveResult? solveResult, - }) : workingDir = _workspaceRoot.dir, + }) : _packages = (root: package, work: package), + workingDir = package.dir, isCachedGlobal = true { if (solveResult != null) { _packageGraph = @@ -410,7 +473,7 @@ class Entrypoint { }) async { workspaceRoot; // This will throw early if pubspec.yaml could not be found. summaryOnly = summaryOnly || _summaryOnlyEnvironment; - final suffix = workspaceRoot.dir == '.' ? '' : ' in ${workspaceRoot.dir}'; + final suffix = workspaceRoot.dir == '.' ? '' : ' in `${workspaceRoot.dir}`'; if (enforceLockfile && !fileExists(lockFilePath)) { throw ApplicationException(''' diff --git a/lib/src/io.dart b/lib/src/io.dart index dfc5e156d3..11f2418065 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart @@ -17,7 +17,7 @@ import 'package:cli_util/cli_util.dart' import 'package:http/http.dart' show ByteStream; import 'package:http_multi_server/http_multi_server.dart'; import 'package:meta/meta.dart'; -import 'package:path/path.dart' as path; +import 'package:path/path.dart' as p; import 'package:pool/pool.dart'; import 'package:stack_trace/stack_trace.dart'; // ignore: prefer_relative_imports @@ -110,7 +110,7 @@ FileStat? tryStatFile(String path) { String canonicalize(String pathString) { var seen = {}; var components = - Queue.from(path.split(path.normalize(path.absolute(pathString)))); + Queue.from(p.split(p.normalize(p.absolute(pathString)))); // The canonical path, built incrementally as we iterate through [components]. var newPath = components.removeFirst(); @@ -119,22 +119,21 @@ String canonicalize(String pathString) { // necessary. A resolved component may also add new components that need to be // resolved in turn. while (components.isNotEmpty) { - seen.add(path.join(newPath, path.joinAll(components))); - var resolvedPath = - _resolveLink(path.join(newPath, components.removeFirst())); - var relative = path.relative(resolvedPath, from: newPath); + seen.add(p.join(newPath, p.joinAll(components))); + var resolvedPath = _resolveLink(p.join(newPath, components.removeFirst())); + var relative = p.relative(resolvedPath, from: newPath); // If the resolved path of the component relative to `newPath` is just ".", // that means component was a symlink pointing to its parent directory. We // can safely ignore such components. if (relative == '.') continue; - var relativeComponents = Queue.from(path.split(relative)); + var relativeComponents = Queue.from(p.split(relative)); // If the resolved path is absolute relative to `newPath`, that means it's // on a different drive. We need to canonicalize the entire target of that // symlink again. - if (path.isAbsolute(relative)) { + if (p.isAbsolute(relative)) { // If we've already tried to canonicalize the new path, we've encountered // a symlink loop. Avoid going infinite by treating the recursive symlink // as the canonical path. @@ -151,7 +150,7 @@ String canonicalize(String pathString) { // Pop directories off `newPath` if the component links upwards in the // directory hierarchy. while (relativeComponents.firstOrNull == '..') { - newPath = path.dirname(newPath); + newPath = p.dirname(newPath); relativeComponents.removeFirst(); } @@ -159,14 +158,14 @@ String canonicalize(String pathString) { // not a link (or is a broken link). We can just add it to `newPath` and // continue resolving the remaining components. if (relativeComponents.length == 1) { - newPath = path.join(newPath, relativeComponents.single); + newPath = p.join(newPath, relativeComponents.single); continue; } // If we've already tried to canonicalize the new path, we've encountered a // symlink loop. Avoid going infinite by treating the recursive symlink as // the canonical path. - var newSubPath = path.join(newPath, path.joinAll(relativeComponents)); + var newSubPath = p.join(newPath, p.joinAll(relativeComponents)); if (seen.contains(newSubPath)) { newPath = newSubPath; continue; @@ -192,8 +191,7 @@ String canonicalize(String pathString) { String _resolveLink(String link) { var seen = {}; while (linkExists(link) && seen.add(link)) { - link = - path.normalize(path.join(path.dirname(link), Link(link).targetSync())); + link = p.normalize(p.join(p.dirname(link), Link(link).targetSync())); } return link; } @@ -520,7 +518,7 @@ void renameDir(String from, String to) { /// If it fails with "destination not empty" we log and continue, assuming /// another process got there before us. void tryRenameDir(String from, String to) { - ensureDir(path.dirname(to)); + ensureDir(p.dirname(to)); try { renameDir(from, to); } on FileSystemException catch (e) { @@ -576,13 +574,13 @@ void createSymlink(String target, String symlink, {bool relative = false}) { // relative path to be relative to the cwd, not the symlink, and will be // confused by forward slashes. if (Platform.isWindows) { - target = path.normalize(path.absolute(target)); + target = p.normalize(p.absolute(target)); } else { // If the directory where we're creating the symlink was itself reached // by traversing a symlink, we want the relative path to be relative to // it's actual location, not the one we went through to get to it. - var symlinkDir = canonicalize(path.dirname(symlink)); - target = path.normalize(path.relative(target, from: symlinkDir)); + var symlinkDir = canonicalize(p.dirname(symlink)); + target = p.normalize(p.relative(target, from: symlinkDir)); } } @@ -607,7 +605,7 @@ void createPackageSymlink( }) { // See if the package has a "lib" directory. If not, there's nothing to // symlink to. - target = path.join(target, 'lib'); + target = p.join(target, 'lib'); if (!dirExists(target)) return; log.fine("Creating ${isSelfLink ? "self" : ""}link for package '$name'."); @@ -662,7 +660,7 @@ final String dartRepoRoot = (() { // as a test or as a pub executable. var url = Platform.script .replace(path: Platform.script.path.replaceAll(_dartRepoRegExp, '')); - return path.fromUri(url); + return p.fromUri(url); })(); /// Displays a message and reads a yes/no confirmation from the user. @@ -1015,10 +1013,10 @@ Future extractFileFromTarGz( String filename, ) async { final reader = TarReader(stream.transform(gzip.decoder)); - filename = path.posix.normalize(filename); + filename = p.posix.normalize(filename); while (await reader.moveNext()) { final entry = reader.current; - if (path.posix.normalize(entry.name) != filename) continue; + if (p.posix.normalize(entry.name) != filename) continue; if (!(entry.type == TypeFlag.reg || entry.type == TypeFlag.regA)) { // Can only read regular files. throw FormatException('$filename is not a file'); @@ -1032,16 +1030,16 @@ Future extractFileFromTarGz( Future extractTarGz(Stream> stream, String destination) async { log.fine('Extracting .tar.gz stream to $destination.'); - destination = path.absolute(destination); + destination = p.absolute(destination); final reader = TarReader(stream.transform(gzip.decoder)); final paths = {}; while (await reader.moveNext()) { final entry = reader.current; - final filePath = path.joinAll([ + final filePath = p.joinAll([ destination, // Tar file names always use forward slashes - ...path.posix.split(entry.name), + ...p.posix.split(entry.name), ]); if (!paths.add(filePath)) { // The tar file contained the same entry twice. Assume it is broken. @@ -1049,7 +1047,7 @@ Future extractTarGz(Stream> stream, String destination) async { throw FormatException('Tar file contained duplicate path ${entry.name}'); } - if (!path.isWithin(destination, filePath)) { + if (!p.isWithin(destination, filePath)) { // The tar contains entries that would be written outside of the // destination. That doesn't happen by accident, assume that the tar file // is malicious. @@ -1057,10 +1055,10 @@ Future extractTarGz(Stream> stream, String destination) async { throw FormatException('Invalid tar entry: ${entry.name}'); } - final parentDirectory = path.dirname(filePath); + final parentDirectory = p.dirname(filePath); bool checkValidTarget(String linkTarget) { - final isValid = path.isWithin(destination, linkTarget); + final isValid = p.isWithin(destination, linkTarget); if (!isValid) { log.fine('Skipping ${entry.name}: Invalid link target'); } @@ -1091,8 +1089,8 @@ Future extractTarGz(Stream> stream, String destination) async { break; case TypeFlag.symlink: // Link to another file in this tar, relative from this entry. - final resolvedTarget = path.joinAll( - [parentDirectory, ...path.posix.split(entry.header.linkName!)], + final resolvedTarget = p.joinAll( + [parentDirectory, ...p.posix.split(entry.header.linkName!)], ); if (!checkValidTarget(resolvedTarget)) { // Don't allow links to files outside of this tar. @@ -1101,7 +1099,7 @@ Future extractTarGz(Stream> stream, String destination) async { ensureDir(parentDirectory); createSymlink( - path.relative(resolvedTarget, from: parentDirectory), + p.relative(resolvedTarget, from: parentDirectory), filePath, ); break; @@ -1109,12 +1107,12 @@ Future extractTarGz(Stream> stream, String destination) async { // We generate hardlinks as symlinks too, but their linkName is relative // to the root of the tar file (unlike symlink entries, whose linkName // is relative to the entry itself). - final fromDestination = path.join(destination, entry.header.linkName); + final fromDestination = p.join(destination, entry.header.linkName); if (!checkValidTarget(fromDestination)) { break; // Link points outside of the tar file. } - final fromFile = path.relative(fromDestination, from: parentDirectory); + final fromFile = p.relative(fromDestination, from: parentDirectory); ensureDir(parentDirectory); createSymlink(fromFile, filePath); break; @@ -1144,18 +1142,18 @@ ByteStream createTarGz( log.fine(buffer.toString()); ArgumentError.checkNotNull(baseDir, 'baseDir'); - baseDir = path.normalize(path.absolute(baseDir)); + baseDir = p.normalize(p.absolute(baseDir)); final tarContents = Stream.fromIterable( contents.map((entry) { - entry = path.normalize(path.absolute(entry)); - if (!path.isWithin(baseDir, entry)) { + entry = p.normalize(p.absolute(entry)); + if (!p.isWithin(baseDir, entry)) { throw ArgumentError('Entry $entry is not inside $baseDir.'); } - final relative = path.relative(entry, from: baseDir); + final relative = p.relative(entry, from: baseDir); // On Windows, we can't open some files without normalizing them - final file = File(path.normalize(entry)); + final file = File(p.normalize(entry)); final stat = file.statSync(); if (stat.type == FileSystemEntityType.link) { @@ -1166,7 +1164,7 @@ ByteStream createTarGz( return TarEntry( TarHeader( // Ensure paths in tar files use forward slashes - name: path.url.joinAll(path.split(relative)), + name: p.url.joinAll(p.split(relative)), // We want to keep executable bits, but otherwise use the default // file mode mode: _defaultMode | (stat.mode & _executableMask), @@ -1213,7 +1211,7 @@ class PubProcessResult { final String? dartConfigDir = () { if (runningFromTest && Platform.environment.containsKey('_PUB_TEST_CONFIG_DIR')) { - return path.join(Platform.environment['_PUB_TEST_CONFIG_DIR']!, 'dart'); + return p.join(Platform.environment['_PUB_TEST_CONFIG_DIR']!, 'dart'); } try { return applicationConfigHome('dart'); @@ -1231,3 +1229,31 @@ String escapeShellArgument(String x) => RegExp(r'^[a-zA-Z0-9-_=@.^]+$').stringMatch(x) == null ? "'${x.replaceAll(r'\', r'\\').replaceAll("'", r"'\''")}'" : x; + +/// Returns all parent directories of [path], starting from [path] to the root. +/// +/// If [path] is relative the directories will also be. +/// +/// If [from] is passed, directories are made relative to that. +/// +/// Examples: +/// parentDirs('/a/b/c') => ('/a/b/c', '/a/b', '/a', '/') +/// parentDirs('./d/e', from: '/a/b/c') => ('./d/e', './d', '.', '..', '../..', '../../..') +Iterable parentDirs(String path, {String? from}) sync* { + var relative = false; + var d = path; + while (true) { + if (relative) { + yield p.relative(d, from: from); + } else { + yield d; + } + if (p.isRelative(d) && p.equals(d, '.')) { + d = from ?? p.current; + relative = true; + } + final parent = p.dirname(d); + if (parent == d) break; + d = parent; + } +} diff --git a/lib/src/package.dart b/lib/src/package.dart index fa9939442e..4285d1cf92 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -131,22 +131,48 @@ class Package { /// /// `pubspec_overrides.yaml` is only loaded if [withPubspecOverrides] is /// `true`. + /// + /// [alreadyLoadedPubspecs] contains a map from the `p.canonicalize`d + /// directory containing a pubspec, to the loaded [Pubspec] object. + /// + /// This mechanism can be used to avoid loading pubspecs twice. It can also be + /// used to override a pubspec in memory for trying out an alternative + /// resolution. factory Package.load( String? name, String dir, SourceRegistry sources, { bool withPubspecOverrides = false, + Map alreadyLoadedPubspecs = const {}, }) { - final pubspec = Pubspec.load( - dir, - sources, - expectedName: name, - allowOverridesFile: withPubspecOverrides, - containingDescription: RootDescription(dir), - ); + final pubspec = alreadyLoadedPubspecs[p.canonicalize(dir)] ?? + Pubspec.load( + dir, + sources, + expectedName: name, + allowOverridesFile: withPubspecOverrides, + containingDescription: RootDescription(dir), + ); final workspacePackages = pubspec.workspace - .map((e) => Package.load(null, p.join(dir, e), sources)) + .map( + (e) => Package.load( + null, + p.join(dir, e), + sources, + alreadyLoadedPubspecs: alreadyLoadedPubspecs, + withPubspecOverrides: withPubspecOverrides, + ), + ) .toList(); + for (final package in workspacePackages) { + if (package.pubspec.resolution != Resolution.workspace) { + fail(''' +${package.pubspecPath} is inluded in the workspace from ${p.join(dir, 'pubspec.yaml')}, but does not have `resolution: workspace`. + +See $workspacesDocUrl for more information. +'''); + } + } return Package(pubspec, dir, workspacePackages); } diff --git a/lib/src/solver/report.dart b/lib/src/solver/report.dart index da82329815..6154d32358 100644 --- a/lib/src/solver/report.dart +++ b/lib/src/solver/report.dart @@ -153,7 +153,7 @@ $contentHashesDocumentationUrl final dir = _location; if (dir != null) { if (dir != '.') { - suffix = ' in $dir'; + suffix = ' in `$dir`'; } } diff --git a/lib/src/utils.dart b/lib/src/utils.dart index ccfb055969..45871b1198 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -102,6 +102,8 @@ const reservedWords = { 'yield', }; +const workspacesDocUrl = 'https://dart.dev/go/pub-workspaces'; + /// An cryptographically secure instance of [math.Random]. final random = math.Random.secure(); diff --git a/test/add/git/git_test.dart b/test/add/git/git_test.dart index 833dd0f248..19938e3483 100644 --- a/test/add/git/git_test.dart +++ b/test/add/git/git_test.dart @@ -47,7 +47,7 @@ void main() { await pubAdd( args: ['--directory', appPath, 'foo', '--git-url', 'foo.git'], workingDirectory: d.sandbox, - output: contains('Changed 1 dependency in myapp!'), + output: contains('Changed 1 dependency in `myapp`!'), ); await d.dir(cachePath, [ @@ -228,7 +228,7 @@ void main() { 'foo:{"git": {"url":"foo.git", "path":"subdir"}}', ], workingDirectory: d.sandbox, - output: contains('Changed 1 dependency in myapp!'), + output: contains('Changed 1 dependency in `myapp`!'), ); await d.appDir( diff --git a/test/add/path/relative_path_test.dart b/test/add/path/relative_path_test.dart index 5a3dcaa6c9..32ae0d6791 100644 --- a/test/add/path/relative_path_test.dart +++ b/test/add/path/relative_path_test.dart @@ -59,7 +59,7 @@ void main() { await pubAdd( args: ['--directory', appPath, 'foo', '--path', 'foo'], workingDirectory: d.sandbox, - output: contains('Changed 1 dependency in myapp!'), + output: contains('Changed 1 dependency in `myapp`!'), ); await d.appPackageConfigFile([ @@ -180,7 +180,7 @@ void main() { 'bar:{"path":"bar"}', ], workingDirectory: d.sandbox, - output: contains('Changed 2 dependencies in myapp!'), + output: contains('Changed 2 dependencies in `myapp`!'), ); await d.appPackageConfigFile([ diff --git a/test/descriptor.dart b/test/descriptor.dart index 678626a68e..85c2787712 100644 --- a/test/descriptor.dart +++ b/test/descriptor.dart @@ -110,14 +110,25 @@ FileDescriptor libPubspec( String version, { Map? deps, Map? devDeps, + String? resolution, String? sdk, Map? extras, + bool resolutionWorkspace = false, }) { var map = packageMap(name, version, deps, devDeps); + if (resolutionWorkspace && sdk == null) { + sdk = '3.7.0'; + } if (sdk != null) { map['environment'] = {'sdk': sdk}; } - return pubspec({...map, ...extras ?? {}}); + return pubspec( + { + ...map, + if (resolutionWorkspace) 'resolution': 'workspace', + ...extras ?? {}, + }, + ); } /// Describes a file named `pubspec_overrides.yaml` by default, with the given diff --git a/test/get/enforce_lockfile_test.dart b/test/get/enforce_lockfile_test.dart index 9cafa8e807..75c3ee1add 100644 --- a/test/get/enforce_lockfile_test.dart +++ b/test/get/enforce_lockfile_test.dart @@ -84,14 +84,14 @@ Try running `dart pub get` to create `pubspec.lock`. args: ['--enforce-lockfile', '--example'], output: allOf( contains('Got dependencies!'), - contains('Resolving dependencies in $example...'), + contains('Resolving dependencies in `$example`...'), ), error: allOf( contains( - 'Unable to satisfy `$examplePubspec` using `$examplePubspecLock` in $example.', + 'Unable to satisfy `$examplePubspec` using `$examplePubspecLock` in `$example`.', ), contains( - 'To update `$examplePubspecLock` run `dart pub get` in $example without\n' + 'To update `$examplePubspecLock` run `dart pub get` in `$example` without\n' '`--enforce-lockfile`.'), ), exitCode: DATA, diff --git a/test/get/gets_in_example_folder_test.dart b/test/get/gets_in_example_folder_test.dart index 11288d0a13..8e990dab3e 100644 --- a/test/get/gets_in_example_folder_test.dart +++ b/test/get/gets_in_example_folder_test.dart @@ -47,16 +47,16 @@ void main() { Resolving dependencies... Downloading packages... Got dependencies! -Resolving dependencies in $dotExample... +Resolving dependencies in `$dotExample`... Downloading packages... -Got dependencies in $dotExample.''' +Got dependencies in `$dotExample`.''' : ''' Resolving dependencies... Downloading packages... No dependencies changed. -Resolving dependencies in $dotExample... +Resolving dependencies in `$dotExample`... Downloading packages... -Got dependencies in $dotExample.''', +Got dependencies in `$dotExample`.''', ); expect(lockFile.existsSync(), true); expect(exampleLockFile.existsSync(), true); diff --git a/test/get/path/relative_path_test.dart b/test/get/path/relative_path_test.dart index 0f6856abca..8724d9c5c8 100644 --- a/test/get/path/relative_path_test.dart +++ b/test/get/path/relative_path_test.dart @@ -92,7 +92,7 @@ void main() { await pubGet( args: ['--directory', appPath], workingDirectory: d.sandbox, - output: contains('Changed 2 dependencies in myapp!'), + output: contains('Changed 2 dependencies in `myapp`!'), ); await d.appPackageConfigFile([ diff --git a/test/global/activate/installs_dependencies_for_path_test.dart b/test/global/activate/installs_dependencies_for_path_test.dart index ff858085fa..7c9f11e747 100644 --- a/test/global/activate/installs_dependencies_for_path_test.dart +++ b/test/global/activate/installs_dependencies_for_path_test.dart @@ -19,7 +19,7 @@ void main() { ]).create(); var pub = await startPub(args: ['global', 'activate', '-spath', '../foo']); - expect(pub.stdout, emitsThrough('Resolving dependencies in ../foo...')); + expect(pub.stdout, emitsThrough('Resolving dependencies in `../foo`...')); expect(pub.stdout, emitsThrough(startsWith('Activated foo 0.0.0 at path'))); await pub.shouldExit(); diff --git a/test/global/run/missing_path_package_test.dart b/test/global/run/missing_path_package_test.dart index dc98583719..dce42da1a2 100644 --- a/test/global/run/missing_path_package_test.dart +++ b/test/global/run/missing_path_package_test.dart @@ -3,7 +3,6 @@ // BSD-style license that can be found in the LICENSE file. import 'package:path/path.dart' as p; -import 'package:pub/src/exit_codes.dart' as exit_codes; import 'package:pub/src/io.dart'; import 'package:test/test.dart'; @@ -22,11 +21,10 @@ void main() { deleteEntry(p.join(d.sandbox, 'foo')); var pub = await pubRun(global: true, args: ['foo']); - var path = canonicalize(p.join(d.sandbox, 'foo')); expect( pub.stderr, - emits('Could not find a file named "pubspec.yaml" in "$path".'), + emits('The directory `${d.path('foo')}` does not exist.'), ); - await pub.shouldExit(exit_codes.NO_INPUT); + await pub.shouldExit(1); }); } diff --git a/test/pub_get_and_upgrade_test.dart b/test/pub_get_and_upgrade_test.dart index 5000cac1fc..c47081481f 100644 --- a/test/pub_get_and_upgrade_test.dart +++ b/test/pub_get_and_upgrade_test.dart @@ -17,8 +17,9 @@ void main() { await pubCommand( command, - error: RegExp(r'Could not find a file named "pubspec.yaml" ' - r'in "[^\n]*"\.'), + error: RegExp( + 'Found no `pubspec.yaml` file in `${d.path(appPath)}` or parent directories', + ), exitCode: exit_codes.NO_INPUT, ); }); diff --git a/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt b/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt index c448092447..6a8719e686 100644 --- a/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt +++ b/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt @@ -2,74 +2,74 @@ ## Section 0 $ pub add --directory=myapp foo -Resolving dependencies in myapp... +Resolving dependencies in `myapp`... Downloading packages... + foo 1.0.0 -Changed 1 dependency in myapp! -Resolving dependencies in myapp/example... +Changed 1 dependency in `myapp`! +Resolving dependencies in `myapp/example`... Downloading packages... -Got dependencies in myapp/example. +Got dependencies in `myapp/example`. -------------------------------- END OF OUTPUT --------------------------------- ## Section 1 $ pub -C myapp add bar -Resolving dependencies in myapp... +Resolving dependencies in `myapp`... Downloading packages... + bar 1.2.3 -Changed 1 dependency in myapp! -Resolving dependencies in myapp/example... +Changed 1 dependency in `myapp`! +Resolving dependencies in `myapp/example`... Downloading packages... -Got dependencies in myapp/example. +Got dependencies in `myapp/example`. -------------------------------- END OF OUTPUT --------------------------------- ## Section 2 $ pub -C 'myapp/example' get --directory=myapp bar -Resolving dependencies in myapp... +Resolving dependencies in `myapp`... Downloading packages... -Got dependencies in myapp! -Resolving dependencies in myapp/example... +Got dependencies in `myapp`! +Resolving dependencies in `myapp/example`... Downloading packages... -Got dependencies in myapp/example. +Got dependencies in `myapp/example`. -------------------------------- END OF OUTPUT --------------------------------- ## Section 3 $ pub remove bar -C myapp -Resolving dependencies in myapp... +Resolving dependencies in `myapp`... Downloading packages... These packages are no longer being depended on: - bar 1.2.3 -Changed 1 dependency in myapp! -Resolving dependencies in myapp/example... +Changed 1 dependency in `myapp`! +Resolving dependencies in `myapp/example`... Downloading packages... -Got dependencies in myapp/example. +Got dependencies in `myapp/example`. -------------------------------- END OF OUTPUT --------------------------------- ## Section 4 $ pub get bar -C myapp -Resolving dependencies in myapp... +Resolving dependencies in `myapp`... Downloading packages... -Got dependencies in myapp! -Resolving dependencies in myapp/example... +Got dependencies in `myapp`! +Resolving dependencies in `myapp/example`... Downloading packages... -Got dependencies in myapp/example. +Got dependencies in `myapp/example`. -------------------------------- END OF OUTPUT --------------------------------- ## Section 5 $ pub get bar -C 'myapp/example' -Resolving dependencies in myapp/example... +Resolving dependencies in `myapp/example`... Downloading packages... -Got dependencies in myapp/example! +Got dependencies in `myapp/example`! -------------------------------- END OF OUTPUT --------------------------------- ## Section 6 $ pub get bar -C 'myapp/example2' -Resolving dependencies in myapp/example2... +Resolving dependencies in `myapp/example2`... [STDERR] Error on line 1, column 9 of myapp/pubspec.yaml: "name" field doesn't match expected name "myapp". [STDERR] ╷ [STDERR] 1 │ {"name":"test_pkg","version":"1.0.0","homepage":"https://pub.dev","description":"A package, I guess.","environment":{"sdk":">=3.1.2 <=3.2.0"}, dependencies: { foo: ^1.0.0}} @@ -81,38 +81,38 @@ Resolving dependencies in myapp/example2... ## Section 7 $ pub get bar -C 'myapp/broken_dir' -[STDERR] Could not find a file named "pubspec.yaml" in "$SANDBOX/myapp/broken_dir". -[EXIT CODE] 66 +[STDERR] The directory `myapp/broken_dir` does not exist. +[EXIT CODE] 1 -------------------------------- END OF OUTPUT --------------------------------- ## Section 8 $ pub downgrade -C myapp -Resolving dependencies in myapp... +Resolving dependencies in `myapp`... Downloading packages... -No dependencies changed in myapp. -Resolving dependencies in myapp/example... +No dependencies changed in `myapp`. +Resolving dependencies in `myapp/example`... Downloading packages... -Got dependencies in myapp/example. +Got dependencies in `myapp/example`. -------------------------------- END OF OUTPUT --------------------------------- ## Section 9 $ pub upgrade bar -C myapp -Resolving dependencies in myapp... +Resolving dependencies in `myapp`... Downloading packages... -No dependencies changed in myapp. -Resolving dependencies in myapp/example... +No dependencies changed in `myapp`. +Resolving dependencies in `myapp/example`... Downloading packages... -Got dependencies in myapp/example. +Got dependencies in `myapp/example`. -------------------------------- END OF OUTPUT --------------------------------- ## Section 10 $ pub run -C myapp 'bin/app.dart' -Resolving dependencies in myapp... +Resolving dependencies in `myapp`... Downloading packages... -Got dependencies in myapp. +Got dependencies in `myapp`. Building package executable... Built test_pkg:app. Hi @@ -121,9 +121,9 @@ Hi ## Section 11 $ pub publish -C myapp --dry-run -Resolving dependencies in myapp... +Resolving dependencies in `myapp`... Downloading packages... -Got dependencies in myapp! +Got dependencies in `myapp`! Publishing test_pkg 1.0.0 to http://localhost:$PORT: ├── CHANGELOG.md (<1 KB) ├── LICENSE (<1 KB) @@ -158,9 +158,9 @@ $ pub uploader -C myapp add sigurdm@google.com ## Section 13 $ pub deps -C myapp -Resolving dependencies in myapp... +Resolving dependencies in `myapp`... Downloading packages... -Got dependencies in myapp. +Got dependencies in `myapp`. Dart SDK 3.1.2+3 test_pkg 1.0.0 └── foo 1.0.0 diff --git a/test/testdata/goldens/outdated/outdated_test/no pubspec.txt b/test/testdata/goldens/outdated/outdated_test/no pubspec.txt index ba8858a4b6..c8def95446 100644 --- a/test/testdata/goldens/outdated/outdated_test/no pubspec.txt +++ b/test/testdata/goldens/outdated/outdated_test/no pubspec.txt @@ -2,6 +2,6 @@ ## Section 0 $ pub outdated -[STDERR] Could not find a file named "pubspec.yaml" in "$SANDBOX/myapp". +[STDERR] Found no `pubspec.yaml` file in `$SANDBOX/myapp` or parent directories [EXIT CODE] 66 diff --git a/test/testdata/goldens/upgrade/example_warns_about_major_versions_test/pub upgrade --major-versions does not update major versions in example~.txt b/test/testdata/goldens/upgrade/example_warns_about_major_versions_test/pub upgrade --major-versions does not update major versions in example~.txt index d53310efba..b5ea61a386 100644 --- a/test/testdata/goldens/upgrade/example_warns_about_major_versions_test/pub upgrade --major-versions does not update major versions in example~.txt +++ b/test/testdata/goldens/upgrade/example_warns_about_major_versions_test/pub upgrade --major-versions does not update major versions in example~.txt @@ -9,19 +9,19 @@ Changed 1 dependency! Changed 1 constraint in pubspec.yaml: bar: ^1.0.0 -> ^2.0.0 -Resolving dependencies in ./example... +Resolving dependencies in `./example`... Downloading packages... -Got dependencies in ./example. +Got dependencies in `./example`. [STDERR] Running `upgrade --major-versions` only in `.`. Run `dart pub upgrade --major-versions --directory example/` separately. -------------------------------- END OF OUTPUT --------------------------------- ## Section 1 $ pub upgrade --major-versions --directory example -Resolving dependencies in example... +Resolving dependencies in `example`... Downloading packages... > foo 2.0.0 (was 1.0.0) -Changed 1 dependency in example! +Changed 1 dependency in `example`! Changed 1 constraint in pubspec.yaml: foo: ^1.0.0 -> ^2.0.0 diff --git a/test/unpack_test.dart b/test/unpack_test.dart index cdf1f6ead9..ebf83ce7cc 100644 --- a/test/unpack_test.dart +++ b/test/unpack_test.dart @@ -72,7 +72,7 @@ void main() { output: allOf( contains(''' Downloading foo 1.2.3 to `.${s}foo-1.2.3`... -Resolving dependencies in .${s}foo-1.2.3... +Resolving dependencies in `.${s}foo-1.2.3`... '''), contains('To explore type: cd .${s}foo-1.2.3'), contains( @@ -97,7 +97,7 @@ Resolving dependencies in .${s}foo-1.2.3... output: allOf( contains(''' Downloading foo 1.2.3-pre to `../foo-1.2.3-pre`... -Resolving dependencies in ../foo-1.2.3-pre... +Resolving dependencies in `../foo-1.2.3-pre`... '''), contains('To explore type: cd ../foo-1.2.3-pre'), ), diff --git a/test/workspace_test.dart b/test/workspace_test.dart index 60204faf39..0ba6e4268a 100644 --- a/test/workspace_test.dart +++ b/test/workspace_test.dart @@ -26,7 +26,12 @@ void main() { ), dir('pkgs', [ dir('a', [ - libPubspec('a', '1.1.1', devDeps: {'dev_dep': '^1.0.0'}), + libPubspec( + 'a', + '1.1.1', + devDeps: {'dev_dep': '^1.0.0'}, + resolutionWorkspace: true, + ), ]), ]), ]).create(); @@ -62,7 +67,12 @@ void main() { ), dir('pkgs', [ dir('a', [ - libPubspec('a', '1.1.1', deps: {'b': '^2.0.0'}), + libPubspec( + 'a', + '1.1.1', + deps: {'b': '^2.0.0'}, + resolutionWorkspace: true, + ), ]), dir('b', [ libPubspec( @@ -71,6 +81,7 @@ void main() { deps: { 'myapp': {'git': 'somewhere'}, }, + resolutionWorkspace: true, ), ]), ]), @@ -109,7 +120,7 @@ void main() { extras: { 'workspace': ['example'], }, - sdk: '^3.7.0', + resolutionWorkspace: true, ), dir('example', [ libPubspec( @@ -118,6 +129,7 @@ void main() { deps: { 'a': {'path': '..'}, }, + resolutionWorkspace: true, ), ]), ]), @@ -151,7 +163,12 @@ void main() { ), dir('pkgs', [ dir('a', [ - libPubspec('a', '1.1.1', deps: {'myapp': '^0.2.3'}), + libPubspec( + 'a', + '1.1.1', + deps: {'myapp': '^0.2.3'}, + resolutionWorkspace: true, + ), ]), ]), ]).create(); @@ -181,10 +198,53 @@ void main() { deps: { 'myapp': {'posted': 'https://abc'}, }, + resolutionWorkspace: true, ), ]), ]), ]).create(); await pubGet(environment: {'_PUB_TEST_SDK_VERSION': '3.7.0'}); }); + + test('Can resolve from any directory inside the workspace', () async { + await dir(appPath, [ + libPubspec( + 'myapp', + '1.2.3', + extras: { + 'workspace': ['pkgs/a'], + }, + sdk: '^3.7.0', + ), + dir('pkgs', [ + dir('a', [ + libPubspec( + 'a', + '1.1.1', + deps: { + 'myapp': {'posted': 'https://abc'}, + }, + resolutionWorkspace: true, + ), + ]), + ]), + ]).create(); + await pubGet( + environment: {'_PUB_TEST_SDK_VERSION': '3.7.0'}, + workingDirectory: p.join(sandbox, appPath, 'pkgs'), + output: contains('Resolving dependencies in `..`...'), + ); + await pubGet( + environment: {'_PUB_TEST_SDK_VERSION': '3.7.0'}, + workingDirectory: p.join(sandbox, appPath, 'pkgs', 'a'), + output: contains('Resolving dependencies in `../..`...'), + ); + + await pubGet( + args: ['-C$appPath/pkgs'], + environment: {'_PUB_TEST_SDK_VERSION': '3.7.0'}, + workingDirectory: sandbox, + output: contains('Resolving dependencies in `$appPath`...'), + ); + }); } From 9996cf8ce723b5f0520b28007a97c58534f5ac75 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Mon, 18 Mar 2024 14:13:55 +0000 Subject: [PATCH 02/10] Remove TODOS --- lib/src/entrypoint.dart | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index cd20235392..558986c93a 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -59,8 +59,6 @@ class Entrypoint { /// /// [workspaceRoot] will be the package in the nearest parent directory that /// has `resolution: null` - // TODO(https://github.com/dart-lang/pub/issues/4127): make this actually - // true. final String workingDir; static ({Package root, Package work}) _loadWorkspace( @@ -145,8 +143,6 @@ See $workspacesDocUrl for more information.''', /// /// It will be the package in the nearest parent directory to `workingDir`. /// Example: if a workspace looks like this: - // TODO(https://github.com/dart-lang/pub/issues/4127): make this actually - // true. /// /// foo/ pubspec.yaml # contains `workspace: [- 'bar'] bar/ pubspec.yaml # /// contains `resolution: workspace`. From 3d70553e11cc35f437e50152d5105b771edaeb6e Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 19 Mar 2024 08:46:58 +0000 Subject: [PATCH 03/10] Handle .. --- lib/src/io.dart | 4 ++-- test/workspace_test.dart | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/src/io.dart b/lib/src/io.dart index 11f2418065..ab16cdc474 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart @@ -1248,8 +1248,8 @@ Iterable parentDirs(String path, {String? from}) sync* { } else { yield d; } - if (p.isRelative(d) && p.equals(d, '.')) { - d = from ?? p.current; + if (!p.isWithin(from ?? p.current, d)) { + d = p.normalize(p.join(from ?? p.current, d)); relative = true; } final parent = p.dirname(d); diff --git a/test/workspace_test.dart b/test/workspace_test.dart index 0ba6e4268a..affa0a4309 100644 --- a/test/workspace_test.dart +++ b/test/workspace_test.dart @@ -246,5 +246,16 @@ void main() { workingDirectory: sandbox, output: contains('Resolving dependencies in `$appPath`...'), ); + + await pubGet( + args: ['-C..'], + environment: {'_PUB_TEST_SDK_VERSION': '3.7.0'}, + workingDirectory: p.join( + sandbox, + appPath, + 'pkgs', + ), + output: contains('Resolving dependencies in `..`...'), + ); }); } From 909f8a1e3338c708f19ffa66339f33b21f1a1f8d Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 19 Mar 2024 08:47:23 +0000 Subject: [PATCH 04/10] Clarify "root" in doc --- lib/src/io.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/src/io.dart b/lib/src/io.dart index ab16cdc474..fabafa3dd3 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart @@ -1230,7 +1230,8 @@ String escapeShellArgument(String x) => ? "'${x.replaceAll(r'\', r'\\').replaceAll("'", r"'\''")}'" : x; -/// Returns all parent directories of [path], starting from [path] to the root. +/// Returns all parent directories of [path], starting from [path] to the +/// filesystem root. /// /// If [path] is relative the directories will also be. /// From 763e8bba6a17a183ba5448df36d2222c6335e475 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 19 Mar 2024 08:47:44 +0000 Subject: [PATCH 05/10] Normalize instead of canonicalize in message. --- lib/src/entrypoint.dart | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index 558986c93a..f3db6f0ccb 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -105,7 +105,7 @@ class Entrypoint { } if (pubspecsMet.isEmpty) { throw FileException( - 'Found no `pubspec.yaml` file in `${p.canonicalize(workingDir)}` or parent directories', + 'Found no `pubspec.yaml` file in `${p.normalize(p.absolute(workingDir))}` or parent directories', p.join(workingDir, 'pubspec.yaml'), ); } else { @@ -121,7 +121,11 @@ See $workspacesDocUrl for more information.''', } } + /// Stores the result of [_loadWorkspace]. + /// Only access via [workspaceRoot], [workPackage] and [canFindWorkspaceRoot]. ({Package root, Package work})? _packages; + + /// Only access via [workspaceRoot], [workPackage] and [canFindWorkspaceRoot]. ({Package root, Package work}) get _getPackages => _packages ??= _loadWorkspace(workingDir, cache); @@ -130,6 +134,8 @@ See $workspacesDocUrl for more information.''', /// For a global package, this is the activated package. Package get workspaceRoot => _getPackages.root; + /// True if we can find a `pubspec.yaml` to resolve in [workingDir] or any + /// parent directory. bool get canFindWorkspaceRoot { try { _getPackages; From a496fb931304ed0dc0c9a6753940cc87ee4f459e Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 19 Mar 2024 11:27:24 +0000 Subject: [PATCH 06/10] Use p.separator in test-expectation --- test/workspace_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/workspace_test.dart b/test/workspace_test.dart index affa0a4309..d603f5da52 100644 --- a/test/workspace_test.dart +++ b/test/workspace_test.dart @@ -234,10 +234,11 @@ void main() { workingDirectory: p.join(sandbox, appPath, 'pkgs'), output: contains('Resolving dependencies in `..`...'), ); + final s = p.separator; await pubGet( environment: {'_PUB_TEST_SDK_VERSION': '3.7.0'}, workingDirectory: p.join(sandbox, appPath, 'pkgs', 'a'), - output: contains('Resolving dependencies in `../..`...'), + output: contains('Resolving dependencies in `..$s..`...'), ); await pubGet( From 4e56d6c85c66cdce3bceb11394e532413e20882b Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Thu, 21 Mar 2024 11:46:26 +0000 Subject: [PATCH 07/10] Fix expectation --- test/pub_get_and_upgrade_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/pub_get_and_upgrade_test.dart b/test/pub_get_and_upgrade_test.dart index c47081481f..61dfa453fa 100644 --- a/test/pub_get_and_upgrade_test.dart +++ b/test/pub_get_and_upgrade_test.dart @@ -17,7 +17,7 @@ void main() { await pubCommand( command, - error: RegExp( + error: contains( 'Found no `pubspec.yaml` file in `${d.path(appPath)}` or parent directories', ), exitCode: exit_codes.NO_INPUT, From f0b9ca5f39b6616d4b72d0e2bc87b2b27f4cd3e2 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Thu, 21 Mar 2024 11:58:17 +0000 Subject: [PATCH 08/10] Document _loadWorkspace --- lib/src/entrypoint.dart | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index f3db6f0ccb..cd63e275aa 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -61,6 +61,22 @@ class Entrypoint { /// has `resolution: null` final String workingDir; + /// Finds the [workspaceRoot] and [workPackage] based on [workingDir]. + /// + /// Works by iterating through the parent directories from [workingDir]. + /// + /// [workPackage] is the package of first dir we find with a `pubspec.yaml` + /// file. + /// + /// [workspaceRoot] is the package of the first dir we find with a + /// `pubspec.yaml` that does not have `resolution: workspace`. + /// + /// [workPackage] and [workspaceRoot] can be the same. And will always be the + /// same when no `workspace` is involved. + /// = + /// If [workingDir] doesn't exist, [fail]. + /// + /// If no `pubspec.yaml` is found without `resolution: workspace` we [fail]. static ({Package root, Package work}) _loadWorkspace( String workingDir, SystemCache cache, From 8b9eeba6f1c81a7e70744469addadfa7b94b3e5c Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Thu, 21 Mar 2024 11:59:29 +0000 Subject: [PATCH 09/10] Remove bare expression --- lib/src/entrypoint.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index cd63e275aa..891126d555 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -111,7 +111,6 @@ class Entrypoint { withPubspecOverrides: true, ); for (final package in root.transitiveWorkspace) { - pubspecsMet.entries.first; if (identical(pubspecsMet.entries.first.value, package.pubspec)) { return (root: root, work: package); } From 8c21037a9daf3e8c6d1c9e4d442be998987aec3a Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Fri, 22 Mar 2024 09:21:27 +0000 Subject: [PATCH 10/10] Make package.load take pubspec loading function --- lib/src/entrypoint.dart | 32 ++++++++++++++++++++++++++------ lib/src/package.dart | 36 ++++++++++++++++++++++-------------- lib/src/source/git.dart | 2 +- lib/src/source/hosted.dart | 4 ++-- lib/src/system_cache.dart | 4 ++-- test/ascii_tree_test.dart | 8 +++----- 6 files changed, 56 insertions(+), 30 deletions(-) diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index 891126d555..45b7cfdf59 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -104,10 +104,21 @@ class Entrypoint { final Package root; if (pubspec.resolution == Resolution.none) { root = Package.load( - null, dir, cache.sources, - alreadyLoadedPubspecs: pubspecsMet, + loadPubspec: ( + path, { + expectedName, + required withPubspecOverrides, + }) => + pubspecsMet[p.canonicalize(path)] ?? + Pubspec.load( + path, + cache.sources, + expectedName: expectedName, + allowOverridesFile: withPubspecOverrides, + containingDescription: RootDescription(path), + ), withPubspecOverrides: true, ); for (final package in root.transitiveWorkspace) { @@ -268,9 +279,9 @@ See $workspacesDocUrl for more information.''', var packages = { for (var packageEntry in packageConfig.nonInjectedPackages) packageEntry.name: Package.load( - packageEntry.name, packageEntry.resolvedRootDir(packageConfigPath), cache.sources, + expectedName: packageEntry.name, ), }; packages[workspaceRoot.name] = workspaceRoot; @@ -334,10 +345,19 @@ See $workspacesDocUrl for more information.''', // Then override the one of the workPackage. existingPubspecs[p.canonicalize(workPackage.dir)] = pubspec; final newWorkspaceRoot = Package.load( - null, workspaceRoot.dir, cache.sources, - alreadyLoadedPubspecs: existingPubspecs, + loadPubspec: ( + dir, { + expectedName, + required withPubspecOverrides, + }) => + existingPubspecs[p.canonicalize(dir)] ?? + Pubspec.load( + dir, + cache.sources, + containingDescription: RootDescription(dir), + ), ); final newWorkPackage = newWorkspaceRoot.transitiveWorkspace .firstWhere((package) => package.dir == workPackage.dir); @@ -1058,7 +1078,7 @@ To update `$lockFilePath` run `$topLevelProgram pub get`$suffix without } var touchedLockFile = false; late final lockFile = _loadLockFile(lockFilePath, cache); - late final root = Package.load(null, dir, cache.sources); + late final root = Package.load(dir, cache.sources); if (!lockfileNewerThanPubspecs) { if (isLockFileUpToDate(lockFile, root)) { diff --git a/lib/src/package.dart b/lib/src/package.dart index 4285d1cf92..568567917a 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart @@ -125,6 +125,8 @@ class Package { /// Loads the package whose root directory is [packageDir]. /// + /// Will also load the workspace sub-packages of this package (recursively). + /// /// [name] is the expected name of that package (e.g. the name given in the /// dependency), or `null` if the package being loaded is the entrypoint /// package. @@ -132,34 +134,40 @@ class Package { /// `pubspec_overrides.yaml` is only loaded if [withPubspecOverrides] is /// `true`. /// - /// [alreadyLoadedPubspecs] contains a map from the `p.canonicalize`d - /// directory containing a pubspec, to the loaded [Pubspec] object. + /// [loadPubspec] if given will be used to obtain a pubspec from a path. Also + /// for the workspace children. /// /// This mechanism can be used to avoid loading pubspecs twice. It can also be /// used to override a pubspec in memory for trying out an alternative /// resolution. factory Package.load( - String? name, String dir, SourceRegistry sources, { bool withPubspecOverrides = false, - Map alreadyLoadedPubspecs = const {}, + String? expectedName, + Pubspec Function( + String path, { + String? expectedName, + required bool withPubspecOverrides, + })? loadPubspec, }) { - final pubspec = alreadyLoadedPubspecs[p.canonicalize(dir)] ?? - Pubspec.load( - dir, - sources, - expectedName: name, - allowOverridesFile: withPubspecOverrides, - containingDescription: RootDescription(dir), - ); + loadPubspec ??= + (path, {expectedName, required withPubspecOverrides}) => Pubspec.load( + path, + sources, + containingDescription: RootDescription(path), + ); + final pubspec = loadPubspec( + dir, + withPubspecOverrides: withPubspecOverrides, + expectedName: expectedName, + ); final workspacePackages = pubspec.workspace .map( (e) => Package.load( - null, p.join(dir, e), sources, - alreadyLoadedPubspecs: alreadyLoadedPubspecs, + loadPubspec: loadPubspec, withPubspecOverrides: withPubspecOverrides, ), ) diff --git a/lib/src/source/git.dart b/lib/src/source/git.dart index 99bf59bff4..7f863aa3d9 100644 --- a/lib/src/source/git.dart +++ b/lib/src/source/git.dart @@ -443,7 +443,7 @@ class GitSource extends CachedSource { var packageDir = p.join(revisionCachePath, relative); try { - return Package.load(null, packageDir, cache.sources); + return Package.load(packageDir, cache.sources); } catch (error, stackTrace) { log.error('Failed to load package', error, stackTrace); var name = p.basename(revisionCachePath).split('-').first; diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart index 7a2c9dea33..b51baa928f 100644 --- a/lib/src/source/hosted.dart +++ b/lib/src/source/hosted.dart @@ -1277,7 +1277,7 @@ class HostedSource extends CachedSource { var packages = []; for (var entry in listDir(serverDir)) { try { - packages.add(Package.load(null, entry, cache.sources)); + packages.add(Package.load(entry, cache.sources)); } catch (error, stackTrace) { log.error('Failed to load package', error, stackTrace); final id = _idForBasename( @@ -1384,7 +1384,7 @@ class HostedSource extends CachedSource { .where(_looksLikePackageDir) .map((entry) { try { - return Package.load(null, entry, cache.sources); + return Package.load(entry, cache.sources); } catch (error, stackTrace) { log.fine('Failed to load package from $entry:\n' '$error\n' diff --git a/lib/src/system_cache.dart b/lib/src/system_cache.dart index 222c375b0c..e20a60b669 100644 --- a/lib/src/system_cache.dart +++ b/lib/src/system_cache.dart @@ -111,16 +111,16 @@ Consider setting the `PUB_CACHE` variable manually. /// /// Throws an [ArgumentError] if [id] has an invalid source. Package load(PackageId id) { - return Package.load(id.name, getDirectory(id), sources); + return Package.load(getDirectory(id), sources, expectedName: id.name); } Package loadCached(PackageId id) { final source = id.description.description.source; if (source is CachedSource) { return Package.load( - id.name, source.getDirectoryInCache(id, this), sources, + expectedName: id.name, ); } else { throw ArgumentError('Call only on Cached ids.'); diff --git a/test/ascii_tree_test.dart b/test/ascii_tree_test.dart index d0a921bd42..b406f725e5 100644 --- a/test/ascii_tree_test.dart +++ b/test/ascii_tree_test.dart @@ -61,11 +61,9 @@ void main() { file('path.dart', bytes(100)), ]), ]).create(); - var files = Package.load( - null, - path(appPath), - (name) => throw UnimplementedError(), - ).listFiles(); + var files = + Package.load(path(appPath), (name) => throw UnimplementedError()) + .listFiles(); ctx.expectNextSection( tree.fromFiles(files, baseDir: path(appPath), showFileSizes: true), );