diff --git a/lib/src/pubspec.dart b/lib/src/pubspec.dart index a0e2ab2b62d..41832940e21 100644 --- a/lib/src/pubspec.dart +++ b/lib/src/pubspec.dart @@ -331,7 +331,7 @@ environment: Pubspec( String name, { - Version? version, + super.version, Iterable? dependencies, Iterable? devDependencies, Iterable? dependencyOverrides, @@ -364,11 +364,7 @@ environment: // This is a dummy value. Dependencies should already be resolved, so we // never need to do relative resolutions. _containingDescription = ResolvedRootDescription.fromDir('.'), - super( - fields == null ? YamlMap() : YamlMap.wrap(fields), - name: name, - version: version, - ); + super(fields == null ? YamlMap() : YamlMap.wrap(fields), name: name); /// Returns a Pubspec object for an already-parsed map representing its /// contents. diff --git a/lib/src/solver/version_solver.dart b/lib/src/solver/version_solver.dart index 1293b9a7395..e8d2b69067d 100644 --- a/lib/src/solver/version_solver.dart +++ b/lib/src/solver/version_solver.dart @@ -126,7 +126,7 @@ class VersionSolver { ); try { - return await _systemCache.hosted.withPrefetching(() async { + return await _systemCache.hosted.withPrefetching(_systemCache, () async { String? next = _root.name; while (next != null) { _propagate(next); diff --git a/lib/src/source/git.dart b/lib/src/source/git.dart index a74e306da26..9dd19fec14b 100644 --- a/lib/src/source/git.dart +++ b/lib/src/source/git.dart @@ -266,21 +266,6 @@ class GitSource extends CachedSource { return p.url.normalize(parsed.toString()); } - /// Limit the number of concurrent git operations to 1. - // TODO(sigurdm): Use RateLimitedScheduler. - final Pool _pool = Pool(1); - - /// A map from revision cache locations to futures that will complete once - /// they're finished being cloned. - /// - /// This lets us avoid race conditions when getting multiple different - /// packages from the same repository. - final _revisionCacheClones = >{}; - - /// The paths to the canonical clones of repositories for which "git fetch" - /// has already been run during this run of pub. - final _updatedRepos = {}; - /// Given a Git repo that contains a pub package, gets the name of the pub /// package. /// @@ -305,7 +290,7 @@ class GitSource extends CachedSource { containingDir: relativeTo, tagPattern: tagPattern, ); - return await _pool.withResource(() async { + return await cache.gitCache.pool.withResource(() async { await _ensureRepoCache(description, cache); final path = _repoCachePath(description, cache); @@ -370,7 +355,7 @@ class GitSource extends CachedSource { if (description is! GitDescription) { throw StateError('Called with wrong ref'); } - return await _pool.withResource(() async { + return await cache.gitCache.pool.withResource(() async { await _ensureRepoCache(description, cache); final path = _repoCachePath(description, cache); final result = []; @@ -417,7 +402,7 @@ class GitSource extends CachedSource { if (description is! ResolvedGitDescription) { throw StateError('Called with wrong ref'); } - final pubspec = await _pool.withResource( + final pubspec = await cache.gitCache.pool.withResource( () => _describeUncached(id.toRef(), description.resolvedRef, cache), ); if (pubspec.version != id.version) { @@ -430,8 +415,6 @@ class GitSource extends CachedSource { return pubspec; } - final Map<(PackageRef, String), Pubspec> _pubspecAtRevisionCache = {}; - /// Like [describeUncached], but takes a separate [ref] and Git [revision] /// rather than a single ID. Future _describeUncached( @@ -443,7 +426,10 @@ class GitSource extends CachedSource { if (description is! GitDescription) { throw ArgumentError('Wrong source'); } - return _pubspecAtRevisionCache[(ref, revision)] ??= await () async { + return cache.gitCache.pubspecAtRevisionCache[( + ref, + revision, + )] ??= await () async { await _ensureRevision(description, revision, cache); final resolvedDescription = ResolvedGitDescription(description, revision); return Pubspec.parse( @@ -473,7 +459,7 @@ class GitSource extends CachedSource { PackageId id, SystemCache cache, ) async { - return await _pool.withResource(() async { + return await cache.gitCache.pool.withResource(() async { var didUpdate = false; final ref = id.toRef(); final description = ref.description; @@ -495,25 +481,28 @@ class GitSource extends CachedSource { final revisionCachePath = _revisionCachePath(id, cache); final path = description.path; - await _revisionCacheClones.putIfAbsent(revisionCachePath, () async { - if (!entryExists(revisionCachePath)) { - await _cloneViaTemp( - _repoCachePath(description, cache), - revisionCachePath, - cache, - ); - await git.run([ - 'config', - 'remote.origin.lfsurl', - description.url, - ], workingDir: revisionCachePath); - await _checkOut(revisionCachePath, resolvedRef); - _writePackageList(revisionCachePath, [path]); - didUpdate = true; - } else { - didUpdate |= _updatePackageList(revisionCachePath, path); - } - }); + await cache.gitCache.revisionCacheClones.putIfAbsent( + revisionCachePath, + () async { + if (!entryExists(revisionCachePath)) { + await _cloneViaTemp( + _repoCachePath(description, cache), + revisionCachePath, + cache, + ); + await git.run([ + 'config', + 'remote.origin.lfsurl', + description.url, + ], workingDir: revisionCachePath); + await _checkOut(revisionCachePath, resolvedRef); + _writePackageList(revisionCachePath, [path]); + didUpdate = true; + } else { + didUpdate |= _updatePackageList(revisionCachePath, path); + } + }, + ); return DownloadPackageResult(id, didUpdate: didUpdate); }); } @@ -646,7 +635,7 @@ class GitSource extends CachedSource { SystemCache cache, ) async { final path = _repoCachePath(description, cache); - if (_updatedRepos.contains(path)) return false; + if (cache.gitCache.updatedRepos.contains(path)) return false; await _deleteGitRepoIfInvalid(path); @@ -672,7 +661,7 @@ class GitSource extends CachedSource { SystemCache cache, ) async { final path = _repoCachePath(description, cache); - if (_updatedRepos.contains(path)) return false; + if (cache.gitCache.updatedRepos.contains(path)) return false; await _deleteGitRepoIfInvalid(path); @@ -693,14 +682,14 @@ class GitSource extends CachedSource { SystemCache cache, ) async { final path = _repoCachePath(description, cache); - assert(!_updatedRepos.contains(path)); + assert(!cache.gitCache.updatedRepos.contains(path)); try { await _cloneViaTemp(description.url, path, cache, mirror: true); } catch (_) { await _deleteGitRepoIfInvalid(path); rethrow; } - _updatedRepos.add(path); + cache.gitCache.updatedRepos.add(path); } /// Runs "git fetch" in the canonical clone of the repository referred to by @@ -714,9 +703,9 @@ class GitSource extends CachedSource { SystemCache cache, ) async { final path = _repoCachePath(description, cache); - if (_updatedRepos.contains(path)) return false; + if (cache.gitCache.updatedRepos.contains(path)) return false; await git.run([_gitDirArg(path), 'fetch'], workingDir: path); - _updatedRepos.add(path); + cache.gitCache.updatedRepos.add(path); return true; } @@ -1226,3 +1215,22 @@ RegExp compileTagPattern(String tagPattern) { r'$', ); } + +/// State that is cached for the GitSource. +final class GitSourceCache { + /// Limit the number of concurrent git operations to 1. + final pool = Pool(1); + + /// A map from revision cache locations to futures that will complete once + /// they're finished being cloned. + /// + /// This lets us avoid race conditions when getting multiple different + /// packages from the same repository. + final revisionCacheClones = >{}; + + /// The paths to the canonical clones of repositories for which "git fetch" + /// has already been run during this run of pub. + final updatedRepos = {}; + + final pubspecAtRevisionCache = <(PackageRef, String), Pubspec>{}; +} diff --git a/lib/src/source/hosted.dart b/lib/src/source/hosted.dart index d13db32e3a5..09dc7ffd368 100644 --- a/lib/src/source/hosted.dart +++ b/lib/src/source/hosted.dart @@ -8,7 +8,7 @@ import 'dart:io' as io; import 'dart:io'; import 'dart:typed_data'; -import 'package:collection/collection.dart' show IterableExtension, maxBy; +import 'package:collection/collection.dart' show IterableExtension; import 'package:crypto/crypto.dart'; import 'package:http/http.dart' as http; import 'package:meta/meta.dart'; @@ -372,10 +372,7 @@ class HostedSource extends CachedSource { r'^[a-zA-Z_]+[a-zA-Z0-9_]*$', ); - late final RateLimitedScheduler<_RefAndCache, List<_VersionInfo>> _scheduler = - RateLimitedScheduler(_fetchVersions, maxConcurrentOperations: 10); - - List<_VersionInfo> _versionInfoFromPackageListing( + List _versionInfoFromPackageListing( Map body, PackageRef ref, Uri location, @@ -443,7 +440,7 @@ class HostedSource extends CachedSource { isRetracted: retracted, advisoriesUpdated: advisoriesDate, ); - return _VersionInfo( + return HostedVersionInfo( pubspec.version, pubspec, Uri.parse(archiveUrl), @@ -453,7 +450,7 @@ class HostedSource extends CachedSource { }).toList(); } - Future> _fetchVersionsNoPrefetching( + Future> _fetchVersionsNoPrefetching( PackageRef ref, SystemCache cache, ) async { @@ -469,7 +466,7 @@ class HostedSource extends CachedSource { final String bodyText; final dynamic body; - final List<_VersionInfo> result; + final List result; try { // TODO(sigurdm): Implement cancellation of requests. This probably // requires resolution of: https://github.com/dart-lang/http/issues/424. @@ -509,29 +506,35 @@ class HostedSource extends CachedSource { return result; } - Future> _fetchVersions(_RefAndCache refAndCache) async { + Future> fetchVersions( + HostedRefAndCache refAndCache, + ) async { final ref = refAndCache.ref; final description = ref.description; if (description is! HostedDescription) { throw ArgumentError('Wrong source'); } final preschedule = - Zone.current[_prefetchingKey] as void Function(_RefAndCache)?; + Zone.current[_prefetchingKey] as void Function(HostedRefAndCache)?; /// Prefetch the dependencies of the latest version, we are likely to need /// them later. void prescheduleDependenciesOfLatest( - List<_VersionInfo>? listing, + List? listing, SystemCache cache, ) { if (listing == null || listing.isEmpty) return; - final latestVersion = - maxBy<_VersionInfo, Version>(listing, (e) => e.version)!; + var latestVersion = listing.first; + for (final e in listing.skip(1)) { + if (e.version > latestVersion.version) { + latestVersion = e; + } + } final dependencies = latestVersion.pubspec.dependencies.values; unawaited(() async { for (final packageRange in dependencies) { if (packageRange.source is HostedSource) { - preschedule!(_RefAndCache(packageRange.toRef(), cache)); + preschedule!(HostedRefAndCache(packageRange.toRef(), cache)); } } }()); @@ -823,7 +826,8 @@ class HostedSource extends CachedSource { /// Invariant: Entries in this cache are the parsed version of the exact same /// information cached on disk. I.e. if the entry is present in this cache, /// there will not be a newer version on disk. - final Map)> _responseCache = {}; + final Map)> _responseCache = + {}; /// If a cached version listing response for [ref] exists on disk and is less /// than [maxAge] old it is parsed and returned. @@ -832,7 +836,7 @@ class HostedSource extends CachedSource { /// /// If [maxAge] is not given, we will try to get the cached version no matter /// how old it is. - Future?> _cachedVersionListingResponse( + Future?> _cachedVersionListingResponse( PackageRef ref, SystemCache cache, { Duration? maxAge, @@ -954,7 +958,7 @@ class HostedSource extends CachedSource { PackageStatus(); } - Future<_VersionInfo?> _versionInfo( + Future _versionInfo( PackageRef ref, Version version, SystemCache cache, { @@ -967,10 +971,14 @@ class HostedSource extends CachedSource { if (versionListing == null) { return null; } - return versionListing.firstWhereOrNull((l) => l.version == version); + return versionListing.firstWhereOrNull( + (HostedVersionInfo l) => l.version == version, + ); } // Did we already get info for this package? - var versionListing = _scheduler.peek(_RefAndCache(ref, cache)); + var versionListing = cache.hostedCache.scheduler.peek( + HostedRefAndCache(ref, cache), + ); if (maxAge != null) { // Do we have a cached version response on disk? versionListing ??= await _cachedVersionListingResponse( @@ -980,11 +988,11 @@ class HostedSource extends CachedSource { ); } // Otherwise retrieve the info from the host. - versionListing ??= await _scheduler - .schedule(_RefAndCache(ref, cache)) + versionListing ??= await cache.hostedCache.scheduler + .schedule(HostedRefAndCache(ref, cache)) // Failures retrieving the listing here should just be ignored. .catchError( - (_) async => <_VersionInfo>[], + (_) async => [], test: (error) => error is Exception, ); @@ -1056,7 +1064,9 @@ class HostedSource extends CachedSource { return offlineVersions; } - var versionListing = _scheduler.peek(_RefAndCache(ref, cache)); + var versionListing = cache.hostedCache.scheduler.peek( + HostedRefAndCache(ref, cache), + ); if (maxAge != null) { // Do we have a cached version response on disk? versionListing ??= await _cachedVersionListingResponse( @@ -1065,7 +1075,9 @@ class HostedSource extends CachedSource { maxAge: maxAge, ); } - versionListing ??= await _scheduler.schedule(_RefAndCache(ref, cache)); + versionListing ??= await cache.hostedCache.scheduler.schedule( + HostedRefAndCache(ref, cache), + ); return versionListing .map( (i) => PackageId( @@ -1143,7 +1155,9 @@ class HostedSource extends CachedSource { hint: 'Try again without --offline!', ); } - final versions = await _scheduler.schedule(_RefAndCache(id.toRef(), cache)); + final versions = await cache.hostedCache.scheduler.schedule( + HostedRefAndCache(id.toRef(), cache), + ); final url = _listVersionsUrl(id.toRef()); return versions.firstWhereOrNull((i) => i.version == id.version)?.pubspec ?? (throw PackageNotFoundException('Could not find package $id at $url')); @@ -1508,7 +1522,9 @@ class HostedSource extends CachedSource { // a custom package server may include a temporary signature in the // query-string as is the case with signed S3 URLs. And we wish to allow for // such URLs to be used. - final versions = await _scheduler.schedule(_RefAndCache(id.toRef(), cache)); + final versions = await cache.hostedCache.scheduler.schedule( + HostedRefAndCache(id.toRef(), cache), + ); final versionInfo = versions.firstWhereOrNull( (i) => i.version == id.version, ); @@ -1830,8 +1846,13 @@ See $contentHashesDocumentationUrl. /// Enables speculative prefetching of dependencies of packages queried with /// [doGetVersions]. - Future withPrefetching(Future Function() callback) async { - return await _scheduler.withPrescheduling((preschedule) async { + Future withPrefetching( + SystemCache cache, + Future Function() callback, + ) async { + return await cache.hostedCache.scheduler.withPrescheduling(( + preschedule, + ) async { return await runZoned( callback, zoneValues: {_prefetchingKey: preschedule}, @@ -1948,7 +1969,7 @@ class ResolvedHostedDescription extends ResolvedDescription { } /// Information about a package version retrieved from /api/packages/$package< -class _VersionInfo { +class HostedVersionInfo { final Pubspec pubspec; final Uri archiveUrl; final Version version; @@ -1957,7 +1978,7 @@ class _VersionInfo { final Uint8List? archiveSha256; final PackageStatus status; - _VersionInfo( + HostedVersionInfo( this.version, this.pubspec, this.archiveUrl, @@ -2046,15 +2067,16 @@ String _directoryToUrl(String directory) { } // TODO(sigurdm): This is quite inelegant. -class _RefAndCache { +class HostedRefAndCache { final PackageRef ref; final SystemCache cache; - _RefAndCache(this.ref, this.cache); + HostedRefAndCache(this.ref, this.cache); @override int get hashCode => ref.hashCode; @override - bool operator ==(Object other) => other is _RefAndCache && other.ref == ref; + bool operator ==(Object other) => + other is HostedRefAndCache && other.ref == ref; } /// A sink that can only have `add` called once, and that can retrieve the @@ -2157,3 +2179,19 @@ int? _parseCrc32c(Map headers, String fileName) { return null; } + +/// State that is cached for the HostedSource. +final class HostedSourceCache { + /// The scheduler used to fetch version information for packages. + /// + /// This allows rate-limiting requests and speculative pre-fetching of + /// dependencies. + final RateLimitedScheduler> + scheduler; + + HostedSourceCache(HostedSource hosted) + : scheduler = RateLimitedScheduler( + (HostedRefAndCache refAndCache) => hosted.fetchVersions(refAndCache), + maxConcurrentOperations: 10, + ); +} diff --git a/lib/src/system_cache.dart b/lib/src/system_cache.dart index 3eacc07b376..58a53ad3a5c 100644 --- a/lib/src/system_cache.dart +++ b/lib/src/system_cache.dart @@ -6,6 +6,7 @@ import 'dart:convert'; import 'dart:io'; import 'package:crypto/crypto.dart'; + import 'package:pub_semver/pub_semver.dart'; import 'authentication/token_store.dart'; @@ -18,6 +19,7 @@ import 'package_name.dart'; import 'path.dart'; import 'platform_info.dart'; import 'pubspec.dart'; + import 'source.dart'; import 'source/cached.dart'; import 'source/git.dart'; @@ -37,6 +39,12 @@ class SystemCache { String get rootDir => _rootDir ??= defaultDir; String? _rootDir; + /// Cache for HostedSource. + late final hostedCache = HostedSourceCache(hosted); + + /// Cache for GitSource. + final gitCache = GitSourceCache(); + String rootDirForSource(CachedSource source) => p.join(rootDir, source.name); String get tempDir => p.join(rootDir, '_temp'); diff --git a/pubspec.lock b/pubspec.lock index 52755d8de6c..65fe6125e90 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -466,4 +466,4 @@ packages: source: hosted version: "2.2.2" sdks: - dart: ">=3.11.0 <4.0.0" + dart: ">=3.9.0 <4.0.0"