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

Delete directories between workspace package and workspace root #4446

Merged
merged 5 commits into from
Dec 2, 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
34 changes: 32 additions & 2 deletions lib/src/entrypoint.dart
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
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,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
14 changes: 7 additions & 7 deletions pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,23 @@ packages:
dependency: transitive
description:
name: _fe_analyzer_shared
sha256: "45cfa8471b89fb6643fe9bf51bd7931a76b8f5ec2d65de4fb176dba8d4f22c77"
sha256: "16e298750b6d0af7ce8a3ba7c18c69c3785d11b15ec83f6dcd0ad2a0009b3cab"
url: "https://pub.dev"
source: hosted
version: "73.0.0"
version: "76.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: "1f14db053a8c23e260789e9b0980fa27f2680dd640932cae5e1137cce0e46e1e"
url: "https://pub.dev"
source: hosted
version: "6.8.0"
version: "6.11.0"
args:
dependency: "direct main"
description:
Expand Down Expand Up @@ -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:
Expand Down
110 changes: 92 additions & 18 deletions test/workspace_test.dart
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