From ce24a2d9ea9319b8f8253180bd13b220b6163962 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Mon, 30 Sep 2024 11:55:59 +0000 Subject: [PATCH 1/3] Allow overriding `resolution` and `workspace` fields in pubspec_overrides. And use this in `dart pub unpack` for allowing to unpack workspace packages. --- lib/src/command/unpack.dart | 44 +++++++++++++++++++++++++---- lib/src/entrypoint.dart | 19 +++++++------ lib/src/pubspec.dart | 55 ++++++++++++++++++++++--------------- test/unpack_test.dart | 37 +++++++++++++++++++++++++ test/workspace_test.dart | 49 +++++++++++++++++++++++++++++++++ 5 files changed, 167 insertions(+), 37 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/entrypoint.dart b/lib/src/entrypoint.dart index f57aeb9e7..5eafd9ba7 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -111,15 +111,16 @@ class Entrypoint { path, { expectedName, required withPubspecOverrides, - }) => - pubspecsMet[p.canonicalize(path)] ?? - Pubspec.load( - path, - cache.sources, - expectedName: expectedName, - allowOverridesFile: withPubspecOverrides, - containingDescription: RootDescription(path), - ), + }) { + return pubspecsMet[p.canonicalize(path)] ?? + Pubspec.load( + path, + cache.sources, + expectedName: expectedName, + allowOverridesFile: withPubspecOverrides, + containingDescription: RootDescription(path), + ); + }, withPubspecOverrides: true, ); for (final package in root.transitiveWorkspace) { diff --git a/lib/src/pubspec.dart b/lib/src/pubspec.dart index fcd9259e1..184301c0b 100644 --- a/lib/src/pubspec.dart +++ b/lib/src/pubspec.dart @@ -66,20 +66,21 @@ 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, ); } - 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); @@ -97,22 +98,24 @@ class Pubspec extends PubspecBase { /// 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, ); } - 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, ) }; }(); @@ -155,16 +158,6 @@ class Pubspec extends PubspecBase { 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', @@ -382,6 +375,19 @@ class Pubspec extends PubspecBase { ? 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; @@ -833,8 +839,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/unpack_test.dart b/test/unpack_test.dart index 5e4ab2ef0..01e6e7334 100644 --- a/test/unpack_test.dart +++ b/test/unpack_test.dart @@ -125,4 +125,41 @@ 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'}, + ); + }); } diff --git a/test/workspace_test.dart b/test/workspace_test.dart index e7d209565..67b1e3468 100644 --- a/test/workspace_test.dart +++ b/test/workspace_test.dart @@ -1637,6 +1637,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; From abb7c2483f3daf8484bd2569009e81a1a1b8799d Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Tue, 1 Oct 2024 12:41:53 +0000 Subject: [PATCH 2/3] Test that error occurs earlier --- pubspec.lock | 14 +++++++------- test/pubspec_test.dart | 9 +++++++-- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/pubspec.lock b/pubspec.lock index 5051dfbd5..e20fe4db8 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -5,23 +5,23 @@ packages: dependency: transitive description: name: _fe_analyzer_shared - sha256: "45cfa8471b89fb6643fe9bf51bd7931a76b8f5ec2d65de4fb176dba8d4f22c77" + sha256: f6dbf021f4b214d85c79822912c5fcd142a2c4869f01222ad371bc51f9f1c356 url: "https://pub.dev" source: hosted - version: "73.0.0" + version: "74.0.0" _macros: dependency: transitive description: dart source: sdk - version: "0.3.2" + version: "0.3.3" analyzer: dependency: "direct main" description: name: analyzer - sha256: "4959fec185fe70cce007c57e9ab6983101dbe593d2bf8bbfb4453aaec0cf470a" + sha256: f7e8caf82f2d3190881d81012606effdf8a38e6c1ab9e30947149733065f817c url: "https://pub.dev" source: hosted - version: "6.8.0" + version: "6.9.0" args: dependency: "direct main" description: @@ -194,10 +194,10 @@ packages: dependency: transitive description: name: macros - sha256: "0acaed5d6b7eab89f63350bccd82119e6c602df0f391260d0e32b5e23db79536" + sha256: "1d9e801cd66f7ea3663c45fc708450db1fa57f988142c64289142c9b7ee80656" url: "https://pub.dev" source: hosted - version: "0.1.2-main.4" + version: "0.1.3-main.0" matcher: dependency: transitive description: diff --git a/test/pubspec_test.dart b/test/pubspec_test.dart index 5db5bf3a9..f7e55d89c 100644 --- a/test/pubspec_test.dart +++ b/test/pubspec_test.dart @@ -1036,8 +1036,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', () { From 9976f6faa6d6af2209e3a94db93262867d22b3ac Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Mon, 7 Oct 2024 14:13:26 +0000 Subject: [PATCH 3/3] Add text expectation about override file --- lib/src/entrypoint.dart | 19 +++++++++---------- test/unpack_test.dart | 3 +++ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/lib/src/entrypoint.dart b/lib/src/entrypoint.dart index 5eafd9ba7..f57aeb9e7 100644 --- a/lib/src/entrypoint.dart +++ b/lib/src/entrypoint.dart @@ -111,16 +111,15 @@ class Entrypoint { path, { expectedName, required withPubspecOverrides, - }) { - return pubspecsMet[p.canonicalize(path)] ?? - Pubspec.load( - path, - cache.sources, - expectedName: expectedName, - allowOverridesFile: withPubspecOverrides, - containingDescription: RootDescription(path), - ); - }, + }) => + pubspecsMet[p.canonicalize(path)] ?? + Pubspec.load( + path, + cache.sources, + expectedName: expectedName, + allowOverridesFile: withPubspecOverrides, + containingDescription: RootDescription(path), + ), withPubspecOverrides: true, ); for (final package in root.transitiveWorkspace) { diff --git a/test/unpack_test.dart b/test/unpack_test.dart index 01e6e7334..56fa6a0fd 100644 --- a/test/unpack_test.dart +++ b/test/unpack_test.dart @@ -161,5 +161,8 @@ Resolving dependencies in `../foo-1.2.3-pre`... ), 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(); }); }