Skip to content

Commit

Permalink
Delete directories between workspace package and workspace root (#4446)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sigurdm
sigurdm committed Dec 10, 2024
1 parent ab8b269 commit 5281207
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 20 deletions.
34 changes: 32 additions & 2 deletions lib/src/entrypoint.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1335,15 +1335,45 @@ See https://dart.dev/go/sdk-constraint
/// Remove any `pubspec.lock` or `.dart_tool/package_config.json` files in
/// workspace packages that are not the root package.
///
/// Also remove from directories between the workspace package and the
/// workspace root, to prevent stray package configs from shadowing the shared
/// workspace package config.
///
/// This is to avoid surprises if a package is turned into a workspace member
/// but still has an old package config or lockfile.
void _removeStrayLockAndConfigFiles() {
final visited = <String>{
// By adding this to visited we will never go above the workspaceRoot.dir.
p.canonicalize(workspaceRoot.dir),
};
var deletedAny = false;
for (final package in workspaceRoot.transitiveWorkspace) {
if (package.pubspec.resolution == Resolution.workspace) {
deleteEntry(p.join(package.dir, 'pubspec.lock'));
deleteEntry(p.join(package.dir, '.dart_tool', 'package_config.json'));
for (final dir in parentDirs(package.dir)) {
if (!visited.add(p.canonicalize(dir))) {
// No reason to delete from the same directory twice.
break;
}
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-stray-files for details.',
);
}
}

/// Returns a list of changes to constraints of workspace pubspecs updated to
Expand Down
37 changes: 37 additions & 0 deletions lib/src/package.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,9 +396,12 @@ See $workspacesDocUrl for more information.
/// * 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 @@ -461,6 +464,40 @@ 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>{
// By adding this to visited we will never go above the workspaceRoot.dir.
p.canonicalize(root.dir),
};
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 we meet another package directory.
if (includedFrom.containsKey(p.canonicalize(dir))) {
break;
}
if (!visited.add(p.canonicalize(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-stray-files for details.
''');
}
}
Expand Down
110 changes: 92 additions & 18 deletions test/workspace_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,40 @@ 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 `.${s}pkgs${s}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 All @@ -825,34 +858,75 @@ foo:foomain''',
'a',
[
libPubspec('a', '1.1.1', resolutionWorkspace: true),
dir('test_data', []),
],
),
]),
]).create();
// Directories outside the workspace should not be affected.
final outideWorkpace = sandbox;
// Directories of worksace packages should be cleaned.
final aDir = p.join(sandbox, appPath, 'pkgs', 'a');
// Directories between workspace root and workspace packages should
// be cleaned.
final pkgsDir = p.join(sandbox, appPath, 'pkgs');
final strayLockFile = File(p.join(aDir, 'pubspec.lock'));
final strayPackageConfig =
File(p.join(aDir, '.dart_tool', 'package_config.json'));
// Directories inside a workspace package should not be cleaned.
final inside = p.join(aDir, 'test_data');

final unmanagedLockFile = File(p.join(pkgsDir, 'pubspec.lock'));
final unmanagedPackageConfig =
File(p.join(pkgsDir, '.dart_tool', 'package_config.json'));
strayPackageConfig.createSync(recursive: true);
strayLockFile.createSync(recursive: true);
void createLockFileAndPackageConfig(String dir) {
File(p.join(dir, 'pubspec.lock')).createSync(recursive: true);
File(p.join(dir, '.dart_tool', 'package_config.json'))
.createSync(recursive: true);
}

unmanagedPackageConfig.createSync(recursive: true);
unmanagedLockFile.createSync(recursive: true);
void validateLockFileAndPackageConfig(
String dir,
FileSystemEntityType state,
) {
expect(
File(p.join(dir, 'pubspec.lock')).statSync().type,
state,
);
expect(
File(p.join(dir, '.dart_tool', 'package_config.json')).statSync().type,
state,
);
}

await pubGet(environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'});
createLockFileAndPackageConfig(sandbox);
createLockFileAndPackageConfig(aDir);
createLockFileAndPackageConfig(pkgsDir);
createLockFileAndPackageConfig(inside);

expect(strayLockFile.statSync().type, FileSystemEntityType.notFound);
expect(strayPackageConfig.statSync().type, FileSystemEntityType.notFound);
await pubGet(
environment: {'_PUB_TEST_SDK_VERSION': '3.5.0'},
warning: allOf(
contains('Deleting old lock-file: `.${s}pkgs/a${s}pubspec.lock'),
contains(
'Deleting old package config: '
'`.${s}pkgs/a$s.dart_tool${s}package_config.json`',
),
contains('Deleting old lock-file: `.${s}pkgs${s}pubspec.lock'),
contains(
'Deleting old package config: '
'`.${s}pkgs$s.dart_tool${s}package_config.json`',
),
contains(
'See https://dart.dev/go/workspaces-stray-files for details.',
),
),
);

// We only delete stray files from directories that contain an actual
// package.
expect(unmanagedLockFile.statSync().type, FileSystemEntityType.file);
expect(unmanagedPackageConfig.statSync().type, FileSystemEntityType.file);
validateLockFileAndPackageConfig(
outideWorkpace,
FileSystemEntityType.file,
);
validateLockFileAndPackageConfig(aDir, FileSystemEntityType.notFound);
validateLockFileAndPackageConfig(pkgsDir, FileSystemEntityType.notFound);
validateLockFileAndPackageConfig(
inside,
FileSystemEntityType.file,
);
});

test('Reports error if workspace doesn\'t form a tree.', () async {
Expand Down

0 comments on commit 5281207

Please sign in to comment.