Skip to content

Resolve pubspec dependencies relative to the Resolved description of their containing package #4575

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions lib/src/command/add.dart
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,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 @@ -566,7 +568,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 +654,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
4 changes: 3 additions & 1 deletion lib/src/command/dependency_services.dart
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,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
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
2 changes: 1 addition & 1 deletion lib/src/global_packages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,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
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
2 changes: 1 addition & 1 deletion lib/src/source.dart
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ abstract class Source {
PackageRef parseRef(
String name,
Object? description, {
required Description containingDescription,
required ResolvedDescription containingDescription,
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
42 changes: 25 additions & 17 deletions lib/src/source/git.dart
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class GitSource extends CachedSource {
PackageRef parseRef(
String name,
Object? description, {
Description? containingDescription,
ResolvedDescription? containingDescription,
required LanguageVersion languageVersion,
}) {
String url;
Expand Down Expand Up @@ -86,7 +86,7 @@ class GitSource extends CachedSource {
}
}

final containingDir = switch (containingDescription) {
final containingDir = switch (containingDescription?.description) {
RootDescription(path: final path) => path,
PathDescription(path: final path) => path,
_ => null,
Expand Down Expand Up @@ -267,7 +267,7 @@ class GitSource extends CachedSource {
return Pubspec.parse(
await _showFileAtRevision(resolvedDescription, 'pubspec.yaml', cache),
cache.sources,
containingDescription: description,
containingDescription: resolvedDescription,
).name;
});
}
Expand Down Expand Up @@ -338,16 +338,26 @@ class GitSource extends CachedSource {
/// Since we don't have an easy way to read from a remote Git repo, this
/// just installs [id] into the system cache, then describes it from there.
@override
Future<Pubspec> describeUncached(PackageId id, SystemCache cache) {
Future<Pubspec> describeUncached(PackageId id, SystemCache cache) async {
final description = id.description;
if (description is! ResolvedGitDescription) {
throw StateError('Called with wrong ref');
}
return _pool.withResource(
final pubspec = await _pool.withResource(
() => _describeUncached(id.toRef(), description.resolvedRef, cache),
);
if (pubspec.version != id.version) {
throw PackageNotFoundException(
'Expected ${id.name} version ${id.version} '
'at commit ${description.resolvedRef}, '
'found ${pubspec.version}.',
);
}
return pubspec;
}

final Map<(PackageRef, String), Pubspec> _pubspecAtRevisionCache = {};

/// Like [describeUncached], but takes a separate [ref] and Git [revision]
/// rather than a single ID.
Future<Pubspec> _describeUncached(
Expand All @@ -359,18 +369,16 @@ class GitSource extends CachedSource {
if (description is! GitDescription) {
throw ArgumentError('Wrong source');
}
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 _pubspecAtRevisionCache[(ref, revision)] ??= await () async {
await _ensureRevision(description, revision, cache);
final resolvedDescription = ResolvedGitDescription(description, revision);
return Pubspec.parse(
await _showFileAtRevision(resolvedDescription, 'pubspec.yaml', cache),
cache.sources,
expectedName: ref.name,
containingDescription: resolvedDescription,
);
}();
}

/// Clones a Git repo to the local filesystem.
Expand Down
21 changes: 13 additions & 8 deletions lib/src/source/hosted.dart
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ class HostedSource extends CachedSource {
PackageRef parseRef(
String name,
Object? description, {
required Description containingDescription,
required ResolvedDescription containingDescription,
required LanguageVersion languageVersion,
}) {
return PackageRef(
Expand Down Expand Up @@ -416,17 +416,22 @@ class HostedSource extends CachedSource {
if (pubspecData is! Map) {
throw const FormatException('pubspec must be a map');
}

final archiveSha256 = map['archive_sha256'];
if (archiveSha256 != null && archiveSha256 is! String) {
throw const FormatException('archive_sha256 must be a String');
}
final parsedContentHash = _parseContentHash(archiveSha256 as String?);
final pubspec = Pubspec.fromMap(
pubspecData,
cache.sources,
expectedName: ref.name,
location: location,
containingDescription: description,
containingDescription: ResolvedHostedDescription(
description,
sha256: parsedContentHash,
),
);
final archiveSha256 = map['archive_sha256'];
if (archiveSha256 != null && archiveSha256 is! String) {
throw const FormatException('archive_sha256 must be a String');
}
final archiveUrl = map['archive_url'];
if (archiveUrl is! String) {
throw const FormatException('archive_url must be a String');
Expand Down Expand Up @@ -463,7 +468,7 @@ class HostedSource extends CachedSource {
pubspec,
Uri.parse(archiveUrl),
status,
_parseContentHash(archiveSha256 as String?),
parsedContentHash,
);
}).toList();
}
Expand Down Expand Up @@ -1640,7 +1645,7 @@ See $contentHashesDocumentationUrl.
containingDescription:
// Dummy description. As we never use the dependencies, they don't
// need to be resolved.
RootDescription('.'),
ResolvedRootDescription.fromDir('.'),
);
final errors = pubspec.dependencyErrors;
if (errors.isNotEmpty) {
Expand Down
Loading