From c0ff7eac569080db25addebc365623032350ed8e Mon Sep 17 00:00:00 2001 From: Felix Damrau Date: Fri, 31 May 2024 23:13:02 +0200 Subject: [PATCH 1/4] Fix possible standard output deadlock in external git handler --- src/RepoCleaner/Git/External/GitHandler.cs | 24 +++++++++++++++++----- src/RepoCleaner/RepoCleaner.csproj | 2 +- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/RepoCleaner/Git/External/GitHandler.cs b/src/RepoCleaner/Git/External/GitHandler.cs index 009029a..6d32417 100644 --- a/src/RepoCleaner/Git/External/GitHandler.cs +++ b/src/RepoCleaner/Git/External/GitHandler.cs @@ -76,13 +76,27 @@ private static Result> GetBranches(string path) "branch --format \"%(HEAD)\t%(refname)\t%(refname:short)\t%(upstream:track)\t%(authordate:unix)\t%(authorname)\" --all"); var process = Process.Start(processStartInfo) ?? throw new UnreachableException("The git process could not start"); + List standardOutput = []; + List standardError = []; + process.OutputDataReceived += (s, e) => Add(standardOutput, e.Data); + process.ErrorDataReceived += (s, e) => Add(standardError, e.Data); + process.BeginOutputReadLine(); + process.BeginErrorReadLine(); process.WaitForExit(); return process.ExitCode != 0 - ? Result.Fail>(process.StandardError.ReadToEnd()) - : Result.Ok(ParseBranchOutput(process.StandardOutput)); + ? Result.Fail>(string.Join(Environment.NewLine, standardError)) + : Result.Ok(ParseBranchOutput(standardOutput)); + + static void Add(List standardOutput, string? data) + { + if (data is not null) + standardOutput.Add(data); + } } + + private static ProcessStartInfo GetGitProcessStartInfo(string path, string arguments) { return new ProcessStartInfo @@ -92,13 +106,13 @@ private static ProcessStartInfo GetGitProcessStartInfo(string path, string argum UseShellExecute = false, RedirectStandardError = true, RedirectStandardOutput = true, - WorkingDirectory = path + WorkingDirectory = path, }; } - private static IEnumerable ParseBranchOutput(StreamReader standardOutput) + private static IEnumerable ParseBranchOutput(IEnumerable outputLines) { - while (standardOutput.ReadLine() is { } line) + foreach (var line in outputLines) { var split = line .Split("\t") diff --git a/src/RepoCleaner/RepoCleaner.csproj b/src/RepoCleaner/RepoCleaner.csproj index 0353d26..da13d31 100644 --- a/src/RepoCleaner/RepoCleaner.csproj +++ b/src/RepoCleaner/RepoCleaner.csproj @@ -9,7 +9,7 @@ Develix.RepoCleaner win-x64 en - 1.8.0 + 1.8.1 From 7f30b52118c70626508f93342cf5a09e648a34fe Mon Sep 17 00:00:00 2001 From: Felix Damrau Date: Sun, 2 Jun 2024 15:25:01 +0200 Subject: [PATCH 2/4] Cleanup naming --- src/RepoCleaner/Git/External/GitHandler.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/RepoCleaner/Git/External/GitHandler.cs b/src/RepoCleaner/Git/External/GitHandler.cs index 6d32417..e787251 100644 --- a/src/RepoCleaner/Git/External/GitHandler.cs +++ b/src/RepoCleaner/Git/External/GitHandler.cs @@ -78,8 +78,8 @@ private static Result> GetBranches(string path) var process = Process.Start(processStartInfo) ?? throw new UnreachableException("The git process could not start"); List standardOutput = []; List standardError = []; - process.OutputDataReceived += (s, e) => Add(standardOutput, e.Data); - process.ErrorDataReceived += (s, e) => Add(standardError, e.Data); + process.OutputDataReceived += (s, e) => Add(e.Data, standardOutput); + process.ErrorDataReceived += (s, e) => Add(e.Data, standardError); process.BeginOutputReadLine(); process.BeginErrorReadLine(); process.WaitForExit(); @@ -88,10 +88,10 @@ private static Result> GetBranches(string path) ? Result.Fail>(string.Join(Environment.NewLine, standardError)) : Result.Ok(ParseBranchOutput(standardOutput)); - static void Add(List standardOutput, string? data) + static void Add(string? data, List outputLines) { if (data is not null) - standardOutput.Add(data); + outputLines.Add(data); } } From f2c9bc49bb6b9bbe423096db83ad87a9b87aa6fe Mon Sep 17 00:00:00 2001 From: Felix Damrau Date: Sun, 2 Jun 2024 15:38:48 +0200 Subject: [PATCH 3/4] Wrap run process handling in own method --- src/RepoCleaner/Git/External/GitHandler.cs | 50 ++++++++++++---------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/src/RepoCleaner/Git/External/GitHandler.cs b/src/RepoCleaner/Git/External/GitHandler.cs index e787251..c7fcb10 100644 --- a/src/RepoCleaner/Git/External/GitHandler.cs +++ b/src/RepoCleaner/Git/External/GitHandler.cs @@ -61,12 +61,11 @@ private static Result DeleteBranch(string repositoryPath, Branch branch) var arguments = $"branch -D {branch.FriendlyName}"; var processStartInfo = GetGitProcessStartInfo(repositoryPath, arguments); - var process = Process.Start(processStartInfo) ?? throw new InvalidOperationException("The git process could not start"); - process.WaitForExit(); + var (_, errorLines, exitCode) = RunProcess(processStartInfo); - return process.ExitCode == 0 + return exitCode == 0 ? Result.Ok() - : Result.Fail(process.StandardError.ReadToEnd()); + : Result.Fail(string.Join(Environment.NewLine, errorLines)); } private static Result> GetBranches(string path) @@ -75,28 +74,13 @@ private static Result> GetBranches(string path) path, "branch --format \"%(HEAD)\t%(refname)\t%(refname:short)\t%(upstream:track)\t%(authordate:unix)\t%(authorname)\" --all"); - var process = Process.Start(processStartInfo) ?? throw new UnreachableException("The git process could not start"); - List standardOutput = []; - List standardError = []; - process.OutputDataReceived += (s, e) => Add(e.Data, standardOutput); - process.ErrorDataReceived += (s, e) => Add(e.Data, standardError); - process.BeginOutputReadLine(); - process.BeginErrorReadLine(); - process.WaitForExit(); - - return process.ExitCode != 0 - ? Result.Fail>(string.Join(Environment.NewLine, standardError)) - : Result.Ok(ParseBranchOutput(standardOutput)); + var (outputLines, errorLines, exitCode) = RunProcess(processStartInfo); - static void Add(string? data, List outputLines) - { - if (data is not null) - outputLines.Add(data); - } + return exitCode == 0 + ? Result.Ok(ParseBranchOutput(outputLines)) + : Result.Fail>(string.Join(Environment.NewLine, errorLines)); } - - private static ProcessStartInfo GetGitProcessStartInfo(string path, string arguments) { return new ProcessStartInfo @@ -110,6 +94,26 @@ private static ProcessStartInfo GetGitProcessStartInfo(string path, string argum }; } + private static (List outputLines, List errorLines, int exitCode) RunProcess(ProcessStartInfo processStartInfo) + { + using var process = Process.Start(processStartInfo) ?? throw new UnreachableException("The git process could not start"); + List standardOutput = []; + List standardError = []; + process.OutputDataReceived += (s, e) => Add(e.Data, standardOutput); + process.ErrorDataReceived += (s, e) => Add(e.Data, standardError); + process.BeginOutputReadLine(); + process.BeginErrorReadLine(); + process.WaitForExit(); + + return (standardOutput, standardError, process.ExitCode); + + static void Add(string? data, List outputLines) + { + if (data is not null) + outputLines.Add(data); + } + } + private static IEnumerable ParseBranchOutput(IEnumerable outputLines) { foreach (var line in outputLines) From 6d65e272710d36a40120da62c732593cc4160c57 Mon Sep 17 00:00:00 2001 From: Felix Damrau Date: Sun, 2 Jun 2024 16:03:10 +0200 Subject: [PATCH 4/4] Move git branch arguments to const --- src/RepoCleaner/Git/External/GitHandler.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/RepoCleaner/Git/External/GitHandler.cs b/src/RepoCleaner/Git/External/GitHandler.cs index c7fcb10..ba896ec 100644 --- a/src/RepoCleaner/Git/External/GitHandler.cs +++ b/src/RepoCleaner/Git/External/GitHandler.cs @@ -6,6 +6,8 @@ namespace Develix.RepoCleaner.Git.External; internal class GitHandler : IGitHandler { + private const string FormattedGitBranchArguments = "branch --format \"%(HEAD)\t%(refname)\t%(refname:short)\t%(upstream:track)\t%(authordate:unix)\t%(authorname)\" --all"; + public IReadOnlyList DeleteBranches(string repositoryPath, IEnumerable branches) { var results = new List(); @@ -70,9 +72,7 @@ private static Result DeleteBranch(string repositoryPath, Branch branch) private static Result> GetBranches(string path) { - var processStartInfo = GetGitProcessStartInfo( - path, - "branch --format \"%(HEAD)\t%(refname)\t%(refname:short)\t%(upstream:track)\t%(authordate:unix)\t%(authorname)\" --all"); + var processStartInfo = GetGitProcessStartInfo(path, FormattedGitBranchArguments); var (outputLines, errorLines, exitCode) = RunProcess(processStartInfo);