From 4f3f95074cdd20f1106e8bf562d2dfbe538ffeda Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 13:33:47 +0000 Subject: [PATCH 01/16] Initial plan From 92de2998c9470c6d5da14ea519f77dd5fe438465 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 13:42:28 +0000 Subject: [PATCH 02/16] Add ProcessStartOptions class with path resolution and tests Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Kernel32/Interop.GetWindowsDirectoryW.cs | 13 + .../ref/System.Diagnostics.Process.cs | 11 + .../src/System.Diagnostics.Process.csproj | 6 + .../System/Diagnostics/ProcessStartOptions.cs | 348 ++++++++++++++++++ .../tests/ProcessStartOptionsTests.Unix.cs | 203 ++++++++++ .../tests/ProcessStartOptionsTests.Windows.cs | 200 ++++++++++ .../tests/ProcessStartOptionsTests.cs | 224 +++++++++++ .../System.Diagnostics.Process.Tests.csproj | 3 + 8 files changed, 1008 insertions(+) create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs create mode 100644 src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs create mode 100644 src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs create mode 100644 src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs create mode 100644 src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs new file mode 100644 index 00000000000000..05710a47a019f9 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetWindowsDirectoryW.cs @@ -0,0 +1,13 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + [LibraryImport(Libraries.Kernel32, SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] + internal static partial uint GetWindowsDirectoryW(ref char lpBuffer, uint uSize); + } +} diff --git a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs index 17366ae9811736..c6b6565016ea69 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -261,6 +261,17 @@ public ProcessStartInfo(string fileName, System.Collections.Generic.IEnumerable< [System.Diagnostics.CodeAnalysis.AllowNullAttribute] public string WorkingDirectory { get { throw null; } set { } } } + public sealed partial class ProcessStartOptions + { + public ProcessStartOptions(string fileName) { } + public System.Collections.Generic.IList Arguments { get { throw null; } set { } } + public bool CreateNewProcessGroup { get { throw null; } set { } } + public System.Collections.Generic.IDictionary Environment { get { throw null; } } + public string FileName { get { throw null; } } + public System.Collections.Generic.IList InheritedHandles { get { throw null; } set { } } + public bool KillOnParentExit { get { throw null; } set { } } + public string? WorkingDirectory { get { throw null; } set { } } + } [System.ComponentModel.DesignerAttribute("System.Diagnostics.Design.ProcessThreadDesigner, System.Design, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a")] public partial class ProcessThread : System.ComponentModel.Component { diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 986ee67013d3d7..a674056838985d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -12,6 +12,7 @@ $([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) SR.Process_PlatformNotSupported true + $(DefineConstants);WINDOWS @@ -26,6 +27,7 @@ + @@ -136,6 +138,10 @@ Link="Common\Interop\Windows\Kernel32\Interop.GetProcessPriorityBoost.cs" /> + + + /// Specifies options for starting a new process. + /// + public sealed class ProcessStartOptions + { + private readonly string _fileName; + private IList? _arguments; + private DictionaryWrapper? _environment; + private IList? _inheritedHandles; + + /// + /// Gets the application to start. + /// + public string FileName => _fileName; + + /// + /// Gets or sets the command-line arguments to pass to the application. + /// + public IList Arguments + { + get => _arguments ??= new List(); + set => _arguments = value; + } + + /// + /// Gets the environment variables that apply to this process and its child processes. + /// + /// + /// By default, the environment is a copy of the current process environment. + /// + public IDictionary Environment + { + get + { + if (_environment == null) + { + IDictionary envVars = System.Environment.GetEnvironmentVariables(); + + _environment = new DictionaryWrapper(new Dictionary( + envVars.Count, + OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal)); + + // Manual use of IDictionaryEnumerator instead of foreach to avoid DictionaryEntry box allocations. + IDictionaryEnumerator e = envVars.GetEnumerator(); + Debug.Assert(!(e is IDisposable), "Environment.GetEnvironmentVariables should not be IDisposable."); + while (e.MoveNext()) + { + DictionaryEntry entry = e.Entry; + _environment.Add((string)entry.Key, (string?)entry.Value); + } + } + return _environment; + } + } + + /// + /// Gets or sets the working directory for the process to be started. + /// + public string? WorkingDirectory { get; set; } + + /// + /// Gets a list of handles that will be inherited by the child process. + /// + /// + /// + /// Handles do not need to have inheritance enabled beforehand. + /// They are also not duplicated, just added as-is to the child process + /// so the exact same handle values can be used in the child process. + /// + /// + /// On Windows, the implementation will automatically enable inheritance on any handle added to this list + /// by modifying the handle's flags using SetHandleInformation. + /// + /// + /// On Unix, the implementation will modify the copy of every handle in the child process + /// by removing FD_CLOEXEC flag. It happens after the fork and before the exec, so it does not affect parent process. + /// + /// + public IList InheritedHandles + { + get => _inheritedHandles ??= new List(); + set => _inheritedHandles = value; + } + + /// + /// Gets or sets a value indicating whether the child process should be terminated when the parent process exits. + /// + public bool KillOnParentExit { get; set; } + + /// + /// Gets or sets a value indicating whether to create the process in a new process group. + /// + /// + /// + /// Creating a new process group enables sending signals to the process (e.g., SIGINT, SIGQUIT) + /// on Windows and provides process group isolation on all platforms. + /// + /// + /// On Unix systems, child processes in a new process group won't receive signals sent to the parent's + /// process group, which can be useful for background processes that should continue running independently. + /// + /// + public bool CreateNewProcessGroup { get; set; } + + /// + /// Initializes a new instance of the class. + /// + /// The application to start. + /// Thrown when is null or empty. + /// Thrown when cannot be resolved to an existing file. + public ProcessStartOptions(string fileName) + { + ArgumentException.ThrowIfNullOrEmpty(fileName); + + _fileName = ResolvePath(fileName); + } + + /// Resolves a path to the filename. + /// The filename. + /// The resolved path. + /// Thrown when cannot be resolved to an existing file. + internal static string ResolvePath(string filename) + { + // If the filename is a complete path, use it, regardless of whether it exists. + if (Path.IsPathRooted(filename)) + { + // In this case, it doesn't matter whether the file exists or not; + // it's what the caller asked for, so it's what they'll get + return filename; + } + +#if WINDOWS + // From: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw + // "If the file name does not contain an extension, .exe is appended. + // Therefore, if the file name extension is .com, this parameter must include the .com extension. + // If the file name ends in a period (.) with no extension, or if the file name contains a path, .exe is not appended." + + // HasExtension returns false for trailing dot, so we need to check that separately + if (filename[filename.Length - 1] != '.' && !Path.HasExtension(filename)) + { + filename += ".exe"; + } +#endif + + // Then check the executable's directory + string? path = Environment.ProcessPath; + if (path != null) + { + try + { + path = Path.Combine(Path.GetDirectoryName(path)!, filename); + if (File.Exists(path)) + { + return path; + } + } + catch (ArgumentException) { } // ignore any errors in data that may come from the exe path + } + + // Then check the current directory + path = Path.Combine(Directory.GetCurrentDirectory(), filename); + if (File.Exists(path)) + { + return path; + } + +#if WINDOWS + // Windows-specific search locations (from CreateProcessW documentation) + + // Check the 32-bit Windows system directory (It can't change over app lifetime) + path = GetSystemDirectory(); + if (path != null) + { + path = Path.Combine(path, filename); + if (File.Exists(path)) + { + return path; + } + } + + // Check the Windows directory + path = GetWindowsDirectory(); + if (path != null) + { + // Check the 16-bit Windows system directory (System subdirectory of Windows directory) + string systemPath = Path.Combine(path, "System", filename); + if (File.Exists(systemPath)) + { + return systemPath; + } + + // Check the Windows directory itself + path = Path.Combine(path, filename); + if (File.Exists(path)) + { + return path; + } + } +#endif + + // Then check each directory listed in the PATH environment variables + return FindProgramInPath(filename); + } + + /// + /// Gets the path to the program by searching in PATH environment variable. + /// + /// The program name. + /// The full path to the program. + /// Thrown when the program cannot be found in PATH. + private static string FindProgramInPath(string program) + { + string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); + if (pathEnvVar != null) + { +#if WINDOWS + char pathSeparator = ';'; +#else + char pathSeparator = ':'; +#endif + var pathParser = new StringParser(pathEnvVar, pathSeparator, skipEmpty: true); + while (pathParser.MoveNext()) + { + string subPath = pathParser.ExtractCurrent(); + string path = Path.Combine(subPath, program); + if (IsExecutableFile(path)) + { + return path; + } + } + } + + throw new FileNotFoundException("Could not resolve the file.", program); + } + + private static bool IsExecutableFile(string path) + { +#if WINDOWS + return File.Exists(path); +#else + return IsExecutable(path); +#endif + } + +#if !WINDOWS + private static bool IsExecutable(string fullPath) + { + Interop.Sys.FileStatus fileinfo; + + if (Interop.Sys.Stat(fullPath, out fileinfo) < 0) + { + return false; + } + + // Check if the path is a directory. + if ((fileinfo.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR) + { + return false; + } + + const UnixFileMode AllExecute = UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; + + UnixFileMode permissions = ((UnixFileMode)fileinfo.Mode) & AllExecute; + + // Avoid checking user/group when permission. + if (permissions == AllExecute) + { + return true; + } + else if (permissions == 0) + { + return false; + } + + uint euid = Interop.Sys.GetEUid(); + + if (euid == 0) + { + return true; // We're root. + } + + if (euid == fileinfo.Uid) + { + // We own the file. + return (permissions & UnixFileMode.UserExecute) != 0; + } + + bool groupCanExecute = (permissions & UnixFileMode.GroupExecute) != 0; + bool otherCanExecute = (permissions & UnixFileMode.OtherExecute) != 0; + + // Avoid group check when group and other have same permissions. + if (groupCanExecute == otherCanExecute) + { + return groupCanExecute; + } + + if (Interop.Sys.IsMemberOfGroup(fileinfo.Gid)) + { + return groupCanExecute; + } + else + { + return otherCanExecute; + } + } +#endif + +#if WINDOWS + private static string? s_cachedSystemDirectory; + + private static string? GetSystemDirectory() + { + if (s_cachedSystemDirectory == null) + { + Span buffer = stackalloc char[260]; // MAX_PATH + uint length = Interop.Kernel32.GetSystemDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); + if (length > 0 && length < buffer.Length) + { + s_cachedSystemDirectory = new string(buffer.Slice(0, (int)length)); + } + } + return s_cachedSystemDirectory; + } + + private static string? GetWindowsDirectory() + { + Span buffer = stackalloc char[260]; // MAX_PATH + uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); + if (length > 0 && length < buffer.Length) + { + return new string(buffer.Slice(0, (int)length)); + } + return null; + } +#endif + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs new file mode 100644 index 00000000000000..2cb8c3cc5f5488 --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -0,0 +1,203 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO; +using Xunit; + +namespace System.Diagnostics.Tests +{ + [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.FreeBSD | TestPlatforms.OSX)] + public class ProcessStartOptionsTests_Unix + { + [Fact] + public void TestResolvePath_FindsInPath() + { + // sh should be findable in PATH on all Unix systems + ProcessStartOptions options = new ProcessStartOptions("sh"); + Assert.True(File.Exists(options.FileName)); + Assert.Contains("sh", options.FileName); + } + + [Fact] + public void TestResolvePath_DoesNotAddExeExtension() + { + // On Unix, no .exe extension should be added + ProcessStartOptions options = new ProcessStartOptions("sh"); + Assert.False(options.FileName.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)); + } + + [Fact] + public void TestResolvePath_UsesCurrentDirectory() + { + string tempDir = Path.GetTempPath(); + string fileName = "testscript.sh"; + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "#!/bin/sh\necho test"); + // Make it executable + File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + + // Save current directory + string oldDir = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.Equal(fullPath, options.FileName); + } + finally + { + Directory.SetCurrentDirectory(oldDir); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + } + } + + [Fact] + public void TestResolvePath_PathSeparatorIsColon() + { + // Create a temp directory and file + string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + Directory.CreateDirectory(tempDir); + string fileName = "testscript"; + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "#!/bin/sh\necho test"); + // Make it executable + File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + + // Add temp directory to PATH using colon separator + string oldPath = Environment.GetEnvironmentVariable("PATH"); + try + { + Environment.SetEnvironmentVariable("PATH", tempDir + ":" + oldPath); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.Equal(fullPath, options.FileName); + } + finally + { + Environment.SetEnvironmentVariable("PATH", oldPath); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + if (Directory.Exists(tempDir)) + { + Directory.Delete(tempDir, recursive: true); + } + } + } + + [Fact] + public void TestResolvePath_ChecksExecutablePermissions() + { + // Create a file without execute permissions + string tempDir = Path.GetTempPath(); + string fileName = "nonexecutable.sh"; + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "#!/bin/sh\necho test"); + // Explicitly make it non-executable + File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite); + + // Save current directory + string oldDir = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(tempDir); + // Should throw because file is not executable + Assert.Throws(() => new ProcessStartOptions(fileName)); + } + finally + { + Directory.SetCurrentDirectory(oldDir); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + } + } + + [Fact] + public void TestResolvePath_AbsolutePathIsNotModified() + { + string tempFile = Path.GetTempFileName(); + try + { + ProcessStartOptions options = new ProcessStartOptions(tempFile); + Assert.Equal(tempFile, options.FileName); + } + finally + { + if (File.Exists(tempFile)) + { + File.Delete(tempFile); + } + } + } + + [Fact] + public void TestResolvePath_FindsCommonUtilities() + { + // Test common Unix utilities + string[] commonUtils = { "ls", "cat", "echo", "sh" }; + + foreach (string util in commonUtils) + { + ProcessStartOptions options = new ProcessStartOptions(util); + Assert.True(File.Exists(options.FileName), $"{util} should be found and exist"); + Assert.Contains(util, options.FileName); + } + } + + [Fact] + public void TestResolvePath_RejectsDirectories() + { + // Create a directory with executable permissions + string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + Directory.CreateDirectory(tempDir); + + try + { + // Try to use the directory name as a command + string oldDir = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(Path.GetTempPath()); + Assert.Throws(() => new ProcessStartOptions(Path.GetFileName(tempDir))); + } + finally + { + Directory.SetCurrentDirectory(oldDir); + } + } + finally + { + if (Directory.Exists(tempDir)) + { + Directory.Delete(tempDir); + } + } + } + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs new file mode 100644 index 00000000000000..287cb573fedf16 --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -0,0 +1,200 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO; +using Microsoft.DotNet.XUnitExtensions; +using Xunit; + +namespace System.Diagnostics.Tests +{ + public class ProcessStartOptionsTests_Windows + { + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_AddsExeExtension() + { + // Test that .exe is appended when no extension is provided + ProcessStartOptions options = new ProcessStartOptions("notepad"); + Assert.EndsWith(".exe", options.FileName, StringComparison.OrdinalIgnoreCase); + Assert.True(File.Exists(options.FileName)); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_DoesNotAddExeExtensionForTrailingDot() + { + // "If the file name ends in a period (.) with no extension, .exe is not appended." + // This should fail since "notepad." won't exist + Assert.Throws(() => new ProcessStartOptions("notepad.")); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_PreservesComExtension() + { + // The .com extension should be preserved + string fileName = "test.com"; + string tempDir = Path.GetTempPath(); + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "test"); + + // Save current directory + string oldDir = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.EndsWith(".com", options.FileName, StringComparison.OrdinalIgnoreCase); + } + finally + { + Directory.SetCurrentDirectory(oldDir); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_FindsInSystemDirectory() + { + // cmd.exe should be found in system directory + ProcessStartOptions options = new ProcessStartOptions("cmd"); + Assert.True(File.Exists(options.FileName)); + Assert.Contains("system32", options.FileName, StringComparison.OrdinalIgnoreCase); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_FindsInWindowsDirectory() + { + // Some utilities are in Windows directory + // We'll test with a file that's commonly in Windows directory + // Note: This might not exist on all systems, so we make it conditional + try + { + ProcessStartOptions options = new ProcessStartOptions("notepad"); + Assert.True(File.Exists(options.FileName)); + } + catch (FileNotFoundException) + { + // Skip if notepad is not found - it's not critical for this test + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_FindsInPath() + { + // powershell.exe should be findable in PATH on most Windows systems + try + { + ProcessStartOptions options = new ProcessStartOptions("powershell"); + Assert.True(File.Exists(options.FileName)); + Assert.EndsWith(".exe", options.FileName, StringComparison.OrdinalIgnoreCase); + } + catch (FileNotFoundException) + { + // Skip if PowerShell is not found - it might not be in PATH + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_UsesCurrentDirectory() + { + string tempDir = Path.GetTempPath(); + string fileName = "testapp.exe"; + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "test"); + + // Save current directory + string oldDir = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.Equal(fullPath, options.FileName); + } + finally + { + Directory.SetCurrentDirectory(oldDir); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_PathSeparatorIsSemicolon() + { + // Create a temp directory and file + string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); + Directory.CreateDirectory(tempDir); + string fileName = "testexe.exe"; + string fullPath = Path.Combine(tempDir, fileName); + + try + { + File.WriteAllText(fullPath, "test"); + + // Add temp directory to PATH using semicolon separator + string oldPath = Environment.GetEnvironmentVariable("PATH"); + try + { + Environment.SetEnvironmentVariable("PATH", tempDir + ";" + oldPath); + ProcessStartOptions options = new ProcessStartOptions("testexe"); + Assert.Equal(fullPath, options.FileName); + } + finally + { + Environment.SetEnvironmentVariable("PATH", oldPath); + } + } + finally + { + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + if (Directory.Exists(tempDir)) + { + Directory.Delete(tempDir, recursive: true); + } + } + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestResolvePath_AbsolutePathIsNotModified() + { + string tempFile = Path.GetTempFileName(); + try + { + // Rename to remove extension to test that .exe is not added for absolute paths + string noExtFile = Path.ChangeExtension(tempFile, null); + File.Move(tempFile, noExtFile); + tempFile = noExtFile; + + ProcessStartOptions options = new ProcessStartOptions(tempFile); + Assert.Equal(tempFile, options.FileName); + } + finally + { + if (File.Exists(tempFile)) + { + File.Delete(tempFile); + } + } + } + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs new file mode 100644 index 00000000000000..cf80a3cd7181ce --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs @@ -0,0 +1,224 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.IO; +using System.Runtime.InteropServices; +using Microsoft.DotNet.RemoteExecutor; +using Microsoft.DotNet.XUnitExtensions; +using Xunit; + +namespace System.Diagnostics.Tests +{ + public class ProcessStartOptionsTests : ProcessTestBase + { + [Fact] + public void TestConstructor_NullFileName_Throws() + { + Assert.Throws(() => new ProcessStartOptions(null)); + } + + [Fact] + public void TestConstructor_EmptyFileName_Throws() + { + Assert.Throws(() => new ProcessStartOptions(string.Empty)); + } + + [Fact] + public void TestConstructor_NonExistentFile_Throws() + { + string nonExistentFile = "ThisFileDoesNotExist_" + Guid.NewGuid().ToString(); + Assert.Throws(() => new ProcessStartOptions(nonExistentFile)); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void TestConstructor_ResolvesCmdOnWindows() + { + if (!OperatingSystem.IsWindows()) + { + return; + } + + ProcessStartOptions options = new ProcessStartOptions("cmd"); + Assert.Contains("cmd.exe", options.FileName, StringComparison.OrdinalIgnoreCase); + Assert.True(File.Exists(options.FileName)); + } + + [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.FreeBSD | TestPlatforms.OSX)] + [Fact] + public void TestConstructor_ResolvesShOnUnix() + { + ProcessStartOptions options = new ProcessStartOptions("sh"); + Assert.Contains("sh", options.FileName); + Assert.True(File.Exists(options.FileName)); + } + + [Fact] + public void TestConstructor_WithAbsolutePath() + { + string tempFile = Path.GetTempFileName(); + try + { + ProcessStartOptions options = new ProcessStartOptions(tempFile); + Assert.Equal(tempFile, options.FileName); + } + finally + { + File.Delete(tempFile); + } + } + + [Fact] + public void TestArguments_LazyInitialization() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + IList args = options.Arguments; + Assert.NotNull(args); + Assert.Empty(args); + } + + [Fact] + public void TestArguments_CanAddAndModify() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + options.Arguments.Add("arg1"); + options.Arguments.Add("arg2"); + Assert.Equal(2, options.Arguments.Count); + Assert.Equal("arg1", options.Arguments[0]); + Assert.Equal("arg2", options.Arguments[1]); + + options.Arguments = new List { "newArg" }; + Assert.Single(options.Arguments); + Assert.Equal("newArg", options.Arguments[0]); + } + + [Fact] + public void TestEnvironment_LazyInitialization() + { + if (PlatformDetection.IsiOS || PlatformDetection.IstvOS || PlatformDetection.IsMacCatalyst) + { + // Whole list of environment variables can no longer be accessed on non-OSX apple platforms + return; + } + + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + IDictionary env = options.Environment; + Assert.NotNull(env); + Assert.NotEmpty(env); + } + + [Fact] + public void TestEnvironment_CanAddAndModify() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + IDictionary env = options.Environment; + + int originalCount = env.Count; + env["TestKey1"] = "TestValue1"; + env["TestKey2"] = "TestValue2"; + Assert.Equal(originalCount + 2, env.Count); + Assert.Equal("TestValue1", env["TestKey1"]); + Assert.Equal("TestValue2", env["TestKey2"]); + + env.Remove("TestKey1"); + Assert.Equal(originalCount + 1, env.Count); + Assert.False(env.ContainsKey("TestKey1")); + } + + [Fact] + public void TestEnvironment_CaseInsensitivityOnWindows() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + IDictionary env = options.Environment; + + env["TestKey"] = "TestValue"; + + if (OperatingSystem.IsWindows()) + { + Assert.True(env.ContainsKey("testkey")); + Assert.Equal("TestValue", env["TESTKEY"]); + } + else + { + Assert.False(env.ContainsKey("testkey")); + } + } + + [Fact] + public void TestInheritedHandles_LazyInitialization() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + IList handles = options.InheritedHandles; + Assert.NotNull(handles); + Assert.Empty(handles); + } + + [Fact] + public void TestInheritedHandles_CanSet() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + List newHandles = new List(); + options.InheritedHandles = newHandles; + Assert.Same(newHandles, options.InheritedHandles); + } + + [Fact] + public void TestWorkingDirectory_DefaultIsNull() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + Assert.Null(options.WorkingDirectory); + } + + [Fact] + public void TestWorkingDirectory_CanSet() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + string tempDir = Path.GetTempPath(); + options.WorkingDirectory = tempDir; + Assert.Equal(tempDir, options.WorkingDirectory); + } + + [Fact] + public void TestKillOnParentExit_DefaultIsFalse() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + Assert.False(options.KillOnParentExit); + } + + [Fact] + public void TestKillOnParentExit_CanSet() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + options.KillOnParentExit = true; + Assert.True(options.KillOnParentExit); + } + + [Fact] + public void TestCreateNewProcessGroup_DefaultIsFalse() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + Assert.False(options.CreateNewProcessGroup); + } + + [Fact] + public void TestCreateNewProcessGroup_CanSet() + { + ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + options.CreateNewProcessGroup = true; + Assert.True(options.CreateNewProcessGroup); + } + + private string GetCurrentProcessName() + { + // Get a valid executable path for testing + if (OperatingSystem.IsWindows()) + { + return Path.Combine(Environment.SystemDirectory, "cmd.exe"); + } + else + { + return "/bin/sh"; + } + } + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj index 93cb8d34b091e1..63523ef48c86fb 100644 --- a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj +++ b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj @@ -28,6 +28,7 @@ + @@ -40,6 +41,7 @@ + + From 0c9d3e24d4f6e8107da0ef0c4af7c1703c16b40c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 14:07:48 +0000 Subject: [PATCH 03/16] Fix build errors and test failures in ProcessStartOptions Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System.Diagnostics.Process.csproj | 4 ++-- .../System/Diagnostics/ProcessStartOptions.cs | 17 +++++++++++++---- .../tests/ProcessStartOptionsTests.cs | 2 +- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index a674056838985d..78eee4897b0b89 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -49,6 +49,8 @@ Link="Common\System\Text\ValueStringBuilder.cs" /> + @@ -241,8 +243,6 @@ - InheritedHandles /// /// /// - /// Creating a new process group enables sending signals to the process (e.g., SIGINT, SIGQUIT) + /// Creating a new process group enables sending signals to the process (e.g., SIGINT, SIGQUIT) /// on Windows and provides process group isolation on all platforms. /// /// - /// On Unix systems, child processes in a new process group won't receive signals sent to the parent's + /// On Unix systems, child processes in a new process group won't receive signals sent to the parent's /// process group, which can be useful for background processes that should continue running independently. /// /// @@ -154,13 +155,17 @@ internal static string ResolvePath(string filename) #endif // Then check the executable's directory - string? path = Environment.ProcessPath; + string? path = System.Environment.ProcessPath; if (path != null) { try { path = Path.Combine(Path.GetDirectoryName(path)!, filename); +#if WINDOWS if (File.Exists(path)) +#else + if (IsExecutableFile(path)) +#endif { return path; } @@ -170,7 +175,11 @@ internal static string ResolvePath(string filename) // Then check the current directory path = Path.Combine(Directory.GetCurrentDirectory(), filename); +#if WINDOWS if (File.Exists(path)) +#else + if (IsExecutableFile(path)) +#endif { return path; } @@ -221,7 +230,7 @@ internal static string ResolvePath(string filename) /// Thrown when the program cannot be found in PATH. private static string FindProgramInPath(string program) { - string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); + string? pathEnvVar = System.Environment.GetEnvironmentVariable("PATH"); if (pathEnvVar != null) { #if WINDOWS diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs index cf80a3cd7181ce..da8c649e8037a7 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs @@ -15,7 +15,7 @@ public class ProcessStartOptionsTests : ProcessTestBase [Fact] public void TestConstructor_NullFileName_Throws() { - Assert.Throws(() => new ProcessStartOptions(null)); + Assert.Throws(() => new ProcessStartOptions(null)); } [Fact] From 794a8a206dffbbbf4bb406b95b3f328798616647 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 14:09:54 +0000 Subject: [PATCH 04/16] Improve code comments based on review feedback Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/ProcessStartOptions.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index f96cf7d9d8abf6..ea6e9799952ab9 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -187,7 +187,7 @@ internal static string ResolvePath(string filename) #if WINDOWS // Windows-specific search locations (from CreateProcessW documentation) - // Check the 32-bit Windows system directory (It can't change over app lifetime) + // Check the system directory (e.g., System32) path = GetSystemDirectory(); if (path != null) { @@ -202,7 +202,7 @@ internal static string ResolvePath(string filename) path = GetWindowsDirectory(); if (path != null) { - // Check the 16-bit Windows system directory (System subdirectory of Windows directory) + // Check the legacy System subdirectory of Windows directory (for compatibility) string systemPath = Path.Combine(path, "System", filename); if (File.Exists(systemPath)) { @@ -282,7 +282,7 @@ private static bool IsExecutable(string fullPath) UnixFileMode permissions = ((UnixFileMode)fileinfo.Mode) & AllExecute; - // Avoid checking user/group when permission. + // Avoid checking user/group when all execute permissions are set. if (permissions == AllExecute) { return true; From a1b34263e30cf4ce70534bf2e112972b0f0cf880 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 14:11:44 +0000 Subject: [PATCH 05/16] Fix exception documentation and add caching comment Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/ProcessStartOptions.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index ea6e9799952ab9..16a1cf66178a00 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -118,7 +118,8 @@ public IList InheritedHandles /// Initializes a new instance of the class. /// /// The application to start. - /// Thrown when is null or empty. + /// Thrown when is null. + /// Thrown when is empty. /// Thrown when cannot be resolved to an existing file. public ProcessStartOptions(string fileName) { @@ -344,6 +345,7 @@ private static bool IsExecutable(string fullPath) private static string? GetWindowsDirectory() { + // Don't cache Windows directory as it's user-specific and can change during app lifetime Span buffer = stackalloc char[260]; // MAX_PATH uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); if (length > 0 && length < buffer.Length) From 10ff95ae2a5112f28a0c43589abb4dd0b95f7c41 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 16:09:04 +0000 Subject: [PATCH 06/16] Address PR feedback: use Process.ResolvePath, fix tests, add null checks Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/Process.Unix.cs | 6 +- .../System/Diagnostics/ProcessStartOptions.cs | 123 +++++------------- .../tests/ProcessStartOptionsTests.Unix.cs | 19 +-- .../tests/ProcessStartOptionsTests.cs | 15 +-- 4 files changed, 43 insertions(+), 120 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 2dde9eb778cdc9..b2fe96562dfcd9 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -685,7 +685,7 @@ private static string[] CreateEnvp(ProcessStartInfo psi) /// Resolves a path to the filename passed to ProcessStartInfo. /// The filename. /// The resolved path. It can return null in case of URLs. - private static string? ResolvePath(string filename) + internal static string? ResolvePath(string filename) { // Follow the same resolution that Windows uses with CreateProcess: // 1. First try the exact path provided @@ -734,7 +734,7 @@ private static string[] CreateEnvp(ProcessStartInfo psi) /// /// /// - private static string? FindProgramInPath(string program) + internal static string? FindProgramInPath(string program) { string path; string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); @@ -754,7 +754,7 @@ private static string[] CreateEnvp(ProcessStartInfo psi) return null; } - private static bool IsExecutable(string fullPath) + internal static bool IsExecutable(string fullPath) { Interop.Sys.FileStatus fileinfo; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 16a1cf66178a00..b70b042bc96ca5 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -31,7 +31,11 @@ public sealed class ProcessStartOptions public IList Arguments { get => _arguments ??= new List(); - set => _arguments = value; + set + { + ArgumentNullException.ThrowIfNull(value); + _arguments = value; + } } /// @@ -91,7 +95,11 @@ public IList Arguments public IList InheritedHandles { get => _inheritedHandles ??= new List(); - set => _inheritedHandles = value; + set + { + ArgumentNullException.ThrowIfNull(value); + _inheritedHandles = value; + } } /// @@ -134,6 +142,21 @@ public ProcessStartOptions(string fileName) /// Thrown when cannot be resolved to an existing file. internal static string ResolvePath(string filename) { +#if WINDOWS + return ResolvePathWindows(filename); +#else + string? resolved = Process.ResolvePath(filename); + if (resolved == null) + { + throw new FileNotFoundException("Could not resolve the file.", filename); + } + return resolved; +#endif + } + +#if WINDOWS + private static string ResolvePathWindows(string filename) + { // If the filename is a complete path, use it, regardless of whether it exists. if (Path.IsPathRooted(filename)) { @@ -142,7 +165,6 @@ internal static string ResolvePath(string filename) return filename; } -#if WINDOWS // From: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw // "If the file name does not contain an extension, .exe is appended. // Therefore, if the file name extension is .com, this parameter must include the .com extension. @@ -153,7 +175,6 @@ internal static string ResolvePath(string filename) { filename += ".exe"; } -#endif // Then check the executable's directory string? path = System.Environment.ProcessPath; @@ -162,11 +183,7 @@ internal static string ResolvePath(string filename) try { path = Path.Combine(Path.GetDirectoryName(path)!, filename); -#if WINDOWS if (File.Exists(path)) -#else - if (IsExecutableFile(path)) -#endif { return path; } @@ -176,16 +193,11 @@ internal static string ResolvePath(string filename) // Then check the current directory path = Path.Combine(Directory.GetCurrentDirectory(), filename); -#if WINDOWS if (File.Exists(path)) -#else - if (IsExecutableFile(path)) -#endif { return path; } -#if WINDOWS // Windows-specific search locations (from CreateProcessW documentation) // Check the system directory (e.g., System32) @@ -217,10 +229,9 @@ internal static string ResolvePath(string filename) return path; } } -#endif // Then check each directory listed in the PATH environment variables - return FindProgramInPath(filename); + return FindProgramInPathWindows(filename); } /// @@ -229,22 +240,17 @@ internal static string ResolvePath(string filename) /// The program name. /// The full path to the program. /// Thrown when the program cannot be found in PATH. - private static string FindProgramInPath(string program) + private static string FindProgramInPathWindows(string program) { string? pathEnvVar = System.Environment.GetEnvironmentVariable("PATH"); if (pathEnvVar != null) { -#if WINDOWS - char pathSeparator = ';'; -#else - char pathSeparator = ':'; -#endif - var pathParser = new StringParser(pathEnvVar, pathSeparator, skipEmpty: true); + var pathParser = new StringParser(pathEnvVar, ';', skipEmpty: true); while (pathParser.MoveNext()) { string subPath = pathParser.ExtractCurrent(); string path = Path.Combine(subPath, program); - if (IsExecutableFile(path)) + if (File.Exists(path)) { return path; } @@ -253,77 +259,6 @@ private static string FindProgramInPath(string program) throw new FileNotFoundException("Could not resolve the file.", program); } - - private static bool IsExecutableFile(string path) - { -#if WINDOWS - return File.Exists(path); -#else - return IsExecutable(path); -#endif - } - -#if !WINDOWS - private static bool IsExecutable(string fullPath) - { - Interop.Sys.FileStatus fileinfo; - - if (Interop.Sys.Stat(fullPath, out fileinfo) < 0) - { - return false; - } - - // Check if the path is a directory. - if ((fileinfo.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR) - { - return false; - } - - const UnixFileMode AllExecute = UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; - - UnixFileMode permissions = ((UnixFileMode)fileinfo.Mode) & AllExecute; - - // Avoid checking user/group when all execute permissions are set. - if (permissions == AllExecute) - { - return true; - } - else if (permissions == 0) - { - return false; - } - - uint euid = Interop.Sys.GetEUid(); - - if (euid == 0) - { - return true; // We're root. - } - - if (euid == fileinfo.Uid) - { - // We own the file. - return (permissions & UnixFileMode.UserExecute) != 0; - } - - bool groupCanExecute = (permissions & UnixFileMode.GroupExecute) != 0; - bool otherCanExecute = (permissions & UnixFileMode.OtherExecute) != 0; - - // Avoid group check when group and other have same permissions. - if (groupCanExecute == otherCanExecute) - { - return groupCanExecute; - } - - if (Interop.Sys.IsMemberOfGroup(fileinfo.Gid)) - { - return groupCanExecute; - } - else - { - return otherCanExecute; - } - } #endif #if WINDOWS diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 2cb8c3cc5f5488..79ba8ced885c37 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -15,7 +15,7 @@ public void TestResolvePath_FindsInPath() // sh should be findable in PATH on all Unix systems ProcessStartOptions options = new ProcessStartOptions("sh"); Assert.True(File.Exists(options.FileName)); - Assert.Contains("sh", options.FileName); + Assert.EndsWith("sh", options.FileName); } [Fact] @@ -33,27 +33,20 @@ public void TestResolvePath_UsesCurrentDirectory() string fileName = "testscript.sh"; string fullPath = Path.Combine(tempDir, fileName); + string oldDir = Directory.GetCurrentDirectory(); try { File.WriteAllText(fullPath, "#!/bin/sh\necho test"); // Make it executable File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); - // Save current directory - string oldDir = Directory.GetCurrentDirectory(); - try - { - Directory.SetCurrentDirectory(tempDir); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.Equal(fullPath, options.FileName); - } - finally - { - Directory.SetCurrentDirectory(oldDir); - } + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.Equal(fullPath, options.FileName); } finally { + Directory.SetCurrentDirectory(oldDir); if (File.Exists(fullPath)) { File.Delete(fullPath); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs index da8c649e8037a7..4bd075a8227948 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs @@ -31,16 +31,11 @@ public void TestConstructor_NonExistentFile_Throws() Assert.Throws(() => new ProcessStartOptions(nonExistentFile)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))] public void TestConstructor_ResolvesCmdOnWindows() { - if (!OperatingSystem.IsWindows()) - { - return; - } - ProcessStartOptions options = new ProcessStartOptions("cmd"); - Assert.Contains("cmd.exe", options.FileName, StringComparison.OrdinalIgnoreCase); + Assert.EndsWith("cmd.exe", options.FileName); Assert.True(File.Exists(options.FileName)); } @@ -69,7 +64,7 @@ public void TestConstructor_WithAbsolutePath() } [Fact] - public void TestArguments_LazyInitialization() + public void TestArguments_DefaultIsEmpty() { ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); IList args = options.Arguments; @@ -145,7 +140,7 @@ public void TestEnvironment_CaseInsensitivityOnWindows() } [Fact] - public void TestInheritedHandles_LazyInitialization() + public void TestInheritedHandles_DefaultIsEmpty() { ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); IList handles = options.InheritedHandles; @@ -217,7 +212,7 @@ private string GetCurrentProcessName() } else { - return "/bin/sh"; + return Environment.ProcessPath ?? "/bin/sh"; } } } From fc0fd30aad754d6c470d2bc8565375eedcecd9ec Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 17:16:30 +0000 Subject: [PATCH 07/16] Refactor: ProcessStartOptions.ResolvePath as main impl, Process calls it Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/Process.Unix.cs | 51 +-------------- .../System/Diagnostics/ProcessStartOptions.cs | 64 ++++++++++--------- 2 files changed, 37 insertions(+), 78 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index b2fe96562dfcd9..3d1ab17a9abb1d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -446,7 +446,7 @@ private bool StartCore(ProcessStartInfo startInfo) } else { - filename = ResolvePath(startInfo.FileName); + filename = ProcessStartOptions.ResolvePath(startInfo.FileName); argv = ParseArgv(startInfo); if (Directory.Exists(filename)) { @@ -682,59 +682,12 @@ private static string[] CreateEnvp(ProcessStartInfo psi) } } - /// Resolves a path to the filename passed to ProcessStartInfo. - /// The filename. - /// The resolved path. It can return null in case of URLs. - internal static string? ResolvePath(string filename) - { - // Follow the same resolution that Windows uses with CreateProcess: - // 1. First try the exact path provided - // 2. Then try the file relative to the executable directory - // 3. Then try the file relative to the current directory - // 4. then try the file in each of the directories specified in PATH - // Windows does additional Windows-specific steps between 3 and 4, - // and we ignore those here. - - // If the filename is a complete path, use it, regardless of whether it exists. - if (Path.IsPathRooted(filename)) - { - // In this case, it doesn't matter whether the file exists or not; - // it's what the caller asked for, so it's what they'll get - return filename; - } - - // Then check the executable's directory - string? path = Environment.ProcessPath; - if (path != null) - { - try - { - path = Path.Combine(Path.GetDirectoryName(path)!, filename); - if (File.Exists(path)) - { - return path; - } - } - catch (ArgumentException) { } // ignore any errors in data that may come from the exe path - } - - // Then check the current directory - path = Path.Combine(Directory.GetCurrentDirectory(), filename); - if (File.Exists(path)) - { - return path; - } - - // Then check each directory listed in the PATH environment variables - return FindProgramInPath(filename); - } - /// /// Gets the path to the program /// /// /// - internal static string? FindProgramInPath(string program) + private static string? FindProgramInPath(string program) { string path; string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index b70b042bc96ca5..1cdac7cbbbb042 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -31,11 +31,7 @@ public sealed class ProcessStartOptions public IList Arguments { get => _arguments ??= new List(); - set - { - ArgumentNullException.ThrowIfNull(value); - _arguments = value; - } + set => _arguments = value; } /// @@ -95,11 +91,7 @@ public IList Arguments public IList InheritedHandles { get => _inheritedHandles ??= new List(); - set - { - ArgumentNullException.ThrowIfNull(value); - _inheritedHandles = value; - } + set => _inheritedHandles = value; } /// @@ -142,21 +134,6 @@ public ProcessStartOptions(string fileName) /// Thrown when cannot be resolved to an existing file. internal static string ResolvePath(string filename) { -#if WINDOWS - return ResolvePathWindows(filename); -#else - string? resolved = Process.ResolvePath(filename); - if (resolved == null) - { - throw new FileNotFoundException("Could not resolve the file.", filename); - } - return resolved; -#endif - } - -#if WINDOWS - private static string ResolvePathWindows(string filename) - { // If the filename is a complete path, use it, regardless of whether it exists. if (Path.IsPathRooted(filename)) { @@ -165,6 +142,7 @@ private static string ResolvePathWindows(string filename) return filename; } +#if WINDOWS // From: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw // "If the file name does not contain an extension, .exe is appended. // Therefore, if the file name extension is .com, this parameter must include the .com extension. @@ -175,6 +153,7 @@ private static string ResolvePathWindows(string filename) { filename += ".exe"; } +#endif // Then check the executable's directory string? path = System.Environment.ProcessPath; @@ -183,7 +162,11 @@ private static string ResolvePathWindows(string filename) try { path = Path.Combine(Path.GetDirectoryName(path)!, filename); +#if WINDOWS if (File.Exists(path)) +#else + if (IsExecutableFile(path)) +#endif { return path; } @@ -193,11 +176,16 @@ private static string ResolvePathWindows(string filename) // Then check the current directory path = Path.Combine(Directory.GetCurrentDirectory(), filename); +#if WINDOWS if (File.Exists(path)) +#else + if (IsExecutableFile(path)) +#endif { return path; } +#if WINDOWS // Windows-specific search locations (from CreateProcessW documentation) // Check the system directory (e.g., System32) @@ -229,9 +217,10 @@ private static string ResolvePathWindows(string filename) return path; } } +#endif // Then check each directory listed in the PATH environment variables - return FindProgramInPathWindows(filename); + return FindProgramInPath(filename); } /// @@ -240,17 +229,22 @@ private static string ResolvePathWindows(string filename) /// The program name. /// The full path to the program. /// Thrown when the program cannot be found in PATH. - private static string FindProgramInPathWindows(string program) + private static string FindProgramInPath(string program) { string? pathEnvVar = System.Environment.GetEnvironmentVariable("PATH"); if (pathEnvVar != null) { - var pathParser = new StringParser(pathEnvVar, ';', skipEmpty: true); +#if WINDOWS + char pathSeparator = ';'; +#else + char pathSeparator = ':'; +#endif + var pathParser = new StringParser(pathEnvVar, pathSeparator, skipEmpty: true); while (pathParser.MoveNext()) { string subPath = pathParser.ExtractCurrent(); string path = Path.Combine(subPath, program); - if (File.Exists(path)) + if (IsExecutableFile(path)) { return path; } @@ -259,6 +253,18 @@ private static string FindProgramInPathWindows(string program) throw new FileNotFoundException("Could not resolve the file.", program); } + + private static bool IsExecutableFile(string path) + { +#if WINDOWS + return File.Exists(path); +#else + return Process.IsExecutable(path); +#endif + } + +#if !WINDOWS + // Unix executable checking is in Process.IsExecutable #endif #if WINDOWS From f687eb8f1620ea24a22d2b902ffc47c56516ff78 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 11 Feb 2026 19:19:59 +0100 Subject: [PATCH 08/16] address my own feedback --- .../src/System/Diagnostics/Process.Linux.cs | 2 +- .../src/System/Diagnostics/Process.SunOS.cs | 2 +- .../src/System/Diagnostics/Process.Unix.cs | 27 +------- .../System/Diagnostics/ProcessStartOptions.cs | 64 ++++++------------- 4 files changed, 23 insertions(+), 72 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs index 5cf8c5c76a993d..a47c8885afa9e4 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs @@ -108,7 +108,7 @@ private static DateTime BootTime ReadOnlySpan allowedProgramsToRun = ["xdg-open", "gnome-open", "kfmclient"]; foreach (var program in allowedProgramsToRun) { - string? pathToProgram = FindProgramInPath(program); + string? pathToProgram = ProcessStartOptions.FindProgramInPath(program); if (!string.IsNullOrEmpty(pathToProgram)) { return pathToProgram; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.SunOS.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.SunOS.cs index f40076a3882aca..3a3bff81abd0a4 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.SunOS.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.SunOS.cs @@ -38,7 +38,7 @@ internal DateTime StartTimeCore /// Gets execution path private static string? GetPathToOpenFile() { - return FindProgramInPath("xdg-open"); + return ProcessStartOptions.FindProgramInPath("xdg-open"); } /// diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 3d1ab17a9abb1d..fc46f14f918e7b 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -663,7 +663,7 @@ private static string[] CreateEnvp(ProcessStartInfo psi) // find filename on PATH else { - resolvedFilename = FindProgramInPath(filename); + resolvedFilename = ProcessStartOptions.FindProgramInPath(filename); } } @@ -682,31 +682,6 @@ private static string[] CreateEnvp(ProcessStartInfo psi) } } - /// - /// Gets the path to the program - /// - /// - /// - private static string? FindProgramInPath(string program) - { - string path; - string? pathEnvVar = Environment.GetEnvironmentVariable("PATH"); - if (pathEnvVar != null) - { - var pathParser = new StringParser(pathEnvVar, ':', skipEmpty: true); - while (pathParser.MoveNext()) - { - string subPath = pathParser.ExtractCurrent(); - path = Path.Combine(subPath, program); - if (IsExecutable(path)) - { - return path; - } - } - } - return null; - } - internal static bool IsExecutable(string fullPath) { Interop.Sys.FileStatus fileinfo; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 1cdac7cbbbb042..3ba446f13dcf7a 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -31,7 +31,11 @@ public sealed class ProcessStartOptions public IList Arguments { get => _arguments ??= new List(); - set => _arguments = value; + set + { + ArgumentNullException.ThrowIfNull(value); + _arguments = value; + } } /// @@ -91,7 +95,11 @@ public IList Arguments public IList InheritedHandles { get => _inheritedHandles ??= new List(); - set => _inheritedHandles = value; + set + { + ArgumentNullException.ThrowIfNull(value); + _inheritedHandles = value; + } } /// @@ -125,14 +133,10 @@ public ProcessStartOptions(string fileName) { ArgumentException.ThrowIfNullOrEmpty(fileName); - _fileName = ResolvePath(fileName); + _fileName = ResolvePath(fileName) ?? throw new FileNotFoundException("Could not resolve the file.", fileName); ; } - /// Resolves a path to the filename. - /// The filename. - /// The resolved path. - /// Thrown when cannot be resolved to an existing file. - internal static string ResolvePath(string filename) + internal static string? ResolvePath(string filename) { // If the filename is a complete path, use it, regardless of whether it exists. if (Path.IsPathRooted(filename)) @@ -162,11 +166,7 @@ internal static string ResolvePath(string filename) try { path = Path.Combine(Path.GetDirectoryName(path)!, filename); -#if WINDOWS if (File.Exists(path)) -#else - if (IsExecutableFile(path)) -#endif { return path; } @@ -176,11 +176,7 @@ internal static string ResolvePath(string filename) // Then check the current directory path = Path.Combine(Directory.GetCurrentDirectory(), filename); -#if WINDOWS if (File.Exists(path)) -#else - if (IsExecutableFile(path)) -#endif { return path; } @@ -219,54 +215,34 @@ internal static string ResolvePath(string filename) } #endif - // Then check each directory listed in the PATH environment variables return FindProgramInPath(filename); } - /// - /// Gets the path to the program by searching in PATH environment variable. - /// - /// The program name. - /// The full path to the program. - /// Thrown when the program cannot be found in PATH. - private static string FindProgramInPath(string program) + internal static string? FindProgramInPath(string program) { string? pathEnvVar = System.Environment.GetEnvironmentVariable("PATH"); if (pathEnvVar != null) { -#if WINDOWS - char pathSeparator = ';'; -#else - char pathSeparator = ':'; -#endif + char pathSeparator = OperatingSystem.IsWindows() ? ';' : ':'; var pathParser = new StringParser(pathEnvVar, pathSeparator, skipEmpty: true); while (pathParser.MoveNext()) { string subPath = pathParser.ExtractCurrent(); string path = Path.Combine(subPath, program); - if (IsExecutableFile(path)) +#if WINDOWS + if (File.Exists(path)) +#else + if (Process.IsExecutable(path)) +#endif { return path; } } } - throw new FileNotFoundException("Could not resolve the file.", program); - } - - private static bool IsExecutableFile(string path) - { -#if WINDOWS - return File.Exists(path); -#else - return Process.IsExecutable(path); -#endif + return null; } -#if !WINDOWS - // Unix executable checking is in Process.IsExecutable -#endif - #if WINDOWS private static string? s_cachedSystemDirectory; From 5459ec9dc9124fc37bf52c4588a367864e0a67ae Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 18:48:29 +0000 Subject: [PATCH 09/16] Simplify Windows tests: remove ConditionalFact, fix nested try-finally Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessStartOptionsTests.Unix.cs | 16 +++---- .../tests/ProcessStartOptionsTests.Windows.cs | 42 ++++--------------- 2 files changed, 13 insertions(+), 45 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 79ba8ced885c37..7114346ef3b34f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -63,6 +63,7 @@ public void TestResolvePath_PathSeparatorIsColon() string fileName = "testscript"; string fullPath = Path.Combine(tempDir, fileName); + string oldPath = Environment.GetEnvironmentVariable("PATH"); try { File.WriteAllText(fullPath, "#!/bin/sh\necho test"); @@ -70,20 +71,13 @@ public void TestResolvePath_PathSeparatorIsColon() File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); // Add temp directory to PATH using colon separator - string oldPath = Environment.GetEnvironmentVariable("PATH"); - try - { - Environment.SetEnvironmentVariable("PATH", tempDir + ":" + oldPath); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.Equal(fullPath, options.FileName); - } - finally - { - Environment.SetEnvironmentVariable("PATH", oldPath); - } + Environment.SetEnvironmentVariable("PATH", tempDir + ":" + oldPath); + ProcessStartOptions options = new ProcessStartOptions(fileName); + Assert.Equal(fullPath, options.FileName); } finally { + Environment.SetEnvironmentVariable("PATH", oldPath); if (File.Exists(fullPath)) { File.Delete(fullPath); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index 287cb573fedf16..1a00288aea5caa 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -9,7 +9,7 @@ namespace System.Diagnostics.Tests { public class ProcessStartOptionsTests_Windows { - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_AddsExeExtension() { // Test that .exe is appended when no extension is provided @@ -18,7 +18,7 @@ public void TestResolvePath_AddsExeExtension() Assert.True(File.Exists(options.FileName)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_DoesNotAddExeExtensionForTrailingDot() { // "If the file name ends in a period (.) with no extension, .exe is not appended." @@ -26,7 +26,7 @@ public void TestResolvePath_DoesNotAddExeExtensionForTrailingDot() Assert.Throws(() => new ProcessStartOptions("notepad.")); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_PreservesComExtension() { // The .com extension should be preserved @@ -72,37 +72,11 @@ public void TestResolvePath_FindsInSystemDirectory() [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] public void TestResolvePath_FindsInWindowsDirectory() { - // Some utilities are in Windows directory - // We'll test with a file that's commonly in Windows directory - // Note: This might not exist on all systems, so we make it conditional - try - { - ProcessStartOptions options = new ProcessStartOptions("notepad"); - Assert.True(File.Exists(options.FileName)); - } - catch (FileNotFoundException) - { - // Skip if notepad is not found - it's not critical for this test - } - } - - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] - public void TestResolvePath_FindsInPath() - { - // powershell.exe should be findable in PATH on most Windows systems - try - { - ProcessStartOptions options = new ProcessStartOptions("powershell"); - Assert.True(File.Exists(options.FileName)); - Assert.EndsWith(".exe", options.FileName, StringComparison.OrdinalIgnoreCase); - } - catch (FileNotFoundException) - { - // Skip if PowerShell is not found - it might not be in PATH - } + ProcessStartOptions options = new ProcessStartOptions("notepad"); + Assert.True(File.Exists(options.FileName)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_UsesCurrentDirectory() { string tempDir = Path.GetTempPath(); @@ -135,7 +109,7 @@ public void TestResolvePath_UsesCurrentDirectory() } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_PathSeparatorIsSemicolon() { // Create a temp directory and file @@ -174,7 +148,7 @@ public void TestResolvePath_PathSeparatorIsSemicolon() } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [Fact] public void TestResolvePath_AbsolutePathIsNotModified() { string tempFile = Path.GetTempFileName(); From c3517ee471e104bd4b8593e2369913a2e142c098 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 07:51:04 +0000 Subject: [PATCH 10/16] Refactor tests: partial classes, remove Test prefix, use new() syntax, Theory for utils Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessStartOptionsTests.Unix.cs | 106 ++++++------------ .../tests/ProcessStartOptionsTests.Windows.cs | 92 +++++++-------- .../tests/ProcessStartOptionsTests.cs | 104 ++++++----------- 3 files changed, 106 insertions(+), 196 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 7114346ef3b34f..ee8db2da0474f9 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -7,27 +7,35 @@ namespace System.Diagnostics.Tests { [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.FreeBSD | TestPlatforms.OSX)] - public class ProcessStartOptionsTests_Unix + public partial class ProcessStartOptionsTests { [Fact] - public void TestResolvePath_FindsInPath() + public void Constructor_ResolvesShOnUnix() + { + ProcessStartOptions options = new("sh"); + Assert.True(File.Exists(options.FileName)); + Assert.Equal("/bin/sh", options.FileName); + } + + [Fact] + public void ResolvePath_FindsInPath() { // sh should be findable in PATH on all Unix systems - ProcessStartOptions options = new ProcessStartOptions("sh"); + ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); Assert.EndsWith("sh", options.FileName); } [Fact] - public void TestResolvePath_DoesNotAddExeExtension() + public void ResolvePath_DoesNotAddExeExtension() { // On Unix, no .exe extension should be added - ProcessStartOptions options = new ProcessStartOptions("sh"); + ProcessStartOptions options = new("sh"); Assert.False(options.FileName.EndsWith(".exe", StringComparison.OrdinalIgnoreCase)); } [Fact] - public void TestResolvePath_UsesCurrentDirectory() + public void ResolvePath_UsesCurrentDirectory() { string tempDir = Path.GetTempPath(); string fileName = "testscript.sh"; @@ -41,8 +49,8 @@ public void TestResolvePath_UsesCurrentDirectory() File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); Directory.SetCurrentDirectory(tempDir); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.Equal(fullPath, options.FileName); + ProcessStartOptions options = new(fileName); + Assert.Equal(Path.GetFullPath(fullPath), options.FileName); } finally { @@ -55,7 +63,7 @@ public void TestResolvePath_UsesCurrentDirectory() } [Fact] - public void TestResolvePath_PathSeparatorIsColon() + public void ResolvePath_PathSeparatorIsColon() { // Create a temp directory and file string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); @@ -72,8 +80,8 @@ public void TestResolvePath_PathSeparatorIsColon() // Add temp directory to PATH using colon separator Environment.SetEnvironmentVariable("PATH", tempDir + ":" + oldPath); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.Equal(fullPath, options.FileName); + ProcessStartOptions options = new(fileName); + Assert.Equal(Path.GetFullPath(fullPath), options.FileName); } finally { @@ -90,48 +98,12 @@ public void TestResolvePath_PathSeparatorIsColon() } [Fact] - public void TestResolvePath_ChecksExecutablePermissions() - { - // Create a file without execute permissions - string tempDir = Path.GetTempPath(); - string fileName = "nonexecutable.sh"; - string fullPath = Path.Combine(tempDir, fileName); - - try - { - File.WriteAllText(fullPath, "#!/bin/sh\necho test"); - // Explicitly make it non-executable - File.SetUnixFileMode(fullPath, UnixFileMode.UserRead | UnixFileMode.UserWrite); - - // Save current directory - string oldDir = Directory.GetCurrentDirectory(); - try - { - Directory.SetCurrentDirectory(tempDir); - // Should throw because file is not executable - Assert.Throws(() => new ProcessStartOptions(fileName)); - } - finally - { - Directory.SetCurrentDirectory(oldDir); - } - } - finally - { - if (File.Exists(fullPath)) - { - File.Delete(fullPath); - } - } - } - - [Fact] - public void TestResolvePath_AbsolutePathIsNotModified() + public void ResolvePath_AbsolutePathIsNotModified() { string tempFile = Path.GetTempFileName(); try { - ProcessStartOptions options = new ProcessStartOptions(tempFile); + ProcessStartOptions options = new(tempFile); Assert.Equal(tempFile, options.FileName); } finally @@ -143,43 +115,35 @@ public void TestResolvePath_AbsolutePathIsNotModified() } } - [Fact] - public void TestResolvePath_FindsCommonUtilities() + [Theory] + [InlineData("ls")] + [InlineData("cat")] + [InlineData("echo")] + [InlineData("sh")] + public void ResolvePath_FindsCommonUtilities(string utilName) { - // Test common Unix utilities - string[] commonUtils = { "ls", "cat", "echo", "sh" }; - - foreach (string util in commonUtils) - { - ProcessStartOptions options = new ProcessStartOptions(util); - Assert.True(File.Exists(options.FileName), $"{util} should be found and exist"); - Assert.Contains(util, options.FileName); - } + ProcessStartOptions options = new(utilName); + Assert.True(File.Exists(options.FileName), $"{utilName} should be found and exist"); + Assert.Contains(utilName, options.FileName); } [Fact] - public void TestResolvePath_RejectsDirectories() + public void ResolvePath_RejectsDirectories() { // Create a directory with executable permissions string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); Directory.CreateDirectory(tempDir); + string oldDir = Directory.GetCurrentDirectory(); try { // Try to use the directory name as a command - string oldDir = Directory.GetCurrentDirectory(); - try - { - Directory.SetCurrentDirectory(Path.GetTempPath()); - Assert.Throws(() => new ProcessStartOptions(Path.GetFileName(tempDir))); - } - finally - { - Directory.SetCurrentDirectory(oldDir); - } + Directory.SetCurrentDirectory(Path.GetTempPath()); + Assert.Throws(() => new ProcessStartOptions(Path.GetFileName(tempDir))); } finally { + Directory.SetCurrentDirectory(oldDir); if (Directory.Exists(tempDir)) { Directory.Delete(tempDir); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index 1a00288aea5caa..fd75560d0677c0 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -7,52 +7,52 @@ namespace System.Diagnostics.Tests { - public class ProcessStartOptionsTests_Windows + public partial class ProcessStartOptionsTests { - [Fact] - public void TestResolvePath_AddsExeExtension() + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void Constructor_ResolvesCmdOnWindows() + { + ProcessStartOptions options = new("cmd"); + Assert.EndsWith("cmd.exe", options.FileName); + Assert.True(File.Exists(options.FileName)); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + public void ResolvePath_AddsExeExtension() { // Test that .exe is appended when no extension is provided - ProcessStartOptions options = new ProcessStartOptions("notepad"); + ProcessStartOptions options = new("notepad"); Assert.EndsWith(".exe", options.FileName, StringComparison.OrdinalIgnoreCase); Assert.True(File.Exists(options.FileName)); } [Fact] - public void TestResolvePath_DoesNotAddExeExtensionForTrailingDot() + public void ResolvePath_DoesNotAddExeExtensionForTrailingDot() { // "If the file name ends in a period (.) with no extension, .exe is not appended." // This should fail since "notepad." won't exist - Assert.Throws(() => new ProcessStartOptions("notepad.")); + Assert.Throws(() => new("notepad.")); } [Fact] - public void TestResolvePath_PreservesComExtension() + public void ResolvePath_PreservesComExtension() { // The .com extension should be preserved string fileName = "test.com"; string tempDir = Path.GetTempPath(); string fullPath = Path.Combine(tempDir, fileName); + string oldDir = Directory.GetCurrentDirectory(); try { File.WriteAllText(fullPath, "test"); - - // Save current directory - string oldDir = Directory.GetCurrentDirectory(); - try - { - Directory.SetCurrentDirectory(tempDir); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.EndsWith(".com", options.FileName, StringComparison.OrdinalIgnoreCase); - } - finally - { - Directory.SetCurrentDirectory(oldDir); - } + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new(fileName); + Assert.EndsWith(".com", options.FileName, StringComparison.OrdinalIgnoreCase); } finally { + Directory.SetCurrentDirectory(oldDir); if (File.Exists(fullPath)) { File.Delete(fullPath); @@ -61,47 +61,39 @@ public void TestResolvePath_PreservesComExtension() } [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] - public void TestResolvePath_FindsInSystemDirectory() + public void ResolvePath_FindsInSystemDirectory() { // cmd.exe should be found in system directory - ProcessStartOptions options = new ProcessStartOptions("cmd"); + ProcessStartOptions options = new("cmd"); Assert.True(File.Exists(options.FileName)); Assert.Contains("system32", options.FileName, StringComparison.OrdinalIgnoreCase); } [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] - public void TestResolvePath_FindsInWindowsDirectory() + public void ResolvePath_FindsInWindowsDirectory() { - ProcessStartOptions options = new ProcessStartOptions("notepad"); + ProcessStartOptions options = new("notepad"); Assert.True(File.Exists(options.FileName)); } [Fact] - public void TestResolvePath_UsesCurrentDirectory() + public void ResolvePath_UsesCurrentDirectory() { string tempDir = Path.GetTempPath(); string fileName = "testapp.exe"; string fullPath = Path.Combine(tempDir, fileName); + string oldDir = Directory.GetCurrentDirectory(); try { File.WriteAllText(fullPath, "test"); - - // Save current directory - string oldDir = Directory.GetCurrentDirectory(); - try - { - Directory.SetCurrentDirectory(tempDir); - ProcessStartOptions options = new ProcessStartOptions(fileName); - Assert.Equal(fullPath, options.FileName); - } - finally - { - Directory.SetCurrentDirectory(oldDir); - } + Directory.SetCurrentDirectory(tempDir); + ProcessStartOptions options = new(fileName); + Assert.Equal(fullPath, options.FileName); } finally { + Directory.SetCurrentDirectory(oldDir); if (File.Exists(fullPath)) { File.Delete(fullPath); @@ -110,7 +102,7 @@ public void TestResolvePath_UsesCurrentDirectory() } [Fact] - public void TestResolvePath_PathSeparatorIsSemicolon() + public void ResolvePath_PathSeparatorIsSemicolon() { // Create a temp directory and file string tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()); @@ -118,25 +110,17 @@ public void TestResolvePath_PathSeparatorIsSemicolon() string fileName = "testexe.exe"; string fullPath = Path.Combine(tempDir, fileName); + string oldPath = Environment.GetEnvironmentVariable("PATH"); try { File.WriteAllText(fullPath, "test"); - - // Add temp directory to PATH using semicolon separator - string oldPath = Environment.GetEnvironmentVariable("PATH"); - try - { - Environment.SetEnvironmentVariable("PATH", tempDir + ";" + oldPath); - ProcessStartOptions options = new ProcessStartOptions("testexe"); - Assert.Equal(fullPath, options.FileName); - } - finally - { - Environment.SetEnvironmentVariable("PATH", oldPath); - } + Environment.SetEnvironmentVariable("PATH", tempDir + ";" + oldPath); + ProcessStartOptions options = new("testexe"); + Assert.Equal(fullPath, options.FileName); } finally { + Environment.SetEnvironmentVariable("PATH", oldPath); if (File.Exists(fullPath)) { File.Delete(fullPath); @@ -149,7 +133,7 @@ public void TestResolvePath_PathSeparatorIsSemicolon() } [Fact] - public void TestResolvePath_AbsolutePathIsNotModified() + public void ResolvePath_AbsolutePathIsNotModified() { string tempFile = Path.GetTempFileName(); try @@ -159,7 +143,7 @@ public void TestResolvePath_AbsolutePathIsNotModified() File.Move(tempFile, noExtFile); tempFile = noExtFile; - ProcessStartOptions options = new ProcessStartOptions(tempFile); + ProcessStartOptions options = new(tempFile); Assert.Equal(tempFile, options.FileName); } finally diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs index 4bd075a8227948..75b361e3337877 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.cs @@ -10,51 +10,34 @@ namespace System.Diagnostics.Tests { - public class ProcessStartOptionsTests : ProcessTestBase + public partial class ProcessStartOptionsTests { [Fact] - public void TestConstructor_NullFileName_Throws() + public void Constructor_NullFileName_Throws() { Assert.Throws(() => new ProcessStartOptions(null)); } [Fact] - public void TestConstructor_EmptyFileName_Throws() + public void Constructor_EmptyFileName_Throws() { Assert.Throws(() => new ProcessStartOptions(string.Empty)); } [Fact] - public void TestConstructor_NonExistentFile_Throws() + public void Constructor_NonExistentFile_Throws() { string nonExistentFile = "ThisFileDoesNotExist_" + Guid.NewGuid().ToString(); Assert.Throws(() => new ProcessStartOptions(nonExistentFile)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))] - public void TestConstructor_ResolvesCmdOnWindows() - { - ProcessStartOptions options = new ProcessStartOptions("cmd"); - Assert.EndsWith("cmd.exe", options.FileName); - Assert.True(File.Exists(options.FileName)); - } - - [PlatformSpecific(TestPlatforms.Linux | TestPlatforms.FreeBSD | TestPlatforms.OSX)] [Fact] - public void TestConstructor_ResolvesShOnUnix() - { - ProcessStartOptions options = new ProcessStartOptions("sh"); - Assert.Contains("sh", options.FileName); - Assert.True(File.Exists(options.FileName)); - } - - [Fact] - public void TestConstructor_WithAbsolutePath() + public void Constructor_WithAbsolutePath() { string tempFile = Path.GetTempFileName(); try { - ProcessStartOptions options = new ProcessStartOptions(tempFile); + ProcessStartOptions options = new(tempFile); Assert.Equal(tempFile, options.FileName); } finally @@ -64,18 +47,18 @@ public void TestConstructor_WithAbsolutePath() } [Fact] - public void TestArguments_DefaultIsEmpty() + public void Arguments_DefaultIsEmpty() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); IList args = options.Arguments; Assert.NotNull(args); Assert.Empty(args); } [Fact] - public void TestArguments_CanAddAndModify() + public void Arguments_CanAddAndModify() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); options.Arguments.Add("arg1"); options.Arguments.Add("arg2"); Assert.Equal(2, options.Arguments.Count); @@ -88,24 +71,9 @@ public void TestArguments_CanAddAndModify() } [Fact] - public void TestEnvironment_LazyInitialization() + public void Environment_CanAddAndModify() { - if (PlatformDetection.IsiOS || PlatformDetection.IstvOS || PlatformDetection.IsMacCatalyst) - { - // Whole list of environment variables can no longer be accessed on non-OSX apple platforms - return; - } - - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); - IDictionary env = options.Environment; - Assert.NotNull(env); - Assert.NotEmpty(env); - } - - [Fact] - public void TestEnvironment_CanAddAndModify() - { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); IDictionary env = options.Environment; int originalCount = env.Count; @@ -121,9 +89,9 @@ public void TestEnvironment_CanAddAndModify() } [Fact] - public void TestEnvironment_CaseInsensitivityOnWindows() + public void Environment_CaseSensitivityIsPlatformSpecific() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); IDictionary env = options.Environment; env["TestKey"] = "TestValue"; @@ -140,80 +108,74 @@ public void TestEnvironment_CaseInsensitivityOnWindows() } [Fact] - public void TestInheritedHandles_DefaultIsEmpty() + public void InheritedHandles_DefaultIsEmpty() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); IList handles = options.InheritedHandles; Assert.NotNull(handles); Assert.Empty(handles); } [Fact] - public void TestInheritedHandles_CanSet() + public void InheritedHandles_CanSet() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); List newHandles = new List(); options.InheritedHandles = newHandles; Assert.Same(newHandles, options.InheritedHandles); } [Fact] - public void TestWorkingDirectory_DefaultIsNull() + public void WorkingDirectory_DefaultIsNull() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); Assert.Null(options.WorkingDirectory); } [Fact] - public void TestWorkingDirectory_CanSet() + public void WorkingDirectory_CanSet() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); string tempDir = Path.GetTempPath(); options.WorkingDirectory = tempDir; Assert.Equal(tempDir, options.WorkingDirectory); } [Fact] - public void TestKillOnParentExit_DefaultIsFalse() + public void KillOnParentExit_DefaultIsFalse() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); Assert.False(options.KillOnParentExit); } [Fact] - public void TestKillOnParentExit_CanSet() + public void KillOnParentExit_CanSet() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); options.KillOnParentExit = true; Assert.True(options.KillOnParentExit); } [Fact] - public void TestCreateNewProcessGroup_DefaultIsFalse() + public void CreateNewProcessGroup_DefaultIsFalse() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); Assert.False(options.CreateNewProcessGroup); } [Fact] - public void TestCreateNewProcessGroup_CanSet() + public void CreateNewProcessGroup_CanSet() { - ProcessStartOptions options = new ProcessStartOptions(GetCurrentProcessName()); + ProcessStartOptions options = new(GetCurrentProcessName()); options.CreateNewProcessGroup = true; Assert.True(options.CreateNewProcessGroup); } private string GetCurrentProcessName() { - // Get a valid executable path for testing - if (OperatingSystem.IsWindows()) - { - return Path.Combine(Environment.SystemDirectory, "cmd.exe"); - } - else - { - return Environment.ProcessPath ?? "/bin/sh"; - } + return Environment.ProcessPath ?? (OperatingSystem.IsWindows() + ? Path.Combine(Environment.SystemDirectory, "cmd.exe") + : "/bin/sh"); } } } From bbea91e0a3a5eaaafe1e7b24460f40d9e3a5dd3b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 08:03:00 +0000 Subject: [PATCH 11/16] Fix test conditions: remove redundant IsWindows check, use GetFullPath for sh Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessStartOptionsTests.Unix.cs | 2 +- .../tests/ProcessStartOptionsTests.Windows.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index ee8db2da0474f9..820411c954e747 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -14,7 +14,7 @@ public void Constructor_ResolvesShOnUnix() { ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); - Assert.Equal("/bin/sh", options.FileName); + Assert.Equal(Path.GetFullPath("/bin/sh"), options.FileName); } [Fact] diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index fd75560d0677c0..eb9fa34225029f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -9,7 +9,7 @@ namespace System.Diagnostics.Tests { public partial class ProcessStartOptionsTests { - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] public void Constructor_ResolvesCmdOnWindows() { ProcessStartOptions options = new("cmd"); From d9d381fef63440991a40ecfab8197cc9cd4e6b77 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 09:53:18 +0000 Subject: [PATCH 12/16] Fix test failures: use ResolveTarget helper for symlinks, fix lambda syntax Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessStartOptionsTests.Unix.cs | 10 ++++++++-- .../tests/ProcessStartOptionsTests.Windows.cs | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 820411c954e747..325d580d9a6673 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -14,7 +14,7 @@ public void Constructor_ResolvesShOnUnix() { ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); - Assert.Equal(Path.GetFullPath("/bin/sh"), options.FileName); + Assert.Equal(ResolveTarget("/bin/sh"), options.FileName); } [Fact] @@ -23,7 +23,13 @@ public void ResolvePath_FindsInPath() // sh should be findable in PATH on all Unix systems ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); - Assert.EndsWith("sh", options.FileName); + Assert.Equal(ResolveTarget("/bin/sh"), options.FileName); + } + + private static string ResolveTarget(string path) + { + FileSystemInfo? target = File.ResolveLinkTarget(path, returnFinalTarget: true); + return target?.FullName ?? path; } [Fact] diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index eb9fa34225029f..06371aae1bcd88 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -31,7 +31,7 @@ public void ResolvePath_DoesNotAddExeExtensionForTrailingDot() { // "If the file name ends in a period (.) with no extension, .exe is not appended." // This should fail since "notepad." won't exist - Assert.Throws(() => new("notepad.")); + Assert.Throws(() => new ProcessStartOptions("notepad.")); } [Fact] From b4d6e6495d7c730be47f45b1a724d775894b0f45 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 10:53:03 +0000 Subject: [PATCH 13/16] Fix Unix test failures: use EndsWith instead of exact path comparison Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessStartOptionsTests.Unix.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 325d580d9a6673..cf357680eb9da6 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -14,7 +14,8 @@ public void Constructor_ResolvesShOnUnix() { ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); - Assert.Equal(ResolveTarget("/bin/sh"), options.FileName); + // Verify the resolved path ends with "sh" (could be /bin/sh, /usr/bin/sh, etc.) + Assert.EndsWith("sh", options.FileName); } [Fact] @@ -23,7 +24,8 @@ public void ResolvePath_FindsInPath() // sh should be findable in PATH on all Unix systems ProcessStartOptions options = new("sh"); Assert.True(File.Exists(options.FileName)); - Assert.Equal(ResolveTarget("/bin/sh"), options.FileName); + // Verify the resolved path ends with "sh" (could be /bin/sh, /usr/bin/sh, etc.) + Assert.EndsWith("sh", options.FileName); } private static string ResolveTarget(string path) From 98d420113a6a647a22812a15db5e47770c17c621 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 14:34:26 +0000 Subject: [PATCH 14/16] Address code review: add file check, fix .exe logic, update comments, remove dead code Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/Resources/Strings.resx | 3 +++ .../src/System/Diagnostics/ProcessStartOptions.cs | 14 +++++++++++--- .../tests/ProcessStartOptionsTests.Unix.cs | 6 ------ .../tests/ProcessStartOptionsTests.Windows.cs | 7 ++++--- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index 3cc0d1e1d09e61..67d05840e9796b 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -336,4 +336,7 @@ Invalid performance counter data with type '{0}'. + + Could not resolve the file. + \ No newline at end of file diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 3ba446f13dcf7a..1d65eb12dc2076 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -133,7 +133,12 @@ public ProcessStartOptions(string fileName) { ArgumentException.ThrowIfNullOrEmpty(fileName); - _fileName = ResolvePath(fileName) ?? throw new FileNotFoundException("Could not resolve the file.", fileName); ; + string? resolved = ResolvePath(fileName); + if (resolved == null || !File.Exists(resolved)) + { + throw new FileNotFoundException(SR.FileNotFoundResolvePath, fileName); + } + _fileName = resolved; } internal static string? ResolvePath(string filename) @@ -153,7 +158,9 @@ public ProcessStartOptions(string fileName) // If the file name ends in a period (.) with no extension, or if the file name contains a path, .exe is not appended." // HasExtension returns false for trailing dot, so we need to check that separately - if (filename[filename.Length - 1] != '.' && !Path.HasExtension(filename)) + if (filename[filename.Length - 1] != '.' + && string.IsNullOrEmpty(Path.GetDirectoryName(filename)) + && !Path.HasExtension(filename)) { filename += ".exe"; } @@ -262,7 +269,8 @@ public ProcessStartOptions(string fileName) private static string? GetWindowsDirectory() { - // Don't cache Windows directory as it's user-specific and can change during app lifetime + // We don't cache the Windows directory; GetWindowsDirectoryW returns a stable system-wide path, + // and this method is not expected to be called frequently enough to warrant additional caching. Span buffer = stackalloc char[260]; // MAX_PATH uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); if (length > 0 && length < buffer.Length) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index cf357680eb9da6..22a2c535077f6a 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -28,12 +28,6 @@ public void ResolvePath_FindsInPath() Assert.EndsWith("sh", options.FileName); } - private static string ResolveTarget(string path) - { - FileSystemInfo? target = File.ResolveLinkTarget(path, returnFinalTarget: true); - return target?.FullName ?? path; - } - [Fact] public void ResolvePath_DoesNotAddExeExtension() { diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs index 06371aae1bcd88..1600c71e5ebfb4 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Windows.cs @@ -17,7 +17,7 @@ public void Constructor_ResolvesCmdOnWindows() Assert.True(File.Exists(options.FileName)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer), nameof(PlatformDetection.IsNotWindowsServerCore))] public void ResolvePath_AddsExeExtension() { // Test that .exe is appended when no extension is provided @@ -66,10 +66,11 @@ public void ResolvePath_FindsInSystemDirectory() // cmd.exe should be found in system directory ProcessStartOptions options = new("cmd"); Assert.True(File.Exists(options.FileName)); - Assert.Contains("system32", options.FileName, StringComparison.OrdinalIgnoreCase); + string systemDirectory = Environment.SystemDirectory; + Assert.StartsWith(systemDirectory, options.FileName, StringComparison.OrdinalIgnoreCase); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer), nameof(PlatformDetection.IsNotWindowsServerCore))] public void ResolvePath_FindsInWindowsDirectory() { ProcessStartOptions options = new("notepad"); From 5b024d4a24cdb6d65c2fb48f15258b0f6ca79657 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 14:54:27 +0000 Subject: [PATCH 15/16] Cache Windows directory value for performance Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../System/Diagnostics/ProcessStartOptions.cs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 1d65eb12dc2076..90d5acbf378f63 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -252,6 +252,7 @@ public ProcessStartOptions(string fileName) #if WINDOWS private static string? s_cachedSystemDirectory; + private static string? s_cachedWindowsDirectory; private static string? GetSystemDirectory() { @@ -269,15 +270,16 @@ public ProcessStartOptions(string fileName) private static string? GetWindowsDirectory() { - // We don't cache the Windows directory; GetWindowsDirectoryW returns a stable system-wide path, - // and this method is not expected to be called frequently enough to warrant additional caching. - Span buffer = stackalloc char[260]; // MAX_PATH - uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); - if (length > 0 && length < buffer.Length) + if (s_cachedWindowsDirectory == null) { - return new string(buffer.Slice(0, (int)length)); + Span buffer = stackalloc char[260]; // MAX_PATH + uint length = Interop.Kernel32.GetWindowsDirectoryW(ref MemoryMarshal.GetReference(buffer), (uint)buffer.Length); + if (length > 0 && length < buffer.Length) + { + s_cachedWindowsDirectory = new string(buffer.Slice(0, (int)length)); + } } - return null; + return s_cachedWindowsDirectory; } #endif } From add4af64bb0ef4b754ca90ac8a8b1dfab3210f61 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Feb 2026 19:54:45 +0000 Subject: [PATCH 16/16] Remove DictionaryWrapper and fix Unix path test for macOS symlinks Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/ProcessStartOptions.cs | 6 +++--- .../tests/ProcessStartOptionsTests.Unix.cs | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs index 90d5acbf378f63..fb12746af8399a 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartOptions.cs @@ -17,7 +17,7 @@ public sealed class ProcessStartOptions { private readonly string _fileName; private IList? _arguments; - private DictionaryWrapper? _environment; + private Dictionary? _environment; private IList? _inheritedHandles; /// @@ -52,9 +52,9 @@ public IList Arguments { IDictionary envVars = System.Environment.GetEnvironmentVariables(); - _environment = new DictionaryWrapper(new Dictionary( + _environment = new Dictionary( envVars.Count, - OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal)); + OperatingSystem.IsWindows() ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal); // Manual use of IDictionaryEnumerator instead of foreach to avoid DictionaryEntry box allocations. IDictionaryEnumerator e = envVars.GetEnumerator(); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs index 22a2c535077f6a..9aae9937f37f7d 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartOptionsTests.Unix.cs @@ -52,7 +52,8 @@ public void ResolvePath_UsesCurrentDirectory() Directory.SetCurrentDirectory(tempDir); ProcessStartOptions options = new(fileName); - Assert.Equal(Path.GetFullPath(fullPath), options.FileName); + // Use Path.GetFullPath on both sides to handle symlinks (e.g., /tmp -> /private/tmp on macOS) + Assert.Equal(Path.GetFullPath(fullPath), Path.GetFullPath(options.FileName)); } finally {