GitProcess: allow GIT_TRACE to point to full path

microsoft · Oct 4, 2018 · f5530c7 · f5530c7
1 parent 8dba3b4
commit f5530c7
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 18 deletions.
diff --git a/GVFS/GVFS.Common/Git/GitProcess.cs b/GVFS/GVFS.Common/Git/GitProcess.cs
@@ -1,6 +1,5 @@
 using GVFS.Common.FileSystem;
 using GVFS.Common.Tracing;
-using Microsoft.Win32;
 using System;
 using System.Collections.Generic;
 using System.ComponentModel;
@@ -149,7 +148,7 @@ public bool IsValidRepo()
             Result result = this.InvokeGitAgainstDotGitFolder("rev-parse --show-toplevel");
             return !result.HasErrors;
         }
-                
+
         public Result RevParse(string gitRef)
         {
             return this.InvokeGitAgainstDotGitFolder("rev-parse " + gitRef);
@@ -405,11 +404,20 @@ public Process GetGitProcess(string command, string workingDirectory, string dot
 
             // Removing trace variables that might change git output and break parsing
             // List of environment variables: https://git-scm.com/book/gr/v2/Git-Internals-Environment-Variables
-            foreach (string key in processInfo.EnvironmentVariables.Keys.Cast<string>()
-                .Where(x => x.StartsWith("GIT_TRACE", StringComparison.OrdinalIgnoreCase))
-                .ToList())
+            foreach (string key in processInfo.EnvironmentVariables.Keys.Cast<string>())
             {
-                processInfo.EnvironmentVariables.Remove(key);
+                // If GIT_TRACE is set to a fully-rooted path, then Git sends the trace
+                // output to that path instead of stdout (GIT_TRACE=1) or stderr (GIT_TRACE=2).
+                if (key.StartsWith("GIT_TRACE", StringComparison.OrdinalIgnoreCase))
+                {
+                    string value = processInfo.EnvironmentVariables[key];
+
+                    if (value.IndexOfAny(Path.GetInvalidPathChars()) >= 0
+                        || !Path.IsPathRooted(processInfo.EnvironmentVariables[key]))
+                    {
+                        processInfo.EnvironmentVariables.Remove(key);
+                    }
+                }
             }
 
             processInfo.EnvironmentVariables["GIT_TERMINAL_PROMPT"] = "0";
@@ -601,7 +609,7 @@ private Result InvokeGitAgainstDotGitFolder(
                 parseStdOutLine: parseStdOutLine,
                 timeoutMs: -1);
         }
-        
+
         public class Result
         {
             public const int SuccessCode = 0;

diff --git a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitCommandsTests.cs b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitCommandsTests.cs
@@ -3,6 +3,7 @@
 using GVFS.Tests.Should;
 using NUnit.Framework;
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Runtime.CompilerServices;
 
@@ -23,7 +24,7 @@ public class GitCommandsTests : GitRepoTests
         private static readonly string RenameFilePathTo = Path.Combine("GVFS", "GVFS.Common", "Physical", "FileSystem", "FileProperties2.cs");
         private static readonly string RenameFolderPathFrom = Path.Combine("GVFS", "GVFS.Common", "PrefetchPacks");
         private static readonly string RenameFolderPathTo = Path.Combine("GVFS", "GVFS.Common", "PrefetchPacksRenamed");
-       
+
         public GitCommandsTests() : base(enlistmentPerTest: false)
         {
         }
@@ -45,6 +46,75 @@ public void StatusTest()
             this.ValidateGitCommand("status");
         }
 
+        [TestCase]
+        public void StatusWithGitTraceToPathTest()
+        {
+            Dictionary<string, string> environmentVariables = new Dictionary<string, string>();
+            string expectedPath = Path.Combine(this.Enlistment.EnlistmentRoot, "trace-log-expected.txt");
+            string actualPath = Path.Combine(this.Enlistment.EnlistmentRoot, "trace-log-actual.txt");
+
+            string command = "status";
+            string controlRepoRoot = this.ControlGitRepo.RootPath;
+            string gvfsRepoRoot = this.Enlistment.RepoRoot;
+
+            environmentVariables["GIT_TRACE"] = expectedPath;
+            ProcessResult expectedResult = GitProcess.InvokeProcess(controlRepoRoot, command, environmentVariables);
+
+            environmentVariables["GIT_TRACE"] = actualPath;
+            ProcessResult actualResult = GitHelpers.InvokeGitAgainstGVFSRepo(gvfsRepoRoot, command, environmentVariables);
+
+            GitHelpers.ValidateGitOutput(command, expectedResult, actualResult);
+
+            this.FileSystem.FileExists(expectedPath).ShouldBeTrue();
+            this.FileSystem.FileExists(actualPath).ShouldBeTrue();
+
+            string lineToFind = "trace: built-in: git status";
+            this.FileSystem.ReadAllText(expectedResult.Output).IndexOf(lineToFind).ShouldBeAtLeast(0);
+            this.FileSystem.ReadAllText(actualResult.Output).IndexOf(lineToFind).ShouldBeAtLeast(0);
+        }
+
+        [TestCase]
+        public void StatusWithGitTraceToStderrTest()
+        {
+            Dictionary<string, string> environmentVariables = new Dictionary<string, string>();
+            environmentVariables["GIT_TRACE"] = "2";
+
+            string command = "status";
+            string controlRepoRoot = this.ControlGitRepo.RootPath;
+            string gvfsRepoRoot = this.Enlistment.RepoRoot;
+
+            ProcessResult expectedResult = GitProcess.InvokeProcess(controlRepoRoot, command, environmentVariables);
+            ProcessResult actualResult = GitHelpers.InvokeGitAgainstGVFSRepo(gvfsRepoRoot, command, environmentVariables);
+
+            GitHelpers.ValidateGitOutput(command, expectedResult, actualResult);
+
+            string.IsNullOrWhiteSpace(expectedResult.Errors).ShouldBeFalse();
+            string.IsNullOrWhiteSpace(actualResult.Errors).ShouldBeTrue();
+        }
+
+        [TestCase]
+        public void StatusWithGitTraceInvalidCharactersTest()
+        {
+            Dictionary<string, string> environmentVariables = new Dictionary<string, string>();
+
+            if (Path.GetInvalidPathChars().Length == 0)
+            {
+                return;
+            }
+
+            string tracePath = Path.Combine(this.Enlistment.EnlistmentRoot, $"trace-log{Path.GetInvalidPathChars()[0]}.txt");
+            environmentVariables["GIT_TRACE"] = tracePath;
+
+            string command = "status";
+            string controlRepoRoot = this.ControlGitRepo.RootPath;
+            string gvfsRepoRoot = this.Enlistment.RepoRoot;
+
+            ProcessResult expectedResult = GitProcess.InvokeProcess(controlRepoRoot, command, environmentVariables);
+            ProcessResult actualResult = GitHelpers.InvokeGitAgainstGVFSRepo(gvfsRepoRoot, command, environmentVariables);
+
+            GitHelpers.ValidateGitOutput(command, expectedResult, actualResult);
+        }
+
         [TestCase]
         public void StatusShortTest()
         {
@@ -263,27 +333,27 @@ public void RenameFilesWithNameAheadOfDot()
         {
             this.FolderShouldExistAndHaveFile("GitCommandsTests", "RenameFileTests", "1", "#test");
             this.MoveFile(
-                Path.Combine("GitCommandsTests", "RenameFileTests", "1", "#test"), 
+                Path.Combine("GitCommandsTests", "RenameFileTests", "1", "#test"),
                 Path.Combine("GitCommandsTests", "RenameFileTests", "1", "#testRenamed"));
 
             this.FolderShouldExistAndHaveFile("GitCommandsTests", "RenameFileTests", "2", "$test");
             this.MoveFile(
-                Path.Combine("GitCommandsTests", "RenameFileTests", "2", "$test"), 
+                Path.Combine("GitCommandsTests", "RenameFileTests", "2", "$test"),
                 Path.Combine("GitCommandsTests", "RenameFileTests", "2", "$testRenamed"));
 
             this.FolderShouldExistAndHaveFile("GitCommandsTests", "RenameFileTests", "3", ")");
             this.MoveFile(
-                Path.Combine("GitCommandsTests", "RenameFileTests", "3", ")"), 
+                Path.Combine("GitCommandsTests", "RenameFileTests", "3", ")"),
                 Path.Combine("GitCommandsTests", "RenameFileTests", "3", ")Renamed"));
 
             this.FolderShouldExistAndHaveFile("GitCommandsTests", "RenameFileTests", "4", "+.test");
             this.MoveFile(
-                Path.Combine("GitCommandsTests", "RenameFileTests", "4", "+.test"), 
+                Path.Combine("GitCommandsTests", "RenameFileTests", "4", "+.test"),
                 Path.Combine("GitCommandsTests", "RenameFileTests", "4", "+.testRenamed"));
 
             this.FolderShouldExistAndHaveFile("GitCommandsTests", "RenameFileTests", "5", "-.test");
             this.MoveFile(
-                Path.Combine("GitCommandsTests", "RenameFileTests", "5", "-.test"), 
+                Path.Combine("GitCommandsTests", "RenameFileTests", "5", "-.test"),
                 Path.Combine("GitCommandsTests", "RenameFileTests", "5", "-.testRenamed"));
 
             this.ValidateGitCommand("status");

diff --git a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitRepoTests.cs b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitRepoTests.cs
@@ -3,8 +3,7 @@
 using GVFS.FunctionalTests.Tools;
 using GVFS.Tests.Should;
 using NUnit.Framework;
-using NUnit.Framework.Interfaces;
-using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 

diff --git a/GVFS/GVFS.FunctionalTests/Tools/GitHelpers.cs b/GVFS/GVFS.FunctionalTests/Tools/GitHelpers.cs
@@ -86,16 +86,21 @@ public static void ValidateGitCommand(
             ProcessResult expectedResult = GitProcess.InvokeProcess(controlRepoRoot, command);
             ProcessResult actualResult = GitHelpers.InvokeGitAgainstGVFSRepo(gvfsRepoRoot, command);
 
-            ErrorsShouldMatch(command, expectedResult, actualResult);
-            actualResult.Output.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries)
-                .ShouldMatchInOrder(expectedResult.Output.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries), LinesAreEqual, command + " Output Lines");
+            ValidateGitOutput(command, expectedResult, actualResult);
 
             if (command != "status")
             {
                 ValidateGitCommand(enlistment, controlGitRepo, "status");
             }
         }
 
+        public static void ValidateGitOutput(string command, ProcessResult expectedResult, ProcessResult actualResult)
+        {
+            ErrorsShouldMatch(command, expectedResult, actualResult);
+            actualResult.Output.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries)
+                .ShouldMatchInOrder(expectedResult.Output.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries), LinesAreEqual, command + " Output Lines");
+        }
+
         /// <summary>
         /// Acquire the GVFSLock. This method will return once the GVFSLock has been acquired.
         /// </summary>