Skip to content
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
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ Without a `WorkingDirectory` the process inherits the current directory of the c

The type is `AbsoluteDirectoryPath` rather than a string on purpose. A relative directory would have to be resolved against the caller's current directory — the process-global state this option exists to avoid depending on, since it is shared by every thread and races with concurrent calls.

A relative executable path that names a directory, such as `./build.sh` or `tools/build.sh`, is resolved against `WorkingDirectory`, the way `cd repo && ./build.sh` works in a shell. Without that, the process would start whichever `build.sh` sits in the caller's current directory. A bare name such as `git` or `dotnet` has no directory part, so it is still found through the usual search, and an absolute path is used as given.

### Environment Variables

`EnvironmentVariables` is an overlay on the inherited environment, not a replacement: a name you do not list keeps whatever the calling process had. A `null` value removes a variable, which is how you unset something the parent had set:
Expand Down
50 changes: 50 additions & 0 deletions RunCommand.Test/RunCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,56 @@
"Expected the process to start in the directory it was given.");
}

[TestMethod]
[DataRow("")]
[DataRow("sub")]
public async Task ExecuteAsyncShouldResolveARelativeExecutablePathAgainstTheWorkingDirectory(string subdirectory)
{
// A same-named script sits in the caller's current directory too, so running the wrong copy
// shows up in the output rather than as a start failure
bool isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
string scriptName = $"{nameof(RunCommandTests)}-{Guid.NewGuid():N}{(isWindows ? ".cmd" : ".sh")}";
string relativePath = string.IsNullOrEmpty(subdirectory) ? $".{Path.DirectorySeparatorChar}{scriptName}" : $"{subdirectory}/{scriptName}";

string workingDirectory = CreateDirectoryForTest(nameof(ExecuteAsyncShouldResolveARelativeExecutablePathAgainstTheWorkingDirectory) + subdirectory);
string intendedScript = Path.Join(workingDirectory, subdirectory, scriptName);
string callerScript = Path.Join(Environment.CurrentDirectory, subdirectory, scriptName);
_ = Directory.CreateDirectory(Path.GetDirectoryName(intendedScript)!);
_ = Directory.CreateDirectory(Path.GetDirectoryName(callerScript)!);
WriteScript(intendedScript, "working-directory");
WriteScript(callerScript, "caller");

try
{
List<string> output = [];
int exitCode = await RunCommand.ExecuteAsync(
relativePath,
[],
new LineOutputHandler(onStandardOutput: output.Add),
new CommandOptions { WorkingDirectory = AbsoluteDirectoryPath.Create(workingDirectory) }).ConfigureAwait(false);

Check warning on line 769 in RunCommand.Test/RunCommandTests.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Consider using the overload that accepts a CancellationToken and pass 'TestContext.CancellationToken'

See more on https://sonarcloud.io/project/issues?id=ktsu-dev_RunCommand&issues=AaGUIYJCJDrY7EFXyMfL&open=AaGUIYJCJDrY7EFXyMfL&pullRequest=131

Assert.AreEqual(0, exitCode, "Expected the script to run successfully.");
Assert.AreEqual("working-directory", string.Concat(output).Trim(), "Expected the script in the working directory to run, not the caller's copy.");
}
finally
{
File.Delete(callerScript);
File.Delete(intendedScript);
}
}

private static void WriteScript(string path, string message)
{
if (OperatingSystem.IsWindows())
{
File.WriteAllText(path, $"@echo {message}\r\n");
return;
}

File.WriteAllText(path, $"#!/bin/sh\necho {message}\n");
File.SetUnixFileMode(path, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute);
}

[TestMethod]
public async Task ExecuteAsyncShouldInheritTheCurrentDirectoryWhenNoWorkingDirectoryIsGiven()
{
Expand Down
29 changes: 28 additions & 1 deletion RunCommand/RunCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
/// </summary>
/// <param name="command">The command to execute.</param>
/// <returns>The exit code of the executed process.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 26 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static int Execute(string command) =>
ExecuteAsync(command).Result;
Expand All @@ -34,7 +34,7 @@
/// <param name="command">The command to execute.</param>
/// <param name="outputHandler">The handler for processing command output.</param>
/// <returns>The exit code of the executed process.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 37 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static int Execute(string command, OutputHandler outputHandler) =>
ExecuteAsync(command, outputHandler).Result;
Expand All @@ -45,7 +45,7 @@
/// <param name="command">The command to execute.</param>
/// <param name="elevation">The privilege level under which to run the command.</param>
/// <returns>The exit code of the executed process.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 48 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static int Execute(string command, Elevation elevation) =>
ExecuteAsync(command, elevation).Result;
Expand All @@ -61,7 +61,7 @@
/// </param>
/// <param name="elevation">The privilege level under which to run the command.</param>
/// <returns>The exit code of the executed process.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 64 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static int Execute(string command, OutputHandler outputHandler, Elevation elevation) =>
ExecuteAsync(command, outputHandler, elevation).Result;
Expand Down Expand Up @@ -108,7 +108,7 @@
/// </summary>
/// <param name="command">The command to execute.</param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 111 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command)
=> await ExecuteAsync(command, new OutputHandler()).ConfigureAwait(false);
Expand All @@ -119,7 +119,7 @@
/// <param name="command">The command to execute.</param>
/// <param name="outputHandler">The handler for processing command output.</param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 122 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command, OutputHandler outputHandler)
=> await ExecuteAsync(command, outputHandler, Elevation.Default).ConfigureAwait(false);
Expand All @@ -130,7 +130,7 @@
/// <param name="command">The command to execute.</param>
/// <param name="elevation">The privilege level under which to run the command.</param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 133 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command, Elevation elevation)
=> await ExecuteAsync(command, new(), elevation).ConfigureAwait(false);
Expand All @@ -146,7 +146,7 @@
/// </param>
/// <param name="elevation">The privilege level under which to run the command.</param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 149 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command, OutputHandler outputHandler, Elevation elevation)
=> await ExecuteAsync(command, outputHandler, elevation, CancellationToken.None).ConfigureAwait(false);
Expand All @@ -159,7 +159,7 @@
/// A token that, when cancelled, terminates the running process and its children.
/// </param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 162 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command, CancellationToken cancellationToken)
=> await ExecuteAsync(command, new OutputHandler(), cancellationToken).ConfigureAwait(false);
Expand All @@ -174,7 +174,7 @@
/// A token that, when cancelled, terminates the running process and its children.
/// </param>
/// <returns>A task representing the asynchronous operation with the process exit code.</returns>
[Obsolete("A command string is split on its first space, which cannot handle an executable path "

Check warning on line 177 in RunCommand/RunCommand.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Do not forget to remove this deprecated code someday.
+ "containing spaces. Use the overload taking a file name and an argument list instead.")]
public static async Task<int> ExecuteAsync(string command, OutputHandler outputHandler, CancellationToken cancellationToken)
=> await ExecuteAsync(command, outputHandler, Elevation.Default, cancellationToken).ConfigureAwait(false);
Expand Down Expand Up @@ -316,6 +316,33 @@
return await RunAsync(startInfo, outputHandler, useElevation, cancellationToken).ConfigureAwait(false);
}

/// <summary>
/// Resolves a relative executable path that names a directory, such as <c>./build.sh</c> or
/// <c>tools/build.sh</c>, against the working directory the command runs in.
/// </summary>
/// <param name="fileName">The executable as the caller gave it.</param>
/// <param name="workingDirectory">The command's working directory, or <see langword="null"/> if none was set.</param>
/// <param name="isWindows">Whether a backslash also separates directories.</param>
/// <returns>The path to start.</returns>
/// <remarks>
/// Process.Start resolves a relative path against the calling process's current directory, not
/// against <see cref="ProcessStartInfo.WorkingDirectory"/>, so without this <c>./build.sh</c> ran
/// the caller's copy, or failed to start, whatever <see cref="CommandOptions.WorkingDirectory"/>
/// said. Resolving it here does what <c>cd dir &amp;&amp; ./build.sh</c> does in a shell. A bare name
/// such as <c>git</c> has no directory part and keeps the usual search, and an absolute path is
/// already unambiguous.
/// </remarks>
private static string ResolveFileName(string fileName, string? workingDirectory, bool isWindows)
{
if (workingDirectory is null || Path.IsPathRooted(fileName))
{
return fileName;
}

bool hasDirectoryPart = fileName.Contains('/') || (isWindows && fileName.Contains('\\'));
return hasDirectoryPart ? Path.GetFullPath(Path.Combine(workingDirectory, fileName)) : fileName;
}

private static ProcessStartInfo CreateStartInfo(string fileName, OutputHandler outputHandler, CommandOptions options, out bool useElevation)
{
bool isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
Expand Down Expand Up @@ -345,7 +372,7 @@

ProcessStartInfo startInfo = new()
{
FileName = fileName,
FileName = ResolveFileName(fileName, options.WorkingDirectory?.WeakString, isWindows),
CreateNoWindow = true,
};

Expand Down
Loading