Merged PR 569687: Fix some frontend hash check problems.

1. Check the nuget download file using the hashtype given in the hash configured instead of the build's default hashtype.
2. Do not record the hash of the downloaded file in download resolver if the file is not up to date.

Related work items: #1761395, #1761396

Public/Src/Cache/ContentStore/Hashing/ContentHash.cs

@@ -40,9 +40,13 @@ namespace BuildXL.Cache.ContentStore.Hashing
         /// </summary>
         public const int SerializedLength = MaxHashByteLength + 1;
 
+        /// <summary>
+        /// HashType.
+        /// </summary>
+        public readonly HashType _hashType;
+
         internal const char SerializedDelimiter = ':';
 
-        private readonly HashType _hashType;
         private readonly ReadOnlyFixedBytes _bytes;
 
         /// <summary>

Public/Src/FrontEnd/Core/FrontEndHostController.Nuget.cs

@@ -528,7 +528,21 @@ namespace BuildXL.FrontEnd.Core
             {
                 try
                 {
-                    var existingContentHash = await ContentHashingUtilities.HashFileAsync(targetFilePath);
+                    // Make sure we honor the expectedContentHash.Value.HashType 
+                    // We compute the file's hash using the same hash type(algorithm) as the expected hash instead of the build's default hash type.
+                    // So the comparison of the existing hash and expected hash is meaningful.
+                    ContentHash existingContentHash;
+                    using (
+                        var fs = FileUtilities.CreateFileStream(
+                            targetFilePath,
+                            FileMode.Open,
+                            FileAccess.Read,
+                            FileShare.Delete | FileShare.Read,
+                            FileOptions.SequentialScan))
+                    {
+                        existingContentHash = await ContentHashingUtilities.HashContentStreamAsync(fs, expectedContentHash.Value.HashType);
+                    }
+
                     if (existingContentHash == expectedContentHash.Value)
                     {
                         m_logger.DownloadToolIsUpToDate(loggingContext, friendlyName, targetFilePath, expectedContentHash.Value.ToHex());

Public/Src/FrontEnd/Download/DownloadResolver.cs

@@ -244,25 +244,31 @@ namespace BuildXL.FrontEnd.Download
             try
             {
                 var downloadFilePath = downloadData.DownloadedFilePath.ToString(m_context.PathTable);
-                // Check if the file already exists and matches the exected hash.
+                // Check if the file already exists and matches the expected hash.
                 if (File.Exists(downloadFilePath))
                 {
                     var expectedHashType = downloadData.ContentHash.HasValue
                         ? downloadData.ContentHash.Value.HashType
                         : HashTypeParser(downloadData.Settings.Hash);
-                   
-                    // Compare actual hash to compare if we need to download again.
-                    var actualHash = await GetContentHashAsync(downloadData.DownloadedFilePath, expectedHashType);
+                    
+                    // We don't record the file until we know it is the one used in this build.
+                    var recordFileAccess = false;
+                    var actualHash = await GetContentHashAsync(downloadData.DownloadedFilePath, expectedHashType, recordFileAccess);
 
+                    // Compare actual hash to compare if we need to download again.
                     // Compare against the static hash value.
                     if (downloadData.ContentHash.HasValue && actualHash == downloadData.ContentHash.Value)
                     {
+                        // Record the file with the build's default hasher.
+                        m_frontEndHost.Engine.RecordFrontEndFile(downloadData.DownloadedFilePath, Name);
                         return new EvaluationResult(FileArtifact.CreateSourceFile(downloadData.DownloadedFilePath));
                     }
 
                     var incrementalState = await DownloadIncrementalState.TryLoadAsync(m_logger, m_context, downloadData);
                     if (incrementalState != null && incrementalState.ContentHash == actualHash)
                     {
+                        // Record the file with the build's default hasher.
+                        m_frontEndHost.Engine.RecordFrontEndFile(downloadData.DownloadedFilePath, Name);
                         return new EvaluationResult(FileArtifact.CreateSourceFile(downloadData.DownloadedFilePath));
                     }
                 }
@@ -370,7 +376,9 @@ namespace BuildXL.FrontEnd.Download
         private async Task<EvaluationResult> ValidateAndStoreIncrementalDownloadStateAsync(DownloadData downloadData)
         {
             // If the hash is given in the download setting, use the corresponding hashType(hash algorithm) to get the content hash of the downloaded file.
-            var downloadedHash = await GetContentHashAsync(downloadData.DownloadedFilePath, HashTypeParser(downloadData.Settings.Hash));
+            // We don't record the file until we know it is the correct one and will be used in this build.
+            var recordFileAccess = false;
+            var downloadedHash = await GetContentHashAsync(downloadData.DownloadedFilePath, HashTypeParser(downloadData.Settings.Hash), recordFileAccess);
             
             if (downloadData.ContentHash.HasValue)
             {
@@ -400,6 +408,8 @@ namespace BuildXL.FrontEnd.Download
                 }
             }
 
+            // Record the file with the build's default hasher.
+            m_frontEndHost.Engine.RecordFrontEndFile(downloadData.DownloadedFilePath, Name);
             return new EvaluationResult(FileArtifact.CreateSourceFile(downloadData.DownloadedFilePath));
         }
 
@@ -770,10 +780,13 @@ namespace BuildXL.FrontEnd.Download
             return Task.FromResult<bool?>(true);
         }
 
-        private async Task<ContentHash> GetContentHashAsync(AbsolutePath path, HashType hashType = HashType.Unknown)
+        private async Task<ContentHash> GetContentHashAsync(AbsolutePath path, HashType hashType = HashType.Unknown, bool recordFile = true)
         {
-            m_frontEndHost.Engine.RecordFrontEndFile(path, Name);
-
+            if (recordFile)
+            {
+                m_frontEndHost.Engine.RecordFrontEndFile(path, Name);
+            }
+            
             // We don't call GetFileContentHashAsync() to get the existing hash, since it register the file. 
             // This has been done in RecordFrontEndFile with the default hasher, re-register it with the specified hasher will cause error.
             using (

Public/Src/FrontEnd/Nuget/Tracing/Log.cs

@@ -47,8 +47,8 @@ namespace BuildXL.FrontEnd.Nuget.Tracing
             EventLevel = Level.Warning,
             Keywords = (ushort)Keywords.UserMessage,
             EventTask = (ushort)Tasks.Parser,
-            Message = "Configured downloadHash '{hash}' for tool '{toolName}' is invalid. Expected '{hashType}' with {length} bytes. Attempt to download tool without hash guard.")]
-        public abstract void NugetDownloadInvalidHash(LoggingContext context, string toolName, string hash, string hashType, int length);
+            Message = "Configured downloadHash '{hash}' for tool '{toolName}' is invalid. Attempt to download tool without hash guard.")]
+        public abstract void NugetDownloadInvalidHash(LoggingContext context, string toolName, string hash);
 
         [GeneratedEvent(
             (ushort)LogEventId.NugetFailedToCleanTargetFolder,

Public/Src/FrontEnd/Nuget/WorkspaceNugetModuleResolver.cs

@@ -1677,15 +1677,13 @@ namespace BuildXL.FrontEnd.Nuget
             expectedHash = null;
             if (!string.IsNullOrEmpty(artifactLocation.Hash))
             {
-                if (!ContentHashingUtilities.TryParse(artifactLocation.Hash, out var contentHash))
+                if (!ContentHash.TryParse(artifactLocation.Hash, out var contentHash))
                 {
                     // TODO: better provenance for configuration settings.
                     Logger.Log.NugetDownloadInvalidHash(
                         m_context.LoggingContext,
                         "nuget.exe",
-                        artifactLocation.Hash,
-                        ContentHashingUtilities.HashInfo.HashType.ToString(),
-                        ContentHashingUtilities.HashInfo.ByteLength);
+                        artifactLocation.Hash);
                     return false;
                 }
 

config.dsc

@@ -45,7 +45,7 @@ config({
             // but when it fails to download (e.g. from a share) the build is aborted. Consider making the failure non-blocking.
             configuration: {
                 toolUrl: "https://dist.nuget.org/win-x86-commandline/v4.9.4/NuGet.exe",
-                hash: "17E8C8C0CDCCA3A6D1EE49836847148C4623ACEA5E6E36E10B691DA7FDC4C39200"
+                hash: "VSO0:17E8C8C0CDCCA3A6D1EE49836847148C4623ACEA5E6E36E10B691DA7FDC4C39200"
             },
 
             repositories: importFile(f`config.microsoftInternal.dsc`).isMicrosoftInternal