Skip to content

Commit 99e84bc

Browse files
authored
Resolve pubspec dependencies relative to the Resolved description of their containing package (#4575)
1 parent 0264dd0 commit 99e84bc

17 files changed

+194
-143
lines changed

lib/src/command/add.dart

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,9 @@ Specify multiple sdk packages with descriptors.''');
275275
location: Uri.parse(entrypoint.workPackage.pubspecPath),
276276
overridesFileContents: overridesFileContents,
277277
overridesLocation: Uri.file(overridesPath),
278-
containingDescription: RootDescription(entrypoint.workPackage.dir),
278+
containingDescription: ResolvedRootDescription.fromDir(
279+
entrypoint.workPackage.dir,
280+
),
279281
),
280282
)
281283
.acquireDependencies(
@@ -566,7 +568,7 @@ Specify multiple sdk packages with descriptors.''');
566568
ref = cache.sdk.parseRef(
567569
packageName,
568570
argResults.sdk,
569-
containingDescription: RootDescription(p.current),
571+
containingDescription: ResolvedRootDescription.fromDir(p.current),
570572
);
571573
} else {
572574
ref = PackageRef(
@@ -652,7 +654,7 @@ Specify multiple sdk packages with descriptors.''');
652654
cache.sources,
653655
// Resolve relative paths relative to current, not where the
654656
// pubspec.yaml is.
655-
containingDescription: RootDescription(p.current),
657+
containingDescription: ResolvedRootDescription.fromDir(p.current),
656658
);
657659
} on FormatException catch (e) {
658660
usageException('Failed parsing package specification: ${e.message}');

lib/src/command/dependency_services.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,9 @@ class DependencyServicesApplyCommand extends PubCommand {
480480
updatedPubspecs[package.dir].toString(),
481481
cache.sources,
482482
location: toUri(package.pubspecPath),
483-
containingDescription: RootDescription(package.dir),
483+
containingDescription: ResolvedRootDescription(
484+
RootDescription(package.dir),
485+
),
484486
),
485487
);
486488
// Resolve versions, this will update transitive dependencies that were

lib/src/command/lish.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,9 @@ the \$PUB_HOSTED_URL environment variable.''');
395395
),
396396
),
397397
cache.sources,
398-
containingDescription: RootDescription(p.dirname(archive)),
398+
containingDescription: ResolvedRootDescription.fromDir(
399+
p.dirname(archive),
400+
),
399401
);
400402
} on FormatException catch (e) {
401403
dataError('Failed to read pubspec.yaml from archive: ${e.message}');

lib/src/command/unpack.dart

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,9 @@ in a directory `foo-<version>`.
144144
final pubspec = Pubspec.load(
145145
destinationDir,
146146
cache.sources,
147-
containingDescription: RootDescription(destinationDir),
147+
containingDescription: ResolvedRootDescription.fromDir(
148+
destinationDir,
149+
),
148150
);
149151
final buffer = StringBuffer();
150152
if (pubspec.resolution != Resolution.none) {
@@ -212,7 +214,7 @@ Creating `pubspec_overrides.yaml` to resolve it without those overrides.''');
212214
// Resolve relative paths relative to current, not where the
213215
// pubspec.yaml is.
214216
location: p.toUri(p.join(p.current, 'descriptor')),
215-
containingDescription: RootDescription('.'),
217+
containingDescription: ResolvedRootDescription.fromDir('.'),
216218
);
217219
} on FormatException catch (e) {
218220
usageException('Failed parsing package specification: ${e.message}');

lib/src/entrypoint.dart

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ class Entrypoint {
9797
pubspec = Pubspec.load(
9898
dir,
9999
cache.sources,
100-
containingDescription: RootDescription(dir),
100+
containingDescription: ResolvedRootDescription.fromDir(dir),
101101
allowOverridesFile: true,
102102
);
103103
} on FileException {
@@ -116,7 +116,9 @@ class Entrypoint {
116116
cache.sources,
117117
expectedName: expectedName,
118118
allowOverridesFile: withPubspecOverrides,
119-
containingDescription: RootDescription(path),
119+
containingDescription: ResolvedRootDescription.fromDir(
120+
path,
121+
),
120122
),
121123
withPubspecOverrides: true,
122124
);

lib/src/global_packages.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ class GlobalPackages {
115115
if (path != null) 'path': path,
116116
if (ref != null) 'ref': ref,
117117
},
118-
containingDescription: RootDescription(p.current),
118+
containingDescription: ResolvedRootDescription.fromDir(p.current),
119119
languageVersion: LanguageVersion.fromVersion(sdk.version),
120120
);
121121
} on FormatException catch (e) {

lib/src/pubspec.dart

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ class Pubspec extends PubspecBase {
6161

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

6666
/// Directories of packages that should resolve together with this package.
6767
late List<String> workspace = () {
@@ -279,7 +279,7 @@ environment:
279279
SourceRegistry sources, {
280280
String? expectedName,
281281
bool allowOverridesFile = false,
282-
required Description containingDescription,
282+
required ResolvedDescription containingDescription,
283283
}) {
284284
final pubspecPath = p.join(packageDir, pubspecYamlFilename);
285285
final overridesPath = p.join(packageDir, pubspecOverridesFilename);
@@ -324,7 +324,7 @@ environment:
324324
sources,
325325
expectedName: expectedName,
326326
allowOverridesFile: withPubspecOverrides,
327-
containingDescription: RootDescription(dir),
327+
containingDescription: ResolvedRootDescription.fromDir(dir),
328328
);
329329
}
330330

@@ -362,7 +362,7 @@ environment:
362362
_overridesFileFields = null,
363363
// This is a dummy value. Dependencies should already be resolved, so we
364364
// never need to do relative resolutions.
365-
_containingDescription = RootDescription('.'),
365+
_containingDescription = ResolvedRootDescription.fromDir('.'),
366366
super(
367367
fields == null ? YamlMap() : YamlMap.wrap(fields),
368368
name: name,
@@ -382,7 +382,7 @@ environment:
382382
YamlMap? overridesFields,
383383
String? expectedName,
384384
Uri? location,
385-
required Description containingDescription,
385+
required ResolvedDescription containingDescription,
386386
}) : _overridesFileFields = overridesFields,
387387
_includeDefaultSdkConstraint = true,
388388
_givenSdkConstraints = null,
@@ -432,7 +432,7 @@ environment:
432432
Uri? location,
433433
String? overridesFileContents,
434434
Uri? overridesLocation,
435-
required Description containingDescription,
435+
required ResolvedDescription containingDescription,
436436
}) {
437437
final YamlMap pubspecMap;
438438
YamlMap? overridesFileMap;
@@ -576,7 +576,7 @@ Map<String, PackageRange> _parseDependencies(
576576
SourceRegistry sources,
577577
LanguageVersion languageVersion,
578578
String? packageName,
579-
Description containingDescription, {
579+
ResolvedDescription containingDescription, {
580580
_FileType fileType = _FileType.pubspec,
581581
}) {
582582
final dependencies = <String, PackageRange>{};

lib/src/source.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ abstract class Source {
8181
PackageRef parseRef(
8282
String name,
8383
Object? description, {
84-
required Description containingDescription,
84+
required ResolvedDescription containingDescription,
8585
required LanguageVersion languageVersion,
8686
});
8787

lib/src/source/cached.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ abstract class CachedSource extends Source {
3232
packageDir,
3333
cache.sources,
3434
expectedName: id.name,
35-
containingDescription: id.description.description,
35+
containingDescription: id.description,
3636
);
3737
}
3838

lib/src/source/git.dart

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class GitSource extends CachedSource {
3737
PackageRef parseRef(
3838
String name,
3939
Object? description, {
40-
Description? containingDescription,
40+
ResolvedDescription? containingDescription,
4141
required LanguageVersion languageVersion,
4242
}) {
4343
String url;
@@ -86,7 +86,7 @@ class GitSource extends CachedSource {
8686
}
8787
}
8888

89-
final containingDir = switch (containingDescription) {
89+
final containingDir = switch (containingDescription?.description) {
9090
RootDescription(path: final path) => path,
9191
PathDescription(path: final path) => path,
9292
_ => null,
@@ -267,7 +267,7 @@ class GitSource extends CachedSource {
267267
return Pubspec.parse(
268268
await _showFileAtRevision(resolvedDescription, 'pubspec.yaml', cache),
269269
cache.sources,
270-
containingDescription: description,
270+
containingDescription: resolvedDescription,
271271
).name;
272272
});
273273
}
@@ -338,16 +338,26 @@ class GitSource extends CachedSource {
338338
/// Since we don't have an easy way to read from a remote Git repo, this
339339
/// just installs [id] into the system cache, then describes it from there.
340340
@override
341-
Future<Pubspec> describeUncached(PackageId id, SystemCache cache) {
341+
Future<Pubspec> describeUncached(PackageId id, SystemCache cache) async {
342342
final description = id.description;
343343
if (description is! ResolvedGitDescription) {
344344
throw StateError('Called with wrong ref');
345345
}
346-
return _pool.withResource(
346+
final pubspec = await _pool.withResource(
347347
() => _describeUncached(id.toRef(), description.resolvedRef, cache),
348348
);
349+
if (pubspec.version != id.version) {
350+
throw PackageNotFoundException(
351+
'Expected ${id.name} version ${id.version} '
352+
'at commit ${description.resolvedRef}, '
353+
'found ${pubspec.version}.',
354+
);
355+
}
356+
return pubspec;
349357
}
350358

359+
final Map<(PackageRef, String), Pubspec> _pubspecAtRevisionCache = {};
360+
351361
/// Like [describeUncached], but takes a separate [ref] and Git [revision]
352362
/// rather than a single ID.
353363
Future<Pubspec> _describeUncached(
@@ -359,18 +369,16 @@ class GitSource extends CachedSource {
359369
if (description is! GitDescription) {
360370
throw ArgumentError('Wrong source');
361371
}
362-
await _ensureRevision(description, revision, cache);
363-
364-
return Pubspec.parse(
365-
await _showFileAtRevision(
366-
ResolvedGitDescription(description, revision),
367-
'pubspec.yaml',
368-
cache,
369-
),
370-
cache.sources,
371-
expectedName: ref.name,
372-
containingDescription: ref.description,
373-
);
372+
return _pubspecAtRevisionCache[(ref, revision)] ??= await () async {
373+
await _ensureRevision(description, revision, cache);
374+
final resolvedDescription = ResolvedGitDescription(description, revision);
375+
return Pubspec.parse(
376+
await _showFileAtRevision(resolvedDescription, 'pubspec.yaml', cache),
377+
cache.sources,
378+
expectedName: ref.name,
379+
containingDescription: resolvedDescription,
380+
);
381+
}();
374382
}
375383

376384
/// Clones a Git repo to the local filesystem.

lib/src/source/hosted.dart

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ class HostedSource extends CachedSource {
229229
PackageRef parseRef(
230230
String name,
231231
Object? description, {
232-
required Description containingDescription,
232+
required ResolvedDescription containingDescription,
233233
required LanguageVersion languageVersion,
234234
}) {
235235
return PackageRef(
@@ -416,17 +416,22 @@ class HostedSource extends CachedSource {
416416
if (pubspecData is! Map) {
417417
throw const FormatException('pubspec must be a map');
418418
}
419+
420+
final archiveSha256 = map['archive_sha256'];
421+
if (archiveSha256 != null && archiveSha256 is! String) {
422+
throw const FormatException('archive_sha256 must be a String');
423+
}
424+
final parsedContentHash = _parseContentHash(archiveSha256 as String?);
419425
final pubspec = Pubspec.fromMap(
420426
pubspecData,
421427
cache.sources,
422428
expectedName: ref.name,
423429
location: location,
424-
containingDescription: description,
430+
containingDescription: ResolvedHostedDescription(
431+
description,
432+
sha256: parsedContentHash,
433+
),
425434
);
426-
final archiveSha256 = map['archive_sha256'];
427-
if (archiveSha256 != null && archiveSha256 is! String) {
428-
throw const FormatException('archive_sha256 must be a String');
429-
}
430435
final archiveUrl = map['archive_url'];
431436
if (archiveUrl is! String) {
432437
throw const FormatException('archive_url must be a String');
@@ -463,7 +468,7 @@ class HostedSource extends CachedSource {
463468
pubspec,
464469
Uri.parse(archiveUrl),
465470
status,
466-
_parseContentHash(archiveSha256 as String?),
471+
parsedContentHash,
467472
);
468473
}).toList();
469474
}
@@ -1640,7 +1645,7 @@ See $contentHashesDocumentationUrl.
16401645
containingDescription:
16411646
// Dummy description. As we never use the dependencies, they don't
16421647
// need to be resolved.
1643-
RootDescription('.'),
1648+
ResolvedRootDescription.fromDir('.'),
16441649
);
16451650
final errors = pubspec.dependencyErrors;
16461651
if (errors.isNotEmpty) {

0 commit comments

Comments
 (0)