Skip to content

Commit

Permalink
Do not throw if Git cmd exits with 0
Browse files Browse the repository at this point in the history 
- Ignore StandardError output
- Use it only as exception text on non-zero exit code

makes gitextensions#9056 less intercepting, fixes gitextensions#9462

Except for the actual changes in Executable.cs, this is just renaming
the argument to match the changed functionality.
  • Loading branch information
@mstv @gerhardol
mstv authored and gerhardol committed Sep 9, 2021
1 parent 4fd3843 commit 3284b1a
Show file tree
Hide file tree
Showing 21 changed files with 92 additions and 95 deletions.
2 changes: 1 addition & 1 deletion GitCommands/CommitDataManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ private bool TryGetCommitLog(string commitId, string format, [NotNullWhen(return

ExecutionResult exec = GetModule().GitExecutable.Execute(arguments,
outputEncoding: GitModule.LosslessEncoding,
cache: cache ? GitModule.GitCommandCache : null, throwOnErrorOutput: false);
cache: cache ? GitModule.GitCommandCache : null, throwOnErrorExit: false);

if (!exec.ExitedSuccessfully)
{
Expand Down
2 changes: 1 addition & 1 deletion GitCommands/FileHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public static bool IsBinaryFileName(GitModule module, string? fileName)
"--",
fileName.Quote()
};
GitUIPluginInterfaces.ExecutionResult result = module.GitExecutable.Execute(cmd, throwOnErrorOutput: false);
GitUIPluginInterfaces.ExecutionResult result = module.GitExecutable.Execute(cmd, throwOnErrorExit: false);
if (!result.ExitedSuccessfully)
{
return null;
Expand Down
2 changes: 1 addition & 1 deletion GitCommands/Git/AheadBehindDataProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public AheadBehindDataProvider(Func<IExecutable> getGitExecutable)
"refs/heads/" + branchName
};

ExecutionResult result = GetGitExecutable().Execute(aheadBehindGitCommand, outputEncoding: encoding, throwOnErrorOutput: false);
ExecutionResult result = GetGitExecutable().Execute(aheadBehindGitCommand, outputEncoding: encoding, throwOnErrorExit: false);
if (!result.ExitedSuccessfully || string.IsNullOrEmpty(result.StandardOutput))
{
return null;
Expand Down
39 changes: 18 additions & 21 deletions GitCommands/Git/Executable.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public Executable(Func<string> fileNameProvider, string workingDir = "")
bool redirectOutput = false,
Encoding? outputEncoding = null,
bool useShellExecute = false,
bool throwOnErrorOutput = true)
bool throwOnErrorExit = true)
{
// TODO should we set these on the child process only?
EnvironmentConfiguration.SetEnvironmentVariables();
Expand All @@ -44,7 +44,7 @@ public Executable(Func<string> fileNameProvider, string workingDir = "")

var fileName = _fileNameProvider();

return new ProcessWrapper(fileName, args, _workingDir, createWindow, redirectInput, redirectOutput, outputEncoding, useShellExecute, throwOnErrorOutput);
return new ProcessWrapper(fileName, args, _workingDir, createWindow, redirectInput, redirectOutput, outputEncoding, useShellExecute, throwOnErrorExit);
}

#region ProcessWrapper
Expand All @@ -64,7 +64,7 @@ private sealed class ProcessWrapper : IProcess
private readonly ProcessOperation _logOperation;
private readonly bool _redirectInput;
private readonly bool _redirectOutput;
private readonly bool _throwOnErrorOutput;
private readonly bool _throwOnErrorExit;

private MemoryStream? _emptyStream;
private StreamReader? _emptyReader;
Expand All @@ -79,15 +79,15 @@ private sealed class ProcessWrapper : IProcess
bool redirectOutput,
Encoding? outputEncoding,
bool useShellExecute,
bool throwOnErrorOutput)
bool throwOnErrorExit)
{
Debug.Assert(redirectOutput == (outputEncoding is not null), "redirectOutput == (outputEncoding is not null)");
_redirectInput = redirectInput;
_redirectOutput = redirectOutput;
_throwOnErrorOutput = throwOnErrorOutput;
_throwOnErrorExit = throwOnErrorExit;

Encoding errorEncoding = outputEncoding;
if (throwOnErrorOutput)
if (throwOnErrorExit)
{
errorEncoding ??= Encoding.Default;
}
Expand All @@ -103,7 +103,7 @@ private sealed class ProcessWrapper : IProcess
CreateNoWindow = !createWindow,
RedirectStandardInput = redirectInput,
RedirectStandardOutput = redirectOutput,
RedirectStandardError = redirectOutput || throwOnErrorOutput,
RedirectStandardError = redirectOutput || throwOnErrorExit,
StandardOutputEncoding = outputEncoding,
StandardErrorEncoding = errorEncoding,
FileName = fileName,
Expand Down Expand Up @@ -151,20 +151,17 @@ private void OnProcessExit(object sender, EventArgs eventArgs)
int exitCode = _process.ExitCode;
_logOperation.LogProcessEnd(exitCode);

if (_throwOnErrorOutput)
if (_throwOnErrorExit && exitCode != 0)
{
string errorOutput = _process.StandardError.ReadToEnd().Trim();
if (exitCode != 0 || errorOutput.Length > 0)
{
ExternalOperationException ex
= new(command: _process.StartInfo.FileName,
_process.StartInfo.Arguments,
_process.StartInfo.WorkingDirectory,
exitCode,
new Exception(errorOutput));
_logOperation.LogProcessEnd(ex);
_exitTaskCompletionSource.TrySetException(ex);
}
ExternalOperationException ex
= new(command: _process.StartInfo.FileName,
_process.StartInfo.Arguments,
_process.StartInfo.WorkingDirectory,
exitCode,
new Exception(errorOutput));
_logOperation.LogProcessEnd(ex);
_exitTaskCompletionSource.TrySetException(ex);
}

_exitTaskCompletionSource.TrySetResult(exitCode);
Expand Down Expand Up @@ -211,12 +208,12 @@ public StreamReader StandardError
{
get
{
if (!_redirectOutput && !_throwOnErrorOutput)
if (!_redirectOutput && !_throwOnErrorExit)
{
throw new InvalidOperationException("Process was not created with redirected output.");
}

if (!_throwOnErrorOutput)
if (!_throwOnErrorExit)
{
return _process.StandardError;
}
Expand Down
14 changes: 7 additions & 7 deletions GitCommands/Git/ExecutableExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public static class ExecutableExtensions
redirectInput: input is not null,
redirectOutput: true,
outputEncoding,
throwOnErrorOutput: true);
throwOnErrorExit: true);
if (input is not null)
{
await process.StandardInput.BaseStream.WriteAsync(input, 0, input.Length);
Expand Down Expand Up @@ -239,7 +239,7 @@ string ComposeOutput()
/// <param name="writeInput">A callback that writes bytes 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="throwOnErrorExit">A flag configuring whether to throw an exception if the exit code is not 0.</param>
/// <param name="cancellationToken">An optional token to cancel the asynchronous operation.</param>
/// <returns>An <see cref="ExecutionResult"/> object that gives access to exit code, standard output and standard error values.</returns>
[MustUseReturnValue("If execution result is not required, use " + nameof(RunCommand) + " instead")]
Expand All @@ -250,11 +250,11 @@ string ComposeOutput()
Encoding? outputEncoding = null,
CommandCache? cache = null,
bool stripAnsiEscapeCodes = true,
bool throwOnErrorOutput = true,
bool throwOnErrorExit = true,
CancellationToken cancellationToken = default)
{
return GitUI.ThreadHelper.JoinableTaskFactory.Run(
() => executable.ExecuteAsync(arguments, writeInput, outputEncoding, cache, stripAnsiEscapeCodes, throwOnErrorOutput, cancellationToken));
() => executable.ExecuteAsync(arguments, writeInput, outputEncoding, cache, stripAnsiEscapeCodes, throwOnErrorExit, cancellationToken));
}

/// <summary>
Expand All @@ -265,7 +265,7 @@ string ComposeOutput()
/// <param name="writeInput">A callback that writes bytes 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="throwOnErrorExit">A flag configuring whether to throw an exception if the exit code is not 0.</param>
/// <param name="cancellationToken">An optional token to cancel the asynchronous operation.</param>
/// <returns>A task that yields an <see cref="ExecutionResult"/> object that gives access to exit code, standard output and standard error values.</returns>
public static async Task<ExecutionResult> ExecuteAsync(
Expand All @@ -275,7 +275,7 @@ string ComposeOutput()
Encoding? outputEncoding = null,
CommandCache? cache = null,
bool stripAnsiEscapeCodes = true,
bool throwOnErrorOutput = true,
bool throwOnErrorExit = true,
CancellationToken cancellationToken = default)
{
outputEncoding ??= _defaultOutputEncoding.Value;
Expand All @@ -288,7 +288,7 @@ string ComposeOutput()
exitCode: 0);
}

using IProcess process = executable.Start(arguments, createWindow: false, redirectInput: writeInput is not null, redirectOutput: true, outputEncoding, throwOnErrorOutput: throwOnErrorOutput);
using IProcess process = executable.Start(arguments, createWindow: false, redirectInput: writeInput is not null, redirectOutput: true, outputEncoding, throwOnErrorExit: throwOnErrorExit);
MemoryStream outputBuffer = new();
MemoryStream errorBuffer = new();
var outputTask = process.StandardOutput.BaseStream.CopyToAsync(outputBuffer);
Expand Down
Loading

0 comments on commit 3284b1a

Please sign in to comment.