Support for resolving all packages in workspace together (#4154)
diff --git a/lib/src/command/add.dart b/lib/src/command/add.dart index a59ecd6..07f1c36 100644 --- a/lib/src/command/add.dart +++ b/lib/src/command/add.dart
@@ -208,7 +208,11 @@ solveResult = await resolveVersions( SolveType.upgrade, cache, - Package(resolutionPubspec, entrypoint.rootDir), + Package( + resolutionPubspec, + entrypoint.rootDir, + entrypoint.root.workspaceChildren, + ), ); } on GitException { final name = updates.first.ref.name; @@ -356,6 +360,7 @@ dependencies: dependencies, devDependencies: devDependencies, dependencyOverrides: dependencyOverrides, + workspace: original.workspace, ); }
diff --git a/lib/src/command/dependency_services.dart b/lib/src/command/dependency_services.dart index 451643f..f095722 100644 --- a/lib/src/command/dependency_services.dart +++ b/lib/src/command/dependency_services.dart
@@ -69,13 +69,21 @@ final breakingPubspec = stripVersionBounds(compatiblePubspec); final compatiblePackagesResult = await _tryResolve( - Package(compatiblePubspec, entrypoint.rootDir), + Package( + compatiblePubspec, + entrypoint.rootDir, + entrypoint.root.workspaceChildren, + ), cache, additionalConstraints: additionalConstraints, ); final breakingPackagesResult = await _tryResolve( - Package(breakingPubspec, entrypoint.rootDir), + Package( + breakingPubspec, + entrypoint.rootDir, + entrypoint.root.workspaceChildren, + ), cache, additionalConstraints: additionalConstraints, ); @@ -101,6 +109,7 @@ sdkConstraints: compatiblePubspec.sdkConstraints, dependencies: compatiblePubspec.dependencies.values, devDependencies: compatiblePubspec.devDependencies.values, + workspace: compatiblePubspec.workspace, ); final dependencySet = _dependencySetOfPackage(singleBreakingPubspec, package); @@ -111,7 +120,11 @@ .toRef() .withConstraint(stripUpperBound(package.toRange().constraint)); final singleBreakingPackagesResult = await _tryResolve( - Package(singleBreakingPubspec, entrypoint.rootDir), + Package( + singleBreakingPubspec, + entrypoint.rootDir, + entrypoint.root.workspaceChildren, + ), cache, ); singleBreakingVersion = singleBreakingPackagesResult @@ -128,7 +141,11 @@ ); final smallestUpgradeResult = await _tryResolve( - Package(atLeastCurrentPubspec, entrypoint.rootDir), + Package( + atLeastCurrentPubspec, + entrypoint.rootDir, + entrypoint.root.workspaceChildren, + ), cache, solveType: SolveType.downgrade, additionalConstraints: additionalConstraints, @@ -216,7 +233,14 @@ final currentPackages = fileExists(entrypoint.lockFilePath) ? entrypoint.lockFile.packages.values.toList() - : (await _tryResolve(Package(pubspec, entrypoint.rootDir), cache) ?? + : (await _tryResolve( + Package( + pubspec, + entrypoint.rootDir, + entrypoint.root.workspaceChildren, + ), + cache, + ) ?? <PackageId>[]); final dependencies = <Object>[]; @@ -428,6 +452,7 @@ location: toUri(entrypoint.pubspecPath), ), entrypoint.rootDir, + entrypoint.root.workspaceChildren, ), lockFile: updatedLockfile, ); @@ -720,6 +745,7 @@ dependencies: rootPubspec.dependencies.values, devDependencies: rootPubspec.devDependencies.values, sdkConstraints: rootPubspec.sdkConstraints, + workspace: rootPubspec.workspace, ); final dependencySet = _dependencySetOfPackage(pubspec, package); @@ -734,7 +760,7 @@ ? SolveType.downgrade : SolveType.get, cache, - Package(pubspec, entrypoint.rootDir), + Package(pubspec, entrypoint.rootDir, entrypoint.root.workspaceChildren), lockFile: lockFile, additionalConstraints: additionalConstraints, );
diff --git a/lib/src/command/outdated.dart b/lib/src/command/outdated.dart index d0a42f1..9def6fe 100644 --- a/lib/src/command/outdated.dart +++ b/lib/src/command/outdated.dart
@@ -146,7 +146,11 @@ 'Resolving', () async { final upgradablePackagesResult = await _tryResolve( - Package(upgradablePubspec, entrypoint.rootDir), + Package( + upgradablePubspec, + entrypoint.rootDir, + entrypoint.root.workspaceChildren, + ), cache, lockFile: entrypoint.lockFile, ); @@ -154,7 +158,11 @@ upgradablePackages = upgradablePackagesResult ?? []; final resolvablePackagesResult = await _tryResolve( - Package(resolvablePubspec, entrypoint.rootDir), + Package( + resolvablePubspec, + entrypoint.rootDir, + entrypoint.root.workspaceChildren, + ), cache, lockFile: entrypoint.lockFile, );
diff --git a/lib/src/command/remove.dart b/lib/src/command/remove.dart index b27bb1b..5490098 100644 --- a/lib/src/command/remove.dart +++ b/lib/src/command/remove.dart
@@ -130,6 +130,7 @@ dependencies: dependencies.values, devDependencies: devDependencies.values, dependencyOverrides: overrides.values, + workspace: original.workspace, ); }
diff --git a/lib/src/command/upgrade.dart b/lib/src/command/upgrade.dart index 7a32bd6..cddd6c7 100644 --- a/lib/src/command/upgrade.dart +++ b/lib/src/command/upgrade.dart
@@ -278,7 +278,11 @@ return await resolveVersions( SolveType.upgrade, cache, - Package(resolvablePubspec, entrypoint.rootDir), + Package( + resolvablePubspec, + entrypoint.rootDir, + entrypoint.root.workspaceChildren, + ), ); }, condition: _shouldShowSpinner, @@ -331,6 +335,7 @@ Package( _updatedPubspec(newPubspecText, entrypoint), entrypoint.rootDir, + entrypoint.root.workspaceChildren, ), ); changes = tighten(
diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index fb599ef..dc83f55 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart
@@ -234,8 +234,10 @@ Entrypoint( this.rootDir, this.cache, { - Pubspec? pubspec, - }) : _root = pubspec == null ? null : Package(pubspec, rootDir), + ({Pubspec pubspec, List<Package> workspacePackages})? preloaded, + }) : _root = preloaded == null + ? null + : Package(preloaded.pubspec, rootDir, preloaded.workspacePackages), globalDir = null { if (p.isWithin(cache.rootDir, rootDir)) { fail('Cannot operate on packages inside the cache.'); @@ -251,7 +253,11 @@ _example, _packageGraph, cache, - Package(pubspec, rootDir), + Package( + pubspec, + rootDir, + root.workspaceChildren, + ), globalDir, ); } @@ -330,14 +336,20 @@ } if (!isGlobal) { - entries.add( - PackageConfigEntry( - name: root.name, - rootUri: p.toUri('../'), - packageUri: p.toUri('lib/'), - languageVersion: root.pubspec.languageVersion, - ), - ); + /// Run through the entire workspace transitive closure and add an entry + /// for each package. + for (final package in root.transitiveWorkspace) { + entries.add( + PackageConfigEntry( + name: package.name, + rootUri: p.toUri( + p.relative(package.dir, from: p.join(rootDir, '.dart_tool')), + ), + packageUri: p.toUri('lib/'), + languageVersion: package.pubspec.languageVersion, + ), + ); + } } final packageConfig = PackageConfig(
diff --git a/lib/src/global_packages.dart b/lib/src/global_packages.dart index 83b4831..4cfbc7e 100644 --- a/lib/src/global_packages.dart +++ b/lib/src/global_packages.dart
@@ -219,6 +219,7 @@ sources: cache.sources, ), _packageDir(name), + [], ); // Resolve it and download its dependencies.
diff --git a/lib/src/package.dart b/lib/src/package.dart index 7a67e7e..286fc90 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart
@@ -44,6 +44,22 @@ /// The parsed pubspec associated with this package. final Pubspec pubspec; + /// The (non-transitive) workspace packages. + final List<Package> workspaceChildren; + + /// The transitive closure of [workspaceChildren] rooted at this package. + /// + /// Includes this package. + Iterable<Package> get transitiveWorkspace sync* { + final stack = [this]; + + while (stack.isNotEmpty) { + final current = stack.removeLast(); + yield current; + stack.addAll(current.workspaceChildren); + } + } + /// The immediate dependencies this package specifies in its pubspec. Map<String, PackageRange> get dependencies => pubspec.dependencies; @@ -119,14 +135,17 @@ expectedName: name, allowOverridesFile: withPubspecOverrides, ); - return Package(pubspec, dir); + final workspacePackages = pubspec.workspace + .map((e) => Package.load(null, p.join(dir, e), sources)) + .toList(); + return Package(pubspec, dir, workspacePackages); } /// Creates a package with [pubspec] associated with [dir]. /// /// For temporary resolution attempts [pubspec] does not have to correspond /// to the one at disk. - Package(this.pubspec, this.dir); + Package(this.pubspec, this.dir, this.workspaceChildren); /// Given a relative path within this package, returns its absolute path. ///
diff --git a/lib/src/package_graph.dart b/lib/src/package_graph.dart index a375493..0aa61b1 100644 --- a/lib/src/package_graph.dart +++ b/lib/src/package_graph.dart
@@ -37,11 +37,12 @@ ) { final packages = { for (final id in result.packages) - id.name: id.name == entrypoint.root.name + id.name: id.isRoot ? entrypoint.root : Package( result.pubspecs[id.name]!, entrypoint.cache.getDirectory(id), + [], ), };
diff --git a/lib/src/pubspec_utils.dart b/lib/src/pubspec_utils.dart index 3259d6e..a8a3d69 100644 --- a/lib/src/pubspec_utils.dart +++ b/lib/src/pubspec_utils.dart
@@ -22,6 +22,7 @@ dependencies: original.dependencies.values, devDependencies: [], // explicitly give empty list, to prevent lazy parsing dependencyOverrides: original.dependencyOverrides.values, + workspace: original.workspace, ); } @@ -36,6 +37,7 @@ dependencies: original.dependencies.values, devDependencies: original.devDependencies.values, dependencyOverrides: [], + workspace: original.workspace, ); } @@ -85,6 +87,7 @@ dependencies: stripBounds(original.dependencies), devDependencies: stripBounds(original.devDependencies), dependencyOverrides: original.dependencyOverrides.values, + workspace: original.workspace, ); } @@ -121,6 +124,7 @@ dependencies: fixBounds(original.dependencies), devDependencies: fixBounds(original.devDependencies), dependencyOverrides: original.dependencyOverrides.values, + workspace: original.workspace, ); }
diff --git a/lib/src/solver/package_lister.dart b/lib/src/solver/package_lister.dart index f233aad..04b8da3 100644 --- a/lib/src/solver/package_lister.dart +++ b/lib/src/solver/package_lister.dart
@@ -15,6 +15,7 @@ import '../package_name.dart'; import '../pubspec.dart'; import '../sdk.dart'; +import '../source/root.dart'; import '../system_cache.dart'; import '../utils.dart'; import 'incompatibility.dart'; @@ -28,7 +29,7 @@ final PackageRef _ref; /// Only used when _ref is root. - final Pubspec? _rootPubspec; + final Package? _rootPackage; /// The version of this package in the lockfile. /// @@ -84,13 +85,23 @@ /// All versions of the package, sorted by [Version.compareTo]. Future<List<PackageId>> get _versions => _versionsMemo.runOnce(() async { - var cachedVersions = (await withDependencyType( - _dependencyType, - () => _systemCache.getVersions( - _ref, - allowedRetractedVersion: _allowedRetractedVersion, - ), - )) + var cachedVersions = _ref.isRoot + ? [ + PackageId( + _ref.name, + _rootPackage!.pubspec.version, + ResolvedRootDescription( + _ref.description as RootDescription, + ), + ), + ] + : (await withDependencyType( + _dependencyType, + () => _systemCache.getVersions( + _ref, + allowedRetractedVersion: _allowedRetractedVersion, + ), + )) ..sort((id1, id2) => id1.version.compareTo(id2.version)); _cachedVersions = cachedVersions; return cachedVersions; @@ -114,7 +125,7 @@ bool downgrade = false, this.sdkOverrides = const {}, }) : _isDowngrade = downgrade, - _rootPubspec = null; + _rootPackage = null; /// Creates a package lister for the root [package]. PackageLister.root( @@ -132,7 +143,7 @@ _isDowngrade = false, _allowedRetractedVersion = null, sdkOverrides = sdkOverrides ?? {}, - _rootPubspec = package.pubspec; + _rootPackage = package; /// Returns the number of versions of this package that match [constraint]. Future<int> countVersions(VersionConstraint constraint) async { @@ -198,11 +209,14 @@ /// dependencies, this will return incompatibilities that reflect that. It /// won't return incompatibilities that have already been returned by a /// previous call to [incompatibilitiesFor]. + /// + /// For a root package, incompatibilities for its dev-dependencies and + /// workspace-children are also added. Future<List<Incompatibility>> incompatibilitiesFor(PackageId id) async { if (_knownInvalidVersions.allows(id.version)) return const []; Pubspec pubspec; if (id.isRoot) { - pubspec = _rootPubspec!; + pubspec = _rootPackage!.pubspec; } else { try { pubspec = await withDependencyType( @@ -259,9 +273,16 @@ if (id.isRoot) ...pubspec.devDependencies.values .where((range) => !_overriddenPackages.contains(range.name)), - if (id.isRoot) ...pubspec.dependencyOverrides.values, + if (id.isRoot) ...[ + ..._rootPackage!.workspaceChildren.map((p) { + return PackageRange( + PackageRef(p.name, RootDescription(p.dir)), + VersionConstraint.any, + ); + }), + ...pubspec.dependencyOverrides.values, + ], ]; - return entries.map((range) => _dependency(depender, range)).toList(); }
diff --git a/lib/src/solver/solve_suggestions.dart b/lib/src/solver/solve_suggestions.dart index 0ffcc00..fbff1c9 100644 --- a/lib/src/solver/solve_suggestions.dart +++ b/lib/src/solver/solve_suggestions.dart
@@ -184,8 +184,13 @@ stripLowerBound: true, ); - final result = - await _tryResolve(Package(relaxedPubspec, entrypoint.rootDir)); + final result = await _tryResolve( + Package( + relaxedPubspec, + entrypoint.rootDir, + entrypoint.root.workspaceChildren, + ), + ); if (result == null) { return null; } @@ -223,8 +228,13 @@ final relaxedPubspec = stripVersionBounds(originalPubspec, stripLowerBound: stripLowerBound); - final result = - await _tryResolve(Package(relaxedPubspec, entrypoint.rootDir)); + final result = await _tryResolve( + Package( + relaxedPubspec, + entrypoint.rootDir, + entrypoint.root.workspaceChildren, + ), + ); if (result == null) { return null; }
diff --git a/lib/src/solver/version_solver.dart b/lib/src/solver/version_solver.dart index e54814b..3f12427 100644 --- a/lib/src/solver/version_solver.dart +++ b/lib/src/solver/version_solver.dart
@@ -62,6 +62,11 @@ /// The entrypoint package, whose dependencies seed the version solve process. final Package _root; + /// Mapping all root packages in the workspace from their name. + late final Map<String, Package> _rootPackages = { + for (final package in _root.transitiveWorkspace) package.name: package, + }; + /// The lockfile, indicating which package versions were previously selected. final LockFile _lockFile; @@ -457,7 +462,7 @@ var pubspecs = <String, Pubspec>{}; for (var id in decisions) { if (id.isRoot) { - pubspecs[id.name] = _root.pubspec; + pubspecs[id.name] = _rootPackages[id.name]!.pubspec; } else { pubspecs[id.name] = await _systemCache.describe(id); } @@ -516,7 +521,7 @@ return _packageListers.putIfAbsent(ref, () { if (ref.isRoot) { return PackageLister.root( - _root, + _rootPackages[ref.name]!, _systemCache, sdkOverrides: _sdkOverrides, ); @@ -529,7 +534,10 @@ ..._dependencyOverrides.keys, // If the package is overridden, ignore its dependencies back onto the // root package. - if (_dependencyOverrides.containsKey(package.name)) _root.name, + if (_dependencyOverrides.containsKey(package.name)) ...[ + _root.name, + ..._root.workspaceChildren.map((e) => e.name), + ], }; return PackageLister(
diff --git a/lib/src/system_cache.dart b/lib/src/system_cache.dart index f8204b0..78ecd73 100644 --- a/lib/src/system_cache.dart +++ b/lib/src/system_cache.dart
@@ -185,9 +185,6 @@ Duration? maxAge, Version? allowedRetractedVersion, }) async { - if (ref.isRoot) { - throw ArgumentError('Cannot get versions for the root package.'); - } var versions = await ref.source.doGetVersions(ref, maxAge, this); versions = (await Future.wait(
diff --git a/test/workspace_test.dart b/test/workspace_test.dart new file mode 100644 index 0000000..60204fa --- /dev/null +++ b/test/workspace_test.dart
@@ -0,0 +1,190 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:io'; + +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; +import 'package:yaml/yaml.dart'; + +import 'descriptor.dart'; +import 'test_pub.dart'; + +void main() { + test('fetches dev_dependencies of workspace members', () async { + final server = await servePackages(); + server.serve('dev_dep', '1.0.0'); + 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', devDeps: {'dev_dep': '^1.0.0'}), + ]), + ]), + ]).create(); + await pubGet( + environment: {'_PUB_TEST_SDK_VERSION': '3.7.0'}, + output: contains('+ dev_dep'), + ); + final lockfile = loadYaml( + File(p.join(sandbox, appPath, 'pubspec.lock')).readAsStringSync(), + ); + expect(lockfile['packages'].keys, <String>{'dev_dep'}); + await appPackageConfigFile( + [ + packageConfigEntry(name: 'dev_dep', version: '1.0.0'), + packageConfigEntry(name: 'a', path: './pkgs/a'), + ], + generatorVersion: '3.7.0', + ).validate(); + }); + + test( + 'allows dependencies between workspace members, the source is overridden', + () async { + await servePackages(); + await dir(appPath, [ + libPubspec( + 'myapp', + '1.2.3', + extras: { + 'workspace': ['pkgs/a', 'pkgs/b'], + }, + sdk: '^3.7.0', + ), + dir('pkgs', [ + dir('a', [ + libPubspec('a', '1.1.1', deps: {'b': '^2.0.0'}), + ]), + dir('b', [ + libPubspec( + 'b', + '2.1.1', + deps: { + 'myapp': {'git': 'somewhere'}, + }, + ), + ]), + ]), + ]).create(); + await pubGet(environment: {'_PUB_TEST_SDK_VERSION': '3.7.0'}); + final lockfile = loadYaml( + File(p.join(sandbox, appPath, 'pubspec.lock')).readAsStringSync(), + ); + expect(lockfile['packages'].keys, <String>{}); + await appPackageConfigFile( + [ + packageConfigEntry(name: 'a', path: './pkgs/a'), + packageConfigEntry(name: 'b', path: './pkgs/b'), + ], + generatorVersion: '3.7.0', + ).validate(); + }); + + test('allows nested workspaces', () async { + final server = await servePackages(); + server.serve('dev_dep', '1.0.0'); + 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', + extras: { + 'workspace': ['example'], + }, + sdk: '^3.7.0', + ), + dir('example', [ + libPubspec( + 'example', + '2.1.1', + deps: { + 'a': {'path': '..'}, + }, + ), + ]), + ]), + ]), + ]).create(); + await pubGet(environment: {'_PUB_TEST_SDK_VERSION': '3.7.0'}); + final lockfile = loadYaml( + File(p.join(sandbox, appPath, 'pubspec.lock')).readAsStringSync(), + ); + expect(lockfile['packages'].keys, <String>{}); + + await appPackageConfigFile( + [ + packageConfigEntry(name: 'a', path: './pkgs/a'), + packageConfigEntry(name: 'example', path: './pkgs/a/example'), + ], + generatorVersion: '3.7.0', + ).validate(); + }); + + test('checks constraints between workspace members', () async { + await servePackages(); + 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': '^0.2.3'}), + ]), + ]), + ]).create(); + await pubGet( + environment: {'_PUB_TEST_SDK_VERSION': '3.7.0'}, + error: contains( + 'Because myapp depends on a which depends on myapp ^0.2.3, myapp ^0.2.3 is required', + ), + ); + }); + + test('reports errors in workspace pubspec.yamls correctly', () 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'}, + }, + ), + ]), + ]), + ]).create(); + await pubGet(environment: {'_PUB_TEST_SDK_VERSION': '3.7.0'}); + }); +}