Fix global activate --git-ref (#3656)
diff --git a/lib/src/global_packages.dart b/lib/src/global_packages.dart index a39536f..089016a 100644 --- a/lib/src/global_packages.dart +++ b/lib/src/global_packages.dart
@@ -93,7 +93,13 @@ String? path, String? ref, }) async { - var name = await cache.git.getPackageNameFromRepo(repo, ref, path, cache); + var name = await cache.git.getPackageNameFromRepo( + repo, + ref, + path, + cache, + relativeTo: p.current, + ); // TODO(nweiz): Add some special handling for git repos that contain path // dependencies. Their executables shouldn't be cached, and there should
diff --git a/lib/src/source/git.dart b/lib/src/source/git.dart index 4bef7fa..ffee07d 100644 --- a/lib/src/source/git.dart +++ b/lib/src/source/git.dart
@@ -196,17 +196,67 @@ /// Given a Git repo that contains a pub package, gets the name of the pub /// package. + /// + /// Will download the repo to the system cache under the assumption that the + /// package will be downloaded afterwards. Future<String> getPackageNameFromRepo( - String repo, String? ref, String? path, SystemCache cache) { - // Clone the repo to a temp directory. - return withTempDir((tempDir) async { - await _clone(repo, tempDir, shallow: true); - if (ref != null) await _checkOut(tempDir, ref); - var pubspec = Pubspec.load(p.join(tempDir, path), cache.sources); - return pubspec.name; + String url, + String? ref, + String? path, + SystemCache cache, { + required String relativeTo, + }) async { + final description = GitDescription( + url: url, + ref: ref, + path: path, + containingDir: relativeTo, + ); + return await _pool.withResource(() async { + await _ensureRepoCache(description, cache); + var path = _repoCachePath(description, cache); + var revision = await _firstRevision(path, description.ref); + final resolvedDescription = GitResolvedDescription(description, revision); + + return Pubspec.parse( + await _showFileAtRevision(resolvedDescription, 'pubspec.yaml', cache), + cache.sources, + ).name; }); } + /// Lists the file as it is represented at the revision of [description]. + /// + /// Assumes that revision is present in the cache already (can be done with + /// [_ensureRevision]). + Future<String> _showFileAtRevision( + GitResolvedDescription resolvedDescription, + String pathInProject, + SystemCache cache, + ) async { + final description = resolvedDescription.description; + // Normalize the path because Git treats "./" at the beginning of a path + // specially. + var pathInCache = + p.normalize(p.join(p.fromUri(description.path), pathInProject)); + + // Git doesn't recognize backslashes in paths, even on Windows. + if (Platform.isWindows) pathInCache = pathInCache.replaceAll('\\', '/'); + + final repoPath = _repoCachePath(description, cache); + final revision = resolvedDescription.resolvedRef; + + late List<String> lines; + try { + lines = await git + .run(['show', '$revision:$pathInCache'], workingDir: repoPath); + } on git.GitException catch (_) { + fail('Could not find a file named "$pathInCache" in ' + '${GitDescription.prettyUri(description.url)} $revision.'); + } + return lines.join(); + } + @override Future<List<PackageId>> doGetVersions( PackageRef ref, @@ -218,12 +268,10 @@ throw StateError('Called with wrong ref'); } return await _pool.withResource(() async { - await _ensureRepoCache(ref, cache); - var path = _repoCachePath(ref, cache); - var revision = await _firstRevision( - path, description.ref!); // TODO(sigurdm) when can ref be null here? - var pubspec = - await _describeUncached(ref, revision, description.path, cache); + await _ensureRepoCache(description, cache); + var path = _repoCachePath(description, cache); + var revision = await _firstRevision(path, description.ref); + var pubspec = await _describeUncached(ref, revision, cache); return [ PackageId(ref.name, pubspec.version, @@ -243,7 +291,6 @@ return _pool.withResource(() => _describeUncached( id.toRef(), description.resolvedRef, - description.description.path, cache, )); } @@ -253,33 +300,20 @@ Future<Pubspec> _describeUncached( PackageRef ref, String revision, - String path, SystemCache cache, ) async { final description = ref.description; if (description is! GitDescription) { throw ArgumentError('Wrong source'); } - await _ensureRevision(ref, revision, cache); - var repoPath = _repoCachePath(ref, cache); - - // Normalize the path because Git treats "./" at the beginning of a path - // specially. - var pubspecPath = p.normalize(p.join(p.fromUri(path), 'pubspec.yaml')); - - // Git doesn't recognize backslashes in paths, even on Windows. - if (Platform.isWindows) pubspecPath = pubspecPath.replaceAll('\\', '/'); - late List<String> lines; - try { - lines = await git - .run(['show', '$revision:$pubspecPath'], workingDir: repoPath); - } on git.GitException catch (_) { - fail('Could not find a file named "$pubspecPath" in ' - '${GitDescription.prettyUri(description.url)} $revision.'); - } + await _ensureRevision(description, revision, cache); return Pubspec.parse( - lines.join('\n'), + await _showFileAtRevision( + GitResolvedDescription(description, revision), + 'pubspec.yaml', + cache, + ), cache.sources, expectedName: ref.name, ); @@ -318,13 +352,13 @@ final resolvedRef = (id.description as GitResolvedDescription).resolvedRef; - didUpdate |= await _ensureRevision(ref, resolvedRef, cache); + didUpdate |= await _ensureRevision(description, resolvedRef, cache); var revisionCachePath = _revisionCachePath(id, cache); final path = description.path; await _revisionCacheClones.putIfAbsent(revisionCachePath, () async { if (!entryExists(revisionCachePath)) { - await _clone(_repoCachePath(ref, cache), revisionCachePath); + await _clone(_repoCachePath(description, cache), revisionCachePath); await _checkOut(revisionCachePath, resolvedRef); _writePackageList(revisionCachePath, [path]); didUpdate = true; @@ -427,26 +461,24 @@ /// Ensures that the canonical clone of the repository referred to by [ref] /// contains the given Git [revision]. - /// - /// Returns `true` if it had to update anything. Future<bool> _ensureRevision( - PackageRef ref, + GitDescription description, String revision, SystemCache cache, ) async { - var path = _repoCachePath(ref, cache); + var path = _repoCachePath(description, cache); if (_updatedRepos.contains(path)) return false; await _deleteGitRepoIfInvalid(path); - if (!entryExists(path)) await _createRepoCache(ref, cache); + if (!entryExists(path)) await _createRepoCache(description, cache); // Try to list the revision. If it doesn't exist, git will fail and we'll // know we have to update the repository. try { await _firstRevision(path, revision); } on git.GitException catch (_) { - await _updateRepoCache(ref, cache); + await _updateRepoCache(description, cache); return true; } return false; @@ -456,29 +488,31 @@ /// exists and is up-to-date. /// /// Returns `true` if it had to update anything. - Future<bool> _ensureRepoCache(PackageRef ref, SystemCache cache) async { - var path = _repoCachePath(ref, cache); + Future<bool> _ensureRepoCache( + GitDescription description, + SystemCache cache, + ) async { + var path = _repoCachePath(description, cache); if (_updatedRepos.contains(path)) return false; await _deleteGitRepoIfInvalid(path); if (!entryExists(path)) { - await _createRepoCache(ref, cache); + await _createRepoCache(description, cache); return true; } else { - return await _updateRepoCache(ref, cache); + return await _updateRepoCache(description, cache); } } /// Creates the canonical clone of the repository referred to by [ref]. /// /// This assumes that the canonical clone doesn't yet exist. - Future<void> _createRepoCache(PackageRef ref, SystemCache cache) async { - final description = ref.description; - if (description is! GitDescription) { - throw ArgumentError('Wrong source'); - } - var path = _repoCachePath(ref, cache); + Future<void> _createRepoCache( + GitDescription description, + SystemCache cache, + ) async { + var path = _repoCachePath(description, cache); assert(!_updatedRepos.contains(path)); try { await _clone(description.url, path, mirror: true); @@ -496,10 +530,10 @@ /// /// Returns `true` if it had to update anything. Future<bool> _updateRepoCache( - PackageRef ref, + GitDescription description, SystemCache cache, ) async { - var path = _repoCachePath(ref, cache); + var path = _repoCachePath(description, cache); if (_updatedRepos.contains(path)) return false; await git.run(['fetch'], workingDir: path); _updatedRepos.add(path); @@ -590,22 +624,13 @@ String from, String to, { bool mirror = false, - bool shallow = false, - }) { - return Future.sync(() { - // Git on Windows does not seem to automatically create the destination - // directory. - ensureDir(to); - var args = [ - 'clone', - if (mirror) '--mirror', - if (shallow) ...['--depth', '1'], - from, - to - ]; + }) async { + // Git on Windows does not seem to automatically create the destination + // directory. + ensureDir(to); + var args = ['clone', if (mirror) '--mirror', from, to]; - return git.run(args); - }).then((result) => null); + await git.run(args); } /// Checks out the reference [ref] in [repoPath]. @@ -616,27 +641,19 @@ String _revisionCachePath(PackageId id, SystemCache cache) => p.join( cache.rootDirForSource(this), - '${_repoName(id.toRef())}-${(id.description as GitResolvedDescription).resolvedRef}'); + '${_repoName(id.description.description as GitDescription)}-${(id.description as GitResolvedDescription).resolvedRef}'); /// Returns the path to the canonical clone of the repository referred to by /// [id] (the one in `<system cache>/git/cache`). - String _repoCachePath(PackageRef ref, SystemCache cache) { - final description = ref.description; - if (description is! GitDescription) { - throw ArgumentError('Wrong source'); - } - final repoCacheName = '${_repoName(ref)}-${sha1(description.url)}'; + String _repoCachePath(GitDescription description, SystemCache cache) { + final repoCacheName = '${_repoName(description)}-${sha1(description.url)}'; return p.join(cache.rootDirForSource(this), 'cache', repoCacheName); } /// Returns a short, human-readable name for the repository URL in [ref]. /// /// This name is not guaranteed to be unique. - String _repoName(PackageRef ref) { - final description = ref.description; - if (description is! GitDescription) { - throw ArgumentError('Wrong source'); - } + String _repoName(GitDescription description) { var name = p.url.basename(description.url); if (name.endsWith('.git')) { name = name.substring(0, name.length - '.git'.length); @@ -665,7 +682,7 @@ final bool relative; /// The git ref to resolve for finding the commit. - final String? ref; + final String ref; /// Relative path of the package inside the git repo. ///
diff --git a/test/global/activate/git_package_test.dart b/test/global/activate/git_package_test.dart index ad3ff27..6b7215e 100644 --- a/test/global/activate/git_package_test.dart +++ b/test/global/activate/git_package_test.dart
@@ -66,7 +66,10 @@ 'global', 'activate', '-sgit', - '../foo.git', + // Testing with a file:// ur; is important here, as the cloning behavior + // matches that of networked cloning. Specifically a shallow clone will + // not be shallow if it is from a relative url. + '${p.toUri(d.sandbox)}/foo.git', '--git-ref=HEAD~', '--git-path=sub/', ],
diff --git a/test/global/activate/missing_git_repo_test.dart b/test/global/activate/missing_git_repo_test.dart index 8db65c3..315a2e2 100644 --- a/test/global/activate/missing_git_repo_test.dart +++ b/test/global/activate/missing_git_repo_test.dart
@@ -12,8 +12,9 @@ ensureGit(); await runPub( - args: ['global', 'activate', '-sgit', '../nope.git'], - error: contains("repository '../nope.git' does not exist"), - exitCode: exit_codes.UNAVAILABLE); + args: ['global', 'activate', '-sgit', '../nope.git'], + error: contains("/nope.git' does not appear to be a git repository"), + exitCode: exit_codes.UNAVAILABLE, + ); }); }