Skip to content

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
21 changes: 18 additions & 3 deletions lib/src/command/add.dart
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ For example (follow the same format including spaces):
help: 'Path of git package in repository',
hide: true,
);
argParser.addOption(
'git-tag-pattern',
help: 'The tag-pattern to search for versions in repository',
hide: true,
);
argParser.addOption(
'hosted-url',
help: 'URL of package host server',
Expand Down Expand Up @@ -275,7 +280,9 @@ Specify multiple sdk packages with descriptors.''');
location: Uri.parse(entrypoint.workPackage.pubspecPath),
overridesFileContents: overridesFileContents,
overridesLocation: Uri.file(overridesPath),
containingDescription: RootDescription(entrypoint.workPackage.dir),
containingDescription: ResolvedRootDescription.fromDir(
entrypoint.workPackage.dir,
),
),
)
.acquireDependencies(
Expand Down Expand Up @@ -541,6 +548,11 @@ Specify multiple sdk packages with descriptors.''');
if (gitUrl == null) {
usageException('The `--git-url` is required for git dependencies.');
}
if (argResults.gitRef != null && argResults.tagPattern != null) {
usageException(
'Cannot provide both `--git-ref` and `--git-tag-pattern`.',
);
}

/// Process the git options to return the simplest representation to be
/// added to the pubspec.
Expand All @@ -552,6 +564,7 @@ Specify multiple sdk packages with descriptors.''');
containingDir: p.current,
ref: argResults.gitRef,
path: argResults.gitPath,
tagPattern: argResults.tagPattern,
),
);
} on FormatException catch (e) {
Expand All @@ -566,7 +579,7 @@ Specify multiple sdk packages with descriptors.''');
ref = cache.sdk.parseRef(
packageName,
argResults.sdk,
containingDescription: RootDescription(p.current),
containingDescription: ResolvedRootDescription.fromDir(p.current),
);
} else {
ref = PackageRef(
Expand Down Expand Up @@ -652,7 +665,7 @@ Specify multiple sdk packages with descriptors.''');
cache.sources,
// Resolve relative paths relative to current, not where the
// pubspec.yaml is.
containingDescription: RootDescription(p.current),
containingDescription: ResolvedRootDescription.fromDir(p.current),
);
} on FormatException catch (e) {
usageException('Failed parsing package specification: ${e.message}');
Expand Down Expand Up @@ -787,6 +800,8 @@ extension on ArgResults {
bool get isDryRun => flag('dry-run');
String? get gitUrl => this['git-url'] as String?;
String? get gitPath => this['git-path'] as String?;
String? get tagPattern => this['git-tag-pattern'] as String?;

String? get gitRef => this['git-ref'] as String?;
String? get hostedUrl => this['hosted-url'] as String?;
String? get path => this['path'] as String?;
Expand Down
6 changes: 5 additions & 1 deletion lib/src/command/dependency_services.dart
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@ class DependencyServicesApplyCommand extends PubCommand {
} else if (targetRevision != null &&
(lockFileYaml['packages'] as Map).containsKey(targetPackage)) {
final ref = entrypoint.lockFile.packages[targetPackage]!.toRef();

final currentDescription = ref.description as GitDescription;
final updatedRef = PackageRef(
targetPackage,
Expand All @@ -432,6 +433,7 @@ class DependencyServicesApplyCommand extends PubCommand {
path: currentDescription.path,
ref: targetRevision,
containingDir: directory,
tagPattern: currentDescription.tagPattern,
),
);
final versions = await cache.getVersions(updatedRef);
Expand Down Expand Up @@ -480,7 +482,9 @@ class DependencyServicesApplyCommand extends PubCommand {
updatedPubspecs[package.dir].toString(),
cache.sources,
location: toUri(package.pubspecPath),
containingDescription: RootDescription(package.dir),
containingDescription: ResolvedRootDescription(
RootDescription(package.dir),
),
),
);
// Resolve versions, this will update transitive dependencies that were
Expand Down
4 changes: 3 additions & 1 deletion lib/src/command/lish.dart
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,9 @@ the \$PUB_HOSTED_URL environment variable.''');
),
),
cache.sources,
containingDescription: RootDescription(p.dirname(archive)),
containingDescription: ResolvedRootDescription.fromDir(
p.dirname(archive),
),
);
} on FormatException catch (e) {
dataError('Failed to read pubspec.yaml from archive: ${e.message}');
Expand Down
6 changes: 4 additions & 2 deletions lib/src/command/unpack.dart
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ in a directory `foo-<version>`.
final pubspec = Pubspec.load(
destinationDir,
cache.sources,
containingDescription: RootDescription(destinationDir),
containingDescription: ResolvedRootDescription.fromDir(
destinationDir,
),
);
final buffer = StringBuffer();
if (pubspec.resolution != Resolution.none) {
Expand Down Expand Up @@ -212,7 +214,7 @@ Creating `pubspec_overrides.yaml` to resolve it without those overrides.''');
// Resolve relative paths relative to current, not where the
// pubspec.yaml is.
location: p.toUri(p.join(p.current, 'descriptor')),
containingDescription: RootDescription('.'),
containingDescription: ResolvedRootDescription.fromDir('.'),
);
} on FormatException catch (e) {
usageException('Failed parsing package specification: ${e.message}');
Expand Down
7 changes: 3 additions & 4 deletions lib/src/command/upgrade.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import '../package_name.dart';
import '../pubspec.dart';
import '../pubspec_utils.dart';
import '../solver.dart';
import '../source/hosted.dart';
import '../utils.dart';

/// Handles the `upgrade` pub command.
Expand Down Expand Up @@ -248,11 +247,11 @@ be direct 'dependencies' or 'dev_dependencies', following packages are not:
// Mapping from original to changed value.
var changes = <Package, Map<PackageRange, PackageRange>>{};
for (final package in entrypoint.workspaceRoot.transitiveWorkspace) {
final declaredHostedDependencies = [
final declaredUpgradableDependencies = [
...package.dependencies.values,
...package.devDependencies.values,
].where((dep) => dep.source is HostedSource);
for (final dep in declaredHostedDependencies) {
].where((dep) => dep.description.hasMultipleVersions);
for (final dep in declaredUpgradableDependencies) {
final resolvedPackage = resolvedPackages[dep.name]!;
if (!toUpgrade.contains(dep.name)) {
// If we're not trying to upgrade this package, or it wasn't in the
Expand Down
6 changes: 4 additions & 2 deletions lib/src/entrypoint.dart
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class Entrypoint {
pubspec = Pubspec.load(
dir,
cache.sources,
containingDescription: RootDescription(dir),
containingDescription: ResolvedRootDescription.fromDir(dir),
allowOverridesFile: true,
);
} on FileException {
Expand All @@ -116,7 +116,9 @@ class Entrypoint {
cache.sources,
expectedName: expectedName,
allowOverridesFile: withPubspecOverrides,
containingDescription: RootDescription(path),
containingDescription: ResolvedRootDescription.fromDir(
path,
),
),
withPubspecOverrides: true,
);
Expand Down
4 changes: 3 additions & 1 deletion lib/src/global_packages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,15 @@ class GlobalPackages {
required bool overwriteBinStubs,
String? path,
String? ref,
String? tagPattern,
}) async {
final name = await cache.git.getPackageNameFromRepo(
repo,
ref,
path,
cache,
relativeTo: p.current,
tagPattern: tagPattern,
);

// TODO(nweiz): Add some special handling for git repos that contain path
Expand All @@ -115,7 +117,7 @@ class GlobalPackages {
if (path != null) 'path': path,
if (ref != null) 'ref': ref,
},
containingDescription: RootDescription(p.current),
containingDescription: ResolvedRootDescription.fromDir(p.current),
languageVersion: LanguageVersion.fromVersion(sdk.version),
);
} on FormatException catch (e) {
Expand Down
3 changes: 3 additions & 0 deletions lib/src/language_version.dart
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ class LanguageVersion implements Comparable<LanguageVersion> {

bool get supportsWorkspaces => this >= firstVersionWithWorkspaces;

bool get supportsTagPattern => this >= firstVersionWithTagPattern;

bool get forbidsUnknownDescriptionKeys =>
this >= firstVersionForbidingUnknownDescriptionKeys;

Expand Down Expand Up @@ -105,6 +107,7 @@ class LanguageVersion implements Comparable<LanguageVersion> {
static const firstVersionWithNullSafety = LanguageVersion(2, 12);
static const firstVersionWithShorterHostedSyntax = LanguageVersion(2, 15);
static const firstVersionWithWorkspaces = LanguageVersion(3, 5);
static const firstVersionWithTagPattern = LanguageVersion(3, 9);
static const firstVersionForbidingUnknownDescriptionKeys = LanguageVersion(
3,
7,
Expand Down
2 changes: 1 addition & 1 deletion lib/src/package_name.dart
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class PackageRange {
bool get _showVersionConstraint {
if (isRoot) return false;
if (!constraint.isAny) return true;
return description.source.hasMultipleVersions;
return description.hasMultipleVersions;
}

/// Returns a copy of `this` with the same semantics, but with a `^`-style
Expand Down
14 changes: 7 additions & 7 deletions lib/src/pubspec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class Pubspec extends PubspecBase {

/// It is used to resolve relative paths. And to resolve path-descriptions
/// from a git dependency as git-descriptions.
final Description _containingDescription;
final ResolvedDescription _containingDescription;

/// Directories of packages that should resolve together with this package.
late List<String> workspace = () {
Expand Down Expand Up @@ -279,7 +279,7 @@ environment:
SourceRegistry sources, {
String? expectedName,
bool allowOverridesFile = false,
required Description containingDescription,
required ResolvedDescription containingDescription,
}) {
final pubspecPath = p.join(packageDir, pubspecYamlFilename);
final overridesPath = p.join(packageDir, pubspecOverridesFilename);
Expand Down Expand Up @@ -324,7 +324,7 @@ environment:
sources,
expectedName: expectedName,
allowOverridesFile: withPubspecOverrides,
containingDescription: RootDescription(dir),
containingDescription: ResolvedRootDescription.fromDir(dir),
);
}

Expand Down Expand Up @@ -362,7 +362,7 @@ environment:
_overridesFileFields = null,
// This is a dummy value. Dependencies should already be resolved, so we
// never need to do relative resolutions.
_containingDescription = RootDescription('.'),
_containingDescription = ResolvedRootDescription.fromDir('.'),
super(
fields == null ? YamlMap() : YamlMap.wrap(fields),
name: name,
Expand All @@ -382,7 +382,7 @@ environment:
YamlMap? overridesFields,
String? expectedName,
Uri? location,
required Description containingDescription,
required ResolvedDescription containingDescription,
}) : _overridesFileFields = overridesFields,
_includeDefaultSdkConstraint = true,
_givenSdkConstraints = null,
Expand Down Expand Up @@ -432,7 +432,7 @@ environment:
Uri? location,
String? overridesFileContents,
Uri? overridesLocation,
required Description containingDescription,
required ResolvedDescription containingDescription,
}) {
final YamlMap pubspecMap;
YamlMap? overridesFileMap;
Expand Down Expand Up @@ -576,7 +576,7 @@ Map<String, PackageRange> _parseDependencies(
SourceRegistry sources,
LanguageVersion languageVersion,
String? packageName,
Description containingDescription, {
ResolvedDescription containingDescription, {
_FileType fileType = _FileType.pubspec,
}) {
final dependencies = <String, PackageRange>{};
Expand Down
5 changes: 4 additions & 1 deletion lib/src/solver/version_solver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,10 @@ class VersionSolver {
// can't be downgraded.
if (_type == SolveType.downgrade) {
final locked = _lockFile.packages[package];
if (locked != null && !locked.source.hasMultipleVersions) return locked;
if (locked != null &&
!locked.description.description.hasMultipleVersions) {
return locked;
}
}

if (_unlock.isEmpty || _unlock.contains(package)) return null;
Expand Down
13 changes: 6 additions & 7 deletions lib/src/source.dart
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,6 @@ abstract class Source {
/// all sources.
String get name;

/// Whether this source can choose between multiple versions of the same
/// package during version solving.
///
/// Defaults to `false`.
bool get hasMultipleVersions => false;

/// Parses a [PackageRef] from a name and a user-provided [description].
///
/// When a [Pubspec] is parsed, it reads in the description for each
Expand All @@ -81,7 +75,7 @@ abstract class Source {
PackageRef parseRef(
String name,
Object? description, {
required Description containingDescription,
required ResolvedDescription containingDescription,
required LanguageVersion languageVersion,
});

Expand Down Expand Up @@ -190,6 +184,11 @@ abstract class Source {
/// with a version constraint.
abstract class Description {
Source get source;

/// Whether the source can choose between multiple versions of this
/// package during version solving.
bool get hasMultipleVersions;

Object? serializeForPubspec({
required String? containingDir,
required LanguageVersion languageVersion,
Expand Down
2 changes: 1 addition & 1 deletion lib/src/source/cached.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ abstract class CachedSource extends Source {
packageDir,
cache.sources,
expectedName: id.name,
containingDescription: id.description.description,
containingDescription: id.description,
);
}

Expand Down
Loading