Merge pull request #1881 from microsoft/milestones/m266

Release M266

Author: mjcheetham

.github/workflows/build.yaml

@@ -29,7 +29,7 @@ jobs:
     - name: Look for prior successful runs
       id: check
       if: github.event.inputs.git_version == ''
-      uses: actions/github-script@v7
+      uses: actions/github-script@v8
       with:
         github-token: ${{secrets.GITHUB_TOKEN}}
         result-encoding: string
@@ -130,7 +130,7 @@ jobs:
     - name: Skip this job if there is a previous successful run
       if: needs.validate.outputs.skip != ''
       id: skip
-      uses: actions/github-script@v7
+      uses: actions/github-script@v8
       with:
         script: |
           core.info(`Skipping: There already is a successful run: ${{ needs.validate.outputs.skip }}`)
@@ -212,7 +212,7 @@ jobs:
     - name: Skip this job if there is a previous successful run
       if: needs.validate.outputs.skip != ''
       id: skip
-      uses: actions/github-script@v7
+      uses: actions/github-script@v8
       with:
         script: |
           core.info(`Skipping: There already is a successful run: ${{ needs.validate.outputs.skip }}`)

GVFS/GVFS.Common/Git/GitRepo.cs

@@ -113,6 +113,21 @@ public virtual bool TryGetBlobLength(string blobSha, out long size)
             return this.GetLooseBlobState(blobSha, null, out size) == LooseBlobState.Exists;
         }
 
+        /// <summary>
+        /// Try to find the SHAs of subtrees missing from the given tree.
+        /// </summary>
+        /// <param name="treeSha">Tree to look up</param>
+        /// <param name="subtrees">SHAs of subtrees of this tree which are not downloaded yet.</param>
+        /// <returns></returns>
+        public virtual bool TryGetMissingSubTrees(string treeSha, out string[] subtrees)
+        {
+            string[] missingSubtrees = null;
+            var succeeded = this.libgit2RepoInvoker.TryInvoke(repo =>
+                repo.GetMissingSubTrees(treeSha), out missingSubtrees);
+            subtrees = missingSubtrees;
+            return succeeded;
+        }
+
         public void Dispose()
         {
             if (this.libgit2RepoInvoker != null)

GVFS/GVFS.Common/Git/HashingStream.cs

@@ -17,7 +17,7 @@ public HashingStream(Stream stream)
         {
             this.stream = stream;
 
-            this.hash = SHA1.Create();
+            this.hash = SHA1.Create(); // CodeQL [SM02196] SHA-1 is acceptable here because this is Git's hashing algorithm, not used for cryptographic purposes
             this.hashResult = null;
             this.hash.Initialize();
             this.closed = false;

GVFS/GVFS.Common/Git/LibGit2Repo.cs

@@ -1,5 +1,6 @@
 using GVFS.Common.Tracing;
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Runtime.InteropServices;
 
@@ -147,6 +148,82 @@ public virtual bool TryCopyBlob(string sha, Action<Stream, long> writeAction)
             return true;
         }
 
+        /// <summary>
+        /// Get the list of missing subtrees for the given treeSha.
+        /// </summary>
+        /// <param name="treeSha">Tree to look up</param>
+        /// <param name="missingSubtrees">SHAs of subtrees of this tree which are not downloaded yet.</param>
+        public virtual string[] GetMissingSubTrees(string treeSha)
+        {
+            List<string> missingSubtreesList = new List<string>();
+            IntPtr treeHandle;
+            if (Native.RevParseSingle(out treeHandle, this.RepoHandle, treeSha) != Native.SuccessCode
+                || treeHandle == IntPtr.Zero)
+            {
+                return Array.Empty<string>();
+            }
+
+            try
+            {
+                if (Native.Object.GetType(treeHandle) != Native.ObjectTypes.Tree)
+                {
+                    return Array.Empty<string>();
+                }
+
+                uint entryCount = Native.Tree.GetEntryCount(treeHandle);
+                for (uint i = 0; i < entryCount; i++)
+                {
+                    if (this.IsMissingSubtree(treeHandle, i, out string entrySha))
+                    {
+                        missingSubtreesList.Add(entrySha);
+                    }
+                }
+            }
+            finally
+            {
+                Native.Object.Free(treeHandle);
+            }
+
+            return missingSubtreesList.ToArray();
+        }
+
+        /// <summary>
+        /// Determine if the given index of a tree is a subtree and if it is missing.
+        /// If it is a missing subtree, return the SHA of the subtree.
+        /// </summary>
+        private bool IsMissingSubtree(IntPtr treeHandle, uint i, out string entrySha)
+        {
+            entrySha = null;
+            IntPtr entryHandle = Native.Tree.GetEntryByIndex(treeHandle, i);
+            if (entryHandle == IntPtr.Zero)
+            {
+                return false;
+            }
+
+            var entryMode = Native.Tree.GetEntryFileMode(entryHandle);
+            if (entryMode != Native.Tree.TreeEntryFileModeDirectory)
+            {
+                return false;
+            }
+
+            var entryId = Native.Tree.GetEntryId(entryHandle);
+            if (entryId == IntPtr.Zero)
+            {
+                return false;
+            }
+
+            var rawEntrySha = Native.IntPtrToGitOid(entryId);
+            entrySha = rawEntrySha.ToString();
+
+            if (this.ObjectExists(entrySha))
+            {
+                return false;
+            }
+            return true;
+            /* Both the entryHandle and the entryId handle are owned by the treeHandle, so we shouldn't free them or it will lead to corruption of the later entries */
+        }
+
+
         public void Dispose()
         {
             this.Dispose(true);
@@ -247,6 +324,26 @@ public static class Blob
                 [DllImport(Git2NativeLibName, EntryPoint = "git_blob_rawcontent")]
                 public static unsafe extern byte* GetRawContent(IntPtr objectHandle);
             }
+
+            public static class Tree
+            {
+                [DllImport(Git2NativeLibName, EntryPoint = "git_tree_entrycount")]
+                public static extern uint GetEntryCount(IntPtr treeHandle);
+
+                [DllImport(Git2NativeLibName, EntryPoint = "git_tree_entry_byindex")]
+                public static extern IntPtr GetEntryByIndex(IntPtr treeHandle, uint index);
+
+                [DllImport(Git2NativeLibName, EntryPoint = "git_tree_entry_id")]
+                public static extern IntPtr GetEntryId(IntPtr entryHandle);
+
+                /* git_tree_entry_type requires the object to exist, so we can't use it to check if
+                 * a missing entry is a tree. Instead, we can use the file mode to determine if it is a tree. */
+                [DllImport(Git2NativeLibName, EntryPoint = "git_tree_entry_filemode")]
+                public static extern uint GetEntryFileMode(IntPtr entryHandle);
+
+                public const uint TreeEntryFileModeDirectory = 0x4000;
+
+            }
         }
     }
 }

GVFS/GVFS.Common/Http/CacheServerResolver.cs

@@ -54,12 +54,12 @@ public bool TryResolveUrlFromRemote(
             if (cacheServerName.Equals(CacheServerInfo.ReservedNames.Default, StringComparison.OrdinalIgnoreCase))
             {
                 cacheServer =
-                    serverGVFSConfig.CacheServers.FirstOrDefault(cache => cache.GlobalDefault)
+                    serverGVFSConfig?.CacheServers.FirstOrDefault(cache => cache.GlobalDefault)
                     ?? this.CreateNone();
             }
             else
             {
-                cacheServer = serverGVFSConfig.CacheServers.FirstOrDefault(cache =>
+                cacheServer = serverGVFSConfig?.CacheServers.FirstOrDefault(cache =>
                     cache.Name.Equals(cacheServerName, StringComparison.OrdinalIgnoreCase));
 
                 if (cacheServer == null)
@@ -87,7 +87,7 @@ public CacheServerInfo ResolveNameFromRemote(
             }
 
             return
-                serverGVFSConfig.CacheServers.FirstOrDefault(cache => cache.Url.Equals(cacheServerUrl, StringComparison.OrdinalIgnoreCase))
+                serverGVFSConfig?.CacheServers.FirstOrDefault(cache => cache.Url.Equals(cacheServerUrl, StringComparison.OrdinalIgnoreCase))
                 ?? new CacheServerInfo(cacheServerUrl, CacheServerInfo.ReservedNames.UserDefined);
         }
 

GVFS/GVFS.Common/SHA1Util.cs

@@ -21,7 +21,7 @@ public static byte[] SHA1ForUTF8String(string s)
         {
             byte[] bytes = Encoding.UTF8.GetBytes(s);
 
-            using (SHA1 sha1 = SHA1.Create())
+            using (SHA1 sha1 = SHA1.Create()) // CodeQL [SM02196] SHA-1 is acceptable here because this is Git's hashing algorithm, not used for cryptographic purposes
             {
                 return sha1.ComputeHash(bytes);
             }

GVFS/GVFS.Mount/InProcessMount.cs

@@ -27,6 +27,12 @@ public class InProcessMount
         private const int MaxPipeNameLength = 250;
         private const int MutexMaxWaitTimeMS = 500;
 
+        // This is value chosen based on tested scenarios to limit the required download time for
+        // all the trees. This is approximately the amount of trees that can be downloaded in 1 second.
+        // Downloading an entire commit pack also takes around 1 second, so this should limit downloading
+        // all the trees in a commit to ~2-3 seconds.
+        private const int MissingTreeThresholdForDownloadingCommitPack = 200;
+
         private readonly bool showDebugWindow;
 
         private FileSystemCallbacks fileSystemCallbacks;
@@ -47,7 +53,6 @@ public class InProcessMount
         private ManualResetEvent unmountEvent;
 
         private readonly Dictionary<string, string> treesWithDownloadedCommits = new Dictionary<string,string>();
-        private DateTime lastCommitPackDownloadTime = DateTime.MinValue;
 
         // True if InProcessMount is calling git reset as part of processing
         // a folder dehydrate request
@@ -518,13 +523,14 @@ private void HandleDownloadObjectRequest(NamedPipeMessages.Message message, Name
                     if (this.ShouldDownloadCommitPack(objectSha, out string commitSha)
                         && this.gitObjects.TryDownloadCommit(commitSha))
                     {
-                        this.DownloadedCommitPack(objectSha: objectSha, commitSha: commitSha);
+                        this.DownloadedCommitPack(commitSha);
                         response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.SuccessResult);
                         // FUTURE: Should the stats be updated to reflect all the trees in the pack?
                         // FUTURE: Should we try to clean up duplicate trees or increase depth of the commit download?
                     }
                     else if (this.gitObjects.TryDownloadAndSaveObject(objectSha, GVFSGitObjects.RequestSource.NamedPipeMessage) == GitObjects.DownloadAndSaveObjectResult.Success)
                     {
+                        this.UpdateTreesForDownloadedCommits(objectSha);
                         response = new NamedPipeMessages.DownloadObject.Response(NamedPipeMessages.DownloadObject.SuccessResult);
                     }
                     else
@@ -548,7 +554,7 @@ private void HandleDownloadObjectRequest(NamedPipeMessages.Message message, Name
                          * Otherwise, the trees for the commit may be needed soon depending on the context. 
                          * e.g. git log (without a pathspec) doesn't need trees, but git checkout does.
                          * 
-                         * Save the tree/commit so if the tree is requested soon we can download all the trees for the commit in a batch.
+                         * Save the tree/commit so if more trees are requested we can download all the trees for the commit in a batch.
                          */
                         this.treesWithDownloadedCommits[treeSha] = objectSha;
                     }
@@ -561,28 +567,67 @@ private void HandleDownloadObjectRequest(NamedPipeMessages.Message message, Name
         private bool PrefetchHasBeenDone()
         {
             var prefetchPacks = this.gitObjects.ReadPackFileNames(this.enlistment.GitPackRoot, GVFSConstants.PrefetchPackPrefix);
-            return prefetchPacks.Length > 0;
+            var result = prefetchPacks.Length > 0;
+            if (result)
+            {
+                this.treesWithDownloadedCommits.Clear();
+            }
+            return result;
         }
 
         private bool ShouldDownloadCommitPack(string objectSha, out string commitSha)
         {
-
             if (!this.treesWithDownloadedCommits.TryGetValue(objectSha, out commitSha)
                 || this.PrefetchHasBeenDone())
             {
                 return false;
             }
 
-            /* This is a heuristic to prevent downloading multiple packs related to git history commands,
-             * since commits downloaded close together likely have similar trees. */
-            var timePassed = DateTime.UtcNow - this.lastCommitPackDownloadTime;
-            return (timePassed > TimeSpan.FromMinutes(5));
+            /* This is a heuristic to prevent downloading multiple packs related to git history commands.
+             * Closely related commits are likely to have similar trees, so we'll find fewer missing trees in them.
+             * Conversely, if we know (from previously downloaded missing trees) that a commit has a lot of missing
+             * trees left, we'll probably need to download many more trees for the commit so we should download the pack.
+             */
+            var commitShaLocal = commitSha; // can't use out parameter in lambda
+            int missingTreeCount = this.treesWithDownloadedCommits.Where(x => x.Value == commitShaLocal).Count();
+            return missingTreeCount > MissingTreeThresholdForDownloadingCommitPack;
+        }
+
+        private void UpdateTreesForDownloadedCommits(string objectSha)
+        {
+            /* If we are downloading missing trees, we probably are missing more trees for the commit.
+             * Update our list of trees associated with the commit so we can use the # of missing trees 
+             * as a heuristic to decide whether to batch download all the trees for the commit the
+             * next time a missing one is requested.
+             */
+            if (!this.treesWithDownloadedCommits.TryGetValue(objectSha, out var commitSha)
+                || this.PrefetchHasBeenDone())
+            {
+                return;
+            }
+
+            if (!this.context.Repository.TryGetObjectType(objectSha, out var objectType)
+                || objectType != Native.ObjectTypes.Tree)
+            {
+                return;
+            }
+
+            if (this.context.Repository.TryGetMissingSubTrees(objectSha, out var missingSubTrees))
+            {
+                foreach (var missingSubTree in missingSubTrees)
+                {
+                    this.treesWithDownloadedCommits[missingSubTree] = commitSha;
+                }
+            }
         }
 
-        private void DownloadedCommitPack(string objectSha, string commitSha)
+        private void DownloadedCommitPack(string commitSha)
         {
-            this.lastCommitPackDownloadTime = DateTime.UtcNow;
-            this.treesWithDownloadedCommits.Remove(objectSha);
+            var toRemove = this.treesWithDownloadedCommits.Where(x => x.Value == commitSha).ToList();
+            foreach (var tree in toRemove)
+            {
+                this.treesWithDownloadedCommits.Remove(tree.Key);
+            }
         }
 
         private void HandlePostFetchJobRequest(NamedPipeMessages.Message message, NamedPipeServer.Connection connection)

GVFS/GVFS.ReadObjectHook/main.cpp

@@ -84,13 +84,13 @@ int main(int, char *argv[])
     DisableCRLFTranslationOnStdPipes();
 
     packet_txt_read(packet_buffer, sizeof(packet_buffer));
-    if (strcmp(packet_buffer, "git-read-object-client"))
+    if (strcmp(packet_buffer, "git-read-object-client")) // CodeQL [SM01932] `packet_txt_read()` either NUL-terminates or `die()`s
     {
         die(ReadObjectHookErrorReturnCode::ErrorReadObjectProtocol, "Bad welcome message\n");
     }
 
     packet_txt_read(packet_buffer, sizeof(packet_buffer));
-    if (strcmp(packet_buffer, "version=1"))
+    if (strcmp(packet_buffer, "version=1")) // CodeQL [SM01932] `packet_txt_read()` either NUL-terminates or `die()`s
     {
         die(ReadObjectHookErrorReturnCode::ErrorReadObjectProtocol, "Bad version\n");
     }
@@ -105,7 +105,7 @@ int main(int, char *argv[])
     packet_flush();
 
     packet_txt_read(packet_buffer, sizeof(packet_buffer));
-    if (strcmp(packet_buffer, "capability=get"))
+    if (strcmp(packet_buffer, "capability=get")) // CodeQL [SM01932] `packet_txt_read()` either NUL-terminates or `die()`s
     {
         die(ReadObjectHookErrorReturnCode::ErrorReadObjectProtocol, "Bad capability\n");
     }
@@ -125,13 +125,13 @@ int main(int, char *argv[])
     while (1)
     {
         packet_txt_read(packet_buffer, sizeof(packet_buffer));
-        if (strcmp(packet_buffer, "command=get"))
+        if (strcmp(packet_buffer, "command=get")) // CodeQL [SM01932] `packet_txt_read()` either NUL-terminates or `die()`s
         {
             die(ReadObjectHookErrorReturnCode::ErrorReadObjectProtocol, "Bad command\n");
         }
 
         len = packet_txt_read(packet_buffer, sizeof(packet_buffer));
-        if ((len != SHA1_LENGTH + 5) || strncmp(packet_buffer, "sha1=", 5))
+        if ((len != SHA1_LENGTH + 5) || strncmp(packet_buffer, "sha1=", 5)) // CodeQL [SM01932] `packet_txt_read()` either NUL-terminates or `die()`s
         {
             die(ReadObjectHookErrorReturnCode::ErrorReadObjectProtocol, "Bad sha1 in get command\n");
         }

GVFS/GVFS.Virtualization/Projection/GitIndexProjection.FolderData.cs

@@ -54,7 +54,7 @@ public void Include()
 
             public string HashedChildrenNamesSha()
             {
-                using (HashAlgorithm hash = SHA1.Create())
+                using (HashAlgorithm hash = SHA1.Create()) // CodeQL [SM02196] SHA-1 is acceptable here because this is Git's hashing algorithm, not used for cryptographic purposes
                 {
                     for (int i = 0; i < this.ChildEntries.Count; i++)
                     {