Handle overrides in workspaces (#4249)
diff --git a/lib/src/command/deps.dart b/lib/src/command/deps.dart index 45b3343..a9d370d 100644 --- a/lib/src/command/deps.dart +++ b/lib/src/command/deps.dart
@@ -211,7 +211,7 @@ } await _outputCompactPackages( 'dependency overrides', - root.dependencyOverrides.keys, + root.pubspec.dependencyOverrides.keys, buffer, ); } @@ -268,7 +268,7 @@ } await _outputListSection( 'dependency overrides', - root.dependencyOverrides.keys, + root.pubspec.dependencyOverrides.keys, buffer, ); } @@ -377,7 +377,7 @@ if (_includeDev) { transitive.removeAll(root.devDependencies.keys); } - transitive.removeAll(root.dependencyOverrides.keys); + transitive.removeAll(root.pubspec.dependencyOverrides.keys); } return transitive; } @@ -391,7 +391,7 @@ final nonDevDependencies = [ for (final package in entrypoint.workspaceRoot.transitiveWorkspace) ...[ ...package.dependencies.keys, - ...package.dependencyOverrides.keys, + ...package.pubspec.dependencyOverrides.keys, ], ]; return nonDevDependencies
diff --git a/lib/src/command/upgrade.dart b/lib/src/command/upgrade.dart index 1864aa2..40131f1 100644 --- a/lib/src/command/upgrade.dart +++ b/lib/src/command/upgrade.dart
@@ -234,7 +234,7 @@ } // Skip [dep] if it has a dependency_override. - if (entrypoint.workspaceRoot.dependencyOverrides + if (entrypoint.workspaceRoot.pubspec.dependencyOverrides .containsKey(dep.name)) { dependencyOverriddenDeps.add(dep.name); continue; @@ -293,7 +293,7 @@ // If any of the packages to upgrade are dependency overrides, then we // show a warning. final toUpgradeOverrides = toUpgrade - .where(entrypoint.workspaceRoot.dependencyOverrides.containsKey); + .where(entrypoint.workspaceRoot.allOverridesInWorkspace.containsKey); if (toUpgradeOverrides.isNotEmpty) { log.warning( 'Warning: dependency_overrides prevents upgrades for: '
diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index e4507ce..96c9fe0 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart
@@ -7,7 +7,6 @@ import 'dart:io'; import 'dart:math'; -import 'package:collection/collection.dart'; import 'package:path/path.dart' as p; import 'package:pool/pool.dart'; import 'package:pub_semver/pub_semver.dart'; @@ -568,6 +567,7 @@ type, workspaceRoot.dir, workspaceRoot.pubspec, + workspaceRoot.allOverridesInWorkspace, lockFile, newLockFile, result.availableVersions, @@ -795,8 +795,6 @@ return false; } - final overrides = MapKeySet(root.dependencyOverrides); - // Check that uncached dependencies' pubspecs are also still satisfied, // since they're mutable and may have changed since the last get. for (var id in lockFile.packages.values) { @@ -806,7 +804,8 @@ try { if (cache.load(id).dependencies.values.every( (dep) => - overrides.contains(dep.name) || isDependencyUpToDate(dep), + root.allOverridesInWorkspace.containsKey(dep.name) || + isDependencyUpToDate(dep), )) { continue; }
diff --git a/lib/src/global_packages.dart b/lib/src/global_packages.dart index cc89903..957884c 100644 --- a/lib/src/global_packages.dart +++ b/lib/src/global_packages.dart
@@ -267,6 +267,7 @@ SolveType.get, null, root.pubspec, + root.pubspec.dependencyOverrides, originalLockFile ?? LockFile.empty(), lockFile, result.availableVersions,
diff --git a/lib/src/package.dart b/lib/src/package.dart index e8c97d6..8edf823 100644 --- a/lib/src/package.dart +++ b/lib/src/package.dart
@@ -76,26 +76,32 @@ } } + /// A collection of all overrides in the workspace. + /// + /// Should only be called on the workspace root. + /// + /// We only allow each package to be overridden once, so it is ok to collapse + /// the overrides into a single map. + late final Map<String, PackageRange> allOverridesInWorkspace = { + for (final package in transitiveWorkspace) + ...package.pubspec.dependencyOverrides, + }; + /// The immediate dependencies this package specifies in its pubspec. Map<String, PackageRange> get dependencies => pubspec.dependencies; /// The immediate dev dependencies this package specifies in its pubspec. Map<String, PackageRange> get devDependencies => pubspec.devDependencies; - /// The dependency overrides this package specifies in its pubspec or pubspec - /// overrides. - Map<String, PackageRange> get dependencyOverrides => - pubspec.dependencyOverrides; - /// All immediate dependencies this package specifies. /// - /// This includes regular, dev dependencies, and overrides. + /// This includes regular, dev dependencies, and overrides from this package. Map<String, PackageRange> get immediateDependencies { // Make sure to add overrides last so they replace normal dependencies. return {} ..addAll(dependencies) ..addAll(devDependencies) - ..addAll(dependencyOverrides); + ..addAll(pubspec.dependencyOverrides); } /// Returns a list of paths to all Dart executables in this package's bin
diff --git a/lib/src/solver/package_lister.dart b/lib/src/solver/package_lister.dart index 5f48a28..c38d300 100644 --- a/lib/src/solver/package_lister.dart +++ b/lib/src/solver/package_lister.dart
@@ -131,6 +131,7 @@ PackageLister.root( Package package, this._systemCache, { + required Set<String> overriddenPackages, required Map<String, Version>? sdkOverrides, }) : _ref = PackageRef.root(package), // Treat the package as locked so we avoid the logic for finding the @@ -138,8 +139,7 @@ // package. _locked = PackageId.root(package), _dependencyType = DependencyType.none, - _overriddenPackages = - Set.unmodifiable(package.dependencyOverrides.keys), + _overriddenPackages = overriddenPackages, _isDowngrade = false, _allowedRetractedVersion = null, sdkOverrides = sdkOverrides ?? {},
diff --git a/lib/src/solver/report.dart b/lib/src/solver/report.dart index 0736ce4..4d2aa9b 100644 --- a/lib/src/solver/report.dart +++ b/lib/src/solver/report.dart
@@ -31,6 +31,7 @@ final LockFile _newLockFile; final SystemCache _cache; final bool _dryRun; + final Map<String, PackageRange> _overriddenPackages; /// If quiet only a single summary line is output. final bool _quiet; @@ -52,6 +53,7 @@ this._type, this._location, this._rootPubspec, + this._overriddenPackages, this._previousLockFile, this._newLockFile, this._availableVersions, @@ -339,7 +341,7 @@ final oldId = _previousLockFile.packages[name]; final id = newId ?? oldId!; - final isOverridden = _rootPubspec.dependencyOverrides.containsKey(id.name); + final isOverridden = _overriddenPackages.containsKey(id.name); // If the package was previously a dependency but the dependency has // changed in some way.
diff --git a/lib/src/solver/result.dart b/lib/src/solver/result.dart index 36925e3..2d3ea06 100644 --- a/lib/src/solver/result.dart +++ b/lib/src/solver/result.dart
@@ -20,6 +20,9 @@ /// reachable from the root. final List<PackageId> packages; + /// Names of all dependency overrides in the workspace. + final Set<String> _overriddenPackages; + /// The root package of this resolution. final Package _root; @@ -81,9 +84,7 @@ // Don't factor in overridden dependencies' SDK constraints, because we'll // accept those packages even if their constraints don't match. final nonOverrides = pubspecs.values - .where( - (pubspec) => !_root.dependencyOverrides.containsKey(pubspec.name), - ) + .where((pubspec) => !_overriddenPackages.contains(pubspec.name)) .toList(); final sdkConstraints = <String, VersionConstraint>{}; @@ -101,7 +102,7 @@ }, mainDependencies: MapKeySet(_root.dependencies), devDependencies: MapKeySet(_root.devDependencies), - overriddenDependencies: MapKeySet(_root.dependencyOverrides), + overriddenDependencies: _overriddenPackages, ); } @@ -125,6 +126,7 @@ SolveResult( this._root, + this._overriddenPackages, this._previousLockFile, this.packages, this.pubspecs,
diff --git a/lib/src/solver/version_solver.dart b/lib/src/solver/version_solver.dart index 7a007a7..89019c3 100644 --- a/lib/src/solver/version_solver.dart +++ b/lib/src/solver/version_solver.dart
@@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:math' as math; +import 'package:collection/collection.dart'; import 'package:pub_semver/pub_semver.dart'; import '../exceptions.dart'; @@ -77,6 +78,11 @@ /// name anywhere in the dependency graph. final Map<String, PackageRange> _dependencyOverrides; + /// Names of packages that are overridden in this resolution as a [Set] for + /// convenience. + late final Set<String> _overriddenPackages = + MapKeySet(_root.allOverridesInWorkspace); + /// The set of packages for which the lockfile should be ignored. final Set<String> _unlock; @@ -94,7 +100,7 @@ Iterable<String> unlock, { Map<String, Version> sdkOverrides = const {}, }) : _sdkOverrides = sdkOverrides, - _dependencyOverrides = _root.dependencyOverrides, + _dependencyOverrides = _root.allOverridesInWorkspace, _unlock = {...unlock}; /// Prime the solver with [constraints]. @@ -470,6 +476,7 @@ return SolveResult( _root, + _overriddenPackages, _lockFile, decisions, pubspecs, @@ -523,6 +530,7 @@ return PackageLister.root( _rootPackages[ref.name]!, _systemCache, + overriddenPackages: _overriddenPackages, sdkOverrides: _sdkOverrides, ); } @@ -531,12 +539,12 @@ if (locked != null && locked.toRef() != ref) locked = null; final overridden = <String>{ - ..._dependencyOverrides.keys, + ..._overriddenPackages, // If the package is overridden, ignore its dependencies back onto the // root package. - if (_dependencyOverrides.containsKey(package.name)) ...[ + if (_overriddenPackages.contains(package.name)) ...[ _root.name, - ..._root.workspaceChildren.map((e) => e.name), + ..._root.transitiveWorkspace.map((e) => e.name), ], };
diff --git a/lib/src/validator/dependency_override.dart b/lib/src/validator/dependency_override.dart index afdb349..edefa8c 100644 --- a/lib/src/validator/dependency_override.dart +++ b/lib/src/validator/dependency_override.dart
@@ -13,7 +13,8 @@ class DependencyOverrideValidator extends Validator { @override Future validate() { - final overridden = MapKeySet(package.dependencyOverrides); + final overridden = + MapKeySet(context.entrypoint.workspaceRoot.allOverridesInWorkspace); final dev = MapKeySet(package.devDependencies); if (overridden.difference(dev).isNotEmpty) { final overridesFile = package.pubspec.dependencyOverridesFromOverridesFile
diff --git a/test/workspace_test.dart b/test/workspace_test.dart index 8ca27b9..0cd47de 100644 --- a/test/workspace_test.dart +++ b/test/workspace_test.dart
@@ -1223,6 +1223,39 @@ `a${s}pubspec.yaml` and `b${s}pubspec.yaml` are both called "a".''', ); }); + + test('overrides are applied', () async { + final server = await servePackages(); + server.serve('foo', '1.0.0'); + await dir('foo', [libPubspec('foo', '1.0.1')]).create(); + await dir(appPath, [ + libPubspec( + 'myapp', + '1.2.3', + deps: {'foo': '1.0.0'}, + extras: { + 'workspace': ['a'], + }, + sdk: '^3.5.0', + ), + dir('a', [ + libPubspec( + 'a', + '1.0.0', + extras: { + 'dependency_overrides': { + 'foo': {'path': '../../foo'}, + }, + }, + resolutionWorkspace: true, + ), + ]), + ]).create(); + await pubGet( + environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'}, + output: contains('! foo 1.0.1 from path ..${s}foo (overridden)'), + ); + }); } final s = p.separator;