Skip to content
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

Allow overriding resolution and workspace fields in pubspec_overrides. #4400

Merged
merged 4 commits into from
Oct 7, 2024
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
44 changes: 38 additions & 6 deletions lib/src/command/unpack.dart
Original file line number Diff line number Diff line change
Expand Up @@ -135,18 +135,50 @@ in a directory `foo-<version>`.
await cache.hosted.downloadInto(id, destinationDir, cache);
},
);
final e = Entrypoint(
destinationDir,
cache,
);

if (argResults.flag('resolve')) {
try {
final pubspec = Pubspec.load(
destinationDir,
cache.sources,
containingDescription: RootDescription(destinationDir),
);
final buffer = StringBuffer();
if (pubspec.resolution != Resolution.none) {
log.message(
'''
This package was developed as part of a workspace.

Creating `pubspec_overrides.yaml` to resolve it alone.''',
);
buffer.writeln('resolution:');
}
if (pubspec.dependencyOverrides.isNotEmpty) {
log.message(
'''
This package was developed with dependency_overrides.

Creating `pubspec_overrides.yaml` to resolve it without those overrides.''',
);
buffer.writeln('dependency_overrides:');
}
if (buffer.isNotEmpty) {
writeTextFile(
p.join(destinationDir, 'pubspec_overrides.yaml'),
buffer.toString(),
);
}
final e = Entrypoint(
destinationDir,
cache,
);
await e.acquireDependencies(SolveType.get);
} finally {
log.message('To explore type: cd $destinationDir');
if (e.example != null) {
final exampleDir = p.join(destinationDir, 'example');
if (dirExists(exampleDir)) {
log.message(
'To explore the example type: cd ${e.example!.workspaceRoot.dir}',
'To explore the example type: cd $exampleDir',
);
}
}
Expand Down
55 changes: 33 additions & 22 deletions lib/src/pubspec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,13 @@ class Pubspec extends PubspecBase {
/// Directories of packages that should resolve together with this package.
late List<String> workspace = () {
final result = <String>[];
final r = fields.nodes['workspace'];
if (r != null && !languageVersion.supportsWorkspaces) {
final workspaceNode =
_overridesFileFields?.nodes['workspace'] ?? fields.nodes['workspace'];
if (workspaceNode != null && !languageVersion.supportsWorkspaces) {
_error(
'`workspace` and `resolution` requires at least language version '
'${LanguageVersion.firstVersionWithWorkspaces}',
r.span,
workspaceNode.span,
hint: '''
Consider updating the SDK constraint to:

Expand All @@ -80,12 +81,12 @@ environment:
''',
);
}
if (r == null || r.value == null) return <String>[];
if (workspaceNode == null || workspaceNode.value == null) return <String>[];

if (r is! YamlList) {
_error('"workspace" must be a list of strings', r.span);
if (workspaceNode is! YamlList) {
_error('"workspace" must be a list of strings', workspaceNode.span);
}
for (final t in r.nodes) {
for (final t in workspaceNode.nodes) {
final value = t.value;
if (value is! String) {
_error('"workspace" must be a list of strings', t.span);
Expand All @@ -103,12 +104,14 @@ environment:

/// The resolution mode.
late Resolution resolution = () {
final r = fields.nodes['resolution'];
if (r != null && !languageVersion.supportsWorkspaces) {
final resolutionNode =
_overridesFileFields?.nodes['resolution'] ?? fields.nodes['resolution'];

if (resolutionNode != null && !languageVersion.supportsWorkspaces) {
_error(
'`workspace` and `resolution` requires at least language version '
'${LanguageVersion.firstVersionWithWorkspaces}',
r.span,
resolutionNode.span,
hint: '''
Consider updating the SDK constraint to:

Expand All @@ -117,14 +120,14 @@ environment:
''',
);
}
return switch (r?.value) {
return switch (resolutionNode?.value) {
null => Resolution.none,
'local' => Resolution.local,
'workspace' => Resolution.workspace,
'external' => Resolution.external,
_ => _error(
'"resolution" must be one of `workspace`, `local`, `external`',
r!.span,
resolutionNode!.span,
)
};
}();
Expand Down Expand Up @@ -167,16 +170,6 @@ environment:
if (_dependencyOverrides != null) return _dependencyOverrides!;
final pubspecOverridesFields = _overridesFileFields;
if (pubspecOverridesFields != null) {
pubspecOverridesFields.nodes.forEach((key, _) {
final keyNode = key as YamlNode;
if (!const {'dependency_overrides'}.contains(keyNode.value)) {
throw SourceSpanApplicationException(
'pubspec_overrides.yaml only supports the '
'`dependency_overrides` field.',
keyNode.span,
);
}
});
if (pubspecOverridesFields.containsKey('dependency_overrides')) {
_dependencyOverrides = _parseDependencies(
'dependency_overrides',
Expand Down Expand Up @@ -394,6 +387,19 @@ environment:
? fields
: YamlMap.wrap(fields, sourceUrl: location),
) {
if (overridesFields != null) {
overridesFields.nodes.forEach((key, _) {
final keyNode = key as YamlNode;
if (!const {'dependency_overrides', 'resolution', 'workspace'}
.contains(keyNode.value)) {
throw SourceSpanApplicationException(
'pubspec_overrides.yaml only supports the '
'`dependency_overrides`, `resolution` and `workspace` fields.',
keyNode.span,
);
}
});
}
// If [expectedName] is passed, ensure that the actual 'name' field exists
// and matches the expectation.
if (expectedName == null) return;
Expand Down Expand Up @@ -845,8 +851,13 @@ class SdkConstraint {
}

enum Resolution {
// Still unused.
external,
// This package is a member of a workspace, and should be resolved with a
// pubspec.yaml located higher.
workspace,
// Still unused.
local,
// This package is at the root of a workspace.
none,
}
9 changes: 7 additions & 2 deletions test/pubspec_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1060,8 +1060,13 @@ dependency_overrides:
);
}

final pubspec = parsePubspecOverrides(contents);
expect(() => fn(pubspec), throwsA(expectation));
expect(
() {
final pubspec = parsePubspecOverrides(contents);
fn(pubspec);
},
throwsA(expectation),
);
}

test('allows empty overrides file', () {
Expand Down
40 changes: 40 additions & 0 deletions test/unpack_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,44 @@ Resolving dependencies in `../foo-1.2.3-pre`...
output: contains('Downloading foo 1.0.0 to `.${s}foo-1.0.0`...'),
);
});

test('unpacks and resolve workspace project', () async {
await d.dir(appPath).create();

final server = await servePackages();
server.serve('bar', '1.0.0');
server.serve(
'foo',
'1.0.0',
pubspec: {
'environment': {'sdk': '^3.5.0'},
'resolution': 'workspace',
'workspace': ['example'],
},
contents: [
d.dir('example', [
d.libPubspec(
'example',
'1.0.0',
sdk: '^3.5.0',
deps: {'foo': null, 'bar': '^1.0.0'},
extras: {'resolution': 'workspace'},
),
]),
],
);
await runPub(
args: ['unpack', 'foo:1.0.0'],
output: allOf(
contains('Downloading foo 1.0.0 to `.${s}foo-1.0.0`...'),
contains(
'+ bar',
),
),
environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'},
);
sigurdm marked this conversation as resolved.
Show resolved Hide resolved
await d.dir(appPath, [
d.dir('foo-1.0.0', [d.file('pubspec_overrides.yaml', 'resolution:\n')]),
]).validate();
});
}
49 changes: 49 additions & 0 deletions test/workspace_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1638,6 +1638,55 @@ b a${s}b$s
''',
);
});

test(
'"workspace" and "resolution" fields can be overridden by '
'`pubspec_overrides`',
() async {
final server = await servePackages();
server.serve('foo', '1.0.0');
server.serve('bar', '1.0.0');
await dir(appPath, [
libPubspec(
'myapp',
'1.2.3',
extras: {
'workspace': ['pkgs/a'],
},
sdk: '^3.5.0',
),
dir('pkgs', [
dir('a', [
libPubspec('a', '1.1.1', sdk: '^3.5.0', deps: {'foo': '^1.0.0'}),
file('pubspec_overrides.yaml', 'resolution: workspace'),
]),
dir(
'b',
[
libPubspec(
'b',
'1.0.0',
deps: {'bar': '^1.0.0'},
resolutionWorkspace: true,
),
],
),
]),
]).create();
await pubGet(
environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'},
output: contains('+ foo'),
);
await dir(
appPath,
[file('pubspec_overrides.yaml', 'workspace: ["pkgs/b/"]')],
).create();
await pubGet(
environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'},
output: contains('+ bar'),
);
},
);
}

final s = p.separator;