Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 4 additions & 0 deletions pkgs/hooks_runner/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 1.6.2-wip

- Fix running a relative executable path with a `workingDirectory` on Windows.

## 1.6.1

- Support versions 3.x of `package:package_config`.
Expand Down
38 changes: 37 additions & 1 deletion pkgs/hooks_runner/lib/src/utils/run_process.dart
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,18 @@ Future<RunProcessResult> runProcess({
try {
final stdoutBuffer = StringBuffer();
final stderrBuffer = StringBuffer();
// On Windows, `CreateProcess` (used when `runInShell` is `false`) ignores
// `workingDirectory` when resolving [executable]. Relative paths that
// contain a directory component must be made absolute against
// [workingDirectory] first; otherwise Process.start fails with
// "The system cannot find the file specified".
final executablePath = _resolveExecutablePath(
filesystem: filesystem,
executable: executable,
workingDirectory: workingDirectory,
);
final process = await Process.start(
executable.toFilePath(),
executablePath,
arguments,
workingDirectory: workingDirectory?.toFilePath(),
environment: environment,
Expand Down Expand Up @@ -128,6 +138,32 @@ Future<RunProcessResult> runProcess({
}
}

/// Resolves [executable] against [workingDirectory] when needed.
///
/// On Windows, [Process.start] does not resolve a relative [executable] against
/// [workingDirectory] (unlike POSIX, where the directory is changed before
/// exec). Relative paths that include a directory component are therefore made
/// absolute here. Bare command names (no separators) are left unchanged so
/// `PATH` / `PATHEXT` lookup still works.
String _resolveExecutablePath({
required FileSystem filesystem,
required Uri executable,
required Uri? workingDirectory,
}) {
final executablePath = executable.toFilePath();
if (workingDirectory == null) return executablePath;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also early return if platform is not Windows?


final context = filesystem.path;
if (context.isAbsolute(executablePath)) return executablePath;
// Bare command name — leave for PATH / PATHEXT resolution.
if (!executablePath.contains(r'\') && !executablePath.contains('/')) {
return executablePath;
}
return context.normalize(
context.join(workingDirectory.toFilePath(), executablePath),
);
}

/// Whether [executable] must be launched through `cmd.exe` on Windows.
///
/// `CreateProcess` can only directly launch `.exe`/`.com` binaries. Anything
Expand Down
2 changes: 1 addition & 1 deletion pkgs/hooks_runner/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: hooks_runner
description: >-
This package is the backend that invokes build hooks.

version: 1.6.1
version: 1.6.2-wip

repository: https://github.com/dart-lang/native/tree/main/pkgs/hooks_runner

Expand Down
120 changes: 120 additions & 0 deletions pkgs/hooks_runner/test/utils/run_process_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,96 @@ void main(List<String> args) {
expect(result.stdout, contains('ARGV:first arg|second arg'));
});

test(
'runProcess runs a relative executable path against workingDirectory',
timeout: const Timeout.factor(5),
() async {
// Regression: on Windows, CreateProcess ignores workingDirectory when
// resolving the executable, so relative paths like
// `build\cli\...\app.exe` failed after runInShell was disabled for .exe.
final projectDir = await tempDirForTest(useSpacesInPath: false);
final outDir = Directory.fromUri(
projectDir.resolve('build/cli/bundle/bin/'),
);
await outDir.create(recursive: true);
final exeUri = outDir.uri.resolve(
'echo_args${Platform.isWindows ? '.exe' : ''}',
);
final compileResult = await Process.run(Platform.resolvedExecutable, [
'compile',
'exe',
scriptUri.toFilePath(),
'-o',
exeUri.toFilePath(),
]);
expect(compileResult.exitCode, 0, reason: '${compileResult.stderr}');

final relativeExe = [
'build',
'cli',
'bundle',
'bin',
'echo_args${Platform.isWindows ? '.exe' : ''}',
].join(Platform.pathSeparator);
final result = await runProcess(
executable: Uri.file(relativeExe),
arguments: ['relative-ok'],
workingDirectory: projectDir,
logger: logger,
);

expect(result.exitCode, 0);
expect(result.stdout, contains('ARGC:1'));
expect(result.stdout, contains('ARGV:relative-ok'));
},
);

test(
'runProcess runs a relative executable path against a workingDirectory '
'containing a space',
timeout: const Timeout.factor(5),
() async {
// The resolved absolute path contains a space, so this exercises the
// relative-path resolution combined with `CreateProcess` command-line
// quoting (the original bug scenario under
// `C:\Users\First Last\...`).
final projectDir = await tempDirForTest();
final outDir = Directory.fromUri(
projectDir.resolve('build/cli/bundle/bin/'),
);
await outDir.create(recursive: true);
final exeUri = outDir.uri.resolve(
'echo_args${Platform.isWindows ? '.exe' : ''}',
);
final compileResult = await Process.run(Platform.resolvedExecutable, [
'compile',
'exe',
scriptUri.toFilePath(),
'-o',
exeUri.toFilePath(),
]);
expect(compileResult.exitCode, 0, reason: '${compileResult.stderr}');

final relativeExe = [
'build',
'cli',
'bundle',
'bin',
'echo_args${Platform.isWindows ? '.exe' : ''}',
].join(Platform.pathSeparator);
final result = await runProcess(
executable: Uri.file(relativeExe),
arguments: ['an argument with spaces'],
workingDirectory: projectDir,
logger: logger,
);

expect(result.exitCode, 0);
expect(result.stdout, contains('ARGC:1'));
expect(result.stdout, contains('ARGV:an argument with spaces'));
},
);

test(
'runProcess runs bare .bat shim from working directory on Windows',
() async {
Expand All @@ -101,6 +191,36 @@ void main(List<String> args) {
},
);

test(
'runProcess runs a relative .bat path against workingDirectory on Windows',
() async {
if (!Platform.isWindows) return;

// A relative path with a directory component is made absolute against
// workingDirectory *and* still launched through `cmd.exe` (a `.bat`
// cannot be started by `CreateProcess` directly). The temp dir contains
// a space, so the absolutized path is a quoted token on the cmd line.
final projectDir = await tempDirForTest();
final binDir = Directory.fromUri(projectDir.resolve('bin/'));
await binDir.create(recursive: true);
final batUri = binDir.uri.resolve('shim.bat');
await File.fromUri(batUri).writeAsString(
'@echo off\r\necho SHIM_OK %*\r\n',
);

final result = await runProcess(
executable: Uri.file(r'bin\shim.bat'),
arguments: ['--help'],
workingDirectory: projectDir,
logger: logger,
);

expect(result.exitCode, 0);
expect(result.stdout, contains('SHIM_OK'));
expect(result.stdout, contains('--help'));
},
);

test(
'runProcess runs bare .bat shim from PATH on Windows',
() async {
Expand Down
Loading