Skip to content

fix: complete Windows compatibility for glob and subprocess tests #5

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

Merged
tsavo-at-pieces merged 5 commits into from
Feb 25, 2026
Merged

fix: complete Windows compatibility for glob and subprocess tests #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6ea9830
fix: complete Windows compatibility for glob and subprocess tests
tsavo-at-pieces Feb 25, 2026
451c81d
fix: handle escaped Windows absolute glob patterns
tsavo-at-pieces Feb 25, 2026
2ff616c
fix: normalize absolute glob output in CliArguments
tsavo-at-pieces Feb 25, 2026
8ffb6c9
test: add deep Windows path and environment regression coverage
tsavo-at-pieces Feb 25, 2026
ad0ad96
fix: align new Windows tests with real runner behavior
tsavo-at-pieces Feb 25, 2026
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
25 changes: 23 additions & 2 deletions lib/cli_script.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,28 @@ Script xargs(
/// The glob syntax is the same as that provided by the [Glob] package.
///
/// If [root] is passed, it's used as the root directory for relative globs.
bool _isLikelyWindowsAbsolutePathGlob(String pattern) {
if (!Platform.isWindows || pattern.isEmpty) return false;
if (RegExp(r'^[A-Za-z](\\)?:[\\/]').hasMatch(pattern)) return true;
if (RegExp(r'^[A-Za-z]:[\\/]').hasMatch(pattern)) return true;
if (pattern.startsWith(r'\\') || pattern.startsWith('//')) return true;
const escapedGlobChars = '*?[]{}(),-\\';
if (pattern.startsWith('\\') && pattern.length > 1) {
return !escapedGlobChars.contains(pattern[1]);
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function _isLikelyWindowsAbsolutePathGlob is duplicated in both lib/cli_script.dart and lib/src/cli_arguments.dart with slightly different implementations. This violates the DRY (Don't Repeat Yourself) principle and creates a maintenance burden. The two implementations differ in how they check for escaped glob characters: this file uses string-based comparison with pattern[1], while cli_arguments.dart uses code unit comparison with pattern.codeUnitAt(1). Consider extracting this function to a shared utility module to ensure consistent behavior and easier maintenance.

Suggested change
return !escapedGlobChars.contains(pattern[1]);
return !escapedGlobChars.codeUnits.contains(pattern.codeUnitAt(1));

Copilot uses AI. Check for mistakes.
}
return false;
}

String _normalizeWindowsGlobPattern(String pattern) {
if (!_isLikelyWindowsAbsolutePathGlob(pattern)) return pattern;
final normalizedDrivePrefix = pattern.replaceFirstMapped(RegExp(r'^([A-Za-z])\\:'), (match) => '${match[1]}:');
return normalizedDrivePrefix.replaceAll('\\', '/');
}

Stream<String> ls(String glob, {String? root}) {
final absolute = p.isAbsolute(glob);
return Glob(glob).list(root: root).map((entity) => absolute ? entity.path : p.relative(entity.path, from: root));
final absolute = Platform.isWindows ? _isLikelyWindowsAbsolutePathGlob(glob) : p.isAbsolute(glob);
final normalizedGlob = _normalizeWindowsGlobPattern(glob);
return Glob(
normalizedGlob,
).list(root: root).map((entity) => absolute ? p.normalize(entity.path) : p.relative(entity.path, from: root));
}
67 changes: 63 additions & 4 deletions lib/src/cli_arguments.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,53 @@ class CliArguments {
while (scanner.scanChar($space)) {}
}

/// Glob characters that can be escaped with a backslash.
static const _globEscapedChars = {
$asterisk,
$question,
$lbracket,
$rbracket,
$lbrace,
$rbrace,
$lparen,
$rparen,
$comma,
$dash,
$backslash,
};

/// Returns whether [pattern] is likely an absolute Windows path glob.
///
/// We only normalize absolute Windows-style paths so we don't corrupt
/// shell-style escaped glob characters like `\\*.txt`.
static bool _isLikelyWindowsAbsolutePathGlob(String pattern) {
if (!Platform.isWindows || pattern.isEmpty) return false;
if (RegExp(r'^[A-Za-z](\\)?:[\\/]').hasMatch(pattern)) return true;
if (RegExp(r'^[A-Za-z]:[\\/]').hasMatch(pattern)) return true;
if (pattern.startsWith(r'\\') || pattern.startsWith('//')) return true;
if (pattern.codeUnitAt(0) == $backslash && pattern.length > 1) {
return !_globEscapedChars.contains(pattern.codeUnitAt(1));
}
return false;
}

/// Normalizes a glob pattern to POSIX separators for the glob package.
///
/// `package:glob` expects `/` as path separators on all platforms.
static String _normalizeGlobPattern(String pattern) {
if (!_isLikelyWindowsAbsolutePathGlob(pattern)) return pattern;
// Glob.quote() may escape `C:` as `C\:`, so normalize that first.
final normalizedDrivePrefix = pattern.replaceFirstMapped(RegExp(r'^([A-Za-z])\\:'), (match) => '${match[1]}:');
Copy link

@sentry sentry bot Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The regex r'^([A-Za-z])\\:' incorrectly looks for a double backslash after a Windows drive letter, but Glob.quote() only produces a single backslash, causing the pattern to not match.
Severity: HIGH

Suggested Fix

Change the regular expression from r'^([A-Za-z])\\:' to r'^([A-Za-z])\:'. This will correctly match the single backslash produced by Glob.quote() after a Windows drive letter, allowing path normalization to proceed as intended.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: lib/src/cli_arguments.dart#L103

Potential issue: The regular expression `r'^([A-Za-z])\\:'` is used to normalize Windows
paths that have been escaped by `Glob.quote()`. In a Dart raw string, `\\` matches two
literal backslashes. However, `Glob.quote()` escapes a drive letter like `C:` to `C\:`,
which contains only a single backslash. Because the regex pattern requires two
backslashes, it will fail to match the input. This prevents the path from being
normalized, leaving backslashes instead of forward slashes. Consequently, the `Glob()`
package, which expects forward slashes, will fail to expand the pattern and find any
files, breaking glob matching for absolute paths on Windows.

return normalizedDrivePrefix.replaceAll('\\', '/');
}

static bool _containsGlobSyntax(String pattern) =>
pattern.contains('*') ||
pattern.contains('?') ||
pattern.contains('[') ||
pattern.contains('{') ||
pattern.contains('(');

/// Scans a single argument.
static _Argument _scanArg(StringScanner scanner, {required bool glob}) {
final plainBuffer = StringBuffer();
Expand All @@ -72,8 +119,17 @@ class CliArguments {
while (true) {
final next = scanner.peekChar();
if (next == $space || next == null) {
final glob = isGlobActive ? globBuffer?.toString() : null;
return _Argument(plainBuffer.toString(), glob == null ? null : Glob(glob));
final rawGlob = globBuffer?.toString();
final normalizedGlob = rawGlob == null ? null : _normalizeGlobPattern(rawGlob);
final windowsAbsoluteGlob =
glob &&
!isGlobActive &&
rawGlob != null &&
_isLikelyWindowsAbsolutePathGlob(rawGlob) &&
normalizedGlob != null &&
_containsGlobSyntax(normalizedGlob);
final globPattern = (isGlobActive || windowsAbsoluteGlob) ? normalizedGlob : null;
return _Argument(plainBuffer.toString(), globPattern == null ? null : Glob(globPattern));
} else if (next == $double_quote || next == $single_quote) {
scanner.readChar();

This comment was marked as outdated.

Sign in to view


Expand Down Expand Up @@ -157,9 +213,12 @@ class _Argument {
Future<List<String>> resolve({String? root}) async {
final glob = _glob;
if (glob != null) {
final absolute = p.isAbsolute(glob.pattern);
final absolute = Platform.isWindows
? CliArguments._isLikelyWindowsAbsolutePathGlob(glob.pattern)
: p.isAbsolute(glob.pattern);
final globbed = [
await for (final entity in glob.list(root: root)) absolute ? entity.path : p.relative(entity.path, from: root),
await for (final entity in glob.list(root: root))
absolute ? p.normalize(entity.path) : p.relative(entity.path, from: root),
];
if (globbed.isNotEmpty) return globbed;
}
Expand Down
20 changes: 20 additions & 0 deletions lib/src/script.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,23 @@ class Script {
: withEnv(() => env, environment!);
}

if (!includeParentEnvironment) {
// Pass an explicit map rather than null so subprocesses don't
// implicitly inherit parent environment variables.
environment ??= <String, String>{};

if (Platform.isWindows) {
// Some Windows executables (including Dart in certain contexts)
// require a minimal base environment to spawn reliably.
final windowsBase = <String, String>{};
final systemRoot = Platform.environment['SystemRoot'] ?? Platform.environment['SYSTEMROOT'];
final winDir = Platform.environment['WINDIR'];
if (systemRoot != null && systemRoot.isNotEmpty) windowsBase['SystemRoot'] = systemRoot;
if (winDir != null && winDir.isNotEmpty) windowsBase['WINDIR'] = winDir;
environment = {...windowsBase, ...environment!};
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spread operator creates a plain Map rather than preserving the CanonicalizedMap type that handles case-insensitive keys on Windows. If environment contains keys like 'SYSTEMROOT' or 'systemroot' and windowsBase contains 'SystemRoot', the resulting map could have duplicate entries with different casing. Consider using _newMap() from environment.dart or explicitly creating a CanonicalizedMap, then adding entries from both maps to ensure case-insensitive key handling is preserved on Windows.

Suggested change
environment = {...windowsBase, ...environment!};
// Merge the minimal Windows base environment into the existing map
// without replacing it, so any specialized map implementation
// (such as a case-insensitive CanonicalizedMap) is preserved and
// user-provided values continue to override the base.
for (final entry in windowsBase.entries) {
environment!.putIfAbsent(entry.key, () => entry.value);
}

Copilot uses AI. Check for mistakes.
}
}

final allArgs = [...await parsedExecutableAndArgs.arguments(root: workingDirectory), ...?args];

if (inDebugMode) {
Expand Down Expand Up @@ -733,6 +750,9 @@ class Script {
///
/// Like `collectBytes(stdout)`, but throws a [ScriptException] if the
/// executable returns a non-zero exit code.
///
/// These are the raw bytes emitted by the subprocess. Line endings are
/// platform-native (for example, `\r\n` on Windows and `\n` on Unix).
Future<Uint8List> get outputBytes async {
final result = await collectBytes(stdout);
await done;
Expand Down
83 changes: 81 additions & 2 deletions test/cli_arguments_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import 'dart:io';

import 'package:cli_script/src/cli_arguments.dart';
import 'package:glob/glob.dart';
import 'package:path/path.dart' as p;
import 'package:test/test.dart';
import 'package:test_descriptor/test_descriptor.dart' as d;

Expand Down Expand Up @@ -150,7 +151,8 @@ void main() {
await d.file('bar.txt').create();
await d.file('baz.zip').create();

final pattern = p.join(Glob.quote(d.sandbox), '*.txt');
final base = d.sandbox.replaceAll(Platform.pathSeparator, '/');
final pattern = '${Glob.quote(base)}/*.txt';
final args = await _resolve('ls $pattern', glob: glob);
expect(args.first, equals('ls'));
expect(args.sublist(1), unorderedEquals([d.path('foo.txt'), d.path('bar.txt')]));
Expand All @@ -173,8 +175,85 @@ void main() {
test("returns plain strings for globs that don't match", () async {
expect(await _resolve('ls *.txt', glob: glob), equals(['ls', '*.txt']));
});

test('absolute glob output uses platform path separators', () async {
await d.file('foo.txt').create();
await d.file('bar.txt').create();

final base = d.sandbox.replaceAll(Platform.pathSeparator, '/');
final pattern = '${Glob.quote(base)}/*.txt';
final args = await _resolve('ls $pattern', glob: glob);
expect(args.first, equals('ls'));
for (final path in args.sublist(1)) {
expect(path, matches(Platform.isWindows ? RegExp(r'^(?:[A-Za-z]:[\\/]|\\\\|//|[\\/])') : RegExp(r'^/')));
}
expect(args.sublist(1), unorderedEquals([d.path('foo.txt'), d.path('bar.txt')]));
});
});

group('Windows-specific glob patterns', () {
group('UNC-style absolute glob', () {
test('throws PathNotFoundException when UNC path does not exist', () {
const uncPattern = r'\\nonexistent\share\*.txt';
expect(_resolve('ls $uncPattern', glob: true), throwsA(isA<PathNotFoundException>()));
}, testOn: 'windows');

test('//server/share form throws PathNotFoundException when no match', () {
const uncPattern = '//server/share/*.txt';
expect(_resolve('ls $uncPattern', glob: true), throwsA(isA<PathNotFoundException>()));
}, testOn: 'windows');
});

group('drive-relative glob pattern', () {
test('C:foo\\*.txt backslash escapes asterisk, resolves to C:foo*.txt', () async {
await d.file('foo.txt').create();
const pattern = r'C:foo\*.txt';
final args = await _resolve('ls $pattern', glob: true);
expect(args.first, equals('ls'));
// Backslash consumed as escape; glob matches literal *.txt (none); fallback to plain.
expect(args.sublist(1), equals(['C:foo*.txt']));
}, testOn: 'windows');

test('C:foo/*.txt returns plain pattern when no match', () async {
await d.file('foo.txt').create();
const pattern = 'C:foo/*.txt';
final args = await _resolve('ls $pattern', glob: true);
expect(args.first, equals('ls'));
expect(args.sublist(1), equals([pattern]));
}, testOn: 'windows');
});

group('Glob.quote-style drive prefix escaping', () {
test('C\\:\\path\\*.txt pattern normalizes and expands correctly', () async {
await d.file('foo.txt').create();
await d.file('bar.txt').create();

final quotedBase = Glob.quote(d.sandbox);
final pattern = '$quotedBase/*.txt';
final args = await _resolve('ls $pattern', glob: true);
expect(args.first, equals('ls'));
expect(args.sublist(1), unorderedEquals([d.path('foo.txt'), d.path('bar.txt')]));
}, testOn: 'windows');

test('quoted Windows absolute path with glob expands to file match', () async {
await d.file('foo.txt').create();
await d.file('bar.txt').create();
final quotedPath = '"${d.sandbox.replaceAll(r'\', r'\\')}\\*.txt"';
final args = await _resolve('ls $quotedPath', glob: true);
expect(args.first, equals('ls'));
expect(args.sublist(1), unorderedEquals([d.path('foo.txt'), d.path('bar.txt')]));
}, testOn: 'windows');
});

test('UNC path with glob: false — backslashes consumed as escapes', () async {
const uncPattern = r'\\server\share\*.txt';
final args = await _resolve('ls $uncPattern', glob: false);
expect(args.first, equals('ls'));
// Parser treats \ as escape: \\→\, \s→s, \*→*; result is \servershare*.txt
expect(args.sublist(1), equals([r'\servershare*.txt']));
});
}, testOn: 'windows');

onWindowsOrWithGlobFalse((glob) {
test('ignores glob characters', () async {
await d.file('foo.txt').create();
Expand Down
Loading
Loading