-
Notifications
You must be signed in to change notification settings - Fork 232
Add 'tag_pattern' feature to git dependencies #4427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 2 commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
a3b6b92
Add 'tag_pattern' feature
sigurdm a482697
Test for interdependencies
sigurdm 4cb927e
Fix `pub add --git-tag-pattern`
sigurdm 9bc9e65
Make `hasMultipleVersions` a property of the description
sigurdm 9a6517e
{{version}}
sigurdm 49978b4
merge
sigurdm 71e0f0f
Format
sigurdm 6124471
Validate tag_pattern early
sigurdm 2c7fb43
not coming until 3.9...
sigurdm 6960eb7
windows compatz2
sigurdm 03ba0ef
Windows compatz in tests
sigurdm 4e8de79
Fix test sdk version
sigurdm b57d89a
Escaping
sigurdm 3d33d64
Update lib/src/source/git.dart
sigurdm 8ff2d3f
Fix serializeForPubspec
sigurdm e630352
Merge
sigurdm 3e98819
Don't print ref in pubspec lock when using tag_pattern
sigurdm 8c97b20
Remove TODO
sigurdm da21216
Fill in missing dartdoc
sigurdm b576aed
Fix test expectation
sigurdm 892e066
Use version constraint when adding a git dependency with a tag pattern
sigurdm b2231d1
Test for `add`ing with a `tag_pattern`
sigurdm 0ffc5d1
Test shape of pubspec.lock
sigurdm cad4daf
More tests
sigurdm 2ba15cb
Windows paths in test
sigurdm c5ca0bf
Merge remote-tracking branch 'origin/master' into git_version_by_tag
sigurdm 520b299
Merge
sigurdm d2b4966
Address some of review
sigurdm e1d51ed
Only allow single {{version}} marker
sigurdm 0fd0394
Revert hack
sigurdm bab5b82
lints
sigurdm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,8 @@ class GitSource extends CachedSource { | |
|
||
@override | ||
final name = 'git'; | ||
@override | ||
final hasMultipleVersions = true; | ||
|
||
@override | ||
PackageRef parseRef( | ||
|
@@ -43,6 +45,7 @@ class GitSource extends CachedSource { | |
String url; | ||
String? ref; | ||
String? path; | ||
String? tagPattern; | ||
if (description is String) { | ||
url = description; | ||
} else if (description is! Map) { | ||
|
@@ -72,6 +75,32 @@ class GitSource extends CachedSource { | |
'string.'); | ||
} | ||
path = descriptionPath; | ||
|
||
// TODO: can we avoid relying on key presence? | ||
if (description.containsKey('tag_pattern')) { | ||
sigurdm marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (languageVersion != null && !languageVersion.supportsWorkspaces) { | ||
throw FormatException( | ||
'Using `git: {tagPattern: }` is only supported with a minimum SDK ' | ||
'constraint of ${LanguageVersion.firstVersionWithTagPattern}.', | ||
); | ||
} | ||
switch (description['tag_pattern']) { | ||
case final String descriptionTagPattern: | ||
tagPattern = descriptionTagPattern; | ||
case null: | ||
tagPattern = 'v*'; | ||
sigurdm marked this conversation as resolved.
Show resolved
Hide resolved
|
||
default: | ||
throw const FormatException( | ||
"The 'tagPattern' field of the description " | ||
'must be a string or null.'); | ||
} | ||
} | ||
|
||
if (ref != null && tagPattern != null) { | ||
throw const FormatException( | ||
'A git description cannot have both a ref and a `tagPattern`.', | ||
); | ||
} | ||
} | ||
|
||
final containingDir = switch (containingDescription) { | ||
|
@@ -87,6 +116,7 @@ class GitSource extends CachedSource { | |
containingDir: containingDir, | ||
ref: ref, | ||
path: _validatedPath(path), | ||
tagPattern: tagPattern, | ||
), | ||
); | ||
} | ||
|
@@ -121,6 +151,13 @@ class GitSource extends CachedSource { | |
throw const FormatException("The 'url' field of the description " | ||
'must be a string.'); | ||
} | ||
|
||
final tagPattern = description['tag_pattern']; | ||
if (tagPattern is! String?) { | ||
throw const FormatException("The 'tag_pattern' field of the description " | ||
'must be a string.'); | ||
} | ||
|
||
return PackageId( | ||
name, | ||
version, | ||
|
@@ -132,6 +169,7 @@ class GitSource extends CachedSource { | |
description['path'], | ||
), | ||
containingDir: containingDir, | ||
tagPattern: tagPattern, | ||
), | ||
resolvedRef, | ||
), | ||
|
@@ -230,17 +268,25 @@ class GitSource extends CachedSource { | |
String? path, | ||
SystemCache cache, { | ||
required String relativeTo, | ||
required String? tagPattern, | ||
}) async { | ||
if (ref != null && tagPattern != null) { | ||
fail('Cannot have both a `tagPattern` and a `ref`'); | ||
sigurdm marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
final description = GitDescription( | ||
url: url, | ||
ref: ref, | ||
path: path, | ||
containingDir: relativeTo, | ||
tagPattern: tagPattern, // TODO | ||
sigurdm marked this conversation as resolved.
Show resolved
Hide resolved
|
||
); | ||
return await _pool.withResource(() async { | ||
await _ensureRepoCache(description, cache); | ||
final path = _repoCachePath(description, cache); | ||
final revision = await _firstRevision(path, description.ref); | ||
|
||
final revision = tagPattern != null | ||
? (await _listRevisionsWithTagPattern(path, tagPattern)).last | ||
: await _firstRevision(path, description.ref); | ||
final resolvedDescription = ResolvedGitDescription(description, revision); | ||
|
||
return Pubspec.parse( | ||
|
@@ -297,16 +343,42 @@ class GitSource extends CachedSource { | |
return await _pool.withResource(() async { | ||
await _ensureRepoCache(description, cache); | ||
final path = _repoCachePath(description, cache); | ||
final revision = await _firstRevision(path, description.ref); | ||
final pubspec = await _describeUncached(ref, revision, cache); | ||
final List<String> revisions; | ||
if (description.tagPattern case final String tagPattern) { | ||
revisions = await _listRevisionsWithTagPattern(path, tagPattern); | ||
} else { | ||
revisions = [await _firstRevision(path, description.ref)]; | ||
} | ||
|
||
return [ | ||
PackageId( | ||
ref.name, | ||
pubspec.version, | ||
ResolvedGitDescription(description, revision), | ||
), | ||
]; | ||
final result = <PackageId>[]; | ||
final seenVersions = <Version, String>{}; | ||
for (final revision in revisions) { | ||
final Pubspec pubspec; | ||
try { | ||
pubspec = await _describeUncached(ref, revision, cache); | ||
} on Exception { | ||
log.fine( | ||
'Found no valid pubspec of ${ref.name} at $revision, ignoring,', | ||
); | ||
continue; | ||
} | ||
final prev = seenVersions[pubspec.version]; | ||
if (prev != null) { | ||
log.fine( | ||
'Repeated version ${pubspec.version} of ${ref.name} ' | ||
'at $revision and $prev', | ||
); | ||
} | ||
seenVersions[pubspec.version] = revision; | ||
result.add( | ||
PackageId( | ||
ref.name, | ||
pubspec.version, | ||
ResolvedGitDescription(description, revision), | ||
), | ||
); | ||
} | ||
return result; | ||
}); | ||
} | ||
|
||
|
@@ -327,6 +399,8 @@ class GitSource extends CachedSource { | |
); | ||
} | ||
|
||
final Map<(PackageRef, String), Pubspec> _pubspecAtRevisionCache = {}; | ||
|
||
/// Like [describeUncached], but takes a separate [ref] and Git [revision] | ||
/// rather than a single ID. | ||
Future<Pubspec> _describeUncached( | ||
|
@@ -338,18 +412,20 @@ class GitSource extends CachedSource { | |
if (description is! GitDescription) { | ||
throw ArgumentError('Wrong source'); | ||
} | ||
await _ensureRevision(description, revision, cache); | ||
return _pubspecAtRevisionCache[(ref, revision)] ??= await () async { | ||
await _ensureRevision(description, revision, cache); | ||
|
||
return Pubspec.parse( | ||
await _showFileAtRevision( | ||
ResolvedGitDescription(description, revision), | ||
'pubspec.yaml', | ||
cache, | ||
), | ||
cache.sources, | ||
expectedName: ref.name, | ||
containingDescription: ref.description, | ||
); | ||
return Pubspec.parse( | ||
await _showFileAtRevision( | ||
ResolvedGitDescription(description, revision), | ||
'pubspec.yaml', | ||
cache, | ||
), | ||
cache.sources, | ||
expectedName: ref.name, | ||
containingDescription: ref.description, | ||
); | ||
}(); | ||
} | ||
|
||
/// Clones a Git repo to the local filesystem. | ||
|
@@ -647,6 +723,19 @@ class GitSource extends CachedSource { | |
String _packageListPath(String revisionCachePath) => | ||
p.join(revisionCachePath, '.git/pub-packages'); | ||
|
||
/// | ||
Future<List<String>> _listRevisionsWithTagPattern( | ||
String path, | ||
String tagPattern, | ||
) async { | ||
return (await git.run( | ||
['tag', '--list', tagPattern, '--format', '%(objectname)'], | ||
workingDir: path, | ||
)) | ||
.trim() | ||
.split('\n'); | ||
} | ||
|
||
/// Runs "git rev-list" on [reference] in [path] and returns the first result. | ||
/// | ||
/// This assumes that the canonical clone already exists. | ||
|
@@ -761,6 +850,8 @@ class GitDescription extends Description { | |
/// not allow strings of the form: '[email protected]:dart-lang/pub.git'. | ||
final String url; | ||
|
||
final String? tagPattern; | ||
sigurdm marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/// `true` if [url] was parsed from a relative url. | ||
final bool relative; | ||
|
||
|
@@ -777,6 +868,7 @@ class GitDescription extends Description { | |
required this.relative, | ||
required String? ref, | ||
required String? path, | ||
required this.tagPattern, | ||
}) : ref = ref ?? 'HEAD', | ||
path = path ?? '.'; | ||
|
||
|
@@ -785,13 +877,15 @@ class GitDescription extends Description { | |
required String? ref, | ||
required String? path, | ||
required String? containingDir, | ||
required String? tagPattern, | ||
}) { | ||
final validatedUrl = GitSource._validatedUrl(url, containingDir); | ||
return GitDescription.raw( | ||
url: validatedUrl.url, | ||
relative: validatedUrl.wasRelative, | ||
ref: ref, | ||
path: path, | ||
tagPattern: tagPattern, | ||
); | ||
} | ||
|
||
|
@@ -819,6 +913,7 @@ class GitDescription extends Description { | |
'url': relativeUrl, | ||
if (ref != 'HEAD') 'ref': ref, | ||
if (path != '.') 'path': path, | ||
if (tagPattern != null) 'tag_pattern': tagPattern, | ||
}; | ||
} | ||
|
||
|
@@ -838,6 +933,7 @@ class GitDescription extends Description { | |
relative: relative, | ||
ref: newRef, | ||
path: path, | ||
tagPattern: tagPattern, | ||
); | ||
|
||
@override | ||
|
@@ -883,6 +979,7 @@ class ResolvedGitDescription extends ResolvedDescription { | |
return { | ||
'url': url, | ||
'ref': description.ref, | ||
'tag_pattern': description.tagPattern, | ||
'resolved-ref': resolvedRef, | ||
sigurdm marked this conversation as resolved.
Show resolved
Hide resolved
|
||
'path': description.path, | ||
}; | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.