Merge pull request microsoft#1856 from tyrielv/restore-repro-test

Fix corruption when unstaging changes made by git commands

Author: tyrielv

GVFS/GVFS.Common/Git/GitProcess.cs

@@ -509,6 +509,138 @@ public Result StatusPorcelain()
             return this.InvokeGitInWorkingDirectoryRoot(command, useReadObjectHook: false);
         }
 
+        /// <summary>
+        /// Returns staged file changes (index vs HEAD) as null-separated pairs of
+        /// status and path: "A\0path1\0M\0path2\0D\0path3\0".
+        /// Status codes: A=added, M=modified, D=deleted, R=renamed, C=copied.
+        /// </summary>
+        /// <param name="pathspecs">Inline pathspecs to scope the diff, or null for all.</param>
+        /// <param name="pathspecFromFile">
+        /// Path to a file containing additional pathspecs (one per line), forwarded
+        /// as --pathspec-from-file to git. Null if not used.
+        /// </param>
+        /// <param name="pathspecFileNul">
+        /// When true and pathspecFromFile is set, pathspec entries in the file are
+        /// separated by NUL instead of newline (--pathspec-file-nul).
+        /// </param>
+        public Result DiffCachedNameStatus(string[] pathspecs = null, string pathspecFromFile = null, bool pathspecFileNul = false)
+        {
+            string command = "diff --cached --name-status -z --no-renames";
+
+            if (pathspecFromFile != null)
+            {
+                command += " --pathspec-from-file=" + QuoteGitPath(pathspecFromFile);
+                if (pathspecFileNul)
+                {
+                    command += " --pathspec-file-nul";
+                }
+            }
+
+            if (pathspecs != null && pathspecs.Length > 0)
+            {
+                command += " -- " + string.Join(" ", pathspecs.Select(p => QuoteGitPath(p)));
+            }
+
+            return this.InvokeGitInWorkingDirectoryRoot(command, useReadObjectHook: false);
+        }
+
+        /// <summary>
+        /// Writes the staged (index) version of the specified files to the working
+        /// tree with correct line endings and attributes. Batches multiple paths into
+        /// a single git process invocation where possible, respecting the Windows
+        /// command line length limit.
+        /// </summary>
+        public List<Result> CheckoutIndexForFiles(IEnumerable<string> paths)
+        {
+            // Windows command line limit is 32,767 characters. Leave headroom for
+            // the base command and other arguments.
+            const int MaxCommandLength = 30000;
+            const string BaseCommand = "-c core.hookspath= checkout-index --force --";
+
+            List<Result> results = new List<Result>();
+            StringBuilder command = new StringBuilder(BaseCommand);
+            foreach (string path in paths)
+            {
+                string quotedPath = " " + QuoteGitPath(path);
+
+                if (command.Length + quotedPath.Length > MaxCommandLength && command.Length > BaseCommand.Length)
+                {
+                    // Flush current batch
+                    results.Add(this.InvokeGitInWorkingDirectoryRoot(command.ToString(), useReadObjectHook: false));
+                    command.Clear();
+                    command.Append(BaseCommand);
+                }
+
+                command.Append(quotedPath);
+            }
+
+            // Flush remaining paths
+            if (command.Length > BaseCommand.Length)
+            {
+                results.Add(this.InvokeGitInWorkingDirectoryRoot(command.ToString(), useReadObjectHook: false));
+            }
+
+            return results;
+        }
+
+        /// <summary>
+        /// Wraps a path in double quotes for use as a git command argument,
+        /// escaping any embedded double quotes and any backslashes that
+        /// immediately precede a double quote (to prevent them from being
+        /// interpreted as escape characters by the Windows C runtime argument
+        /// parser). Lone backslashes used as path separators are left as-is.
+        /// </summary>
+        public static string QuoteGitPath(string path)
+        {
+            StringBuilder sb = new StringBuilder(path.Length + 4);
+            sb.Append('"');
+
+            for (int i = 0; i < path.Length; i++)
+            {
+                if (path[i] == '"')
+                {
+                    sb.Append('\\');
+                    sb.Append('"');
+                }
+                else if (path[i] == '\\')
+                {
+                    // Count consecutive backslashes
+                    int backslashCount = 0;
+                    while (i < path.Length && path[i] == '\\')
+                    {
+                        backslashCount++;
+                        i++;
+                    }
+
+                    if (i < path.Length && path[i] == '"')
+                    {
+                        // Backslashes before a quote: double them all, then escape the quote
+                        sb.Append('\\', backslashCount * 2);
+                        sb.Append('\\');
+                        sb.Append('"');
+                    }
+                    else if (i == path.Length)
+                    {
+                        // Backslashes at end of string (before closing quote): double them
+                        sb.Append('\\', backslashCount * 2);
+                    }
+                    else
+                    {
+                        // Backslashes not before a quote: keep as-is (path separators)
+                        sb.Append('\\', backslashCount);
+                        i--; // Re-process current non-backslash char
+                    }
+                }
+                else
+                {
+                    sb.Append(path[i]);
+                }
+            }
+
+            sb.Append('"');
+            return sb.ToString();
+        }
+
         public Result SerializeStatus(bool allowObjectDownloads, string serializePath)
         {
             // specify ignored=matching and --untracked-files=complete

GVFS/GVFS.Common/NamedPipes/UnstageNamedPipeMessages.cs

@@ -0,0 +1,27 @@
+namespace GVFS.Common.NamedPipes
+{
+    public static partial class NamedPipeMessages
+    {
+        public static class PrepareForUnstage
+        {
+            public const string Request = "PreUnstage";
+            public const string SuccessResult = "S";
+            public const string FailureResult = "F";
+
+            public class Response
+            {
+                public Response(string result)
+                {
+                    this.Result = result;
+                }
+
+                public string Result { get; }
+
+                public Message CreateMessage()
+                {
+                    return new Message(this.Result, null);
+                }
+            }
+        }
+    }
+}

GVFS/GVFS.FunctionalTests/Tests/GitCommands/CorruptionReproTests.cs

@@ -0,0 +1,79 @@
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using GVFS.Common;
+using GVFS.FunctionalTests.Properties;
+using GVFS.FunctionalTests.Tests.EnlistmentPerTestCase;
+using NUnit.Framework;
+
+namespace GVFS.FunctionalTests.Tests.GitCommands
+{
+    /// <summary>
+    /// This class is used to reproduce corruption scenarios in the GVFS virtual projection.
+    /// </summary>
+    [Category(Categories.GitCommands)]
+    [TestFixtureSource(typeof(GitRepoTests), nameof(GitRepoTests.ValidateWorkingTree))]
+    public class CorruptionReproTests : GitRepoTests
+    {
+        public CorruptionReproTests(Settings.ValidateWorkingTreeMode validateWorkingTree)
+            : base(enlistmentPerTest: true, validateWorkingTree: validateWorkingTree)
+        {
+        }
+
+        [TestCase]
+        public void ReproCherryPickRestoreCorruption()
+        {
+            // Reproduces a corruption scenario where git commands (like cherry-pick -n)
+            // stage changes directly, bypassing the filesystem. In VFS mode, these staged
+            // files have skip-worktree set and are not in the ModifiedPaths database.
+            // Without the fix, a subsequent "restore --staged" would fail to properly
+            // unstage them, leaving the index and projection in an inconsistent state.
+            //
+            // See https://github.com/microsoft/VFSForGit/issues/1855
+
+            // Based on FunctionalTests/20170206_Conflict_Source
+            const string CherryPickCommit = "51d15f7584e81d59d44c1511ce17d7c493903390";
+            const string StartingCommit = "db95d631e379d366d26d899523f8136a77441914";
+
+            this.ControlGitRepo.Fetch(StartingCommit);
+            this.ControlGitRepo.Fetch(CherryPickCommit);
+
+            this.ValidateGitCommand($"checkout -b FunctionalTests/CherryPickRestoreCorruptionRepro {StartingCommit}");
+
+            // Cherry-pick stages adds, deletes, and modifications without committing.
+            // In VFS mode, these changes are made directly by git in the index — they
+            // are not in ModifiedPaths, so all affected files still have skip-worktree set.
+            this.ValidateGitCommand($"cherry-pick -n {CherryPickCommit}");
+
+            // Restore --staged for a single file first. This verifies that only the
+            // targeted file is added to ModifiedPaths, not all staged files (important
+            // for performance when there are many staged files, e.g. during merge
+            // conflict resolution).
+            //
+            // Before the fix: added files with skip-worktree would be skipped by
+            // restore --staged, remaining stuck as staged in the index.
+            this.ValidateGitCommand("restore --staged Test_ConflictTests/AddedFiles/AddedBySource.txt");
+
+            // Restore --staged for everything remaining. Before the fix:
+            // - Modified files: restored in the index but invisible to git status
+            //   because skip-worktree was set and the file wasn't in ModifiedPaths,
+            //   so git never checked the working tree against the index.
+            // - Deleted files: same issue — deletions became invisible.
+            // - Added files: remained stuck as staged because restore --staged
+            //   skipped them (skip-worktree set), and their ProjFS placeholders
+            //   would later vanish when the projection reverted to HEAD.
+            this.ValidateGitCommand("restore --staged .");
+
+            // Restore the working directory. Before the fix, this step would
+            // silently succeed but leave corrupted state: modified/deleted files
+            // had stale projected content that didn't match HEAD, and added files
+            // (as ProjFS placeholders) would vanish entirely since they're not in
+            // HEAD's tree.
+            this.ValidateGitCommand("restore -- .");
+            this.FilesShouldMatchCheckoutOfSourceBranch();
+        }
+    }
+}

GVFS/GVFS.Hooks/GVFS.Hooks.csproj

@@ -49,6 +49,9 @@
     <Compile Include="..\GVFS.Common\NamedPipes\LockNamedPipeMessages.cs">
       <Link>Common\NamedPipes\LockNamedPipeMessages.cs</Link>
     </Compile>
+    <Compile Include="..\GVFS.Common\NamedPipes\UnstageNamedPipeMessages.cs">
+      <Link>Common\NamedPipes\UnstageNamedPipeMessages.cs</Link>
+    </Compile>
     <Compile Include="..\GVFS.Common\NamedPipes\NamedPipeClient.cs">
       <Link>Common\NamedPipes\NamedPipeClient.cs</Link>
     </Compile>

GVFS/GVFS.Hooks/Program.Unstage.cs

@@ -0,0 +1,106 @@
+using GVFS.Common.NamedPipes;
+using System;
+
+namespace GVFS.Hooks
+{
+    /// <summary>
+    /// Partial class for unstage-related pre-command handling.
+    /// Detects "restore --staged" and "checkout HEAD --" operations and sends
+    /// a PrepareForUnstage message to the GVFS mount process so it can add
+    /// staged files to ModifiedPaths before git clears skip-worktree.
+    /// </summary>
+    public partial class Program
+    {
+        /// <summary>
+        /// Sends a PrepareForUnstage message to the GVFS mount process, which will
+        /// add staged files matching the pathspec to ModifiedPaths so that git will
+        /// clear skip-worktree and process them.
+        /// </summary>
+        private static void SendPrepareForUnstageMessage(string command, string[] args)
+        {
+            UnstageCommandParser.PathspecResult pathspecResult = UnstageCommandParser.GetRestorePathspec(command, args);
+
+            if (pathspecResult.Failed)
+            {
+                ExitWithError(
+                    "VFS for Git was unable to determine the pathspecs for this unstage operation.",
+                    "This can happen when --pathspec-from-file=- (stdin) is used.",
+                    "",
+                    "Instead, pass the paths directly on the command line:",
+                    "  git restore --staged <path1> <path2> ...");
+                return;
+            }
+
+            // Build the message body. Format:
+            //   null/empty          → all staged files (no pathspec)
+            //   "path1\0path2"      → inline pathspecs (null-separated)
+            //   "\nF\n<filepath>"   → --pathspec-from-file (mount forwards to git)
+            //   "\nFZ\n<filepath>"  → --pathspec-from-file with --pathspec-file-nul
+            // The leading \n distinguishes file-reference bodies from inline pathspecs.
+            string body;
+            if (pathspecResult.PathspecFromFile != null)
+            {
+                string prefix = pathspecResult.PathspecFileNul ? "\nFZ\n" : "\nF\n";
+                body = prefix + pathspecResult.PathspecFromFile;
+
+                // If there are also inline pathspecs, append them after another \n
+                if (!string.IsNullOrEmpty(pathspecResult.InlinePathspecs))
+                {
+                    body += "\n" + pathspecResult.InlinePathspecs;
+                }
+            }
+            else
+            {
+                body = pathspecResult.InlinePathspecs;
+            }
+
+            string message = string.IsNullOrEmpty(body)
+                ? NamedPipeMessages.PrepareForUnstage.Request
+                : NamedPipeMessages.PrepareForUnstage.Request + "|" + body;
+
+            bool succeeded = false;
+            string failureMessage = null;
+
+            try
+            {
+                using (NamedPipeClient pipeClient = new NamedPipeClient(enlistmentPipename))
+                {
+                    if (pipeClient.Connect())
+                    {
+                        pipeClient.SendRequest(message);
+                        string rawResponse = pipeClient.ReadRawResponse();
+                        if (rawResponse != null && rawResponse.StartsWith(NamedPipeMessages.PrepareForUnstage.SuccessResult))
+                        {
+                            succeeded = true;
+                        }
+                        else
+                        {
+                            failureMessage = "GVFS mount process returned failure for PrepareForUnstage.";
+                        }
+                    }
+                    else
+                    {
+                        failureMessage = "Unable to connect to GVFS mount process.";
+                    }
+                }
+            }
+            catch (Exception e)
+            {
+                failureMessage = "Exception communicating with GVFS: " + e.Message;
+            }
+
+            if (!succeeded && failureMessage != null)
+            {
+                ExitWithError(
+                    failureMessage,
+                    "The unstage operation cannot safely proceed because GVFS was unable to",
+                    "prepare the staged files. This could lead to index corruption.",
+                    "",
+                    "To resolve:",
+                    "  1. Run 'gvfs unmount' and 'gvfs mount' to reset the GVFS state",
+                    "  2. Retry the restore --staged command",
+                    "If the problem persists, run 'gvfs repair' or re-clone the enlistment.");
+            }
+        }
+    }
+}

GVFS/GVFS.Hooks/Program.cs

@@ -10,7 +10,7 @@
 
 namespace GVFS.Hooks
 {
-    public class Program
+    public partial class Program
     {
         private const string PreCommandHook = "pre-command";
         private const string PostCommandHook = "post-command";
@@ -100,6 +100,13 @@ private static void RunPreCommands(string[] args)
                         ProcessHelper.Run("gvfs", "health --status", redirectOutput: false);
                     }
                     break;
+                case "restore":
+                case "checkout":
+                    if (UnstageCommandParser.IsUnstageOperation(command, args))
+                    {
+                        SendPrepareForUnstageMessage(command, args);
+                    }
+                    break;
             }
         }