Skip to content

feat: support negated glob patterns in pub workspaces #4757

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.

Sign up for GitHub

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

Open
GiancarloCante wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: support negated glob patterns in pub workspaces #4757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
86 changes: 56 additions & 30 deletions lib/src/package.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,32 +176,57 @@ class Package {
expectedName: expectedName,
);

final workspacePackages =
pubspec.workspace.expand((workspacePath) {
final packages = <Package>[];
var globHint = '';
if (pubspec.languageVersion.supportsWorkspaceGlobs) {
final Glob glob;
try {
glob = Glob(workspacePath);
} on FormatException catch (e) {
fail('Failed to parse glob `$workspacePath`. $e');
}
for (final globResult in glob.listSync(root: dir)) {
final pubspecPath = p.join(globResult.path, 'pubspec.yaml');
if (!fileExists(pubspecPath)) continue;
packages.add(
Package.load(
globResult.path,
loadPubspec: loadPubspec,
withPubspecOverrides: withPubspecOverrides,
),
);
}
} else {
final List<Package> workspacePackages;
if (pubspec.languageVersion.supportsWorkspaceGlobs) {
final packagesByDir = <String, Package>{};
for (final workspacePath in pubspec.workspace) {
final isNegated = _isNegatedGlob(workspacePath);
final pattern = isNegated ? workspacePath.substring(1) : workspacePath;
final Glob glob;
try {
glob = Glob(pattern);
} on FormatException catch (e) {
fail('Failed to parse glob `$workspacePath`. $e');
}
if (isNegated) {
final matchedDirs = <String>{};
for (final globResult in glob.listSync(root: dir)) {
matchedDirs.add(p.canonicalize(globResult.path));
}
packagesByDir.removeWhere(
(canonicalDir, _) => matchedDirs.contains(canonicalDir),
);
} else {
var found = false;
for (final globResult in glob.listSync(root: dir)) {
final pubspecPath = p.join(globResult.path, 'pubspec.yaml');
if (!fileExists(pubspecPath)) continue;
found = true;
final canonicalDir = p.canonicalize(globResult.path);
packagesByDir[canonicalDir] ??= Package.load(
globResult.path,
loadPubspec: loadPubspec,
withPubspecOverrides: withPubspecOverrides,
);
}
if (!found) {
fail('''
No workspace packages matching `$workspacePath`.
That was included in the workspace of `${p.join(dir, 'pubspec.yaml')}`.
''');
}
}
}
workspacePackages = packagesByDir.values.toList();
} else {
workspacePackages =
pubspec.workspace.expand((workspacePath) {
final packages = <Package>[];
var globHint = '';
final pubspecPath = p.join(dir, workspacePath, 'pubspec.yaml');
if (!fileExists(pubspecPath)) {
if (_looksLikeGlob(workspacePath)) {
if (_looksLikeGlob(workspacePath) ||
_isNegatedGlob(workspacePath)) {
globHint = '''
\n\nGlob syntax is only supported from language version ${LanguageVersion.firstVersionWithWorkspaceGlobs}.
Consider changing the language version of ${p.join(dir, 'pubspec.yaml')} to ${LanguageVersion.firstVersionWithWorkspaceGlobs}.
Expand All @@ -216,15 +241,15 @@ Consider changing the language version of ${p.join(dir, 'pubspec.yaml')} to ${La
),
);
}
}
if (packages.isEmpty) {
fail('''
if (packages.isEmpty) {
fail('''
No workspace packages matching `$workspacePath`.
That was included in the workspace of `${p.join(dir, 'pubspec.yaml')}`.$globHint
''');
}
return packages;
}).toList();
}
return packages;
}).toList();
}
for (final package in workspacePackages) {
if (package.pubspec.resolution != Resolution.workspace) {
fail('''
Expand Down Expand Up @@ -583,6 +608,7 @@ See https://dart.dev/go/workspaces-stray-files for details.
}

bool _looksLikeGlob(String s) => Glob.quote(s) != s;
bool _isNegatedGlob(String s) => s.startsWith('!');
String _useBackSlashesOnWindows(String path) {
if (Platform.isWindows) {
return p.joinAll(p.split(path));
Expand Down
8 changes: 6 additions & 2 deletions lib/src/pubspec.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,14 @@ environment:
if (value is! String) {
_error('"workspace" must be a list of strings', t.span);
}
if (!p.isRelative(value)) {
final valueToValidate =
languageVersion.supportsWorkspaceGlobs && value.startsWith('!')
? value.substring(1)
: value;
if (!p.isRelative(valueToValidate)) {
_error('"workspace" members must be relative paths', t.span);
}
if (p.equals(value, '.') || !p.isWithin('.', value)) {
if (p.equals(valueToValidate, '.') || !p.isWithin('.', valueToValidate)) {
_error('"workspace" members must be subdirectories', t.span);
}
result.add(value);
Expand Down
193 changes: 193 additions & 0 deletions test/workspace_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,64 @@ Packages can only be included in the workspace once.
},
);

test('Reports a failure if a negated workspace pubspec is not nested '
'inside the parent dir', () async {
await dir(appPath, [
libPubspec(
'myapp',
'1.2.3',
sdk: '^3.11.0',
extras: {
'workspace': ['!../'],
},
),
]).create();
await pubGet(
environment: {'_PUB_TEST_SDK_VERSION': '3.11.0'},
error: contains('"workspace" members must be subdirectories'),
exitCode: DATA,
);
});

test('Reports a failure if a negated workspace includes "."', () async {
await dir(appPath, [
libPubspec(
'myapp',
'1.2.3',
sdk: '^3.11.0',
extras: {
'workspace': ['!.'],
},
),
]).create();
await pubGet(
environment: {'_PUB_TEST_SDK_VERSION': '3.11.0'},
error: contains('"workspace" members must be subdirectories'),
exitCode: DATA,
);
});

test(
'Reports a failure if a negated workspace pubspec is not a relative path',
() async {
await dir(appPath, [
libPubspec(
'myapp',
'1.2.3',
sdk: '^3.11.0',
extras: {
'workspace': ['!${p.join(sandbox, appPath, 'a')}'],
},
),
]).create();
await pubGet(
environment: {'_PUB_TEST_SDK_VERSION': '3.11.0'},
error: contains('"workspace" members must be relative paths'),
exitCode: DATA,
);
},
);

test('`upgrade` upgrades all workspace', () async {
final server = await servePackages();
server.serve('foo', '1.0.0');
Expand Down Expand Up @@ -1939,6 +1997,141 @@ Consider changing the language version of .${s}pubspec.yaml to 3.11.'''),
], generatorVersion: '3.11.0'),
]).validate();
});

test('negated glob patterns exclude workspace members', () async {
await dir(appPath, [
libPubspec(
'myapp',
'1.2.3',
extras: {
'workspace': ['pkgs/*', '!pkgs/c'],
},
sdk: '^3.11.0',
),
dir('pkgs', [
dir('a', [libPubspec('a', '1.1.1', resolutionWorkspace: true)]),
dir('b', [libPubspec('b', '1.1.1', resolutionWorkspace: true)]),
dir('c', [libPubspec('c', '0.0.1', resolutionWorkspace: true)]),
]),
]).create();
await pubGet(environment: {'_PUB_TEST_SDK_VERSION': '3.11.0'});
await dir(appPath, [
packageConfigFile([
packageConfigEntry(name: 'myapp', path: '.'),
packageConfigEntry(name: 'a', path: 'pkgs/a'),
packageConfigEntry(name: 'b', path: 'pkgs/b'),
], generatorVersion: '3.11.0'),
]).validate();
});

test(
'negated workspace members can be re-included by later patterns',
() async {
await dir(appPath, [
libPubspec(
'myapp',
'1.2.3',
extras: {
'workspace': ['pkgs/a', '!pkgs/a', 'pkgs/a'],
},
sdk: '^3.11.0',
),
dir('pkgs', [
dir('a', [libPubspec('a', '1.1.1', resolutionWorkspace: true)]),
]),
]).create();
await pubGet(environment: {'_PUB_TEST_SDK_VERSION': '3.11.0'});
await dir(appPath, [
packageConfigFile([
packageConfigEntry(name: 'myapp', path: '.'),
packageConfigEntry(name: 'a', path: 'pkgs/a'),
], generatorVersion: '3.11.0'),
]).validate();
},
);

test(
'negated glob pattern with wildcard excludes multiple packages',
() async {
await dir(appPath, [
libPubspec(
'myapp',
'1.2.3',
extras: {
'workspace': ['pkgs/*', '!pkgs/test_*'],
},
sdk: '^3.11.0',
),
dir('pkgs', [
dir('a', [libPubspec('a', '1.1.1', resolutionWorkspace: true)]),
dir('test_a', [
libPubspec('test_a', '0.0.1', resolutionWorkspace: true),
]),
dir('test_b', [
libPubspec('test_b', '0.0.1', resolutionWorkspace: true),
]),
]),
]).create();
await pubGet(environment: {'_PUB_TEST_SDK_VERSION': '3.11.0'});
await dir(appPath, [
packageConfigFile([
packageConfigEntry(name: 'myapp', path: '.'),
packageConfigEntry(name: 'a', path: 'pkgs/a'),
], generatorVersion: '3.11.0'),
]).validate();
},
);

test('negated pattern that does not match anything is a no-op', () async {
await dir(appPath, [
libPubspec(
'myapp',
'1.2.3',
extras: {
'workspace': ['pkgs/a', '!pkgs/non_existent'],
},
sdk: '^3.11.0',
),
dir('pkgs', [
dir('a', [libPubspec('a', '1.1.1', resolutionWorkspace: true)]),
]),
]).create();
await pubGet(environment: {'_PUB_TEST_SDK_VERSION': '3.11.0'});
await dir(appPath, [
packageConfigFile([
packageConfigEntry(name: 'myapp', path: '.'),
packageConfigEntry(name: 'a', path: 'pkgs/a'),
], generatorVersion: '3.11.0'),
]).validate();
});

test(
'negated patterns in older language versions are treated as literal paths',
() async {
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', resolutionWorkspace: true)]),
]),
]).create();
await pubGet(
environment: {'_PUB_TEST_SDK_VERSION': '3.11.0'},
error: contains('''
No workspace packages matching `!pkgs/a`.
That was included in the workspace of `.${s}pubspec.yaml`.

Glob syntax is only supported from language version 3.11.
Consider changing the language version of .${s}pubspec.yaml to 3.11.'''),
);
},
);
}

final s = p.separator;