Skip to content

Commit

Permalink
Warn when deleting files, and error on stray pubspec.yamls
Browse files Browse the repository at this point in the history
  • Loading branch information
sigurdm committed Nov 29, 2024
1 parent 418e017 commit 188b8fc
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 4 deletions.
20 changes: 18 additions & 2 deletions lib/src/entrypoint.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1343,6 +1343,7 @@ See https://dart.dev/go/sdk-constraint
/// but still has an old package config or lockfile.
void _removeStrayLockAndConfigFiles() {
final visited = <String>{};
var deletedAny = false;
for (final package in workspaceRoot.transitiveWorkspace) {
if (package.pubspec.resolution == Resolution.workspace) {
for (final dir in parentDirs(package.dir)) {
Expand All @@ -1353,11 +1354,26 @@ See https://dart.dev/go/sdk-constraint
// No reason to delete from the same directory twice.
break;
}
deleteEntry(p.join(dir, 'pubspec.lock'));
deleteEntry(p.join(dir, '.dart_tool', 'package_config.json'));
void deleteIfPresent(String path, String type) {
fileExists(path);
log.warning('Deleting old $type: `$path`.');
deleteEntry(path);
deletedAny = true;
}

deleteIfPresent(p.join(dir, 'pubspec.lock'), 'lock-file');
deleteIfPresent(
p.join(dir, '.dart_tool', 'package_config.json'),
'package config',
);
}
}
}
if (deletedAny) {
log.warning(
'See https://dart.dev/go/workspaces-no-inbetween-packages for details.',
);
}
}

/// Returns a list of changes to constraints of workspace pubspecs updated to
Expand Down
35 changes: 35 additions & 0 deletions lib/src/package.dart
Original file line number Diff line number Diff line change
Expand Up @@ -435,9 +435,12 @@ $symlinkResolvedDir => ${p.canonicalize(symlinkResolvedParent)}
/// * If a package name occurs twice.
/// * If two packages in the workspace override the same package name.
/// * A workspace package is overridden.
/// * A pubspec not included in the workspace exists in a directory
/// between the root and a workspace package.
void validateWorkspace(Package root) {
if (root.workspaceChildren.isEmpty) return;

/// Maps the `p.canonicalize`d dir of each workspace-child to its parent.
final includedFrom = <String, String>{};
final stack = [root];

Expand Down Expand Up @@ -500,6 +503,38 @@ Consider removing one of the overrides.
Cannot override workspace packages.
Package `$override` at `${overriddenWorkspacePackage.presentationDir}` is overridden in `${package.pubspecPath}`.
''');
}
}
}

// Check for pubspec.yaml files between the root and any workspace package.
final visited = <String>{};
for (final package in root.transitiveWorkspace) {
// Run through all parent directories until we meet another workspace
// package.
for (final dir in parentDirs(package.dir).skip(1)) {
// Stop
if (includedFrom.containsKey(p.canonicalize(dir)) ||
p.equals(dir, root.dir)) {
break;
}
if (!visited.add(dir)) {
// We have been here before.
break;
}
final pubspecCandidate = p.join(dir, 'pubspec.yaml');
if (fileExists(pubspecCandidate)) {
fail('''
The file `$pubspecCandidate` is located in a directory between the workspace root at
`${root.dir}` and a workspace package at `${package.dir}`. But is not a member of the
workspace.
This blocks the resolution of the package at `${package.dir}`.
Consider removing it.
See https://dart.dev/go/workspaces-no-inbetween-packages for details.
''');
}
}
Expand Down
51 changes: 49 additions & 2 deletions test/workspace_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,39 @@ foo:foomain''',
);
});

test('Removes lock files and package configs from workspace members',
test('Reports error if pubspec inside workspace is not part of the workspace',
() async {
await dir(appPath, [
libPubspec(
'myapp',
'1.2.3',
extras: {
'workspace': ['pkgs/a', 'pkgs/a/example'],
},
sdk: '^3.5.0',
),
dir('pkgs', [
libPubspec('not_in_workspace', '1.0.0'),
dir(
'a',
[
libPubspec('a', '1.1.1', resolutionWorkspace: true),
dir('example', [
libPubspec('example', '0.0.0', resolutionWorkspace: true),
]),
],
),
]),
]).create();
await pubGet(
environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'},
error: contains(
'The file `./pkgs/pubspec.yaml` is located in a directory between the workspace root',
),
);
});

test('Removes lock files and package configs from inside the workspace',
() async {
await dir(appPath, [
libPubspec(
Expand Down Expand Up @@ -865,7 +897,22 @@ foo:foomain''',
createLockFileAndPackageConfig(pkgsDir);
createLockFileAndPackageConfig(inside);

await pubGet(environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'});
await pubGet(
environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'},
warning: allOf(
contains('Deleting old lock-file: `./pkgs/a/pubspec.lock'),
contains(
'Deleting old package config: `./pkgs/a/.dart_tool/package_config.json`',
),
contains('Deleting old lock-file: `./pkgs/pubspec.lock'),
contains(
'Deleting old package config: `./pkgs/.dart_tool/package_config.json`',
),
contains(
'See https://dart.dev/go/workspaces-no-inbetween-packages for details.',
),
),
);

validateLockFileAndPackageConfig(
outideWorkpace,
Expand Down

0 comments on commit 188b8fc

Please sign in to comment.