From eb487b2cf8ecf2f3acf631e7bb5b4ccac06444a9 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Mon, 25 Nov 2024 13:57:58 -0800 Subject: [PATCH 1/8] Refactor RestoreCommand.ExecuteAsync() --- .../RestoreCommand/RestoreCommand.cs | 585 ++++++++++-------- 1 file changed, 321 insertions(+), 264 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index fa59c8df04a..c66f8b46d35 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -157,42 +157,184 @@ public Task ExecuteAsync() return ExecuteAsync(CancellationToken.None); } - public async Task ExecuteAsync(CancellationToken token) + private void InitializeTelemetry(TelemetryActivity telemetry) { - using (var telemetry = TelemetryActivity.Create(parentId: ParentId, eventName: ProjectRestoreInformation)) + telemetry.TelemetryEvent.AddPiiData(ProjectFilePath, _request.Project.FilePath); + bool isPackageSourceMappingEnabled = _request.PackageSourceMapping?.IsEnabled ?? false; + telemetry.TelemetryEvent[PackageSourceMappingIsMappingEnabled] = isPackageSourceMappingEnabled; + telemetry.TelemetryEvent[SourcesCount] = _request.DependencyProviders.RemoteProviders.Count; + int httpSourcesCount = _request.DependencyProviders.RemoteProviders.Where(e => e.IsHttp).Count(); + telemetry.TelemetryEvent[HttpSourcesCount] = httpSourcesCount; + telemetry.TelemetryEvent[LocalSourcesCount] = _request.DependencyProviders.RemoteProviders.Count - httpSourcesCount; + telemetry.TelemetryEvent[FallbackFoldersCount] = _request.DependencyProviders.FallbackPackageFolders.Count; + telemetry.TelemetryEvent[IsLockFileEnabled] = _isLockFileEnabled; + telemetry.TelemetryEvent[UseLegacyDependencyResolver] = _request.Project.RestoreMetadata.UseLegacyDependencyResolver; + telemetry.TelemetryEvent[UsedLegacyDependencyResolver] = !_enableNewDependencyResolver; + var isCpvmEnabled = _request.Project.RestoreMetadata?.CentralPackageVersionsEnabled ?? false; + telemetry.TelemetryEvent[IsCentralVersionManagementEnabled] = isCpvmEnabled; + + if (isCpvmEnabled) { - telemetry.TelemetryEvent.AddPiiData(ProjectFilePath, _request.Project.FilePath); + var isCentralPackageTransitivePinningEnabled = _request.Project.RestoreMetadata?.CentralPackageTransitivePinningEnabled ?? false; + telemetry.TelemetryEvent[IsCentralPackageTransitivePinningEnabled] = isCentralPackageTransitivePinningEnabled; + } - bool isPackageSourceMappingEnabled = _request.PackageSourceMapping?.IsEnabled ?? false; - telemetry.TelemetryEvent[PackageSourceMappingIsMappingEnabled] = isPackageSourceMappingEnabled; - telemetry.TelemetryEvent[SourcesCount] = _request.DependencyProviders.RemoteProviders.Count; - int httpSourcesCount = _request.DependencyProviders.RemoteProviders.Where(e => e.IsHttp).Count(); - telemetry.TelemetryEvent[HttpSourcesCount] = httpSourcesCount; - telemetry.TelemetryEvent[LocalSourcesCount] = _request.DependencyProviders.RemoteProviders.Count - httpSourcesCount; - telemetry.TelemetryEvent[FallbackFoldersCount] = _request.DependencyProviders.FallbackPackageFolders.Count; - telemetry.TelemetryEvent[IsLockFileEnabled] = _isLockFileEnabled; - telemetry.TelemetryEvent[UseLegacyDependencyResolver] = _request.Project.RestoreMetadata.UseLegacyDependencyResolver; - telemetry.TelemetryEvent[UsedLegacyDependencyResolver] = !_enableNewDependencyResolver; - telemetry.TelemetryEvent[TargetFrameworksCount] = _request.Project.RestoreMetadata.TargetFrameworks.Count; - telemetry.TelemetryEvent[RuntimeIdentifiersCount] = _request.Project.RuntimeGraph.Runtimes.Count; - telemetry.TelemetryEvent[TreatWarningsAsErrors] = _request.Project.RestoreMetadata.ProjectWideWarningProperties.AllWarningsAsErrors; + bool auditEnabled = AuditUtility.ParseEnableValue( + _request.Project.RestoreMetadata?.RestoreAuditProperties, + _request.Project.FilePath, + _logger); + telemetry.TelemetryEvent[AuditEnabled] = auditEnabled ? "enabled" : "disabled"; + } - _operationId = telemetry.OperationId; + private async Task HandleNoOpRestoreAsync(TelemetryActivity telemetry, RestoreResultData restoreResultData, Stopwatch restoreTime) + { + telemetry.StartIntervalMeasure(); + bool noOp; + TimeSpan? cacheFileAge; + + if (NuGetEventSource.IsEnabled) TraceEvents.CalcNoOpRestoreStart(_request.Project.FilePath); + (restoreResultData.CacheFile, noOp, cacheFileAge) = EvaluateCacheFile(); + if (NuGetEventSource.IsEnabled) TraceEvents.CalcNoOpRestoreStop(_request.Project.FilePath); + + telemetry.TelemetryEvent[NoOpCacheFileEvaluationResult] = noOp; + telemetry.EndIntervalMeasure(NoOpCacheFileEvaluateDuration); + if (noOp) + { + telemetry.StartIntervalMeasure(); + + var noOpSuccess = NoOpRestoreUtilities.VerifyRestoreOutput(_request, restoreResultData.CacheFile); - var isCpvmEnabled = _request.Project.RestoreMetadata?.CentralPackageVersionsEnabled ?? false; - telemetry.TelemetryEvent[IsCentralVersionManagementEnabled] = isCpvmEnabled; + telemetry.EndIntervalMeasure(NoOpRestoreOutputEvaluationDuration); + telemetry.TelemetryEvent[NoOpRestoreOutputEvaluationResult] = noOpSuccess; - if (isCpvmEnabled) + if (noOpSuccess) { - var isCentralPackageTransitivePinningEnabled = _request.Project.RestoreMetadata?.CentralPackageTransitivePinningEnabled ?? false; - telemetry.TelemetryEvent[IsCentralPackageTransitivePinningEnabled] = isCentralPackageTransitivePinningEnabled; + telemetry.StartIntervalMeasure(); + + // Replay Warnings and Errors from an existing lock file in case of a no-op. + await MSBuildRestoreUtility.ReplayWarningsAndErrorsAsync(restoreResultData.CacheFile.LogMessages, _logger); + + telemetry.EndIntervalMeasure(NoOpReplayLogsDuration); + + restoreTime.Stop(); + telemetry.TelemetryEvent[NoOpResult] = true; + telemetry.TelemetryEvent[RestoreSuccess] = _success; + telemetry.TelemetryEvent[TotalUniquePackagesCount] = restoreResultData.CacheFile.ExpectedPackageFilePaths?.Count ?? -1; + telemetry.TelemetryEvent[NewPackagesInstalledCount] = 0; + if (cacheFileAge.HasValue) { telemetry.TelemetryEvent[NoOpCacheFileAgeDays] = cacheFileAge.Value.TotalDays; } + + return new NoOpRestoreResult( + _success, + _request.LockFilePath, + new Lazy(() => LockFileUtilities.GetLockFile(_request.LockFilePath, _logger)), + restoreResultData.CacheFile, + _request.Project.RestoreMetadata.CacheFilePath, + _request.ProjectStyle, + restoreTime.Elapsed); } + } + + return null; + } + private async Task ShowHttpSourcesError() + { + if (_request.DependencyProviders.RemoteProviders != null) + { + foreach (var remoteProvider in _request.DependencyProviders.RemoteProviders) + { + var source = remoteProvider.Source; + if (source.IsHttp && !source.IsHttps && !source.AllowInsecureConnections) + { + var isErrorEnabled = SdkAnalysisLevelMinimums.IsEnabled(_request.Project.RestoreMetadata.SdkAnalysisLevel, + _request.Project.RestoreMetadata.UsingMicrosoftNETSdk, + SdkAnalysisLevelMinimums.HttpErrorSdkAnalysisLevelMinimumValue); + + if (isErrorEnabled) + { + await _logger.LogAsync(RestoreLogMessage.CreateError(NuGetLogCode.NU1302, + string.Format(CultureInfo.CurrentCulture, Strings.Error_HttpSource_Single, "restore", source.Source))); + } + else + { + await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803, + string.Format(CultureInfo.CurrentCulture, Strings.Warning_HttpServerUsage, "restore", source.Source))); + } + } + } + } + } + + private async Task<(bool, bool)> EvaluateLockFile(TelemetryActivity telemetry, RemoteWalkContext contextForProject, RestoreResultData restoreResultData, bool isLockFileValid, bool regenerateLockFile, CancellationToken token) + { + // evaluate packages.lock.json file + using (telemetry.StartIndependentInterval(EvaluateLockFileDuration)) + { + bool result; + (result, isLockFileValid, restoreResultData.PackagesLockFile) = await EvaluatePackagesLockFileAsync(restoreResultData.PackagesLockFilePath, contextForProject, telemetry); + + telemetry.TelemetryEvent[IsLockFileValidForRestore] = isLockFileValid; + telemetry.TelemetryEvent[LockFileEvaluationResult] = result; + + regenerateLockFile = result; // Ensure that the lock file *does not* get rewritten, when the lock file is out of date and the status is false. + _success &= result; + } + + restoreResultData.RestoreGraphs = null; + if (_success) + { + using (telemetry.StartIndependentInterval(GenerateRestoreGraphDuration)) + { + if (NuGetEventSource.IsEnabled) + TraceEvents.BuildRestoreGraphStart(_request.Project.FilePath); + + if (_enableNewDependencyResolver) + { + restoreResultData.RestoreGraphs = await ExecuteRestoreAsync(_request.DependencyProviders.GlobalPackages, _request.DependencyProviders.FallbackPackageFolders, contextForProject, token, telemetry); + } + else + { + // Restore using the legacy code path if the optimized dependency resolution is disabled. + restoreResultData.RestoreGraphs = await ExecuteLegacyRestoreAsync(_request.DependencyProviders.GlobalPackages, _request.DependencyProviders.FallbackPackageFolders, contextForProject, token, telemetry); + } + + if (NuGetEventSource.IsEnabled) + TraceEvents.BuildRestoreGraphStop(_request.Project.FilePath); + } + } + else + { + // Being in an unsuccessful state before ExecuteRestoreAsync means there was a problem with the + // project or we're in locked mode and out of date. + // For example, project TFM or package versions couldn't be parsed. Although the minimal + // fake package spec generated has no packages requested, it also doesn't have any project TFMs + // and will generate validation errors if we tried to call ExecuteRestoreAsync. So, to avoid + // incorrect validation messages, don't try to restore. It is however, the responsibility for the + // caller of RestoreCommand to have provided at least one AdditionalMessage in RestoreArgs. + // The other scenario is when the lock file is not up to date and we're running locked mode. + // In that case we want to write a `target` for each target framework to avoid missing target errors from the SDK build tasks. + var frameworkRuntimePair = CreateFrameworkRuntimePairs(_request.Project, RequestRuntimeUtility.GetRestoreRuntimes(_request)); + restoreResultData.RestoreGraphs = frameworkRuntimePair.Select(e => + { + return RestoreTargetGraph.Create(_request.Project.RuntimeGraph, Enumerable.Empty>(), contextForProject, _logger, e.Framework, e.RuntimeIdentifier); + }); + } + + return (isLockFileValid, regenerateLockFile); + } + + public async Task ExecuteAsync(CancellationToken token) + { + using (var telemetry = TelemetryActivity.Create(parentId: ParentId, eventName: ProjectRestoreInformation)) + { + InitializeTelemetry(telemetry); + RestoreResultData restoreResultData = new RestoreResultData(); + int httpSourcesCount = _request.DependencyProviders.RemoteProviders.Where(e => e.IsHttp).Count(); bool auditEnabled = AuditUtility.ParseEnableValue( - _request.Project.RestoreMetadata?.RestoreAuditProperties, - _request.Project.FilePath, - _logger); - telemetry.TelemetryEvent[AuditEnabled] = auditEnabled ? "enabled" : "disabled"; + _request.Project.RestoreMetadata?.RestoreAuditProperties, + _request.Project.FilePath, + _logger); + _operationId = telemetry.OperationId; var restoreTime = Stopwatch.StartNew(); @@ -206,59 +348,20 @@ public async Task ExecuteAsync(CancellationToken token) var contextForProject = CreateRemoteWalkContext(_request, _logger); - CacheFile cacheFile = null; + restoreResultData.CacheFile = null; using (telemetry.StartIndependentInterval(NoOpDuration)) { if (NoOpRestoreUtilities.IsNoOpSupported(_request)) { - telemetry.StartIntervalMeasure(); - bool noOp; - TimeSpan? cacheFileAge; - - if (NuGetEventSource.IsEnabled) TraceEvents.CalcNoOpRestoreStart(_request.Project.FilePath); - (cacheFile, noOp, cacheFileAge) = EvaluateCacheFile(); - if (NuGetEventSource.IsEnabled) TraceEvents.CalcNoOpRestoreStop(_request.Project.FilePath); - - telemetry.TelemetryEvent[NoOpCacheFileEvaluationResult] = noOp; - telemetry.EndIntervalMeasure(NoOpCacheFileEvaluateDuration); - if (noOp) + var noOpResult = await HandleNoOpRestoreAsync(telemetry, restoreResultData, restoreTime); + if (noOpResult != null) { - telemetry.StartIntervalMeasure(); - - var noOpSuccess = NoOpRestoreUtilities.VerifyRestoreOutput(_request, cacheFile); - - telemetry.EndIntervalMeasure(NoOpRestoreOutputEvaluationDuration); - telemetry.TelemetryEvent[NoOpRestoreOutputEvaluationResult] = noOpSuccess; - - if (noOpSuccess) - { - telemetry.StartIntervalMeasure(); - - // Replay Warnings and Errors from an existing lock file in case of a no-op. - await MSBuildRestoreUtility.ReplayWarningsAndErrorsAsync(cacheFile.LogMessages, _logger); - - telemetry.EndIntervalMeasure(NoOpReplayLogsDuration); - - restoreTime.Stop(); - telemetry.TelemetryEvent[NoOpResult] = true; - telemetry.TelemetryEvent[RestoreSuccess] = _success; - telemetry.TelemetryEvent[TotalUniquePackagesCount] = cacheFile.ExpectedPackageFilePaths?.Count ?? -1; - telemetry.TelemetryEvent[NewPackagesInstalledCount] = 0; - if (cacheFileAge.HasValue) { telemetry.TelemetryEvent[NoOpCacheFileAgeDays] = cacheFileAge.Value.TotalDays; } - - return new NoOpRestoreResult( - _success, - _request.LockFilePath, - new Lazy(() => LockFileUtilities.GetLockFile(_request.LockFilePath, _logger)), - cacheFile, - _request.Project.RestoreMetadata.CacheFilePath, - _request.ProjectStyle, - restoreTime.Elapsed); - } + return noOpResult; } } } + telemetry.TelemetryEvent[NoOpResult] = false; // Getting here means we did not no-op. if (!await AreCentralVersionRequirementsSatisfiedAsync(_request, httpSourcesCount)) @@ -267,126 +370,51 @@ public async Task ExecuteAsync(CancellationToken token) _success = false; } - if (_request.DependencyProviders.RemoteProviders != null) - { - foreach (var remoteProvider in _request.DependencyProviders.RemoteProviders) - { - var source = remoteProvider.Source; - if (source.IsHttp && !source.IsHttps && !source.AllowInsecureConnections) - { - var isErrorEnabled = SdkAnalysisLevelMinimums.IsEnabled(_request.Project.RestoreMetadata.SdkAnalysisLevel, - _request.Project.RestoreMetadata.UsingMicrosoftNETSdk, - SdkAnalysisLevelMinimums.HttpErrorSdkAnalysisLevelMinimumValue); - - if (isErrorEnabled) - { - await _logger.LogAsync(RestoreLogMessage.CreateError(NuGetLogCode.NU1302, - string.Format(CultureInfo.CurrentCulture, Strings.Error_HttpSource_Single, "restore", source.Source))); - } - else - { - await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803, - string.Format(CultureInfo.CurrentCulture, Strings.Warning_HttpServerUsage, "restore", source.Source))); - } - } - } - } + await ShowHttpSourcesError(); _success &= HasValidPlatformVersions(); - // evaluate packages.lock.json file - var packagesLockFilePath = PackagesLockFileUtilities.GetNuGetLockFilePath(_request.Project); + restoreResultData.PackagesLockFilePath = PackagesLockFileUtilities.GetNuGetLockFilePath(_request.Project); + restoreResultData.PackagesLockFile = null; var isLockFileValid = false; - PackagesLockFile packagesLockFile = null; var regenerateLockFile = true; - - using (telemetry.StartIndependentInterval(EvaluateLockFileDuration)) - { - bool result; - (result, isLockFileValid, packagesLockFile) = await EvaluatePackagesLockFileAsync(packagesLockFilePath, contextForProject, telemetry); - - telemetry.TelemetryEvent[IsLockFileValidForRestore] = isLockFileValid; - telemetry.TelemetryEvent[LockFileEvaluationResult] = result; - - regenerateLockFile = result; // Ensure that the lock file *does not* get rewritten, when the lock file is out of date and the status is false. - _success &= result; - } - - IEnumerable graphs = null; - if (_success) - { - using (telemetry.StartIndependentInterval(GenerateRestoreGraphDuration)) - { - if (NuGetEventSource.IsEnabled) - TraceEvents.BuildRestoreGraphStart(_request.Project.FilePath); - - if (_enableNewDependencyResolver) - { - graphs = await ExecuteRestoreAsync(_request.DependencyProviders.GlobalPackages, _request.DependencyProviders.FallbackPackageFolders, contextForProject, token, telemetry); - } - else - { - // Restore using the legacy code path if the optimized dependency resolution is disabled. - graphs = await ExecuteLegacyRestoreAsync(_request.DependencyProviders.GlobalPackages, _request.DependencyProviders.FallbackPackageFolders, contextForProject, token, telemetry); - } - - if (NuGetEventSource.IsEnabled) - TraceEvents.BuildRestoreGraphStop(_request.Project.FilePath); - } - } - else - { - // Being in an unsuccessful state before ExecuteRestoreAsync means there was a problem with the - // project or we're in locked mode and out of date. - // For example, project TFM or package versions couldn't be parsed. Although the minimal - // fake package spec generated has no packages requested, it also doesn't have any project TFMs - // and will generate validation errors if we tried to call ExecuteRestoreAsync. So, to avoid - // incorrect validation messages, don't try to restore. It is however, the responsibility for the - // caller of RestoreCommand to have provided at least one AdditionalMessage in RestoreArgs. - // The other scenario is when the lock file is not up to date and we're running locked mode. - // In that case we want to write a `target` for each target framework to avoid missing target errors from the SDK build tasks. - var frameworkRuntimePair = CreateFrameworkRuntimePairs(_request.Project, RequestRuntimeUtility.GetRestoreRuntimes(_request)); - graphs = frameworkRuntimePair.Select(e => - { - return RestoreTargetGraph.Create(_request.Project.RuntimeGraph, Enumerable.Empty>(), contextForProject, _logger, e.Framework, e.RuntimeIdentifier); - }); - } + (isLockFileValid, regenerateLockFile) = await EvaluateLockFile(telemetry, contextForProject, restoreResultData, isLockFileValid, regenerateLockFile, token); bool auditRan = false; if (auditEnabled) { - auditRan = await PerformAuditAsync(graphs, telemetry, token); + auditRan = await PerformAuditAsync(restoreResultData.RestoreGraphs, telemetry, token); } telemetry.StartIntervalMeasure(); // Create assets file if (NuGetEventSource.IsEnabled) TraceEvents.BuildAssetsFileStart(_request.Project.FilePath); - LockFile assetsFile = BuildAssetsFile( + restoreResultData.LockFile = BuildAssetsFile( _request.ExistingLockFile, - _request.Project, - graphs, + _request.Project, + restoreResultData.RestoreGraphs, localRepositories, contextForProject); if (NuGetEventSource.IsEnabled) TraceEvents.BuildAssetsFileStop(_request.Project.FilePath); telemetry.EndIntervalMeasure(GenerateAssetsFileDuration); - IList checkResults = null; + restoreResultData.CompatibilityCheckResults = null; telemetry.StartIntervalMeasure(); - _success &= await ValidateRestoreGraphsAsync(graphs, _logger); + _success &= await ValidateRestoreGraphsAsync(restoreResultData.RestoreGraphs, _logger); // Check package compatibility - checkResults = await VerifyCompatibilityAsync( + restoreResultData.CompatibilityCheckResults = await VerifyCompatibilityAsync( _request.Project, _includeFlagGraphs, localRepositories, - assetsFile, - graphs, + restoreResultData.LockFile, + restoreResultData.RestoreGraphs, _request.ValidateRuntimeAssets, _logger); - if (checkResults.Any(r => !r.Success)) + if (restoreResultData.CompatibilityCheckResults.Any(r => !r.Success)) { _success = false; } @@ -394,135 +422,151 @@ await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803, // Generate Targets/Props files - var msbuildOutputFiles = Enumerable.Empty(); - string assetsFilePath = null; - string cacheFilePath = null; + restoreResultData.MSBuildOutputFiles = Enumerable.Empty(); + restoreResultData.AssetFilePath = null; + restoreResultData.CacheFilePath = null; - using (telemetry.StartIndependentInterval(CreateRestoreResultDuration)) - { - // Determine the lock file output path - assetsFilePath = GetAssetsFilePath(assetsFile); + await ProcessRestoreResultAsync( + telemetry, + localRepositories, + contextForProject, + isLockFileValid, + regenerateLockFile, + restoreResultData, + token); - // Determine the cache file output path - cacheFilePath = NoOpRestoreUtilities.GetCacheFilePath(_request, assetsFile); - // Tool restores are unique since the output path is not known until after restore - if (_request.LockFilePath == null - && _request.ProjectStyle == ProjectStyle.DotnetCliTool) - { - _request.LockFilePath = assetsFilePath; - } + restoreTime.Stop(); - if (contextForProject.IsMsBuildBased) - { - msbuildOutputFiles = BuildAssetsUtils.GetMSBuildOutputFiles( - _request.Project, - assetsFile, - graphs, - localRepositories, - _request, - assetsFilePath, - _success, - _logger); - } + // Create result + return new RestoreResult( + _success, + restoreResultData.RestoreGraphs, + restoreResultData.CompatibilityCheckResults, + restoreResultData.MSBuildOutputFiles, + restoreResultData.LockFile, + _request.ExistingLockFile, + restoreResultData.AssetFilePath, + restoreResultData.CacheFile, + restoreResultData.CacheFilePath, + restoreResultData.PackagesLockFilePath, + restoreResultData.PackagesLockFile, + dependencyGraphSpecFilePath: NoOpRestoreUtilities.GetPersistedDGSpecFilePath(_request), + dependencyGraphSpec: _request.DependencyGraphSpec, + _request.ProjectStyle, + restoreTime.Elapsed) + { + AuditRan = auditRan + }; + } + } - // If the request is for a lower lock file version, downgrade it appropriately - DowngradeLockFileIfNeeded(assetsFile); + private async Task ProcessRestoreResultAsync(TelemetryActivity telemetry, List localRepositories, RemoteWalkContext contextForProject, bool isLockFileValid, bool regenerateLockFile, RestoreResultData restoreResultData, CancellationToken token) + { + restoreResultData.MSBuildOutputFiles = Enumerable.Empty(); + restoreResultData.AssetFilePath = null; + restoreResultData.CacheFilePath = null; + using (telemetry.StartIndependentInterval(CreateRestoreResultDuration)) + { + // Determine the lock file output path + restoreResultData.AssetFilePath = GetAssetsFilePath(restoreResultData.LockFile); - // Revert to the original case if needed - await FixCaseForLegacyReaders(graphs, assetsFile, token); + // Determine the cache file output path + restoreResultData.CacheFilePath = NoOpRestoreUtilities.GetCacheFilePath(_request, restoreResultData.LockFile); - // if lock file was still valid then validate package's sha512 hash or else write - // the file if enabled. - if (isLockFileValid) - { - telemetry.StartIntervalMeasure(); - // validate package's SHA512 - _success &= ValidatePackagesSha512(packagesLockFile, assetsFile); - telemetry.EndIntervalMeasure(ValidatePackagesShaDuration); + // Tool restores are unique since the output path is not known until after restore + if (_request.LockFilePath == null + && _request.ProjectStyle == ProjectStyle.DotnetCliTool) + { + _request.LockFilePath = restoreResultData.AssetFilePath; + } - // clear out the existing lock file so that we don't over-write the same file - packagesLockFile = null; - } - else if (_isLockFileEnabled) - { - if (regenerateLockFile) - { - // generate packages.lock.json file if enabled - packagesLockFile = new PackagesLockFileBuilder() - .CreateNuGetLockFile(assetsFile); - } - else - { - packagesLockFile = null; - _logger.LogVerbose(string.Format(CultureInfo.CurrentCulture, Strings.Log_SkippingPackagesLockFileGeneration, packagesLockFilePath)); - } - } + if (contextForProject.IsMsBuildBased) + { + restoreResultData.MSBuildOutputFiles = BuildAssetsUtils.GetMSBuildOutputFiles( + _request.Project, + restoreResultData.LockFile, + restoreResultData.RestoreGraphs, + localRepositories, + _request, + restoreResultData.AssetFilePath, + _success, + _logger); + } - // Write the logs into the assets file - var logsEnumerable = _logger.Errors - .Select(l => AssetsLogMessage.Create(l)); - if (_request.AdditionalMessages != null) - { - logsEnumerable = logsEnumerable.Concat(_request.AdditionalMessages); - } - var logs = logsEnumerable - .ToList(); + // If the request is for a lower lock file version, downgrade it appropriately + DowngradeLockFileIfNeeded(restoreResultData.LockFile); - _success &= !logs.Any(l => l.Level == LogLevel.Error); + // Revert to the original case if needed + await FixCaseForLegacyReaders(restoreResultData.RestoreGraphs, restoreResultData.LockFile, token); - assetsFile.LogMessages = logs; + // if lock file was still valid then validate package's sha512 hash or else write + // the file if enabled. + if (isLockFileValid) + { + telemetry.StartIntervalMeasure(); + // validate package's SHA512 + _success &= ValidatePackagesSha512(restoreResultData.PackagesLockFile, restoreResultData.LockFile); + telemetry.EndIntervalMeasure(ValidatePackagesShaDuration); - if (cacheFile != null) + // clear out the existing lock file so that we don't over-write the same file + restoreResultData.PackagesLockFile = null; + } + else if (_isLockFileEnabled) + { + if (regenerateLockFile) { - cacheFile.Success = _success; - cacheFile.ProjectFilePath = _request.Project.FilePath; - cacheFile.LogMessages = assetsFile.LogMessages; - cacheFile.ExpectedPackageFilePaths = NoOpRestoreUtilities.GetRestoreOutput(_request, assetsFile); - telemetry.TelemetryEvent[TotalUniquePackagesCount] = cacheFile?.ExpectedPackageFilePaths.Count; + // generate packages.lock.json file if enabled + restoreResultData.PackagesLockFile = new PackagesLockFileBuilder() + .CreateNuGetLockFile(restoreResultData.LockFile); } - - var errorCodes = ConcatAsString(new HashSet(logs.Where(l => l.Level == LogLevel.Error).Select(l => l.Code))); - var warningCodes = ConcatAsString(new HashSet(logs.Where(l => l.Level == LogLevel.Warning).Select(l => l.Code))); - - if (!string.IsNullOrEmpty(errorCodes)) + else { - telemetry.TelemetryEvent[ErrorCodes] = errorCodes; + restoreResultData.PackagesLockFile = null; + _logger.LogVerbose(string.Format(CultureInfo.CurrentCulture, Strings.Log_SkippingPackagesLockFileGeneration, restoreResultData.PackagesLockFilePath)); } + } - if (!string.IsNullOrEmpty(warningCodes)) - { - telemetry.TelemetryEvent[WarningCodes] = warningCodes; - } + // Write the logs into the assets file + var logsEnumerable = _logger.Errors + .Select(l => AssetsLogMessage.Create(l)); + if (_request.AdditionalMessages != null) + { + logsEnumerable = logsEnumerable.Concat(_request.AdditionalMessages); + } + var logs = logsEnumerable + .ToList(); + _success &= !logs.Any(l => l.Level == LogLevel.Error); - telemetry.TelemetryEvent[NewPackagesInstalledCount] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Install).Distinct().Count(); + restoreResultData.LockFile.LogMessages = logs; - telemetry.TelemetryEvent[RestoreSuccess] = _success; + if (restoreResultData.CacheFile != null) + { + restoreResultData.CacheFile.Success = _success; + restoreResultData.CacheFile.ProjectFilePath = _request.Project.FilePath; + restoreResultData.CacheFile.LogMessages = restoreResultData.LockFile.LogMessages; + restoreResultData.CacheFile.ExpectedPackageFilePaths = NoOpRestoreUtilities.GetRestoreOutput(_request, restoreResultData.LockFile); + telemetry.TelemetryEvent[TotalUniquePackagesCount] = restoreResultData.CacheFile?.ExpectedPackageFilePaths.Count; } - restoreTime.Stop(); + var errorCodes = ConcatAsString(new HashSet(logs.Where(l => l.Level == LogLevel.Error).Select(l => l.Code))); + var warningCodes = ConcatAsString(new HashSet(logs.Where(l => l.Level == LogLevel.Warning).Select(l => l.Code))); - // Create result - return new RestoreResult( - _success, - graphs, - checkResults, - msbuildOutputFiles, - assetsFile, - _request.ExistingLockFile, - assetsFilePath, - cacheFile, - cacheFilePath, - packagesLockFilePath, - packagesLockFile, - dependencyGraphSpecFilePath: NoOpRestoreUtilities.GetPersistedDGSpecFilePath(_request), - dependencyGraphSpec: _request.DependencyGraphSpec, - _request.ProjectStyle, - restoreTime.Elapsed) + if (!string.IsNullOrEmpty(errorCodes)) { - AuditRan = auditRan - }; + telemetry.TelemetryEvent[ErrorCodes] = errorCodes; + } + + if (!string.IsNullOrEmpty(warningCodes)) + { + telemetry.TelemetryEvent[WarningCodes] = warningCodes; + } + + + telemetry.TelemetryEvent[NewPackagesInstalledCount] = restoreResultData.RestoreGraphs.Where(g => !g.InConflict).SelectMany(g => g.Install).Distinct().Count(); + + telemetry.TelemetryEvent[RestoreSuccess] = _success; } } @@ -1660,5 +1704,18 @@ public static void CalcNoOpRestoreStop(string filePath) NuGetEventSource.Instance.Write(EventNameCalcNoOpRestore, eventOptions, new { FilePath = filePath }); } } + + private class RestoreResultData + { + public IEnumerable RestoreGraphs { get; set; } + public IEnumerable CompatibilityCheckResults { get; set; } + public IEnumerable MSBuildOutputFiles { get; set; } + public LockFile LockFile { get; set; } + public string AssetFilePath { get; set; } + public CacheFile CacheFile { get; set; } + public string CacheFilePath { get; set; } + public string PackagesLockFilePath { get; set; } + public PackagesLockFile PackagesLockFile { get; set; } + } } } From 939fa52c13af2ad7238126b76794a42c17d86f09 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 26 Nov 2024 09:33:33 -0800 Subject: [PATCH 2/8] Cleanup --- .../RestoreCommand/RestoreCommand.cs | 296 +++++++++--------- 1 file changed, 148 insertions(+), 148 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index c66f8b46d35..e9ab229d71f 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -157,19 +157,162 @@ public Task ExecuteAsync() return ExecuteAsync(CancellationToken.None); } - private void InitializeTelemetry(TelemetryActivity telemetry) + public async Task ExecuteAsync(CancellationToken token) + { + using (var telemetry = TelemetryActivity.Create(parentId: ParentId, eventName: ProjectRestoreInformation)) + { + RestoreResultData restoreResultData = new RestoreResultData(); + int httpSourcesCount = _request.DependencyProviders.RemoteProviders.Where(e => e.IsHttp).Count(); + bool auditEnabled = AuditUtility.ParseEnableValue( + _request.Project.RestoreMetadata?.RestoreAuditProperties, + _request.Project.FilePath, + _logger); + InitializeTelemetry(telemetry, httpSourcesCount, auditEnabled); + + var restoreTime = Stopwatch.StartNew(); + + // Local package folders (non-sources) + var localRepositories = new List + { + _request.DependencyProviders.GlobalPackages + }; + + localRepositories.AddRange(_request.DependencyProviders.FallbackPackageFolders); + + var contextForProject = CreateRemoteWalkContext(_request, _logger); + + restoreResultData.CacheFile = null; + + using (telemetry.StartIndependentInterval(NoOpDuration)) + { + if (NoOpRestoreUtilities.IsNoOpSupported(_request)) + { + var noOpResult = await HandleNoOpRestoreAsync(telemetry, restoreResultData, restoreTime); + + if (noOpResult != null) + { + return noOpResult; + } + } + } + + telemetry.TelemetryEvent[NoOpResult] = false; // Getting here means we did not no-op. + + if (!await AreCentralVersionRequirementsSatisfiedAsync(_request, httpSourcesCount)) + { + // the errors will be added to the assets file + _success = false; + } + + await ShowHttpSourcesError(); + + _success &= HasValidPlatformVersions(); + + restoreResultData.PackagesLockFilePath = PackagesLockFileUtilities.GetNuGetLockFilePath(_request.Project); + restoreResultData.PackagesLockFile = null; + var isLockFileValid = false; + var regenerateLockFile = true; + (isLockFileValid, regenerateLockFile) = await EvaluateLockFile(telemetry, contextForProject, restoreResultData, isLockFileValid, regenerateLockFile, token); + + bool auditRan = false; + + if (auditEnabled) + { + auditRan = await PerformAuditAsync(restoreResultData.RestoreGraphs, telemetry, token); + } + + telemetry.StartIntervalMeasure(); + // Create assets file + if (NuGetEventSource.IsEnabled) TraceEvents.BuildAssetsFileStart(_request.Project.FilePath); + restoreResultData.LockFile = BuildAssetsFile( + _request.ExistingLockFile, + _request.Project, + restoreResultData.RestoreGraphs, + localRepositories, + contextForProject); + if (NuGetEventSource.IsEnabled) TraceEvents.BuildAssetsFileStop(_request.Project.FilePath); + telemetry.EndIntervalMeasure(GenerateAssetsFileDuration); + + restoreResultData.CompatibilityCheckResults = null; + + telemetry.StartIntervalMeasure(); + + _success &= await ValidateRestoreGraphsAsync(restoreResultData.RestoreGraphs, _logger); + + // Check package compatibility + restoreResultData.CompatibilityCheckResults = await VerifyCompatibilityAsync( + _request.Project, + _includeFlagGraphs, + localRepositories, + restoreResultData.LockFile, + restoreResultData.RestoreGraphs, + _request.ValidateRuntimeAssets, + _logger); + + if (restoreResultData.CompatibilityCheckResults.Any(r => !r.Success)) + { + _success = false; + } + telemetry.EndIntervalMeasure(ValidateRestoreGraphsDuration); + + + // Generate Targets/Props files + restoreResultData.MSBuildOutputFiles = Enumerable.Empty(); + restoreResultData.AssetFilePath = null; + restoreResultData.CacheFilePath = null; + + await ProcessRestoreResultAsync( + telemetry, + localRepositories, + contextForProject, + isLockFileValid, + regenerateLockFile, + restoreResultData, + token); + + + restoreTime.Stop(); + + // Create result + return new RestoreResult( + _success, + restoreResultData.RestoreGraphs, + restoreResultData.CompatibilityCheckResults, + restoreResultData.MSBuildOutputFiles, + restoreResultData.LockFile, + _request.ExistingLockFile, + restoreResultData.AssetFilePath, + restoreResultData.CacheFile, + restoreResultData.CacheFilePath, + restoreResultData.PackagesLockFilePath, + restoreResultData.PackagesLockFile, + dependencyGraphSpecFilePath: NoOpRestoreUtilities.GetPersistedDGSpecFilePath(_request), + dependencyGraphSpec: _request.DependencyGraphSpec, + _request.ProjectStyle, + restoreTime.Elapsed) + { + AuditRan = auditRan + }; + } + } + + private void InitializeTelemetry(TelemetryActivity telemetry, int httpSourcesCount, bool auditEnabled) { telemetry.TelemetryEvent.AddPiiData(ProjectFilePath, _request.Project.FilePath); bool isPackageSourceMappingEnabled = _request.PackageSourceMapping?.IsEnabled ?? false; telemetry.TelemetryEvent[PackageSourceMappingIsMappingEnabled] = isPackageSourceMappingEnabled; telemetry.TelemetryEvent[SourcesCount] = _request.DependencyProviders.RemoteProviders.Count; - int httpSourcesCount = _request.DependencyProviders.RemoteProviders.Where(e => e.IsHttp).Count(); telemetry.TelemetryEvent[HttpSourcesCount] = httpSourcesCount; telemetry.TelemetryEvent[LocalSourcesCount] = _request.DependencyProviders.RemoteProviders.Count - httpSourcesCount; telemetry.TelemetryEvent[FallbackFoldersCount] = _request.DependencyProviders.FallbackPackageFolders.Count; telemetry.TelemetryEvent[IsLockFileEnabled] = _isLockFileEnabled; telemetry.TelemetryEvent[UseLegacyDependencyResolver] = _request.Project.RestoreMetadata.UseLegacyDependencyResolver; telemetry.TelemetryEvent[UsedLegacyDependencyResolver] = !_enableNewDependencyResolver; + telemetry.TelemetryEvent[TargetFrameworksCount] = _request.Project.RestoreMetadata.TargetFrameworks.Count; + telemetry.TelemetryEvent[RuntimeIdentifiersCount] = _request.Project.RuntimeGraph.Runtimes.Count; + telemetry.TelemetryEvent[TreatWarningsAsErrors] = _request.Project.RestoreMetadata.ProjectWideWarningProperties.AllWarningsAsErrors; + _operationId = telemetry.OperationId; + var isCpvmEnabled = _request.Project.RestoreMetadata?.CentralPackageVersionsEnabled ?? false; telemetry.TelemetryEvent[IsCentralVersionManagementEnabled] = isCpvmEnabled; @@ -179,10 +322,6 @@ private void InitializeTelemetry(TelemetryActivity telemetry) telemetry.TelemetryEvent[IsCentralPackageTransitivePinningEnabled] = isCentralPackageTransitivePinningEnabled; } - bool auditEnabled = AuditUtility.ParseEnableValue( - _request.Project.RestoreMetadata?.RestoreAuditProperties, - _request.Project.FilePath, - _logger); telemetry.TelemetryEvent[AuditEnabled] = auditEnabled ? "enabled" : "disabled"; } @@ -323,149 +462,12 @@ await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803, return (isLockFileValid, regenerateLockFile); } - public async Task ExecuteAsync(CancellationToken token) - { - using (var telemetry = TelemetryActivity.Create(parentId: ParentId, eventName: ProjectRestoreInformation)) - { - InitializeTelemetry(telemetry); - RestoreResultData restoreResultData = new RestoreResultData(); - int httpSourcesCount = _request.DependencyProviders.RemoteProviders.Where(e => e.IsHttp).Count(); - bool auditEnabled = AuditUtility.ParseEnableValue( - _request.Project.RestoreMetadata?.RestoreAuditProperties, - _request.Project.FilePath, - _logger); - _operationId = telemetry.OperationId; - - var restoreTime = Stopwatch.StartNew(); - - // Local package folders (non-sources) - var localRepositories = new List - { - _request.DependencyProviders.GlobalPackages - }; - - localRepositories.AddRange(_request.DependencyProviders.FallbackPackageFolders); - - var contextForProject = CreateRemoteWalkContext(_request, _logger); - - restoreResultData.CacheFile = null; - - using (telemetry.StartIndependentInterval(NoOpDuration)) - { - if (NoOpRestoreUtilities.IsNoOpSupported(_request)) - { - var noOpResult = await HandleNoOpRestoreAsync(telemetry, restoreResultData, restoreTime); - if (noOpResult != null) - { - return noOpResult; - } - } - } - - telemetry.TelemetryEvent[NoOpResult] = false; // Getting here means we did not no-op. - - if (!await AreCentralVersionRequirementsSatisfiedAsync(_request, httpSourcesCount)) - { - // the errors will be added to the assets file - _success = false; - } - - await ShowHttpSourcesError(); - - _success &= HasValidPlatformVersions(); - - restoreResultData.PackagesLockFilePath = PackagesLockFileUtilities.GetNuGetLockFilePath(_request.Project); - restoreResultData.PackagesLockFile = null; - var isLockFileValid = false; - var regenerateLockFile = true; - (isLockFileValid, regenerateLockFile) = await EvaluateLockFile(telemetry, contextForProject, restoreResultData, isLockFileValid, regenerateLockFile, token); - - bool auditRan = false; - if (auditEnabled) - { - auditRan = await PerformAuditAsync(restoreResultData.RestoreGraphs, telemetry, token); - } - - telemetry.StartIntervalMeasure(); - // Create assets file - if (NuGetEventSource.IsEnabled) TraceEvents.BuildAssetsFileStart(_request.Project.FilePath); - restoreResultData.LockFile = BuildAssetsFile( - _request.ExistingLockFile, - _request.Project, - restoreResultData.RestoreGraphs, - localRepositories, - contextForProject); - if (NuGetEventSource.IsEnabled) TraceEvents.BuildAssetsFileStop(_request.Project.FilePath); - telemetry.EndIntervalMeasure(GenerateAssetsFileDuration); - - restoreResultData.CompatibilityCheckResults = null; - - telemetry.StartIntervalMeasure(); - - _success &= await ValidateRestoreGraphsAsync(restoreResultData.RestoreGraphs, _logger); - - // Check package compatibility - restoreResultData.CompatibilityCheckResults = await VerifyCompatibilityAsync( - _request.Project, - _includeFlagGraphs, - localRepositories, - restoreResultData.LockFile, - restoreResultData.RestoreGraphs, - _request.ValidateRuntimeAssets, - _logger); - - if (restoreResultData.CompatibilityCheckResults.Any(r => !r.Success)) - { - _success = false; - } - telemetry.EndIntervalMeasure(ValidateRestoreGraphsDuration); - - - // Generate Targets/Props files - restoreResultData.MSBuildOutputFiles = Enumerable.Empty(); - restoreResultData.AssetFilePath = null; - restoreResultData.CacheFilePath = null; - - await ProcessRestoreResultAsync( - telemetry, - localRepositories, - contextForProject, - isLockFileValid, - regenerateLockFile, - restoreResultData, - token); - - - restoreTime.Stop(); - - // Create result - return new RestoreResult( - _success, - restoreResultData.RestoreGraphs, - restoreResultData.CompatibilityCheckResults, - restoreResultData.MSBuildOutputFiles, - restoreResultData.LockFile, - _request.ExistingLockFile, - restoreResultData.AssetFilePath, - restoreResultData.CacheFile, - restoreResultData.CacheFilePath, - restoreResultData.PackagesLockFilePath, - restoreResultData.PackagesLockFile, - dependencyGraphSpecFilePath: NoOpRestoreUtilities.GetPersistedDGSpecFilePath(_request), - dependencyGraphSpec: _request.DependencyGraphSpec, - _request.ProjectStyle, - restoreTime.Elapsed) - { - AuditRan = auditRan - }; - } - } - private async Task ProcessRestoreResultAsync(TelemetryActivity telemetry, List localRepositories, RemoteWalkContext contextForProject, bool isLockFileValid, bool regenerateLockFile, RestoreResultData restoreResultData, CancellationToken token) { restoreResultData.MSBuildOutputFiles = Enumerable.Empty(); restoreResultData.AssetFilePath = null; restoreResultData.CacheFilePath = null; + using (telemetry.StartIndependentInterval(CreateRestoreResultDuration)) { // Determine the lock file output path @@ -530,15 +532,15 @@ private async Task ProcessRestoreResultAsync(TelemetryActivity telemetry, List AssetsLogMessage.Create(l)); + if (_request.AdditionalMessages != null) { logsEnumerable = logsEnumerable.Concat(_request.AdditionalMessages); } + var logs = logsEnumerable .ToList(); - _success &= !logs.Any(l => l.Level == LogLevel.Error); - restoreResultData.LockFile.LogMessages = logs; if (restoreResultData.CacheFile != null) @@ -563,9 +565,7 @@ private async Task ProcessRestoreResultAsync(TelemetryActivity telemetry, List !g.InConflict).SelectMany(g => g.Install).Distinct().Count(); - telemetry.TelemetryEvent[RestoreSuccess] = _success; } } From e24c0f2c2817b2696d6c3dee711d8c8b9d21f365 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 26 Nov 2024 11:26:26 -0800 Subject: [PATCH 3/8] cleaup --- .../RestoreCommand/RestoreCommand.cs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index e9ab229d71f..7b46593ee44 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -164,9 +164,9 @@ public async Task ExecuteAsync(CancellationToken token) RestoreResultData restoreResultData = new RestoreResultData(); int httpSourcesCount = _request.DependencyProviders.RemoteProviders.Where(e => e.IsHttp).Count(); bool auditEnabled = AuditUtility.ParseEnableValue( - _request.Project.RestoreMetadata?.RestoreAuditProperties, - _request.Project.FilePath, - _logger); + _request.Project.RestoreMetadata?.RestoreAuditProperties, + _request.Project.FilePath, + _logger); InitializeTelemetry(telemetry, httpSourcesCount, auditEnabled); var restoreTime = Stopwatch.StartNew(); @@ -187,7 +187,7 @@ public async Task ExecuteAsync(CancellationToken token) { if (NoOpRestoreUtilities.IsNoOpSupported(_request)) { - var noOpResult = await HandleNoOpRestoreAsync(telemetry, restoreResultData, restoreTime); + var noOpResult = await EvaluateNoOpAsync(telemetry, restoreResultData, restoreTime); if (noOpResult != null) { @@ -208,11 +208,7 @@ public async Task ExecuteAsync(CancellationToken token) _success &= HasValidPlatformVersions(); - restoreResultData.PackagesLockFilePath = PackagesLockFileUtilities.GetNuGetLockFilePath(_request.Project); - restoreResultData.PackagesLockFile = null; - var isLockFileValid = false; - var regenerateLockFile = true; - (isLockFileValid, regenerateLockFile) = await EvaluateLockFile(telemetry, contextForProject, restoreResultData, isLockFileValid, regenerateLockFile, token); + (bool isLockFileValid, bool regenerateLockFile) = await EvaluateLockFile(telemetry, contextForProject, restoreResultData, token); bool auditRan = false; @@ -253,8 +249,8 @@ public async Task ExecuteAsync(CancellationToken token) { _success = false; } - telemetry.EndIntervalMeasure(ValidateRestoreGraphsDuration); + telemetry.EndIntervalMeasure(ValidateRestoreGraphsDuration); // Generate Targets/Props files restoreResultData.MSBuildOutputFiles = Enumerable.Empty(); @@ -270,7 +266,6 @@ await ProcessRestoreResultAsync( restoreResultData, token); - restoreTime.Stop(); // Create result @@ -325,7 +320,7 @@ private void InitializeTelemetry(TelemetryActivity telemetry, int httpSourcesCou telemetry.TelemetryEvent[AuditEnabled] = auditEnabled ? "enabled" : "disabled"; } - private async Task HandleNoOpRestoreAsync(TelemetryActivity telemetry, RestoreResultData restoreResultData, Stopwatch restoreTime) + private async Task EvaluateNoOpAsync(TelemetryActivity telemetry, RestoreResultData restoreResultData, Stopwatch restoreTime) { telemetry.StartIntervalMeasure(); bool noOp; @@ -404,9 +399,14 @@ await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803, } } - private async Task<(bool, bool)> EvaluateLockFile(TelemetryActivity telemetry, RemoteWalkContext contextForProject, RestoreResultData restoreResultData, bool isLockFileValid, bool regenerateLockFile, CancellationToken token) + private async Task<(bool, bool)> EvaluateLockFile(TelemetryActivity telemetry, RemoteWalkContext contextForProject, RestoreResultData restoreResultData, CancellationToken token) { // evaluate packages.lock.json file + restoreResultData.PackagesLockFilePath = PackagesLockFileUtilities.GetNuGetLockFilePath(_request.Project); + restoreResultData.PackagesLockFile = null; + var isLockFileValid = false; + var regenerateLockFile = true; + using (telemetry.StartIndependentInterval(EvaluateLockFileDuration)) { bool result; From dfe603ec858a21d06732d712698c714587420913 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 26 Nov 2024 14:31:49 -0800 Subject: [PATCH 4/8] Refactor more --- .../NuGet.Commands/RestoreCommand/RestoreCommand.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index 7b46593ee44..5ffa1971285 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -210,6 +210,8 @@ public async Task ExecuteAsync(CancellationToken token) (bool isLockFileValid, bool regenerateLockFile) = await EvaluateLockFile(telemetry, contextForProject, restoreResultData, token); + await GenerateRestoreGraphsAsync(telemetry, contextForProject, restoreResultData, token); + bool auditRan = false; if (auditEnabled) @@ -419,6 +421,11 @@ await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803, _success &= result; } + return (isLockFileValid, regenerateLockFile); + } + + private async Task GenerateRestoreGraphsAsync(TelemetryActivity telemetry, RemoteWalkContext contextForProject, RestoreResultData restoreResultData, CancellationToken token) + { restoreResultData.RestoreGraphs = null; if (_success) { @@ -458,8 +465,6 @@ await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803, return RestoreTargetGraph.Create(_request.Project.RuntimeGraph, Enumerable.Empty>(), contextForProject, _logger, e.Framework, e.RuntimeIdentifier); }); } - - return (isLockFileValid, regenerateLockFile); } private async Task ProcessRestoreResultAsync(TelemetryActivity telemetry, List localRepositories, RemoteWalkContext contextForProject, bool isLockFileValid, bool regenerateLockFile, RestoreResultData restoreResultData, CancellationToken token) From dcdf02ecb1bbd999615ce9cc9741d32015979570 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Wed, 27 Nov 2024 14:58:14 -0800 Subject: [PATCH 5/8] remove object --- .../RestoreCommand/RestoreCommand.cs | 204 ++++++++++-------- 1 file changed, 114 insertions(+), 90 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index 5ffa1971285..7093d0e6225 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -161,7 +161,6 @@ public async Task ExecuteAsync(CancellationToken token) { using (var telemetry = TelemetryActivity.Create(parentId: ParentId, eventName: ProjectRestoreInformation)) { - RestoreResultData restoreResultData = new RestoreResultData(); int httpSourcesCount = _request.DependencyProviders.RemoteProviders.Where(e => e.IsHttp).Count(); bool auditEnabled = AuditUtility.ParseEnableValue( _request.Project.RestoreMetadata?.RestoreAuditProperties, @@ -181,13 +180,13 @@ public async Task ExecuteAsync(CancellationToken token) var contextForProject = CreateRemoteWalkContext(_request, _logger); - restoreResultData.CacheFile = null; + CacheFile cacheFile = null; using (telemetry.StartIndependentInterval(NoOpDuration)) { if (NoOpRestoreUtilities.IsNoOpSupported(_request)) { - var noOpResult = await EvaluateNoOpAsync(telemetry, restoreResultData, restoreTime); + (RestoreResult noOpResult, cacheFile) = await EvaluateNoOpAsync(telemetry, cacheFile, restoreTime); if (noOpResult != null) { @@ -208,46 +207,51 @@ public async Task ExecuteAsync(CancellationToken token) _success &= HasValidPlatformVersions(); - (bool isLockFileValid, bool regenerateLockFile) = await EvaluateLockFile(telemetry, contextForProject, restoreResultData, token); + var packagesLockFilePath = PackagesLockFileUtilities.GetNuGetLockFilePath(_request.Project); + PackagesLockFile packagesLockFile = null; + (bool isLockFileValid, bool regenerateLockFile, packagesLockFilePath, packagesLockFile) = await EvaluateLockFile( + telemetry, + contextForProject, + packagesLockFilePath, + packagesLockFile, + token); - await GenerateRestoreGraphsAsync(telemetry, contextForProject, restoreResultData, token); + var graphs = await GenerateRestoreGraphsAsync(telemetry, contextForProject, token); bool auditRan = false; if (auditEnabled) { - auditRan = await PerformAuditAsync(restoreResultData.RestoreGraphs, telemetry, token); + auditRan = await PerformAuditAsync(graphs, telemetry, token); } telemetry.StartIntervalMeasure(); // Create assets file if (NuGetEventSource.IsEnabled) TraceEvents.BuildAssetsFileStart(_request.Project.FilePath); - restoreResultData.LockFile = BuildAssetsFile( + var assetsFile = BuildAssetsFile( _request.ExistingLockFile, _request.Project, - restoreResultData.RestoreGraphs, + graphs, localRepositories, contextForProject); if (NuGetEventSource.IsEnabled) TraceEvents.BuildAssetsFileStop(_request.Project.FilePath); telemetry.EndIntervalMeasure(GenerateAssetsFileDuration); - restoreResultData.CompatibilityCheckResults = null; - telemetry.StartIntervalMeasure(); - _success &= await ValidateRestoreGraphsAsync(restoreResultData.RestoreGraphs, _logger); + _success &= await ValidateRestoreGraphsAsync(graphs, _logger); // Check package compatibility - restoreResultData.CompatibilityCheckResults = await VerifyCompatibilityAsync( + var compatibilityCheckResults = await VerifyCompatibilityAsync( _request.Project, _includeFlagGraphs, localRepositories, - restoreResultData.LockFile, - restoreResultData.RestoreGraphs, + assetsFile, + graphs, _request.ValidateRuntimeAssets, _logger); - if (restoreResultData.CompatibilityCheckResults.Any(r => !r.Success)) + if (compatibilityCheckResults.Any(r => !r.Success)) { _success = false; } @@ -255,17 +259,31 @@ public async Task ExecuteAsync(CancellationToken token) telemetry.EndIntervalMeasure(ValidateRestoreGraphsDuration); // Generate Targets/Props files - restoreResultData.MSBuildOutputFiles = Enumerable.Empty(); - restoreResultData.AssetFilePath = null; - restoreResultData.CacheFilePath = null; - - await ProcessRestoreResultAsync( + var mSBuildOutputFiles = Enumerable.Empty(); + string assetFilePath = null; + string cacheFilePath = null; + + (mSBuildOutputFiles, + assetFilePath, + cacheFilePath, + assetsFile, + graphs, + packagesLockFile, + packagesLockFilePath, + cacheFile) = await ProcessRestoreResultAsync( telemetry, localRepositories, contextForProject, isLockFileValid, regenerateLockFile, - restoreResultData, + mSBuildOutputFiles, + assetFilePath, + cacheFilePath, + assetsFile, + graphs, + packagesLockFile, + packagesLockFilePath, + cacheFile, token); restoreTime.Stop(); @@ -273,16 +291,16 @@ await ProcessRestoreResultAsync( // Create result return new RestoreResult( _success, - restoreResultData.RestoreGraphs, - restoreResultData.CompatibilityCheckResults, - restoreResultData.MSBuildOutputFiles, - restoreResultData.LockFile, + graphs, + compatibilityCheckResults, + mSBuildOutputFiles, + assetsFile, _request.ExistingLockFile, - restoreResultData.AssetFilePath, - restoreResultData.CacheFile, - restoreResultData.CacheFilePath, - restoreResultData.PackagesLockFilePath, - restoreResultData.PackagesLockFile, + assetFilePath, + cacheFile, + cacheFilePath, + packagesLockFilePath, + packagesLockFile, dependencyGraphSpecFilePath: NoOpRestoreUtilities.GetPersistedDGSpecFilePath(_request), dependencyGraphSpec: _request.DependencyGraphSpec, _request.ProjectStyle, @@ -322,14 +340,14 @@ private void InitializeTelemetry(TelemetryActivity telemetry, int httpSourcesCou telemetry.TelemetryEvent[AuditEnabled] = auditEnabled ? "enabled" : "disabled"; } - private async Task EvaluateNoOpAsync(TelemetryActivity telemetry, RestoreResultData restoreResultData, Stopwatch restoreTime) + private async Task<(RestoreResult, CacheFile)> EvaluateNoOpAsync(TelemetryActivity telemetry, CacheFile cacheFile, Stopwatch restoreTime) { telemetry.StartIntervalMeasure(); bool noOp; TimeSpan? cacheFileAge; if (NuGetEventSource.IsEnabled) TraceEvents.CalcNoOpRestoreStart(_request.Project.FilePath); - (restoreResultData.CacheFile, noOp, cacheFileAge) = EvaluateCacheFile(); + (cacheFile, noOp, cacheFileAge) = EvaluateCacheFile(); if (NuGetEventSource.IsEnabled) TraceEvents.CalcNoOpRestoreStop(_request.Project.FilePath); telemetry.TelemetryEvent[NoOpCacheFileEvaluationResult] = noOp; @@ -338,7 +356,7 @@ private async Task EvaluateNoOpAsync(TelemetryActivity telemetry, { telemetry.StartIntervalMeasure(); - var noOpSuccess = NoOpRestoreUtilities.VerifyRestoreOutput(_request, restoreResultData.CacheFile); + var noOpSuccess = NoOpRestoreUtilities.VerifyRestoreOutput(_request, cacheFile); telemetry.EndIntervalMeasure(NoOpRestoreOutputEvaluationDuration); telemetry.TelemetryEvent[NoOpRestoreOutputEvaluationResult] = noOpSuccess; @@ -348,29 +366,29 @@ private async Task EvaluateNoOpAsync(TelemetryActivity telemetry, telemetry.StartIntervalMeasure(); // Replay Warnings and Errors from an existing lock file in case of a no-op. - await MSBuildRestoreUtility.ReplayWarningsAndErrorsAsync(restoreResultData.CacheFile.LogMessages, _logger); + await MSBuildRestoreUtility.ReplayWarningsAndErrorsAsync(cacheFile.LogMessages, _logger); telemetry.EndIntervalMeasure(NoOpReplayLogsDuration); restoreTime.Stop(); telemetry.TelemetryEvent[NoOpResult] = true; telemetry.TelemetryEvent[RestoreSuccess] = _success; - telemetry.TelemetryEvent[TotalUniquePackagesCount] = restoreResultData.CacheFile.ExpectedPackageFilePaths?.Count ?? -1; + telemetry.TelemetryEvent[TotalUniquePackagesCount] = cacheFile.ExpectedPackageFilePaths?.Count ?? -1; telemetry.TelemetryEvent[NewPackagesInstalledCount] = 0; if (cacheFileAge.HasValue) { telemetry.TelemetryEvent[NoOpCacheFileAgeDays] = cacheFileAge.Value.TotalDays; } - return new NoOpRestoreResult( + return (new NoOpRestoreResult( _success, _request.LockFilePath, new Lazy(() => LockFileUtilities.GetLockFile(_request.LockFilePath, _logger)), - restoreResultData.CacheFile, + cacheFile, _request.Project.RestoreMetadata.CacheFilePath, _request.ProjectStyle, - restoreTime.Elapsed); + restoreTime.Elapsed), cacheFile); } } - return null; + return (null, cacheFile); } private async Task ShowHttpSourcesError() @@ -401,18 +419,16 @@ await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803, } } - private async Task<(bool, bool)> EvaluateLockFile(TelemetryActivity telemetry, RemoteWalkContext contextForProject, RestoreResultData restoreResultData, CancellationToken token) + private async Task<(bool, bool, string, PackagesLockFile)> EvaluateLockFile(TelemetryActivity telemetry, RemoteWalkContext contextForProject, string packagesLockFilePath, PackagesLockFile packagesLockFile, CancellationToken token) { // evaluate packages.lock.json file - restoreResultData.PackagesLockFilePath = PackagesLockFileUtilities.GetNuGetLockFilePath(_request.Project); - restoreResultData.PackagesLockFile = null; var isLockFileValid = false; var regenerateLockFile = true; using (telemetry.StartIndependentInterval(EvaluateLockFileDuration)) { bool result; - (result, isLockFileValid, restoreResultData.PackagesLockFile) = await EvaluatePackagesLockFileAsync(restoreResultData.PackagesLockFilePath, contextForProject, telemetry); + (result, isLockFileValid, packagesLockFile) = await EvaluatePackagesLockFileAsync(packagesLockFilePath, contextForProject, telemetry); telemetry.TelemetryEvent[IsLockFileValidForRestore] = isLockFileValid; telemetry.TelemetryEvent[LockFileEvaluationResult] = result; @@ -421,12 +437,13 @@ await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803, _success &= result; } - return (isLockFileValid, regenerateLockFile); + return (isLockFileValid, regenerateLockFile, packagesLockFilePath, packagesLockFile); } - private async Task GenerateRestoreGraphsAsync(TelemetryActivity telemetry, RemoteWalkContext contextForProject, RestoreResultData restoreResultData, CancellationToken token) + private async Task> GenerateRestoreGraphsAsync(TelemetryActivity telemetry, RemoteWalkContext contextForProject, CancellationToken token) { - restoreResultData.RestoreGraphs = null; + IEnumerable graphs = null; + if (_success) { using (telemetry.StartIndependentInterval(GenerateRestoreGraphDuration)) @@ -436,12 +453,12 @@ private async Task GenerateRestoreGraphsAsync(TelemetryActivity telemetry, Remot if (_enableNewDependencyResolver) { - restoreResultData.RestoreGraphs = await ExecuteRestoreAsync(_request.DependencyProviders.GlobalPackages, _request.DependencyProviders.FallbackPackageFolders, contextForProject, token, telemetry); + graphs = await ExecuteRestoreAsync(_request.DependencyProviders.GlobalPackages, _request.DependencyProviders.FallbackPackageFolders, contextForProject, token, telemetry); } else { // Restore using the legacy code path if the optimized dependency resolution is disabled. - restoreResultData.RestoreGraphs = await ExecuteLegacyRestoreAsync(_request.DependencyProviders.GlobalPackages, _request.DependencyProviders.FallbackPackageFolders, contextForProject, token, telemetry); + graphs = await ExecuteLegacyRestoreAsync(_request.DependencyProviders.GlobalPackages, _request.DependencyProviders.FallbackPackageFolders, contextForProject, token, telemetry); } if (NuGetEventSource.IsEnabled) @@ -460,52 +477,63 @@ private async Task GenerateRestoreGraphsAsync(TelemetryActivity telemetry, Remot // The other scenario is when the lock file is not up to date and we're running locked mode. // In that case we want to write a `target` for each target framework to avoid missing target errors from the SDK build tasks. var frameworkRuntimePair = CreateFrameworkRuntimePairs(_request.Project, RequestRuntimeUtility.GetRestoreRuntimes(_request)); - restoreResultData.RestoreGraphs = frameworkRuntimePair.Select(e => + graphs = frameworkRuntimePair.Select(e => { return RestoreTargetGraph.Create(_request.Project.RuntimeGraph, Enumerable.Empty>(), contextForProject, _logger, e.Framework, e.RuntimeIdentifier); }); } + + return graphs; } - private async Task ProcessRestoreResultAsync(TelemetryActivity telemetry, List localRepositories, RemoteWalkContext contextForProject, bool isLockFileValid, bool regenerateLockFile, RestoreResultData restoreResultData, CancellationToken token) + private async Task<(IEnumerable, string, string, LockFile, IEnumerable, PackagesLockFile, string, CacheFile)> ProcessRestoreResultAsync(TelemetryActivity telemetry, + List localRepositories, + RemoteWalkContext contextForProject, + bool isLockFileValid, + bool regenerateLockFile, + IEnumerable mSBuildOutputFiles, + string assetFilePath, + string cacheFilePath, + LockFile assetsFile, + IEnumerable graphs, + PackagesLockFile packagesLockFile, + string packagesLockFilePath, + CacheFile cacheFile, + CancellationToken token) { - restoreResultData.MSBuildOutputFiles = Enumerable.Empty(); - restoreResultData.AssetFilePath = null; - restoreResultData.CacheFilePath = null; - using (telemetry.StartIndependentInterval(CreateRestoreResultDuration)) { // Determine the lock file output path - restoreResultData.AssetFilePath = GetAssetsFilePath(restoreResultData.LockFile); + assetFilePath = GetAssetsFilePath(assetsFile); // Determine the cache file output path - restoreResultData.CacheFilePath = NoOpRestoreUtilities.GetCacheFilePath(_request, restoreResultData.LockFile); + cacheFilePath = NoOpRestoreUtilities.GetCacheFilePath(_request, assetsFile); // Tool restores are unique since the output path is not known until after restore if (_request.LockFilePath == null && _request.ProjectStyle == ProjectStyle.DotnetCliTool) { - _request.LockFilePath = restoreResultData.AssetFilePath; + _request.LockFilePath = assetFilePath; } if (contextForProject.IsMsBuildBased) { - restoreResultData.MSBuildOutputFiles = BuildAssetsUtils.GetMSBuildOutputFiles( + mSBuildOutputFiles = BuildAssetsUtils.GetMSBuildOutputFiles( _request.Project, - restoreResultData.LockFile, - restoreResultData.RestoreGraphs, + assetsFile, + graphs, localRepositories, _request, - restoreResultData.AssetFilePath, + assetFilePath, _success, _logger); } // If the request is for a lower lock file version, downgrade it appropriately - DowngradeLockFileIfNeeded(restoreResultData.LockFile); + DowngradeLockFileIfNeeded(assetsFile); // Revert to the original case if needed - await FixCaseForLegacyReaders(restoreResultData.RestoreGraphs, restoreResultData.LockFile, token); + await FixCaseForLegacyReaders(graphs, assetsFile, token); // if lock file was still valid then validate package's sha512 hash or else write // the file if enabled. @@ -513,24 +541,24 @@ private async Task ProcessRestoreResultAsync(TelemetryActivity telemetry, List l.Level == LogLevel.Error); - restoreResultData.LockFile.LogMessages = logs; + assetsFile.LogMessages = logs; - if (restoreResultData.CacheFile != null) + if (cacheFile != null) { - restoreResultData.CacheFile.Success = _success; - restoreResultData.CacheFile.ProjectFilePath = _request.Project.FilePath; - restoreResultData.CacheFile.LogMessages = restoreResultData.LockFile.LogMessages; - restoreResultData.CacheFile.ExpectedPackageFilePaths = NoOpRestoreUtilities.GetRestoreOutput(_request, restoreResultData.LockFile); - telemetry.TelemetryEvent[TotalUniquePackagesCount] = restoreResultData.CacheFile?.ExpectedPackageFilePaths.Count; + cacheFile.Success = _success; + cacheFile.ProjectFilePath = _request.Project.FilePath; + cacheFile.LogMessages = assetsFile.LogMessages; + cacheFile.ExpectedPackageFilePaths = NoOpRestoreUtilities.GetRestoreOutput(_request, assetsFile); + telemetry.TelemetryEvent[TotalUniquePackagesCount] = cacheFile?.ExpectedPackageFilePaths.Count; } var errorCodes = ConcatAsString(new HashSet(logs.Where(l => l.Level == LogLevel.Error).Select(l => l.Code))); @@ -570,9 +598,18 @@ private async Task ProcessRestoreResultAsync(TelemetryActivity telemetry, List !g.InConflict).SelectMany(g => g.Install).Distinct().Count(); + telemetry.TelemetryEvent[NewPackagesInstalledCount] = graphs.Where(g => !g.InConflict).SelectMany(g => g.Install).Distinct().Count(); telemetry.TelemetryEvent[RestoreSuccess] = _success; } + + return (mSBuildOutputFiles, + assetFilePath, + cacheFilePath, + assetsFile, + graphs, + packagesLockFile, + packagesLockFilePath, + cacheFile); } /// Run NuGetAudit on the project's resolved restore graphs, and log messages and telemetry with the results. @@ -1709,18 +1746,5 @@ public static void CalcNoOpRestoreStop(string filePath) NuGetEventSource.Instance.Write(EventNameCalcNoOpRestore, eventOptions, new { FilePath = filePath }); } } - - private class RestoreResultData - { - public IEnumerable RestoreGraphs { get; set; } - public IEnumerable CompatibilityCheckResults { get; set; } - public IEnumerable MSBuildOutputFiles { get; set; } - public LockFile LockFile { get; set; } - public string AssetFilePath { get; set; } - public CacheFile CacheFile { get; set; } - public string CacheFilePath { get; set; } - public string PackagesLockFilePath { get; set; } - public PackagesLockFile PackagesLockFile { get; set; } - } } } From 94bee3823875d025c213884d3926ce77fd0c0bcc Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Wed, 27 Nov 2024 15:05:31 -0800 Subject: [PATCH 6/8] reduce changes --- .../RestoreCommand/RestoreCommand.cs | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index 7093d0e6225..fe843aa7464 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -228,7 +228,7 @@ public async Task ExecuteAsync(CancellationToken token) telemetry.StartIntervalMeasure(); // Create assets file if (NuGetEventSource.IsEnabled) TraceEvents.BuildAssetsFileStart(_request.Project.FilePath); - var assetsFile = BuildAssetsFile( + LockFile assetsFile = BuildAssetsFile( _request.ExistingLockFile, _request.Project, graphs, @@ -242,7 +242,7 @@ public async Task ExecuteAsync(CancellationToken token) _success &= await ValidateRestoreGraphsAsync(graphs, _logger); // Check package compatibility - var compatibilityCheckResults = await VerifyCompatibilityAsync( + IList checkResults = await VerifyCompatibilityAsync( _request.Project, _includeFlagGraphs, localRepositories, @@ -251,7 +251,7 @@ public async Task ExecuteAsync(CancellationToken token) _request.ValidateRuntimeAssets, _logger); - if (compatibilityCheckResults.Any(r => !r.Success)) + if (checkResults.Any(r => !r.Success)) { _success = false; } @@ -259,12 +259,12 @@ public async Task ExecuteAsync(CancellationToken token) telemetry.EndIntervalMeasure(ValidateRestoreGraphsDuration); // Generate Targets/Props files - var mSBuildOutputFiles = Enumerable.Empty(); - string assetFilePath = null; + var msbuildOutputFiles = Enumerable.Empty(); + string assetsFilePath = null; string cacheFilePath = null; - (mSBuildOutputFiles, - assetFilePath, + (msbuildOutputFiles, + assetsFilePath, cacheFilePath, assetsFile, graphs, @@ -276,8 +276,8 @@ public async Task ExecuteAsync(CancellationToken token) contextForProject, isLockFileValid, regenerateLockFile, - mSBuildOutputFiles, - assetFilePath, + msbuildOutputFiles, + assetsFilePath, cacheFilePath, assetsFile, graphs, @@ -292,11 +292,11 @@ public async Task ExecuteAsync(CancellationToken token) return new RestoreResult( _success, graphs, - compatibilityCheckResults, - mSBuildOutputFiles, + checkResults, + msbuildOutputFiles, assetsFile, _request.ExistingLockFile, - assetFilePath, + assetsFilePath, cacheFile, cacheFilePath, packagesLockFilePath, @@ -491,7 +491,7 @@ private async Task> GenerateRestoreGraphsAsync(T RemoteWalkContext contextForProject, bool isLockFileValid, bool regenerateLockFile, - IEnumerable mSBuildOutputFiles, + IEnumerable msbuildOutputFiles, string assetFilePath, string cacheFilePath, LockFile assetsFile, @@ -518,7 +518,7 @@ private async Task> GenerateRestoreGraphsAsync(T if (contextForProject.IsMsBuildBased) { - mSBuildOutputFiles = BuildAssetsUtils.GetMSBuildOutputFiles( + msbuildOutputFiles = BuildAssetsUtils.GetMSBuildOutputFiles( _request.Project, assetsFile, graphs, @@ -602,7 +602,7 @@ private async Task> GenerateRestoreGraphsAsync(T telemetry.TelemetryEvent[RestoreSuccess] = _success; } - return (mSBuildOutputFiles, + return (msbuildOutputFiles, assetFilePath, cacheFilePath, assetsFile, From c8bf25a7baabd94b30883ee1cf5fc86cf0043341 Mon Sep 17 00:00:00 2001 From: Nigusu Solomon Yenework <59111203+Nigusu-Allehu@users.noreply.github.com> Date: Mon, 2 Dec 2024 20:41:08 -0800 Subject: [PATCH 7/8] whitespace --- src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index fe843aa7464..02862b3749e 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -443,7 +443,6 @@ await _logger.LogAsync(RestoreLogMessage.CreateWarning(NuGetLogCode.NU1803, private async Task> GenerateRestoreGraphsAsync(TelemetryActivity telemetry, RemoteWalkContext contextForProject, CancellationToken token) { IEnumerable graphs = null; - if (_success) { using (telemetry.StartIndependentInterval(GenerateRestoreGraphDuration)) From 46d31ab4dd17fefcf0e912b155e77fdc21cdb610 Mon Sep 17 00:00:00 2001 From: Nigusu Yenework Date: Tue, 3 Dec 2024 15:56:44 -0800 Subject: [PATCH 8/8] cleanup --- .../RestoreCommand/RestoreCommand.cs | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index 02862b3749e..db923840be7 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -259,13 +259,9 @@ public async Task ExecuteAsync(CancellationToken token) telemetry.EndIntervalMeasure(ValidateRestoreGraphsDuration); // Generate Targets/Props files - var msbuildOutputFiles = Enumerable.Empty(); - string assetsFilePath = null; - string cacheFilePath = null; - - (msbuildOutputFiles, - assetsFilePath, - cacheFilePath, + (IEnumerable msbuildOutputFiles, + string assetsFilePath, + string cacheFilePath, assetsFile, graphs, packagesLockFile, @@ -276,9 +272,6 @@ public async Task ExecuteAsync(CancellationToken token) contextForProject, isLockFileValid, regenerateLockFile, - msbuildOutputFiles, - assetsFilePath, - cacheFilePath, assetsFile, graphs, packagesLockFile, @@ -490,9 +483,6 @@ private async Task> GenerateRestoreGraphsAsync(T RemoteWalkContext contextForProject, bool isLockFileValid, bool regenerateLockFile, - IEnumerable msbuildOutputFiles, - string assetFilePath, - string cacheFilePath, LockFile assetsFile, IEnumerable graphs, PackagesLockFile packagesLockFile, @@ -500,6 +490,10 @@ private async Task> GenerateRestoreGraphsAsync(T CacheFile cacheFile, CancellationToken token) { + string assetFilePath = null; + string cacheFilePath = null; + var msbuildOutputFiles = Enumerable.Empty(); + using (telemetry.StartIndependentInterval(CreateRestoreResultDuration)) { // Determine the lock file output path