From 39bcc028278bf42d029b607a0eb668a6a769b167 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Tue, 17 Dec 2019 07:11:59 -0500 Subject: [PATCH 01/15] Remove WindowsProcessSignaler's reliance on creating/deleting a temp directory --- MedallionShell/MedallionShell.csproj | 6 +++--- MedallionShell/Signals/WindowsProcessSignaler.cs | 9 +++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/MedallionShell/MedallionShell.csproj b/MedallionShell/MedallionShell.csproj index dd25b4b..792086f 100644 --- a/MedallionShell/MedallionShell.csproj +++ b/MedallionShell/MedallionShell.csproj @@ -6,8 +6,8 @@ Medallion.Shell - 1.6.0 - 1.6.0.0 + 1.6.1-alpha01 + 1.6.1.0 1.6.0.0 Michael Adelson A lightweight, cross-platform library that simplifies working with processes in .NET @@ -38,7 +38,7 @@ True - + diff --git a/MedallionShell/Signals/WindowsProcessSignaler.cs b/MedallionShell/Signals/WindowsProcessSignaler.cs index f91729a..73b3528 100644 --- a/MedallionShell/Signals/WindowsProcessSignaler.cs +++ b/MedallionShell/Signals/WindowsProcessSignaler.cs @@ -112,11 +112,9 @@ private static async Task SendSignalFromCurrentProcess(int processId, Nati private static async Task DeploySignalerExeAsync() { - var tempDirectoryName = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N")); - Directory.CreateDirectory(tempDirectoryName); - const string SignalerExeName = "MedallionShell.ProcessSignaler.exe"; - var exePath = Path.Combine(tempDirectoryName, SignalerExeName); - using (var resourceStream = Helpers.GetMedallionShellAssembly().GetManifestResourceStream(SignalerExeName)) + const string SignalerExeNameWithoutExtension = "MedallionShell.ProcessSignaler"; + var exePath = Path.Combine(Path.GetTempPath(), $"{SignalerExeNameWithoutExtension}_{Guid.NewGuid():N}.exe"); + using (var resourceStream = Helpers.GetMedallionShellAssembly().GetManifestResourceStream(SignalerExeNameWithoutExtension + ".exe")) using (var fileStream = new FileStream(exePath, FileMode.CreateNew, FileAccess.Write, FileShare.None, Constants.ByteBufferSize, useAsync: true)) { await resourceStream.CopyToAsync(fileStream).ConfigureAwait(false); @@ -142,7 +140,6 @@ public void Dispose() if (toDelete != null) { File.Delete(toDelete); - Directory.Delete(System.IO.Path.GetDirectoryName(toDelete)); } } } From 9d4b6576b73f62e2242e8625424597cd2d7a734e Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sat, 21 Dec 2019 08:37:16 -0500 Subject: [PATCH 02/15] Fix versioning --- MedallionShell/MedallionShell.csproj | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/MedallionShell/MedallionShell.csproj b/MedallionShell/MedallionShell.csproj index 792086f..ae742bf 100644 --- a/MedallionShell/MedallionShell.csproj +++ b/MedallionShell/MedallionShell.csproj @@ -6,16 +6,15 @@ Medallion.Shell - 1.6.1-alpha01 + 1.6.1 1.6.1.0 - 1.6.0.0 + 1.6.1.0 Michael Adelson A lightweight, cross-platform library that simplifies working with processes in .NET Copyright © 2017 Michael Adelson MIT process async shell stdin stdout stderr pipe redirect signal kill ctrlc controlc https://github.com/madelson/MedallionShell - See https://github.com/madelson/MedallionShell#release-notes From e23140b47f422a34add1970f7d14c1c007b7660b Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sat, 21 Dec 2019 11:24:04 -0500 Subject: [PATCH 03/15] Try to fix travis build with newer .net --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index f4371a8..365c96e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ solution: MedallionShell.sln mono: latest # recommended here: https://travis-ci.community/t/c-builds-are-failing-net-core-installation-error/3063 dist: xenial -dotnet: 2.2.1 +dotnet: 3.1.0 install: - nuget restore MedallionShell.sln - nuget install NUnit.Console -Version 3.10.0 -OutputDirectory testrunner From 60b6d4ed51ad9e210e6592bf7aa341b43300f4ff Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sat, 21 Dec 2019 11:34:11 -0500 Subject: [PATCH 04/15] Switch to dotnet latest on travis --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 365c96e..a9cbd29 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ solution: MedallionShell.sln mono: latest # recommended here: https://travis-ci.community/t/c-builds-are-failing-net-core-installation-error/3063 dist: xenial -dotnet: 3.1.0 +dotnet: latest install: - nuget restore MedallionShell.sln - nuget install NUnit.Console -Version 3.10.0 -OutputDirectory testrunner From d07edcd326e3295f5d41d732256d2a64e8256b24 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sat, 21 Dec 2019 11:48:20 -0500 Subject: [PATCH 05/15] dotnet version fixes --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index a9cbd29..8d228ff 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ solution: MedallionShell.sln mono: latest # recommended here: https://travis-ci.community/t/c-builds-are-failing-net-core-installation-error/3063 dist: xenial -dotnet: latest +dotnet: 3.1 install: - nuget restore MedallionShell.sln - nuget install NUnit.Console -Version 3.10.0 -OutputDirectory testrunner From 78fbc2a289037dda4fa06911cddf986f14d580c2 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sat, 21 Dec 2019 12:16:56 -0500 Subject: [PATCH 06/15] Run tests on dotnet 3.1 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 8d228ff..675a6b2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,4 +13,4 @@ script: - msbuild /p:Configuration=Release MedallionShell.ProcessSignaler/MedallionShell.ProcessSignaler.csproj - msbuild /p:Configuration=Release MedallionShell.sln - mono ./testrunner/NUnit.ConsoleRunner.3.10.0/tools/nunit3-console.exe ./MedallionShell.Tests/bin/Release/net46/MedallionShell.Tests.dll - - dotnet test --no-build -f netcoreapp2.2 -c Release + - dotnet test --no-build -f netcoreapp3.1 -c Release From fecf83316f40882c9fff0203aaaaf25693f4f6fd Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sat, 21 Dec 2019 15:18:40 -0500 Subject: [PATCH 07/15] Temporary change to TestProcessKeepsWritingAfterOutputIsClosed to debug linux failure --- MedallionShell.Tests/GeneralTest.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/MedallionShell.Tests/GeneralTest.cs b/MedallionShell.Tests/GeneralTest.cs index e969726..11337c1 100644 --- a/MedallionShell.Tests/GeneralTest.cs +++ b/MedallionShell.Tests/GeneralTest.cs @@ -575,7 +575,7 @@ public void TestCommandOption() [Test] public void TestProcessKeepsWritingAfterOutputIsClosed() { - var command = TestShell.Run(SampleCommand, new[] { "pipe" }, options: o => o.ThrowOnError()); + var command = TestShell.Run(SampleCommand, new[] { "pipe" }/*, options: o => o.ThrowOnError()*/); command.StandardOutput.Dispose(); for (var i = 0; i < 100; ++i) { @@ -585,6 +585,8 @@ public void TestProcessKeepsWritingAfterOutputIsClosed() command.StandardInput.Dispose(); command.Task.Wait(TimeSpan.FromSeconds(1000)).ShouldEqual(true); + Console.WriteLine($"Result: {command.Result.ExitCode}, STDERR: {command.Result.StandardError}"); + Assert.IsTrue(command.Result.Success); } private IEnumerable ErrorLines() From 254f90f1cf81b9f23536b9c2dcb0aff79afab89e Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sat, 21 Dec 2019 15:47:14 -0500 Subject: [PATCH 08/15] Switched assertion to be different for mono runtime. --- MedallionShell.Tests/GeneralTest.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/MedallionShell.Tests/GeneralTest.cs b/MedallionShell.Tests/GeneralTest.cs index 11337c1..fffef8c 100644 --- a/MedallionShell.Tests/GeneralTest.cs +++ b/MedallionShell.Tests/GeneralTest.cs @@ -575,7 +575,7 @@ public void TestCommandOption() [Test] public void TestProcessKeepsWritingAfterOutputIsClosed() { - var command = TestShell.Run(SampleCommand, new[] { "pipe" }/*, options: o => o.ThrowOnError()*/); + var command = TestShell.Run(SampleCommand, new[] { "pipe" }); command.StandardOutput.Dispose(); for (var i = 0; i < 100; ++i) { @@ -585,8 +585,7 @@ public void TestProcessKeepsWritingAfterOutputIsClosed() command.StandardInput.Dispose(); command.Task.Wait(TimeSpan.FromSeconds(1000)).ShouldEqual(true); - Console.WriteLine($"Result: {command.Result.ExitCode}, STDERR: {command.Result.StandardError}"); - Assert.IsTrue(command.Result.Success); + command.Result.Success.ShouldEqual(Type.GetType("Mono.Runtime") == null); } private IEnumerable ErrorLines() From ccde498e2657e93ae9a0e3989c9aa7891574a421 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sat, 21 Dec 2019 16:26:59 -0500 Subject: [PATCH 09/15] Fix mono compat in test --- MedallionShell.Tests/GeneralTest.cs | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/MedallionShell.Tests/GeneralTest.cs b/MedallionShell.Tests/GeneralTest.cs index fffef8c..a2ec355 100644 --- a/MedallionShell.Tests/GeneralTest.cs +++ b/MedallionShell.Tests/GeneralTest.cs @@ -581,11 +581,26 @@ public void TestProcessKeepsWritingAfterOutputIsClosed() { command.StandardInput.WriteLine(new string('a', i)); } - command.Task.IsCompleted.ShouldEqual(false); - command.StandardInput.Dispose(); - command.Task.Wait(TimeSpan.FromSeconds(1000)).ShouldEqual(true); - command.Result.Success.ShouldEqual(Type.GetType("Mono.Runtime") == null); + // workaround for https://github.com/mono/mono/issues/18279; so far + // I've encountered this only on Mono Linux + if (Type.GetType("Mono.Runtime") != null + && !RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + command.StandardInput.Dispose(); + command.Task.Wait(TimeSpan.FromSeconds(1000)).ShouldEqual(true); + command.Result.ExitCode.ShouldEqual(1); + // SampleCommand fails because it's attempt to write to Console.Out fails hard + Assert.That(command.Result.StandardError, Does.Contain("System.IO.IOException: Write fault")); + } + else + { + command.Task.IsCompleted.ShouldEqual(false); + + command.StandardInput.Dispose(); + command.Task.Wait(TimeSpan.FromSeconds(1000)).ShouldEqual(true); + command.Result.Success.ShouldEqual(Type.GetType("Mono.Runtime") == null); + } } private IEnumerable ErrorLines() From d2fcd118706c1bb64304a8a02f818edc9ade9a2c Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sat, 21 Dec 2019 22:27:53 -0500 Subject: [PATCH 10/15] Update to C# 8 + some additional dependency and signaling cleanup --- .../MedallionShell.ProcessSignaler.csproj | 17 +++++- MedallionShell.Tests/AttachingTests.cs | 13 +++-- MedallionShell.Tests/CommandLineSyntaxTest.cs | 4 +- MedallionShell.Tests/GeneralTest.cs | 4 +- .../MedallionShell.Tests.csproj | 9 +-- MedallionShell.Tests/SignalTest.cs | 4 +- MedallionShell.Tests/Streams/PipeTest.cs | 4 +- MedallionShell.Tests/UnitTestHelpers.cs | 6 +- MedallionShell/Command.cs | 41 ++++++-------- MedallionShell/CommandLineSyntax.cs | 2 +- MedallionShell/MedallionShell.csproj | 4 +- MedallionShell/PipedCommand.cs | 14 ++--- MedallionShell/PlatformCompatibilityHelper.cs | 4 +- MedallionShell/ProcessCommand.cs | 55 +++++-------------- MedallionShell/ProcessHelper.cs | 6 +- MedallionShell/Shell.cs | 23 ++++---- MedallionShell/Shims.cs | 15 +++++ MedallionShell/Signals/NativeMethods.cs | 2 +- .../Signals/WindowsProcessSignaler.cs | 34 +++--------- MedallionShell/Streams/Pipe.cs | 34 +++++------- MedallionShell/Streams/PipeHelpers.cs | 4 +- MedallionShell/Utilities/Helpers.cs | 4 +- SampleCommand/SampleCommand.csproj | 3 +- 23 files changed, 135 insertions(+), 171 deletions(-) create mode 100644 MedallionShell/Shims.cs diff --git a/MedallionShell.ProcessSignaler/MedallionShell.ProcessSignaler.csproj b/MedallionShell.ProcessSignaler/MedallionShell.ProcessSignaler.csproj index 42e366c..1038d98 100644 --- a/MedallionShell.ProcessSignaler/MedallionShell.ProcessSignaler.csproj +++ b/MedallionShell.ProcessSignaler/MedallionShell.ProcessSignaler.csproj @@ -4,17 +4,28 @@ net45;netstandard1.3 Exe false - 7.2 + Latest + enable True ..\stylecop.analyzers.ruleset - true + + TRACE;DEBUG + + + + + True + + True + TRACE + + All - diff --git a/MedallionShell.Tests/AttachingTests.cs b/MedallionShell.Tests/AttachingTests.cs index 1db178c..c28afa3 100644 --- a/MedallionShell.Tests/AttachingTests.cs +++ b/MedallionShell.Tests/AttachingTests.cs @@ -24,7 +24,7 @@ public void TestWaitingForAttachedProcessExit() var processCommand = TestShell.Run(SampleCommand, new[] { "sleep", "100" }); Command.TryAttachToProcess(processCommand.ProcessId, out var attachedCommand) .ShouldEqual(true, "Attaching to process failed."); - var commandResult = attachedCommand.Task; + var commandResult = attachedCommand!.Task; commandResult.IsCompleted.ShouldEqual(false, "Task has finished too early."); Thread.Sleep(300); commandResult.IsCompleted.ShouldEqual(true, "Task has not finished on time."); @@ -36,7 +36,7 @@ public void TestGettingExitCodeFromAttachedProcess() var processCommand = TestShell.Run(SampleCommand, new[] { "exit", "16" }); Command.TryAttachToProcess(processCommand.ProcessId, out var attachedCommand) .ShouldEqual(true, "Attaching to process failed."); - var task = attachedCommand.Task; + var task = attachedCommand!.Task; task.Wait(1000).ShouldEqual(true, "Task has not finished on time."); task.Result.ExitCode.ShouldEqual(16, "Exit code was not correct."); } @@ -61,7 +61,7 @@ public void TestKillingAttachedProcess() out var attachedCommand) .ShouldEqual(true, "Attaching to process failed."); - attachedCommand.Kill(); + attachedCommand!.Kill(); attachedCommand.Task.Wait(TimeSpan.FromSeconds(1)) .ShouldEqual(true, "The process is still alive after Kill() has finished."); @@ -81,7 +81,7 @@ public void TestAttachingWithAlreadyCanceledToken() .ShouldEqual(true, "attaching failed"); using (attachedCommand) { - attachedCommand.Process.WaitForExit(1000).ShouldEqual(true, "The process wasn't killed."); + attachedCommand!.Process.WaitForExit(1000).ShouldEqual(true, "The process wasn't killed."); } } @@ -94,10 +94,11 @@ public void TestTimeout() Command.TryAttachToProcess( processId, options => options.Timeout(TimeSpan.FromMilliseconds(150)), - out var attachedCommand); + out var attachedCommand) + .ShouldEqual(true); // the timeout is counted from the moment we attached to the process so it shouldn't throw at this moment - attachedCommand.Task.Wait(100); + attachedCommand!.Task.Wait(100); // but should eventually throw var exception = Assert.Throws( diff --git a/MedallionShell.Tests/CommandLineSyntaxTest.cs b/MedallionShell.Tests/CommandLineSyntaxTest.cs index cbc0181..b0be6ca 100644 --- a/MedallionShell.Tests/CommandLineSyntaxTest.cs +++ b/MedallionShell.Tests/CommandLineSyntaxTest.cs @@ -15,8 +15,8 @@ public class CommandLineSyntaxTest public void TestArgumentValidation([Values] bool isWindowsSyntax) { var syntax = isWindowsSyntax ? new WindowsCommandLineSyntax() : new MonoUnixCommandLineSyntax().As(); - Assert.Throws(() => syntax.CreateArgumentString(null)); - Assert.Throws(() => syntax.CreateArgumentString(new[] { "a", null, "b" })); + Assert.Throws(() => syntax.CreateArgumentString(null!)); + Assert.Throws(() => syntax.CreateArgumentString(new[] { "a", null!, "b" })); } [TestCase(" ")] diff --git a/MedallionShell.Tests/GeneralTest.cs b/MedallionShell.Tests/GeneralTest.cs index a2ec355..1ec8ae9 100644 --- a/MedallionShell.Tests/GeneralTest.cs +++ b/MedallionShell.Tests/GeneralTest.cs @@ -584,7 +584,7 @@ public void TestProcessKeepsWritingAfterOutputIsClosed() // workaround for https://github.com/mono/mono/issues/18279; so far // I've encountered this only on Mono Linux - if (Type.GetType("Mono.Runtime") != null + if (PlatformCompatibilityHelper.IsMono && !RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { command.StandardInput.Dispose(); @@ -599,7 +599,7 @@ public void TestProcessKeepsWritingAfterOutputIsClosed() command.StandardInput.Dispose(); command.Task.Wait(TimeSpan.FromSeconds(1000)).ShouldEqual(true); - command.Result.Success.ShouldEqual(Type.GetType("Mono.Runtime") == null); + command.Result.Success.ShouldEqual(true); } } diff --git a/MedallionShell.Tests/MedallionShell.Tests.csproj b/MedallionShell.Tests/MedallionShell.Tests.csproj index d86cd6d..b4fe639 100644 --- a/MedallionShell.Tests/MedallionShell.Tests.csproj +++ b/MedallionShell.Tests/MedallionShell.Tests.csproj @@ -3,10 +3,11 @@ net46;netcoreapp2.2 false - 7.2 - True - ..\stylecop.analyzers.ruleset - true + Latest + enable + True + ..\stylecop.analyzers.ruleset + true 1591 diff --git a/MedallionShell.Tests/SignalTest.cs b/MedallionShell.Tests/SignalTest.cs index bf4ed6c..8b0fd29 100644 --- a/MedallionShell.Tests/SignalTest.cs +++ b/MedallionShell.Tests/SignalTest.cs @@ -46,7 +46,7 @@ public async Task CanSendControlCToPipeline() public async Task HandlesEdgeCases() { var command = TestShell.Run(SampleCommand, "sleep", "10000"); - Assert.Throws(() => command.TrySignalAsync(null).GetType()); + Assert.Throws(() => command.TrySignalAsync(null!).GetType()); command.Kill(); await command.Task; @@ -99,7 +99,7 @@ public async Task CanSendSignalToSelf(int signal) try { - (await thisCommand.TrySignalAsync(CommandSignal.FromSystemValue(signal))).ShouldEqual(true); + (await thisCommand!.TrySignalAsync(CommandSignal.FromSystemValue(signal))).ShouldEqual(true); manualResetEvent.Wait(TimeSpan.FromSeconds(5)).ShouldEqual(true); } finally diff --git a/MedallionShell.Tests/Streams/PipeTest.cs b/MedallionShell.Tests/Streams/PipeTest.cs index b5bf737..9a6f512 100644 --- a/MedallionShell.Tests/Streams/PipeTest.cs +++ b/MedallionShell.Tests/Streams/PipeTest.cs @@ -260,7 +260,7 @@ public void TestPipeChainWithFixedLengthPipes() asyncWrite.Wait(TimeSpan.FromSeconds(10)).ShouldEqual(true); asyncRead.Wait(TimeSpan.FromSeconds(10)).ShouldEqual(true); asyncRead.Result.ShouldNotEqual(null); - asyncRead.Result.Length.ShouldEqual(longText.Length); + asyncRead.Result!.Length.ShouldEqual(longText.Length); asyncRead.Result.ShouldEqual(longText); } @@ -364,7 +364,7 @@ public static Task WriteTextAsync(this Pipe @this, string text) return new StreamWriter(@this.InputStream) { AutoFlush = true }.WriteAsync(text); } - public static async Task ReadTextAsync(this Pipe @this, int count, CancellationToken token = default(CancellationToken)) + public static async Task ReadTextAsync(this Pipe @this, int count, CancellationToken token = default(CancellationToken)) { var bytes = new byte[count]; var bytesRead = 0; diff --git a/MedallionShell.Tests/UnitTestHelpers.cs b/MedallionShell.Tests/UnitTestHelpers.cs index 1f549ce..af36bf7 100644 --- a/MedallionShell.Tests/UnitTestHelpers.cs +++ b/MedallionShell.Tests/UnitTestHelpers.cs @@ -22,19 +22,19 @@ public static class UnitTestHelpers public static Shell MakeTestShell(Action options) => new Shell(TestShell.Configuration + options); - public static T ShouldEqual(this T @this, T that, string message = null) + public static T ShouldEqual(this T @this, T that, string? message = null) { Assert.AreEqual(that, @this, message); return @this; } - public static T ShouldNotEqual(this T @this, T that, string message = null) + public static T ShouldNotEqual(this T @this, T that, string? message = null) { Assert.AreNotEqual(that, @this, message); return @this; } - public static string ShouldContain(this string haystack, string needle, string message = null) + public static string ShouldContain(this string haystack, string needle, string? message = null) { Assert.IsNotNull(haystack, $"Expected: contains '{needle}'. Was: NULL{(message != null ? $" ({message})" : string.Empty)}"); if (!haystack.Contains(needle)) diff --git a/MedallionShell/Command.cs b/MedallionShell/Command.cs index 8cb68d0..997ed4a 100644 --- a/MedallionShell/Command.cs +++ b/MedallionShell/Command.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.ComponentModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; using System.Text; @@ -382,13 +383,12 @@ public Command RedirectFrom(TextReader reader) /// public static Command operator >(Command command, IEnumerable lines) { - Throw.IfNull(command, "command"); - Throw.IfNull(lines, "lines"); - - var linesCollection = lines as ICollection; - Throw.If(linesCollection == null, "lines: must implement ICollection in order to recieve output"); + Throw.IfNull(command, nameof(command)); + Throw.IfNull(lines, nameof(lines)); - return command.RedirectTo(linesCollection); + return lines is ICollection linesCollection + ? command.RedirectTo(linesCollection) + : throw new ArgumentException("must implement ICollection in order to recieve output", nameof(lines)); } /// @@ -410,13 +410,12 @@ public Command RedirectFrom(TextReader reader) /// public static Command operator >(Command command, IEnumerable chars) { - Throw.IfNull(command, "command"); - Throw.IfNull(chars, "chars"); - - var charCollection = chars as ICollection; - Throw.If(charCollection == null, "chars: must implement ICollection in order to receive output"); + Throw.IfNull(command, nameof(command)); + Throw.IfNull(chars, nameof(chars)); - return command.RedirectTo(charCollection); + return chars is ICollection charCollection + ? command.RedirectTo(charCollection) + : throw new ArgumentException("must implement ICollection in order to receive output", nameof(chars)); } /// @@ -436,26 +435,20 @@ public Command RedirectFrom(TextReader reader) /// /// A convenience method for calling on /// - public static Command Run(string executable, IEnumerable arguments = null, Action options = null) - { - return Shell.Default.Run(executable, arguments, options); - } + public static Command Run(string executable, IEnumerable? arguments = null, Action? options = null) => + Shell.Default.Run(executable, arguments, options); /// /// A convenience method for calling on /// - public static bool TryAttachToProcess(int processId, Action options, out Command attachedCommand) - { - return Shell.Default.TryAttachToProcess(processId, options, out attachedCommand); - } + public static bool TryAttachToProcess(int processId, Action options, [NotNullWhen(returnValue: true)] out Command? attachedCommand) => + Shell.Default.TryAttachToProcess(processId, options, out attachedCommand); /// /// A convenience method for calling on /// - public static bool TryAttachToProcess(int processId, out Command attachedCommand) - { - return Shell.Default.TryAttachToProcess(processId, out attachedCommand); - } + public static bool TryAttachToProcess(int processId, [NotNullWhen(returnValue: true)] out Command? attachedCommand) => + Shell.Default.TryAttachToProcess(processId, out attachedCommand); /// /// A convenience method for calling on diff --git a/MedallionShell/CommandLineSyntax.cs b/MedallionShell/CommandLineSyntax.cs index 036e73f..86329a0 100644 --- a/MedallionShell/CommandLineSyntax.cs +++ b/MedallionShell/CommandLineSyntax.cs @@ -28,7 +28,7 @@ internal static string CreateArgumentString(IEnumerable arguments, Actio var isFirstArgument = true; foreach (var argument in arguments) { - Throw.If(argument == null, nameof(arguments) + ": must not contain null"); + if (argument == null) { throw new ArgumentException("must not contain null", nameof(arguments)); } if (isFirstArgument) { isFirstArgument = false; } else { builder.Append(' '); } diff --git a/MedallionShell/MedallionShell.csproj b/MedallionShell/MedallionShell.csproj index ae742bf..da01294 100644 --- a/MedallionShell/MedallionShell.csproj +++ b/MedallionShell/MedallionShell.csproj @@ -19,7 +19,9 @@ - 4 + 4 + Latest + enable True ..\stylecop.analyzers.ruleset diff --git a/MedallionShell/PipedCommand.cs b/MedallionShell/PipedCommand.cs index 07727b3..2b67923 100644 --- a/MedallionShell/PipedCommand.cs +++ b/MedallionShell/PipedCommand.cs @@ -34,19 +34,13 @@ public override Process Process get { return this.second.Process; } } - private IReadOnlyList processes; - public override IReadOnlyList Processes - { - get { return this.processes ?? (this.processes = this.first.Processes.Concat(this.second.Processes).ToList().AsReadOnly()); } - } + private IReadOnlyList? processes; + public override IReadOnlyList Processes => this.processes ??= this.first.Processes.Concat(this.second.Processes).ToList().AsReadOnly(); public override int ProcessId => this.second.ProcessId; - private IReadOnlyList processIds; - public override IReadOnlyList ProcessIds - { - get { return this.processIds ?? (this.processIds = this.first.ProcessIds.Concat(this.second.ProcessIds).ToList().AsReadOnly()); } - } + private IReadOnlyList? processIds; + public override IReadOnlyList ProcessIds => this.processIds ??= this.first.ProcessIds.Concat(this.second.ProcessIds).ToList().AsReadOnly(); public override Task Task { diff --git a/MedallionShell/PlatformCompatibilityHelper.cs b/MedallionShell/PlatformCompatibilityHelper.cs index 9f9d6be..7d95e7c 100644 --- a/MedallionShell/PlatformCompatibilityHelper.cs +++ b/MedallionShell/PlatformCompatibilityHelper.cs @@ -11,7 +11,7 @@ namespace Medallion.Shell internal static class PlatformCompatibilityHelper { // see http://www.mono-project.com/docs/faq/technical/ - private static readonly bool IsMono = Type.GetType("Mono.Runtime") != null; + public static readonly bool IsMono = Type.GetType("Mono.Runtime") != null; public static bool IsWindows { @@ -78,7 +78,7 @@ public static int SafeGetExitCode(this Process process) /// /// If https://github.com/mono/mono/issues/8478 is ever addressed, we wouldn't need this any more. /// - public static bool SafeStart(this Process process, out StreamWriter standardInput, out StreamReader standardOutput, out StreamReader standardError) + public static bool SafeStart(this Process process, out StreamWriter? standardInput, out StreamReader? standardOutput, out StreamReader? standardError) { var redirectStandardInput = process.StartInfo.RedirectStandardInput; var redirectStandardOutput = process.StartInfo.RedirectStandardOutput; diff --git a/MedallionShell/ProcessCommand.cs b/MedallionShell/ProcessCommand.cs index 91f555b..5df63e2 100644 --- a/MedallionShell/ProcessCommand.cs +++ b/MedallionShell/ProcessCommand.cs @@ -27,7 +27,7 @@ internal ProcessCommand( bool disposeOnExit, TimeSpan timeout, CancellationToken cancellationToken, - Encoding standardInputEncoding) + Encoding? standardInputEncoding) { this.disposeOnExit = disposeOnExit; this.fileName = startInfo.FileName; @@ -39,17 +39,17 @@ internal ProcessCommand( this.process.SafeStart(out var processStandardInput, out var processStandardOutput, out var processStandardError); var ioTasks = new List(capacity: 2); - if (startInfo.RedirectStandardOutput) + if (processStandardOutput != null) { this.standardOutputReader = new InternalProcessStreamReader(processStandardOutput); ioTasks.Add(this.standardOutputReader.Task); } - if (startInfo.RedirectStandardError) + if (processStandardError != null) { this.standardErrorReader = new InternalProcessStreamReader(processStandardError); ioTasks.Add(this.standardErrorReader.Task); } - if (startInfo.RedirectStandardInput) + if (processStandardInput != null) { // unfortunately, changing the encoding can't be done via ProcessStartInfo so we have to do it manually here. // See https://github.com/dotnet/corefx/issues/20497 @@ -113,11 +113,8 @@ public override System.Diagnostics.Process Process } } - private IReadOnlyList processes; - public override IReadOnlyList Processes - { - get { return this.processes ?? (this.processes = new ReadOnlyCollection(new[] { this.Process })); } - } + private IReadOnlyList? processes; + public override IReadOnlyList Processes => this.processes ??= new ReadOnlyCollection(new[] { this.Process }); private readonly object processIdOrExceptionDispatchInfo; public override int ProcessId @@ -135,41 +132,17 @@ public override int ProcessId } } - private IReadOnlyList processIds; - public override IReadOnlyList ProcessIds - { - get { return this.processIds ?? (this.processIds = new ReadOnlyCollection(new[] { this.ProcessId })); } - } + private IReadOnlyList? processIds; + public override IReadOnlyList ProcessIds => this.processIds ??= new ReadOnlyCollection(new[] { this.ProcessId }); - private readonly ProcessStreamWriter standardInput; - public override ProcessStreamWriter StandardInput - { - get - { - Throw.If(this.standardInput == null, "Standard input is not redirected"); - return this.standardInput; - } - } + private readonly ProcessStreamWriter? standardInput; + public override ProcessStreamWriter StandardInput => this.standardInput ?? throw new InvalidOperationException("Standard input is not redirected"); - private readonly InternalProcessStreamReader standardOutputReader; - public override ProcessStreamReader StandardOutput - { - get - { - Throw.If(this.standardOutputReader == null, "Standard output is not redirected"); - return this.standardOutputReader; - } - } + private readonly InternalProcessStreamReader? standardOutputReader; + public override ProcessStreamReader StandardOutput => this.standardOutputReader ?? throw new InvalidOperationException("Standard output is not redirected"); - private readonly InternalProcessStreamReader standardErrorReader; - public override Streams.ProcessStreamReader StandardError - { - get - { - Throw.If(this.standardErrorReader == null, "Standard error is not redirected"); - return this.standardErrorReader; - } - } + private readonly InternalProcessStreamReader? standardErrorReader; + public override Streams.ProcessStreamReader StandardError => this.standardErrorReader ?? throw new InvalidOperationException("Standard error is not redirected"); private readonly Task task; public override Task Task { get { return this.task; } } diff --git a/MedallionShell/ProcessHelper.cs b/MedallionShell/ProcessHelper.cs index 37d0aa1..bcef9e6 100644 --- a/MedallionShell/ProcessHelper.cs +++ b/MedallionShell/ProcessHelper.cs @@ -11,12 +11,12 @@ internal static class ProcessHelper public const string ProcessNotAccessibleWithDisposeOnExitEnabled = "Process can only be accessed when the command is not set to dispose on exit. This is to prevent non-deterministic code which may access the process before or after it exits"; - private static object _boxedCurrentProcessId; + private static object? _boxedCurrentProcessId; public static int CurrentProcessId => (int)LazyInitializer.EnsureInitialized( ref _boxedCurrentProcessId, () => { using (var process = Process.GetCurrentProcess()) { return (object)process.Id; } } - ); + )!; public static void TryKillProcess(Process process) { @@ -54,7 +54,7 @@ public static Task CreateProcessTask( var taskBuilder = new TaskCompletionSource(); var disposables = new List(); - object resultObject = null; + object? resultObject = null; if (cancellationToken.CanBeCanceled) { diff --git a/MedallionShell/Shell.cs b/MedallionShell/Shell.cs index 152390f..0d4e323 100644 --- a/MedallionShell/Shell.cs +++ b/MedallionShell/Shell.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.ComponentModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq; using System.Text; @@ -31,7 +32,7 @@ public Shell(Action options) /// Executes the given with the given and /// /// - public Command Run(string executable, IEnumerable arguments = null, Action options = null) + public Command Run(string executable, IEnumerable? arguments = null, Action? options = null) { Throw.If(string.IsNullOrEmpty(executable), "executable is required"); @@ -77,17 +78,15 @@ public Command Run(string executable, IEnumerable arguments = null, Acti /// giving representing the process and returning /// true if this succeeded, otherwise false. /// - public bool TryAttachToProcess(int processId, out Command attachedCommand) - { - return this.TryAttachToProcess(processId, options: null, attachedCommand: out attachedCommand); - } + public bool TryAttachToProcess(int processId, [NotNullWhen(returnValue: true)] out Command? attachedCommand) => + this.TryAttachToProcess(processId, options: null, attachedCommand: out attachedCommand); /// /// Tries to attach to an already running process, given its /// and , giving representing /// the process and returning true if this succeeded, otherwise false. /// - public bool TryAttachToProcess(int processId, Action options, out Command attachedCommand) + public bool TryAttachToProcess(int processId, Action? options, [NotNullWhen(returnValue: true)] out Command? attachedCommand) { var finalOptions = this.GetOptions(options); if (finalOptions.ProcessStreamEncoding != null || finalOptions.StartInfoInitializers.Count != 0) @@ -97,7 +96,7 @@ public bool TryAttachToProcess(int processId, Action options, out } attachedCommand = null; - Process process = null; + Process? process = null; try { process = Process.GetProcessById(processId); @@ -150,7 +149,7 @@ public Command Run(string executable, params object[] arguments) public static Shell Default { get; } = new Shell(_ => { }); #endregion - private Options GetOptions(Action additionalConfiguration) + private Options GetOptions(Action? additionalConfiguration) { var builder = new Options(); this.Configuration.Invoke(builder); @@ -170,13 +169,13 @@ internal Options() this.RestoreDefaults(); } - internal List> StartInfoInitializers { get; private set; } - internal List> CommandInitializers { get; private set; } - internal CommandLineSyntax CommandLineSyntax { get; private set; } + internal List> StartInfoInitializers { get; private set; } = default!; // assigned in RestoreDefaults + internal List> CommandInitializers { get; private set; } = default!; // assigned in RestoreDefaults + internal CommandLineSyntax CommandLineSyntax { get; private set; } = default!; // assigned in RestoreDefaults internal bool ThrowExceptionOnError { get; private set; } internal bool DisposeProcessOnExit { get; private set; } internal TimeSpan ProcessTimeout { get; private set; } - internal Encoding ProcessStreamEncoding { get; private set; } + internal Encoding? ProcessStreamEncoding { get; private set; } internal CancellationToken ProcessCancellationToken { get; private set; } #region ---- Builder methods ---- diff --git a/MedallionShell/Shims.cs b/MedallionShell/Shims.cs new file mode 100644 index 0000000..946b9db --- /dev/null +++ b/MedallionShell/Shims.cs @@ -0,0 +1,15 @@ +using System; +using System.Collections.Generic; +using System.Text; + +namespace System.Diagnostics.CodeAnalysis +{ + [SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1649:File name should match first type name", Justification = "Shim")] + [System.AttributeUsage(System.AttributeTargets.Parameter, Inherited = false)] + internal sealed class NotNullWhenAttribute : Attribute + { + public NotNullWhenAttribute(bool returnValue) { this.ReturnValue = returnValue; } + + public bool ReturnValue { get; } + } +} diff --git a/MedallionShell/Signals/NativeMethods.cs b/MedallionShell/Signals/NativeMethods.cs index bb8d07a..2abcff5 100644 --- a/MedallionShell/Signals/NativeMethods.cs +++ b/MedallionShell/Signals/NativeMethods.cs @@ -26,7 +26,7 @@ public enum CtrlType : uint public static extern uint GetConsoleProcessList(uint[] lpdwProcessList, uint dwProcessCount); [DllImport("kernel32.dll", SetLastError = true)] - public static extern bool SetConsoleCtrlHandler(ConsoleCtrlDelegate handlerRoutine, bool add); + public static extern bool SetConsoleCtrlHandler(ConsoleCtrlDelegate? handlerRoutine, bool add); [DllImport("kernel32.dll", SetLastError = true)] public static extern bool GenerateConsoleCtrlEvent(CtrlType dwCtrlEvent, uint dwProcessGroupId); diff --git a/MedallionShell/Signals/WindowsProcessSignaler.cs b/MedallionShell/Signals/WindowsProcessSignaler.cs index 73b3528..7507d08 100644 --- a/MedallionShell/Signals/WindowsProcessSignaler.cs +++ b/MedallionShell/Signals/WindowsProcessSignaler.cs @@ -31,11 +31,16 @@ public static async Task TrySignalAsync(int processId, NativeMethods.CtrlT return await SendSignalFromCurrentProcess(processId, signal).ConfigureAwait(false); } - using (var file = await DeploySignalerExeAsync().ConfigureAwait(false)) + var exeFile = await DeploySignalerExeAsync().ConfigureAwait(false); + try { - var command = Command.Run(file.Path, new object[] { processId, (int)signal }); + var command = Command.Run(exeFile, new object[] { processId, (int)signal }); return (await command.Task.ConfigureAwait(false)).Success; } + finally + { + File.Delete(exeFile); + } } internal static bool HasSameConsole(int processId) @@ -110,7 +115,7 @@ private static async Task SendSignalFromCurrentProcess(int processId, Nati } } - private static async Task DeploySignalerExeAsync() + private static async Task DeploySignalerExeAsync() { const string SignalerExeNameWithoutExtension = "MedallionShell.ProcessSignaler"; var exePath = Path.Combine(Path.GetTempPath(), $"{SignalerExeNameWithoutExtension}_{Guid.NewGuid():N}.exe"); @@ -119,29 +124,8 @@ private static async Task DeploySignalerExeAsync() { await resourceStream.CopyToAsync(fileStream).ConfigureAwait(false); } - - return new TemporaryExeFile(exePath); - } - - private class TemporaryExeFile : IDisposable - { - private string _path; - public TemporaryExeFile(string path) - { - this._path = path; - } - - public string Path => this._path; - - public void Dispose() - { - var toDelete = Interlocked.Exchange(ref this._path, null); - if (toDelete != null) - { - File.Delete(toDelete); - } - } + return exePath; } } } diff --git a/MedallionShell/Streams/Pipe.cs b/MedallionShell/Streams/Pipe.cs index 73a7321..9cbf9ea 100644 --- a/MedallionShell/Streams/Pipe.cs +++ b/MedallionShell/Streams/Pipe.cs @@ -30,7 +30,7 @@ internal sealed class Pipe private byte[] buffer = Empty; private int start, count; private bool writerClosed, readerClosed; - private SemaphoreSlim spaceAvailableSignal; + private SemaphoreSlim? spaceAvailableSignal; private Task readTask = CompletedZeroTask; private Task writeTask = CompletedZeroTask; @@ -190,7 +190,7 @@ private async Task WriteNoLockAsync(byte[] buffer, int offset, int count, TimeSp } // acquire the semaphore - var acquired = await this.spaceAvailableSignal.WaitAsync(timeoutToUse, cancellationTokenToUse).ConfigureAwait(false); + var acquired = await this.spaceAvailableSignal!.WaitAsync(timeoutToUse, cancellationTokenToUse).ConfigureAwait(false); if (!acquired) { throw new TimeoutException("Timed out writing to the pipe"); @@ -447,13 +447,10 @@ private void InternalCloseReadSideNoLock() #region ---- Dispose ---- private void CleanupNoLock() { - this.buffer = null; - this.readTask = null; + this.buffer = Empty; + this.writeTask = this.readTask = CompletedZeroTask; this.bytesAvailableSignal.Dispose(); - if (this.spaceAvailableSignal != null) - { - this.spaceAvailableSignal.Dispose(); - } + this.spaceAvailableSignal?.Dispose(); } #endregion @@ -549,9 +546,9 @@ public override int EndRead(IAsyncResult asyncResult) public override void EndWrite(IAsyncResult asyncResult) { - Throw.IfNull(asyncResult, "asyncResult"); - var writeResult = asyncResult as AsyncWriteResult; - Throw.If(writeResult == null || writeResult.Stream != this, "asyncResult: must be created by this stream's BeginWrite method"); + Throw.IfNull(asyncResult, nameof(asyncResult)); + var writeResult = asyncResult as AsyncWriteResult + ?? throw new ArgumentException("must be created by this stream's BeginWrite method", nameof(asyncResult)); writeResult.WriteTask.Wait(); } #endif @@ -647,10 +644,8 @@ public override int WriteTimeout } } - private static NotSupportedException WriteOnly([CallerMemberName] string memberName = null) - { + private static NotSupportedException WriteOnly([CallerMemberName] string memberName = "") => throw new NotSupportedException(memberName + ": the stream is write only"); - } } #endregion @@ -735,10 +730,9 @@ protected override void Dispose(bool disposing) #if !NETSTANDARD1_3 public override int EndRead(IAsyncResult asyncResult) { - Throw.IfNull(asyncResult, "asyncResult"); - var readResult = asyncResult as AsyncReadResult; - Throw.If(readResult == null || readResult.Stream != this, "asyncResult: must be created by this stream's BeginRead method"); - + Throw.IfNull(asyncResult, nameof(asyncResult)); + var readResult = asyncResult as AsyncReadResult + ?? throw new ArgumentException("must be created by this stream's BeginRead method", nameof(asyncResult)); return readResult.ReadTask.Result; } @@ -841,10 +835,8 @@ public override int WriteTimeout set { throw ReadOnly(); } } - private static NotSupportedException ReadOnly([CallerMemberName] string memberName = null) - { + private static NotSupportedException ReadOnly([CallerMemberName] string memberName = "") => throw new NotSupportedException(memberName + ": the stream is read only"); - } } #endregion } diff --git a/MedallionShell/Streams/PipeHelpers.cs b/MedallionShell/Streams/PipeHelpers.cs index e43aa65..af05c34 100644 --- a/MedallionShell/Streams/PipeHelpers.cs +++ b/MedallionShell/Streams/PipeHelpers.cs @@ -38,7 +38,7 @@ public static async Task CopyToAsyncWithAutoFlush(this Stream source, Stream des // the overhead of flushing on each write call var buffer = new byte[Constants.ByteBufferSize]; - Task flushTask = null; + Task? flushTask = null; try { while (true) @@ -83,7 +83,7 @@ public static async Task CopyToAsyncWithAutoFlush(this Stream source, Stream des } } - public static async Task PipeAsync(this IDisposable @this, Func pipeTaskFactory, bool leaveOpen, Action extraDisposeAction = null) + public static async Task PipeAsync(this IDisposable @this, Func pipeTaskFactory, bool leaveOpen, Action? extraDisposeAction = null) { try { diff --git a/MedallionShell/Utilities/Helpers.cs b/MedallionShell/Utilities/Helpers.cs index 9556d75..ba2eccd 100644 --- a/MedallionShell/Utilities/Helpers.cs +++ b/MedallionShell/Utilities/Helpers.cs @@ -81,10 +81,8 @@ public static void IfInvalidBuffer(T[] buffer, int offset, int count) } } - public static NotSupportedException NotSupported([CallerMemberName] string memberName = null) - { + public static NotSupportedException NotSupported([CallerMemberName] string memberName = "") => throw new NotSupportedException(memberName); - } } internal static class Throw diff --git a/SampleCommand/SampleCommand.csproj b/SampleCommand/SampleCommand.csproj index dc9a822..9d9b025 100644 --- a/SampleCommand/SampleCommand.csproj +++ b/SampleCommand/SampleCommand.csproj @@ -4,7 +4,8 @@ net46;netcoreapp2.2 Exe false - 7.2 + Latest + enable True ..\stylecop.analyzers.ruleset true From 44adf1b10f8abc2829554f90531512c36c14260b Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sat, 21 Dec 2019 22:30:25 -0500 Subject: [PATCH 11/15] Update appveyor to VS2019 --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 9a976b2..c321996 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,5 +1,5 @@ version: 1.0.{build} -image: Visual Studio 2017 +image: Visual Studio 2019 build_script: # see comment in .travis.yml for why we need to build ProcessSignaler explicitly - cmd: >- From e746ae4b0d354776bd31b5a1a943b93846675240 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sun, 22 Dec 2019 17:45:50 -0500 Subject: [PATCH 12/15] Implement better error messages for accessing standardoutput/standarderror on iocommands (fix #59). --- MedallionShell.Tests/IoCommandTest.cs | 72 +++++++++++++++++++++++++++ MedallionShell/Command.cs | 30 +++++------ MedallionShell/CommandResult.cs | 17 ++++--- MedallionShell/IoCommand.cs | 68 ++++++++++++++++++------- 4 files changed, 148 insertions(+), 39 deletions(-) create mode 100644 MedallionShell.Tests/IoCommandTest.cs diff --git a/MedallionShell.Tests/IoCommandTest.cs b/MedallionShell.Tests/IoCommandTest.cs new file mode 100644 index 0000000..78a6996 --- /dev/null +++ b/MedallionShell.Tests/IoCommandTest.cs @@ -0,0 +1,72 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Medallion.Shell.Tests; +using NUnit.Framework; + +namespace MedallionShell.Tests +{ + using static UnitTestHelpers; + + public class IOCommandTest + { + [Test] + public void TestStandardOutCannotBeAccessedAfterRedirectingIt() + { + var output = new List(); + var command = TestShell.Run(SampleCommand, "argecho", "a"); + var ioCommand = command.RedirectTo(output); + + var errorMessage = Assert.Throws(() => ioCommand.StandardOutput.GetHashCode()).Message; + errorMessage.ShouldEqual("StandardOutput is unavailable because it is already being piped to System.Collections.Generic.List`1[System.String]"); + + Assert.DoesNotThrow(() => command.StandardOutput.GetHashCode()); + + Assert.Throws(() => ioCommand.Result.StandardOutput.GetHashCode()) + .Message + .ShouldEqual(errorMessage); + Assert.Throws(() => command.Result.StandardOutput.GetHashCode()); + + CollectionAssert.AreEqual(new[] { "a" }, output); + ioCommand.Result.StandardError.ShouldEqual(command.Result.StandardError).ShouldEqual(string.Empty); + } + + [Test] + public void TestStandardErrorCannotBeAccessedAfterRedirectingIt() + { + var output = new List(); + var command = TestShell.Run(SampleCommand, "argecho", "a"); + var ioCommand = command.RedirectStandardErrorTo(output); + + var errorMessage = Assert.Throws(() => ioCommand.StandardError.GetHashCode()).Message; + errorMessage.ShouldEqual("StandardError is unavailable because it is already being piped to System.Collections.Generic.List`1[System.String]"); + + Assert.DoesNotThrow(() => command.StandardError.GetHashCode()); + + Assert.Throws(() => ioCommand.Result.StandardError.GetHashCode()) + .Message + .ShouldEqual(errorMessage); + Assert.Throws(() => command.Result.StandardError.GetHashCode()); + + Assert.IsEmpty(output); + ioCommand.Result.StandardOutput.ShouldEqual(command.Result.StandardOutput).ShouldEqual("a\r\n"); + } + + [Test] + public void TestStandardInputCannotBeAccessedAfterRedirectingIt() + { + var command = TestShell.Run(SampleCommand, "echo"); + var ioCommand = command.RedirectFrom(new[] { "a" }); + + var errorMessage = Assert.Throws(() => ioCommand.StandardInput.GetHashCode()).Message; + errorMessage.ShouldEqual("StandardInput is unavailable because it is already being piped from System.String[]"); + + Assert.DoesNotThrow(() => command.StandardInput.GetHashCode()); + + ioCommand.Result.StandardOutput.ShouldEqual(command.Result.StandardOutput).ShouldEqual("a\r\n"); + ioCommand.Result.StandardError.ShouldEqual(command.Result.StandardError).ShouldEqual(string.Empty); + } + } +} diff --git a/MedallionShell/Command.cs b/MedallionShell/Command.cs index 997ed4a..49f30be 100644 --- a/MedallionShell/Command.cs +++ b/MedallionShell/Command.cs @@ -136,7 +136,7 @@ public Command RedirectTo(Stream stream) { Throw.IfNull(stream, nameof(stream)); - return new IoCommand(this, this.StandardOutput.PipeToAsync(stream, leaveStreamOpen: true), ">", stream); + return new IOCommand(this, this.StandardOutput.PipeToAsync(stream, leaveStreamOpen: true), StandardIOStream.Out, stream); } /// @@ -148,7 +148,7 @@ public Command RedirectStandardErrorTo(Stream stream) { Throw.IfNull(stream, nameof(stream)); - return new IoCommand(this, this.StandardError.PipeToAsync(stream, leaveStreamOpen: true), "2>", stream); + return new IOCommand(this, this.StandardError.PipeToAsync(stream, leaveStreamOpen: true), StandardIOStream.Error, stream); } /// @@ -160,7 +160,7 @@ public Command RedirectFrom(Stream stream) { Throw.IfNull(stream, nameof(stream)); - return new IoCommand(this, this.StandardInput.PipeFromAsync(stream, leaveStreamOpen: true), "<", stream); + return new IOCommand(this, this.StandardInput.PipeFromAsync(stream, leaveStreamOpen: true), StandardIOStream.In, stream); } /// @@ -172,7 +172,7 @@ public Command RedirectTo(FileInfo file) { Throw.IfNull(file, nameof(file)); - return new IoCommand(this, this.StandardOutput.PipeToAsync(file), ">", file); + return new IOCommand(this, this.StandardOutput.PipeToAsync(file), StandardIOStream.Out, file); } /// @@ -184,7 +184,7 @@ public Command RedirectStandardErrorTo(FileInfo file) { Throw.IfNull(file, nameof(file)); - return new IoCommand(this, this.StandardError.PipeToAsync(file), "2>", file); + return new IOCommand(this, this.StandardError.PipeToAsync(file), StandardIOStream.Error, file); } /// @@ -196,7 +196,7 @@ public Command RedirectFrom(FileInfo file) { Throw.IfNull(file, nameof(file)); - return new IoCommand(this, this.StandardInput.PipeFromAsync(file), "<", file); + return new IOCommand(this, this.StandardInput.PipeFromAsync(file), StandardIOStream.In, file); } /// @@ -208,7 +208,7 @@ public Command RedirectTo(ICollection lines) { Throw.IfNull(lines, nameof(lines)); - return new IoCommand(this, this.StandardOutput.PipeToAsync(lines), ">", lines.GetType()); + return new IOCommand(this, this.StandardOutput.PipeToAsync(lines), StandardIOStream.Out, lines.GetType()); } /// @@ -220,7 +220,7 @@ public Command RedirectStandardErrorTo(ICollection lines) { Throw.IfNull(lines, nameof(lines)); - return new IoCommand(this, this.StandardError.PipeToAsync(lines), "2>", lines.GetType()); + return new IOCommand(this, this.StandardError.PipeToAsync(lines), StandardIOStream.Error, lines.GetType()); } /// @@ -232,7 +232,7 @@ public Command RedirectFrom(IEnumerable lines) { Throw.IfNull(lines, nameof(lines)); - return new IoCommand(this, this.StandardInput.PipeFromAsync(lines), "<", lines.GetType()); + return new IOCommand(this, this.StandardInput.PipeFromAsync(lines), StandardIOStream.In, lines.GetType()); } /// @@ -244,7 +244,7 @@ public Command RedirectTo(ICollection chars) { Throw.IfNull(chars, nameof(chars)); - return new IoCommand(this, this.StandardOutput.PipeToAsync(chars), ">", chars.GetType()); + return new IOCommand(this, this.StandardOutput.PipeToAsync(chars), StandardIOStream.Out, chars.GetType()); } /// @@ -256,7 +256,7 @@ public Command RedirectStandardErrorTo(ICollection chars) { Throw.IfNull(chars, nameof(chars)); - return new IoCommand(this, this.StandardError.PipeToAsync(chars), "2>", chars.GetType()); + return new IOCommand(this, this.StandardError.PipeToAsync(chars), StandardIOStream.Error, chars.GetType()); } /// @@ -268,7 +268,7 @@ public Command RedirectFrom(IEnumerable chars) { Throw.IfNull(chars, nameof(chars)); - return new IoCommand(this, this.StandardInput.PipeFromAsync(chars), "<", chars.GetType()); + return new IOCommand(this, this.StandardInput.PipeFromAsync(chars), StandardIOStream.In, chars.GetType()); } /// @@ -280,7 +280,7 @@ public Command RedirectTo(TextWriter writer) { Throw.IfNull(writer, nameof(writer)); - return new IoCommand(this, this.StandardOutput.PipeToAsync(writer, leaveWriterOpen: true), ">", writer); + return new IOCommand(this, this.StandardOutput.PipeToAsync(writer, leaveWriterOpen: true), StandardIOStream.Out, writer); } /// @@ -292,7 +292,7 @@ public Command RedirectStandardErrorTo(TextWriter writer) { Throw.IfNull(writer, nameof(writer)); - return new IoCommand(this, this.StandardError.PipeToAsync(writer, leaveWriterOpen: true), "2>", writer); + return new IOCommand(this, this.StandardError.PipeToAsync(writer, leaveWriterOpen: true), StandardIOStream.Error, writer); } /// @@ -304,7 +304,7 @@ public Command RedirectFrom(TextReader reader) { Throw.IfNull(reader, nameof(reader)); - return new IoCommand(this, this.StandardInput.PipeFromAsync(reader, leaveReaderOpen: true), "<", reader); + return new IOCommand(this, this.StandardInput.PipeFromAsync(reader, leaveReaderOpen: true), StandardIOStream.In, reader); } /// diff --git a/MedallionShell/CommandResult.cs b/MedallionShell/CommandResult.cs index 6313917..4773470 100644 --- a/MedallionShell/CommandResult.cs +++ b/MedallionShell/CommandResult.cs @@ -15,30 +15,35 @@ public sealed class CommandResult private readonly Lazy standardOutput, standardError; internal CommandResult(int exitCode, Command command) + : this(exitCode, () => command.StandardOutput.ReadToEnd(), () => command.StandardError.ReadToEnd()) + { + } + + internal CommandResult(int exitCode, Func standardOutput, Func standardError) { this.ExitCode = exitCode; - this.standardOutput = new Lazy(() => command.StandardOutput.ReadToEnd()); - this.standardError = new Lazy(() => command.StandardError.ReadToEnd()); + this.standardOutput = new Lazy(standardOutput); + this.standardError = new Lazy(standardError); } /// /// The exit code of the command's process /// - public int ExitCode { get; private set; } + public int ExitCode { get; } /// /// Returns true iff the exit code is 0 (indicating success) /// - public bool Success { get { return this.ExitCode == 0; } } + public bool Success => this.ExitCode == 0; /// /// If available, the full standard output text of the command /// - public string StandardOutput { get { return this.standardOutput.Value; } } + public string StandardOutput => this.standardOutput.Value; /// /// If available, the full standard error text of the command /// - public string StandardError { get { return this.standardError.Value; } } + public string StandardError => this.standardError.Value; } } diff --git a/MedallionShell/IoCommand.cs b/MedallionShell/IoCommand.cs index badfd61..600c328 100644 --- a/MedallionShell/IoCommand.cs +++ b/MedallionShell/IoCommand.cs @@ -6,26 +6,46 @@ namespace Medallion.Shell { - internal sealed class IoCommand : Command + internal sealed class IOCommand : Command { private readonly Command command; private readonly Task task; + private readonly StandardIOStream standardIOStream; // for toString - private readonly string @operator; private readonly object sourceOrSink; - public IoCommand(Command command, Task ioTask, string @operator, object sourceOrSink) + public IOCommand(Command command, Task ioTask, StandardIOStream standardIOStream, object sourceOrSink) { this.command = command; this.task = this.CreateTask(ioTask); - this.@operator = @operator; + this.standardIOStream = standardIOStream; this.sourceOrSink = sourceOrSink; } private async Task CreateTask(Task ioTask) { await ioTask.ConfigureAwait(false); - return await this.command.Task.ConfigureAwait(false); + var innerResult = await this.command.Task.ConfigureAwait(false); + + // We wrap the inner command's result so that we can apply our stream availability error + // checking (the Ignore() calls). However, we use the inner result's string values since + // accessing those consumes the stream and we want both this result and the inner result + // to have the value. + return new CommandResult( + innerResult.ExitCode, + standardOutput: () => + { + Ignore(this.StandardOutput); + return innerResult.StandardOutput; + }, + standardError: () => + { + Ignore(this.StandardError); + return innerResult.StandardError; + } + ); + + void Ignore(object ignored) { } } public override System.Diagnostics.Process Process @@ -41,20 +61,17 @@ public override IReadOnlyList Processes public override int ProcessId => this.command.ProcessId; public override IReadOnlyList ProcessIds => this.command.ProcessIds; - public override Streams.ProcessStreamWriter StandardInput - { - get { return this.command.StandardInput; } - } + public override Streams.ProcessStreamWriter StandardInput => this.standardIOStream != StandardIOStream.In + ? this.command.StandardInput + : throw new InvalidOperationException($"{nameof(this.StandardInput)} is unavailable because it is already being piped from {this.sourceOrSink}"); - public override Streams.ProcessStreamReader StandardOutput - { - get { return this.command.StandardOutput; } - } + public override Streams.ProcessStreamReader StandardOutput => this.standardIOStream != StandardIOStream.Out + ? this.command.StandardOutput + : throw new InvalidOperationException($"{nameof(this.StandardOutput)} is unavailable because it is already being piped to {this.sourceOrSink}"); - public override Streams.ProcessStreamReader StandardError - { - get { return this.command.StandardError; } - } + public override Streams.ProcessStreamReader StandardError => this.standardIOStream != StandardIOStream.Error + ? this.command.StandardError + : throw new InvalidOperationException($"{nameof(this.StandardError)} is unavailable because it is already being piped to {this.sourceOrSink}"); public override Task Task { @@ -66,11 +83,26 @@ public override void Kill() this.command.Kill(); } - public override string ToString() => $"{this.command} {this.@operator} {this.sourceOrSink}"; + public override string ToString() => $"{this.command} {ToString(this.standardIOStream)} {this.sourceOrSink}"; protected override void DisposeInternal() { this.command.As().Dispose(); } + + private static string ToString(StandardIOStream standardIOStream) => standardIOStream switch + { + StandardIOStream.In => "<", + StandardIOStream.Out => ">", + StandardIOStream.Error => "2>", + _ => throw new InvalidOperationException("should never get here") + }; + } + + internal enum StandardIOStream + { + In, + Out, + Error, } } From 10750530d003b661b992f865df8ebd677fbbc413 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sun, 22 Dec 2019 18:08:31 -0500 Subject: [PATCH 13/15] Use cross-platform line endings in test --- MedallionShell.Tests/IoCommandTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MedallionShell.Tests/IoCommandTest.cs b/MedallionShell.Tests/IoCommandTest.cs index 78a6996..330632c 100644 --- a/MedallionShell.Tests/IoCommandTest.cs +++ b/MedallionShell.Tests/IoCommandTest.cs @@ -51,7 +51,7 @@ public void TestStandardErrorCannotBeAccessedAfterRedirectingIt() Assert.Throws(() => command.Result.StandardError.GetHashCode()); Assert.IsEmpty(output); - ioCommand.Result.StandardOutput.ShouldEqual(command.Result.StandardOutput).ShouldEqual("a\r\n"); + ioCommand.Result.StandardOutput.ShouldEqual(command.Result.StandardOutput).ShouldEqual($"a{Environment.NewLine}"); } [Test] @@ -65,7 +65,7 @@ public void TestStandardInputCannotBeAccessedAfterRedirectingIt() Assert.DoesNotThrow(() => command.StandardInput.GetHashCode()); - ioCommand.Result.StandardOutput.ShouldEqual(command.Result.StandardOutput).ShouldEqual("a\r\n"); + ioCommand.Result.StandardOutput.ShouldEqual(command.Result.StandardOutput).ShouldEqual($"a{Environment.NewLine}"); ioCommand.Result.StandardError.ShouldEqual(command.Result.StandardError).ShouldEqual(string.Empty); } } From ccedc227dbed362291dfeea78c546f62d404864e Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Mon, 23 Dec 2019 08:23:12 -0500 Subject: [PATCH 14/15] Release notes --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 51122b6..ebdcfa5 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,11 @@ Windows: [![Build status](https://ci.appveyor.com/api/projects/status/9idbmymiat Linux: [![Build Status](https://travis-ci.com/madelson/MedallionShell.svg?branch=master)](https://travis-ci.com/madelson/MedallionShell) ## Release Notes +- 1.6.1 + - Fixes transient error in signaling on Windows machines with slow disks ([#61](https://github.com/madelson/MedallionShell/issues/61)) + - Reduces dependency footprint for .NET Standard 2.0 ([#56](https://github.com/madelson/MedallionShell/issues/56)) + - Improves error messaging when trying to access the standard IO streams of Commands when those streams have been piped elsewhere ([#59](https://github.com/madelson/MedallionShell/issues/59)) + - Adds C#8 nullable reference type annotations - 1.6.0 - Adds `Command.TryAttachToProcess` API for creating a `Command` attached to an already-running process ([#30](https://github.com/madelson/MedallionShell/issues/30)). Thanks [konrad-kruczynski](konrad-kruczynski) for coming up with the idea and implementing! - Adds `Command.TrySignal` API which provides cross-platform support for the CTRL+C (SIGINT) signal as well as support for OS-specific signals ([#35](https://github.com/madelson/MedallionShell/issues/35)) From 78020f93512584546ae9af7ca245b97527545760 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Mon, 23 Dec 2019 08:26:10 -0500 Subject: [PATCH 15/15] Additional readme changes --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ebdcfa5..86ff9a4 100644 --- a/README.md +++ b/README.md @@ -117,7 +117,7 @@ Linux: [![Build Status](https://travis-ci.com/madelson/MedallionShell.svg?branch - 1.6.1 - Fixes transient error in signaling on Windows machines with slow disks ([#61](https://github.com/madelson/MedallionShell/issues/61)) - Reduces dependency footprint for .NET Standard 2.0 ([#56](https://github.com/madelson/MedallionShell/issues/56)) - - Improves error messaging when trying to access the standard IO streams of Commands when those streams have been piped elsewhere ([#59](https://github.com/madelson/MedallionShell/issues/59)) + - Improves error messaging when trying to access the standard IO streams of Commands when those streams have been piped elsewhere ([#59](https://github.com/madelson/MedallionShell/issues/59), [#41](https://github.com/madelson/MedallionShell/issues/41)) - Adds C#8 nullable reference type annotations - 1.6.0 - Adds `Command.TryAttachToProcess` API for creating a `Command` attached to an already-running process ([#30](https://github.com/madelson/MedallionShell/issues/30)). Thanks [konrad-kruczynski](konrad-kruczynski) for coming up with the idea and implementing!