From 1efd3f5e274e153637d99698b0ee454f6f2550ab Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Mon, 7 Oct 2024 16:26:28 +0200 Subject: [PATCH] Allow overriding `resolution` and `workspace` fields in pubspec_overrides. (#4400) --- lib/src/command/unpack.dart | 44 +++++++++++++++++++++++++---- lib/src/pubspec.dart | 55 ++++++++++++++++++++++--------------- test/pubspec_test.dart | 9 ++++-- test/unpack_test.dart | 40 +++++++++++++++++++++++++++ test/workspace_test.dart | 49 +++++++++++++++++++++++++++++++++ 5 files changed, 167 insertions(+), 30 deletions(-) diff --git a/lib/src/command/unpack.dart b/lib/src/command/unpack.dart index 2ee228fab..3ec9ddff6 100644 --- a/lib/src/command/unpack.dart +++ b/lib/src/command/unpack.dart @@ -135,18 +135,50 @@ in a directory `foo-`. 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', ); } } diff --git a/lib/src/pubspec.dart b/lib/src/pubspec.dart index 15427683b..8c40b6d71 100644 --- a/lib/src/pubspec.dart +++ b/lib/src/pubspec.dart @@ -66,12 +66,13 @@ class Pubspec extends PubspecBase { /// Directories of packages that should resolve together with this package. late List workspace = () { final result = []; - 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: @@ -80,12 +81,12 @@ environment: ''', ); } - if (r == null || r.value == null) return []; + if (workspaceNode == null || workspaceNode.value == null) return []; - 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); @@ -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: @@ -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, ) }; }(); @@ -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', @@ -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; @@ -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, } diff --git a/test/pubspec_test.dart b/test/pubspec_test.dart index 8dbff1876..c181fcc0c 100644 --- a/test/pubspec_test.dart +++ b/test/pubspec_test.dart @@ -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', () { diff --git a/test/unpack_test.dart b/test/unpack_test.dart index 5e4ab2ef0..56fa6a0fd 100644 --- a/test/unpack_test.dart +++ b/test/unpack_test.dart @@ -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'}, + ); + await d.dir(appPath, [ + d.dir('foo-1.0.0', [d.file('pubspec_overrides.yaml', 'resolution:\n')]), + ]).validate(); + }); } diff --git a/test/workspace_test.dart b/test/workspace_test.dart index 1dc9a636c..b1336d453 100644 --- a/test/workspace_test.dart +++ b/test/workspace_test.dart @@ -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;