Skip to content

Commit

Permalink
wip gitextensions#9056 throwOnErrorOutput
Browse files Browse the repository at this point in the history
  • Loading branch information
gerhardol committed Aug 12, 2021
1 parent 9717359 commit 25c5fe6
Show file tree
Hide file tree
Showing 11 changed files with 64 additions and 109 deletions.
9 changes: 7 additions & 2 deletions GitCommands/FileHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,13 @@ public static bool IsBinaryFileName(GitModule module, string? fileName)
"--",
fileName.Quote()
};
string result = module.GitExecutable.GetOutput(cmd, throwOnErrorOutput: false);
var lines = result.Split(Delimiters.NullAndLineFeed);
GitUIPluginInterfaces.ExecutionResult result = module.GitExecutable.Execute(cmd, throwOnErrorOutput: false);
if (!result.ExitedSuccessfully)
{
return null;
}

var lines = result.StandardOutput.Split(Delimiters.NullAndLineFeed);
Dictionary<string, string> attributes = new();
for (int i = 0; i < lines.Length - 2; i += 3)
{
Expand Down
75 changes: 6 additions & 69 deletions GitCommands/Git/ExecutableExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ public static class ExecutableExtensions
/// <param name="outputEncoding">The text encoding to use when decoding bytes read from the process's standard output and standard error streams, or <c>null</c> if the default encoding is to be used.</param>
/// <param name="cache">A <see cref="CommandCache"/> to use if command results may be cached, otherwise <c>null</c>.</param>
/// <param name="stripAnsiEscapeCodes">A flag indicating whether ANSI escape codes should be removed from output strings.</param>
/// <param name="throwOnErrorOutput">A flag configuring whether to throw an exception if the exit code is not 0 or if the output to StandardError is not empty.</param>
/// <returns>The concatenation of standard output and standard error. To receive these outputs separately, use <see cref="Execute"/> instead.</returns>
[MustUseReturnValue("If output text is not required, use " + nameof(RunCommand) + " instead")]
public static string GetOutput(
Expand All @@ -43,11 +42,10 @@ public static class ExecutableExtensions
byte[]? input = null,
Encoding? outputEncoding = null,
CommandCache? cache = null,
bool stripAnsiEscapeCodes = true,
bool throwOnErrorOutput = true)
bool stripAnsiEscapeCodes = true)
{
return GitUI.ThreadHelper.JoinableTaskFactory.Run(
() => executable.GetOutputAsync(arguments, input, outputEncoding, cache, stripAnsiEscapeCodes, throwOnErrorOutput));
() => executable.GetOutputAsync(arguments, input, outputEncoding, cache, stripAnsiEscapeCodes));
}

/// <summary>
Expand Down Expand Up @@ -90,16 +88,14 @@ public static class ExecutableExtensions
/// <param name="outputEncoding">The text encoding to use when decoding bytes read from the process's standard output and standard error streams, or <c>null</c> if the default encoding is to be used.</param>
/// <param name="cache">A <see cref="CommandCache"/> to use if command results may be cached, otherwise <c>null</c>.</param>
/// <param name="stripAnsiEscapeCodes">A flag indicating whether ANSI escape codes should be removed from output strings.</param>
/// <param name="throwOnErrorOutput">A flag configuring whether to throw an exception if the exit code is not 0 or if the output to StandardError is not empty.</param>
/// <returns>A task that yields the concatenation of standard output and standard error. To receive these outputs separately, use <see cref="ExecuteAsync"/> instead.</returns>
public static async Task<string> GetOutputAsync(
this IExecutable executable,
ArgumentString arguments = default,
byte[]? input = null,
Encoding? outputEncoding = null,
CommandCache? cache = null,
bool stripAnsiEscapeCodes = true,
bool throwOnErrorOutput = true)
bool stripAnsiEscapeCodes = true)
{
outputEncoding ??= _defaultOutputEncoding.Value;

Expand All @@ -114,23 +110,21 @@ public static class ExecutableExtensions
redirectInput: input is not null,
redirectOutput: true,
outputEncoding,
throwOnErrorOutput: throwOnErrorOutput);
throwOnErrorOutput: true);
if (input is not null)
{
await process.StandardInput.BaseStream.WriteAsync(input, 0, input.Length);
process.StandardInput.Close();
}

MemoryStream outputBuffer = new();
MemoryStream errorBuffer = new();
var outputTask = process.StandardOutput.BaseStream.CopyToAsync(outputBuffer);
var errorTask = process.StandardError.BaseStream.CopyToAsync(errorBuffer);
var exitTask = process.WaitForExitAsync();

await Task.WhenAll(outputTask, errorTask, exitTask);
await Task.WhenAll(outputTask, exitTask);

output = outputBuffer.ToArray();
error = errorBuffer.ToArray();
error = Array.Empty<byte>();

if (cache is not null && await exitTask == 0)
{
Expand Down Expand Up @@ -231,63 +225,6 @@ string ComposeOutput()
return await process.WaitForExitAsync() == 0;
}

/// <summary>
/// Launches a process for the executable and returns output lines as they become available.
/// </summary>
/// <param name="executable">The executable from which to launch a process.</param>
/// <param name="arguments">The arguments to pass to the executable.</param>
/// <param name="input">Bytes to be written to the process's standard input stream, or <c>null</c> if no input is required.</param>
/// <param name="outputEncoding">The text encoding to use when decoding bytes read from the process's standard output and standard error streams, or <c>null</c> if the default encoding is to be used.</param>
/// <param name="stripAnsiEscapeCodes">A flag indicating whether ANSI escape codes should be removed from output strings.</param>
/// <param name="throwOnErrorOutput">A flag configuring whether to throw an exception if the exit code is not 0 or if the output to StandardError is not empty.</param>
/// <param name="includeErrorOutput">If not throwing on error, include the StandardError output in the response.</param>
/// <returns>An enumerable sequence of lines that yields lines as they become available. Lines from standard output are returned first, followed by lines from standard error.</returns>
[MustUseReturnValue("If output lines are not required, use " + nameof(RunCommand) + " instead")]
public static IEnumerable<string> GetOutputLines(
this IExecutable executable,
ArgumentString arguments = default,
byte[]? input = null,
Encoding? outputEncoding = null,
bool stripAnsiEscapeCodes = true,
bool throwOnErrorOutput = true,
bool includeErrorOutput = false)
{
outputEncoding ??= _defaultOutputEncoding.Value;

using var process = executable.Start(arguments, createWindow: false, redirectInput: input is not null, redirectOutput: true, outputEncoding, throwOnErrorOutput: throwOnErrorOutput);
if (input is not null)
{
process.StandardInput.BaseStream.Write(input, 0, input.Length);
process.StandardInput.Close();
}

while (true)
{
var line = process.StandardOutput.ReadLine();

if (line is null)
{
break;
}

yield return CleanString(stripAnsiEscapeCodes, line);
}

while (includeErrorOutput)
{
var line = process.StandardError.ReadLine();

if (line is null)
{
break;
}

yield return CleanString(stripAnsiEscapeCodes, line);
}

process.WaitForExit();
}

/// <summary>
/// Launches a process for the executable and returns an object detailing exit code, standard output and standard error values.
/// </summary>
Expand Down
38 changes: 23 additions & 15 deletions GitCommands/Git/GitModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -693,10 +693,9 @@ public async Task<List<ConflictData>> GetConflictsAsync(string? filename = "")
filename.QuoteNE()
};

var unmerged = (await _gitExecutable
.GetOutputAsync(args, throwOnErrorOutput: false) // ignore non-zero exit code, e.g. in case of missing submodule
.ConfigureAwait(false))
.Split(Delimiters.NullAndLineFeed, StringSplitOptions.RemoveEmptyEntries);
// ignore non-zero exit code, e.g. in case of missing submodule
ExecutionResult result = await _gitExecutable.ExecuteAsync(args, throwOnErrorOutput: false).ConfigureAwait(false);
var unmerged = result.StandardOutput.Split(Delimiters.NullAndLineFeed, StringSplitOptions.RemoveEmptyEntries);

var item = new ConflictedFileData[3];

Expand Down Expand Up @@ -778,7 +777,8 @@ public async Task<List<ConflictData>> GetConflictsAsync(string? filename = "")
$"^{child}",
"--count"
};
string output = _gitExecutable.GetOutput(args, cache: cache ? GitCommandCache : null, throwOnErrorOutput: throwOnErrorOutput);
ExecutionResult result = _gitExecutable.Execute(args, cache: cache ? GitCommandCache : null, throwOnErrorOutput: throwOnErrorOutput);
string output = result.StandardOutput;

if (int.TryParse(output, out var commitCount))
{
Expand Down Expand Up @@ -880,7 +880,7 @@ public void RunMergeTool(string? fileName = "", string? customTool = null)
{ !string.IsNullOrWhiteSpace(fileName), "--" },
fileName.ToPosixPath().QuoteNE()
};
using var process = _gitExecutable.Start(args, createWindow: true, throwOnErrorOutput: true);
using var process = _gitExecutable.Start(args, createWindow: true);
process.WaitForExit();
}

Expand Down Expand Up @@ -1074,7 +1074,8 @@ public bool TryResolvePartialCommitId(string objectIdPrefix, [NotNullWhen(return
"--quiet",
$"{objectIdPrefix}^{{commit}}"
};
string output = _gitExecutable.GetOutput(args, throwOnErrorOutput: false).Trim();
ExecutionResult result = _gitExecutable.Execute(args, throwOnErrorOutput: false);
string output = result.StandardOutput.Trim();

if (output.StartsWith(objectIdPrefix) && ObjectId.TryParse(output, out objectId))
{
Expand Down Expand Up @@ -1132,7 +1133,7 @@ public bool ExistsMergeCommit(string? startRev, string? endRev)
};

// Could fail if pulling interactively from remote where the specified branch does not exist
return _gitExecutable.GetOutputLines(args, throwOnErrorOutput: false).Any();
return _gitExecutable.Execute(args, throwOnErrorOutput: false).StandardOutput.LazySplit('\n').Any();
}

public ConfigFile GetSubmoduleConfigFile()
Expand Down Expand Up @@ -1192,7 +1193,8 @@ IGitModule IGitModule.GetSubmodule(string submoduleName)
}

GitArgumentBuilder args = new("submodule") { "status" };
var lines = _gitExecutable.GetOutputLines(args);
ExecutionResult result = _gitExecutable.Execute(args);
var lines = result.StandardOutput.LazySplit('\n');

string? lastLine = null;

Expand Down Expand Up @@ -2135,7 +2137,9 @@ public string AddRemote(string? name, string? path)
public IReadOnlyList<string> GetRemoteNames()
{
return _gitExecutable
.GetOutputLines("remote")
.Execute("remote")
.StandardOutput
.LazySplit('\n')
.Where(r => !string.IsNullOrWhiteSpace(r))
.ToList();
}
Expand Down Expand Up @@ -2306,12 +2310,13 @@ public IReadOnlyList<GitStash> GetStashes(bool noLocks = false)
: null;

bool nonZeroGitExitCode = firstId == ObjectId.WorkTreeId && secondId is not null && !isTracked;
string patch = await _gitExecutable.GetOutputAsync(
ExecutionResult result = await _gitExecutable.ExecuteAsync(
args,
cache: cache,
outputEncoding: LosslessEncoding,
throwOnErrorOutput: !nonZeroGitExitCode);

string patch = result.StandardOutput;
IReadOnlyList<Patch> patches = PatchProcessor.CreatePatchesFromString(patch, new Lazy<Encoding>(() => encoding)).ToList();

return GetPatch(patches, fileName, oldFileName);
Expand Down Expand Up @@ -2962,9 +2967,10 @@ public IReadOnlyList<string> GetMergedRemoteBranches()
const string refsPrefix = "refs/";

var remotes = GetRemoteNames();
ExecutionResult result = _gitExecutable.Execute(GitCommandHelpers.MergedBranchesCmd(includeRemote: true));
var lines = result.StandardOutput.LazySplit('\n');

return _gitExecutable
.GetOutputLines(GitCommandHelpers.MergedBranchesCmd(includeRemote: true))
return lines
.Select(b => b.Trim())
.Where(b => b.StartsWith(remoteBranchPrefixForMergedBranches))
.Select(b => string.Concat(refsPrefix, b))
Expand Down Expand Up @@ -3659,7 +3665,8 @@ public string OpenWithDifftool(string? filename, string? oldFileName = "", strin
a,
b
};
string output = _gitExecutable.GetOutput(args, cache: GitCommandCache, throwOnErrorOutput: false);
ExecutionResult result = _gitExecutable.Execute(args, cache: GitCommandCache, throwOnErrorOutput: false);
string output = result.StandardOutput;

return ObjectId.TryParse(output, offset: 0, out var objectId)
? objectId
Expand Down Expand Up @@ -4146,7 +4153,8 @@ private static GitItemStatus createErrorGitItemStatus(string gitOutput)
GetDateParameter("--until", until)
};

var lines = _gitExecutable.GetOutputLines(args);
ExecutionResult result = _gitExecutable.Execute(args);
var lines = result.StandardOutput.Split('\n');

foreach (var line in lines)
{
Expand Down
3 changes: 2 additions & 1 deletion GitCommands/Git/SystemEncodingReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ public Encoding Read()
controlStr
};

string? s = _module.GitExecutable.GetOutput(arguments, outputEncoding: Encoding.UTF8, throwOnErrorOutput: false);
ExecutionResult result = _module.GitExecutable.Execute(arguments, outputEncoding: Encoding.UTF8, throwOnErrorOutput: false);
string? s = result.AllOutput;
if (s is not null && s.IndexOf(controlStr) != -1)
{
systemEncoding = new UTF8Encoding(false);
Expand Down
3 changes: 2 additions & 1 deletion GitUI/Script/ScriptRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ private static CommandStatus RunScriptInternal(IWin32Window owner, IGitModule mo
command = command.Replace(NavigateToPrefix, string.Empty);
if (!string.IsNullOrEmpty(command))
{
string revisionRef = new Executable(command, module.WorkingDir).GetOutputLines(argument).FirstOrDefault();
ExecutionResult result = new Executable(command, module.WorkingDir).Execute(argument);
string revisionRef = result.StandardOutput.Split('\n').FirstOrDefault();

if (revisionRef is not null)
{
Expand Down
6 changes: 4 additions & 2 deletions GitUI/UserControls/RevisionGrid/RevisionGridControl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,8 @@ string BuildPathFilter(string? path)
};

HashSet<string?> setOfFileNames = new();
var lines = Module.GitExecutable.GetOutputLines(args, outputEncoding: GitModule.LosslessEncoding, throwOnErrorOutput: false);
ExecutionResult result = Module.GitExecutable.Execute(args, outputEncoding: GitModule.LosslessEncoding, throwOnErrorOutput: false);
var lines = result.StandardOutput.LazySplit('\n');

// TODO Check the exit code and warn the user that rename detection could not be done.

Expand Down Expand Up @@ -1502,7 +1503,8 @@ private IEnumerable<ObjectId> TryGetParents(ObjectId objectId)
objectId
};

foreach (var line in Module.GitExecutable.GetOutputLines(args, throwOnErrorOutput: false))
ExecutionResult result = Module.GitExecutable.Execute(args, throwOnErrorOutput: false);
foreach (var line in result.StandardOutput.LazySplit('\n'))
{
if (ObjectId.TryParse(line, out var parentId))
{
Expand Down
3 changes: 2 additions & 1 deletion Plugins/Statistics/GitImpact/ImpactLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ where submodule.IsValidGitWorkingDir()

private void LoadModuleInfo(string command, IGitModule module, CancellationToken token)
{
using var lineEnumerator = module.GitExecutable.GetOutputLines(command).GetEnumerator();
ExecutionResult result = module.GitExecutable.Execute(command);
using var lineEnumerator = result.StandardOutput.Split('\n').ToList().GetEnumerator();

// Analyze commit listing
while (!token.IsCancellationRequested && lineEnumerator.MoveNext())
Expand Down
4 changes: 2 additions & 2 deletions UnitTests/CommonTestUtils/GitModuleTestHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public void AddSubmodule(GitModuleTestHelper subModuleHelper, string path)
// Submodules require at least one commit
subModuleHelper.Module.GitExecutable.GetOutput(@"commit --allow-empty -m ""Initial empty commit""");

Module.GitExecutable.GetOutput(GitCommandHelpers.AddSubmoduleCmd(subModuleHelper.Module.WorkingDir.ToPosixPath(), path, null, true), throwOnErrorOutput: false);
Module.GitExecutable.Execute(GitCommandHelpers.AddSubmoduleCmd(subModuleHelper.Module.WorkingDir.ToPosixPath(), path, null, true), throwOnErrorOutput: false);
Module.GitExecutable.GetOutput(@"commit -am ""Add submodule""");
}

Expand All @@ -146,7 +146,7 @@ public void AddSubmodule(GitModuleTestHelper subModuleHelper, string path)
/// <returns>All submodule Modules</returns>
public IEnumerable<GitModule> GetSubmodulesRecursive()
{
Module.GitExecutable.GetOutput(@"submodule update --init --recursive", throwOnErrorOutput: false);
Module.GitExecutable.Execute(@"submodule update --init --recursive", throwOnErrorOutput: false);
var paths = Module.GetSubmodulesLocalPaths(recursive: true);
return paths.Select(path =>
{
Expand Down
2 changes: 1 addition & 1 deletion UnitTests/GitCommands.Tests/Config/ConfigFileTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ private string GetConfigValue(string cfgFile, string key)
// Quote the key without unescaping internal quotes
$"\"{key}\""
};
return Module.GitExecutable.GetOutput(args, throwOnErrorOutput: false).TrimEnd('\n');
return Module.GitExecutable.Execute(args, throwOnErrorOutput: false).StandardOutput.TrimEnd('\n');
}

private void CheckValueIsEqual(ConfigFile configFile, string key, string expectedValue)
Expand Down
4 changes: 2 additions & 2 deletions UnitTests/GitCommands.Tests/GitModuleTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -745,13 +745,13 @@ public void GetSubmodulesLocalPaths()
var child = moduleTestHelpers[i];

// Add child as submodule of parent
parent.Module.GitExecutable.GetOutput(GitCommandHelpers.AddSubmoduleCmd(child.Module.WorkingDir.ToPosixPath(), $"repo{i}", null, true), throwOnErrorOutput: false);
parent.Module.GitExecutable.Execute(GitCommandHelpers.AddSubmoduleCmd(child.Module.WorkingDir.ToPosixPath(), $"repo{i}", null, true), throwOnErrorOutput: false);
parent.Module.GitExecutable.GetOutput(@"commit -am ""Add submodule""");
}

// Init all modules of root
var root = moduleTestHelpers[0];
root.Module.GitExecutable.GetOutput(@"submodule update --init --recursive", throwOnErrorOutput: false);
root.Module.GitExecutable.Execute(@"submodule update --init --recursive", throwOnErrorOutput: false);

var paths = root.Module.GetSubmodulesLocalPaths(recursive: true);
Assert.AreEqual(new string[] { "repo1", "repo1/repo2", "repo1/repo2/repo3" }, paths, $"Modules: {string.Join(" ", paths)}");
Expand Down
Loading

0 comments on commit 25c5fe6

Please sign in to comment.