diff --git a/src/GitHub.Api/Git/Tasks/GitCommitTask.cs b/src/GitHub.Api/Git/Tasks/GitCommitTask.cs index 68c5b4085..b6d17809b 100644 --- a/src/GitHub.Api/Git/Tasks/GitCommitTask.cs +++ b/src/GitHub.Api/Git/Tasks/GitCommitTask.cs @@ -15,7 +15,7 @@ public GitCommitTask(string message, string body, Guard.ArgumentNotNullOrWhiteSpace(message, "message"); Name = TaskName; - arguments = "commit "; + arguments = "-c i18n.commitencoding=utf8 commit "; arguments += String.Format(" -m \"{0}\"", message); if (!String.IsNullOrEmpty(body)) arguments += String.Format(" -m \"{0}\"", body); diff --git a/src/GitHub.Api/Git/Tasks/GitLogTask.cs b/src/GitHub.Api/Git/Tasks/GitLogTask.cs index eb09b8f5c..d416ae660 100644 --- a/src/GitHub.Api/Git/Tasks/GitLogTask.cs +++ b/src/GitHub.Api/Git/Tasks/GitLogTask.cs @@ -15,7 +15,7 @@ public GitLogTask(IGitObjectFactory gitObjectFactory, public override string ProcessArguments { - get { return @"log --pretty=format:""%H%n%P%n%aN%n%aE%n%aI%n%cN%n%cE%n%cI%n%B---GHUBODYEND---"" --name-status"; } + get { return @"-c i18n.logoutputencoding=utf8 -c core.quotepath=false log --pretty=format:""%H%n%P%n%aN%n%aE%n%aI%n%cN%n%cE%n%cI%n%B---GHUBODYEND---"" --name-status"; } } } } diff --git a/src/GitHub.Api/Git/Tasks/GitStatusTask.cs b/src/GitHub.Api/Git/Tasks/GitStatusTask.cs index a574cef6a..7ee47111f 100644 --- a/src/GitHub.Api/Git/Tasks/GitStatusTask.cs +++ b/src/GitHub.Api/Git/Tasks/GitStatusTask.cs @@ -15,7 +15,7 @@ public GitStatusTask(IGitObjectFactory gitObjectFactory, public override string ProcessArguments { - get { return "status -b -u --ignored --porcelain"; } + get { return "-c i18n.logoutputencoding=utf8 -c core.quotepath=false status -b -u --ignored --porcelain"; } } public override TaskAffinity Affinity { get { return TaskAffinity.Exclusive; } } } diff --git a/src/GitHub.Api/GitHub.Api.csproj b/src/GitHub.Api/GitHub.Api.csproj index 3679250d7..2327c99ed 100644 --- a/src/GitHub.Api/GitHub.Api.csproj +++ b/src/GitHub.Api/GitHub.Api.csproj @@ -108,6 +108,7 @@ + diff --git a/src/GitHub.Api/Helpers/NewlineSplitStringBuilder.cs b/src/GitHub.Api/Helpers/NewlineSplitStringBuilder.cs new file mode 100644 index 000000000..df648482e --- /dev/null +++ b/src/GitHub.Api/Helpers/NewlineSplitStringBuilder.cs @@ -0,0 +1,57 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; + +namespace GitHub.Unity.Helpers +{ + class NewlineSplitStringBuilder + { + private StringBuilder builder; + + public string[] Append(string value) + { + if (value == null) + { + var singleResult = builder?.ToString(); + if (singleResult == null) + { + return new string[0]; + } + return new[] { singleResult }; + } + + var splitValues = value.Split(new[] { "\r\n", "\n" }, StringSplitOptions.None); + + if (splitValues.Length == 0) + { + throw new ArgumentOutOfRangeException(); + } + + if (builder == null) + { + builder = new StringBuilder(); + } + + builder.Append(splitValues[0]); + + if (splitValues.Length == 1) + { + return new string[0]; + } + + var results = new string[splitValues.Length - 1]; + results[0] = builder.ToString(); + + for (var index = 1; index < splitValues.Length - 1; index++) + { + results[index] = splitValues[index]; + } + + builder = new StringBuilder(splitValues[splitValues.Length - 1]); + + return results; + } + } +} + diff --git a/src/GitHub.Api/NewTaskSystem/ProcessTask.cs b/src/GitHub.Api/NewTaskSystem/ProcessTask.cs index f929fd96b..b233a9bdc 100644 --- a/src/GitHub.Api/NewTaskSystem/ProcessTask.cs +++ b/src/GitHub.Api/NewTaskSystem/ProcessTask.cs @@ -3,9 +3,11 @@ using System.ComponentModel; using System.Diagnostics; using System.IO; +using System.Linq; using System.Text; using System.Threading; using System.Threading.Tasks; +using GitHub.Unity.Helpers; namespace GitHub.Unity { @@ -56,7 +58,6 @@ class ProcessWrapper private readonly Action onEnd; private readonly Action onError; private readonly CancellationToken token; - private readonly List errors = new List(); public Process Process { get; } public StreamWriter Input { get; private set; } @@ -78,39 +79,6 @@ public ProcessWrapper(Process process, IOutputProcessor outputProcessor, public void Run() { - if (Process.StartInfo.RedirectStandardOutput) - { - Process.OutputDataReceived += (s, e) => - { - //Logger.Trace("OutputData \"" + (e.Data == null ? "'null'" : e.Data) + "\""); - - string encodedData = null; - if (e.Data != null) - { - encodedData = Encoding.UTF8.GetString(Encoding.Default.GetBytes(e.Data)); - } - outputProcessor.LineReceived(encodedData); - }; - } - - if (Process.StartInfo.RedirectStandardError) - { - Process.ErrorDataReceived += (s, e) => - { - //if (e.Data != null) - //{ - // Logger.Trace("ErrorData \"" + (e.Data == null ? "'null'" : e.Data) + "\""); - //} - - string encodedData = null; - if (e.Data != null) - { - encodedData = Encoding.UTF8.GetString(Encoding.Default.GetBytes(e.Data)); - errors.Add(encodedData); - } - }; - } - try { Process.Start(); @@ -128,54 +96,103 @@ public void Run() sb.AppendFormat("{0}:{1}", env, Process.StartInfo.EnvironmentVariables[env]); sb.AppendLine(); } - onError?.Invoke(ex, String.Format("{0} {1}", ex.Message, sb.ToString())); + onError?.Invoke(ex, $"{ex.Message} {sb}"); onEnd?.Invoke(); return; } - if (Process.StartInfo.RedirectStandardOutput) - Process.BeginOutputReadLine(); - if (Process.StartInfo.RedirectStandardError) - Process.BeginErrorReadLine(); if (Process.StartInfo.RedirectStandardInput) Input = new StreamWriter(Process.StandardInput.BaseStream, new UTF8Encoding(false)); onStart?.Invoke(); - if (Process.StartInfo.CreateNoWindow) { - while (!WaitForExit(500)) + // buffer size refers to https://github.com/Unity-Technologies/mono/blob/unity-5.6-staging/mcs/class/System/System.Diagnostics/Process.cs#L1149-L1157 + const int bufferSize = 8182; + + if (Process.StartInfo.RedirectStandardOutput) { - if (token.IsCancellationRequested) + var outputStream = Process.StandardOutput.BaseStream; + var outputBuffer = new byte[bufferSize]; + var outputEncoding = Process.StartInfo.StandardOutputEncoding; + var splitStringbuilder = new NewlineSplitStringBuilder(); + string[] splitLines = null; + + var bytesRead = outputStream.Read(outputBuffer, 0, bufferSize); + while (bytesRead > 0) { - if (!Process.HasExited) - Process.Kill(); - Process.Close(); - onEnd?.Invoke(); - token.ThrowIfCancellationRequested(); + var encoded = outputEncoding.GetString(outputBuffer, 0, bytesRead); + splitLines = splitStringbuilder.Append(encoded); + foreach (var splitLine in splitLines) + { + outputProcessor.LineReceived(splitLine); + } + + if (token.IsCancellationRequested) + { + if (!Process.HasExited) + Process.Kill(); + + Process.Close(); + onEnd?.Invoke(); + token.ThrowIfCancellationRequested(); + } + + bytesRead = outputStream.Read(outputBuffer, 0, bufferSize); } + + splitLines = splitStringbuilder.Append(null); + //All but the last line, which will always be empty + for (var index = 0; index < splitLines.Length - 1; index++) + { + var splitLine = splitLines[index]; + outputProcessor.LineReceived(splitLine); + } + + outputProcessor.LineReceived(null); } - if (Process.ExitCode != 0 && errors.Count > 0) + if (Process.StartInfo.RedirectStandardError) { - onError?.Invoke(null, String.Join(Environment.NewLine, errors.ToArray())); - } - } - onEnd?.Invoke(); - } + var errorStream = Process.StandardError.BaseStream; + var errorBuffer = new byte[bufferSize]; + var errorEncoding = Process.StartInfo.StandardErrorEncoding; + var errorStringBuilder = new StringBuilder(); - private bool WaitForExit(int milliseconds) - { - //Logger.Debug("WaitForExit - time: {0}ms", milliseconds); + var bytesRead = errorStream.Read(errorBuffer, 0, bufferSize); + while (bytesRead > 0) + { + var encoded = errorEncoding.GetString(errorBuffer, 0, bytesRead); + errorStringBuilder.Append(encoded); - // Workaround for a bug in which some data may still be processed AFTER this method returns true, thus losing the data. - // http://connect.microsoft.com/VisualStudio/feedback/details/272125/waitforexit-and-waitforexit-int32-provide-different-and-undocumented-implementations - bool waitSucceeded = Process.WaitForExit(milliseconds); - if (waitSucceeded) - { - Process.WaitForExit(); + if (token.IsCancellationRequested) + { + if (!Process.HasExited) + Process.Kill(); + + Process.Close(); + onEnd?.Invoke(); + token.ThrowIfCancellationRequested(); + } + + bytesRead = errorStream.Read(errorBuffer, 0, bufferSize); + } + + var errors = errorStringBuilder.ToString().Split(new[] { "\r\n", "\n" }, StringSplitOptions.None).ToArray(); + if (errors.Length > 1) + { + //All but the last line, which will always be empty + errors = errors.Take(errors.Length - 1).ToArray(); + } + + if (Process.ExitCode != 0 && errors.Length > 0) + { + onError?.Invoke(null, string.Join(Environment.NewLine, errors)); + } + } } - return waitSucceeded; + + onEnd?.Invoke(); } } diff --git a/src/tests/IntegrationTests/Process/ProcessManagerIntegrationTests.cs b/src/tests/IntegrationTests/Process/ProcessManagerIntegrationTests.cs index 77c34ff29..20e81b96b 100644 --- a/src/tests/IntegrationTests/Process/ProcessManagerIntegrationTests.cs +++ b/src/tests/IntegrationTests/Process/ProcessManagerIntegrationTests.cs @@ -81,7 +81,7 @@ public async Task LogEntriesTest() }); } - [Test, Category("DoNotRunOnAppVeyor")] + [Test] public async Task RussianLogEntriesTest() { await Initialize(TestRepoMasterCleanUnsynchronizedRussianLanguage); diff --git a/src/tests/UnitTests/Helpers/NewlineSplitStringBuilderTests.cs b/src/tests/UnitTests/Helpers/NewlineSplitStringBuilderTests.cs new file mode 100644 index 000000000..fdf9cf510 --- /dev/null +++ b/src/tests/UnitTests/Helpers/NewlineSplitStringBuilderTests.cs @@ -0,0 +1,40 @@ +using System; +using System.Collections.Generic; +using FluentAssertions; +using GitHub.Unity.Helpers; +using NUnit.Framework; + +namespace UnitTests +{ + [TestFixture] + public class NewlineSplitStringBuilderTests + { + public static TestCaseData[] GetNewlineSplitTestData() + { + return new[] { + new TestCaseData(new string[] {null}, new string[0]).SetName("Null returns nothing"), + new TestCaseData(new string[] { $"ASDF{Environment.NewLine}Hello", null}, new[] {"ASDF", "Hello"}).SetName("Can split whole string"), + new TestCaseData(new string[] { $"ASDF", $"{Environment.NewLine}Hello", null}, new[] {"ASDF", "Hello"}).SetName("Can split string with second beginning newline"), + new TestCaseData(new string[] { $"ASDF{Environment.NewLine}", "Hello", null}, new[] {"ASDF", "Hello"}).SetName("Can split string with first ending newline"), + new TestCaseData(new string[] { $"AS", $"DF{Environment.NewLine}Hello", null}, new[] {"ASDF", "Hello"}).SetName("Can split string with newline contained in second"), + }; + } + + [TestCaseSource("GetNewlineSplitTestData")] + public void NewlineSplitStringBuilderTest(string[] expectedInputs, string[] expectedOutputs) + { + var results = new List(); + var newlineSplitStringBuilder = new NewlineSplitStringBuilder(); + foreach (var expectedInput in expectedInputs) + { + var output = newlineSplitStringBuilder.Append(expectedInput); + if (output != null) + { + results.AddRange(output); + } + } + + results.ShouldAllBeEquivalentTo(expectedOutputs); + } + } +} \ No newline at end of file diff --git a/src/tests/UnitTests/UnitTests.csproj b/src/tests/UnitTests/UnitTests.csproj index 1e6b7c2c8..80e9a10c4 100644 --- a/src/tests/UnitTests/UnitTests.csproj +++ b/src/tests/UnitTests/UnitTests.csproj @@ -80,6 +80,7 @@ +