From 5ab36e31de7ee5b030a768962b82861715fc0889 Mon Sep 17 00:00:00 2001 From: Nikolche Kolev Date: Thu, 14 May 2020 16:08:44 -0700 Subject: [PATCH 1/3] Implement Visual Studio partial restore. Add a solution based up to date check that's particular to VS and the fact that it's a long running process --- .../ISolutionRestoreChecker.cs | 42 + .../NuGet.SolutionRestoreManager.csproj | 2 + .../SolutionRestoreBuildHandler.cs | 14 +- .../SolutionRestoreJob.cs | 55 +- .../SolutionUpToDateChecker.cs | 264 +++++++ .../GlobalSuppressions.cs | 4 - .../Telemetry/RestoreTelemetryEvent.cs | 12 +- .../Utility/BuildAssetsUtils.cs | 25 +- .../Utility/NoOpRestoreUtilities.cs | 4 +- .../Telemetry/RestoreTelemetryServiceTests.cs | 6 +- .../SolutionRestoreBuildHandlerTests.cs | 16 +- .../SolutionUpToDateCheckerTests.cs | 726 ++++++++++++++++++ .../BuildAssetsUtilsTests.cs | 53 +- .../NuGet.Commands.Test/RestoreRunnerTests.cs | 38 +- .../Commands/ProjectJsonTestHelpers.cs | 26 +- 15 files changed, 1207 insertions(+), 80 deletions(-) create mode 100644 src/NuGet.Clients/NuGet.SolutionRestoreManager/ISolutionRestoreChecker.cs create mode 100644 src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionUpToDateChecker.cs create mode 100644 test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/SolutionUpToDateCheckerTests.cs diff --git a/src/NuGet.Clients/NuGet.SolutionRestoreManager/ISolutionRestoreChecker.cs b/src/NuGet.Clients/NuGet.SolutionRestoreManager/ISolutionRestoreChecker.cs new file mode 100644 index 00000000000..33cbfd3e12d --- /dev/null +++ b/src/NuGet.Clients/NuGet.SolutionRestoreManager/ISolutionRestoreChecker.cs @@ -0,0 +1,42 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using System.Threading.Tasks; + +using NuGet.Commands; +using NuGet.ProjectModel; + +namespace NuGet.SolutionRestoreManager +{ + /// + /// An up to date checker for a solution. + /// + public interface ISolutionRestoreChecker + { + /// + /// Given the current dependency graph spec, perform a fast up to date check and return the dirty projects. + /// The checker itself caches the DependencyGraphSpec it is provided & the last restore status, reported through . + /// Accounts for changes in the PackageSpec and marks all the parent projects as dirty as well. + /// Additionally also ensures that the expected output files have the same timestamps as the last time a succesful status was reported through . + /// + /// The current dependency graph spec. + /// Unique ids of the dirty projects + /// Note that this call is stateful. This method may end up caching the dependency graph spec, so do not invoke multiple times. Ideally call should be followed by a call. + IEnumerable PerformUpToDateCheck(DependencyGraphSpec dependencyGraphSpec); + + /// + /// Report the status of all the projects restored. + /// + /// + /// Note that this call is stateful. This method may end up caching the dependency graph spec, so do not invoke multiple times. Ideally call should be followed by a call. + + void ReportStatus(IReadOnlyList restoreSummaries); + + /// + /// Clears any cached values. This is meant to mimic restores that overwrite the incremental restore optimizations. + /// + /// + void CleanCache(); + } +} diff --git a/src/NuGet.Clients/NuGet.SolutionRestoreManager/NuGet.SolutionRestoreManager.csproj b/src/NuGet.Clients/NuGet.SolutionRestoreManager/NuGet.SolutionRestoreManager.csproj index 29c474fd210..c7840b141e9 100644 --- a/src/NuGet.Clients/NuGet.SolutionRestoreManager/NuGet.SolutionRestoreManager.csproj +++ b/src/NuGet.Clients/NuGet.SolutionRestoreManager/NuGet.SolutionRestoreManager.csproj @@ -32,6 +32,7 @@ + @@ -49,6 +50,7 @@ + diff --git a/src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionRestoreBuildHandler.cs b/src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionRestoreBuildHandler.cs index a2565cd509a..6a61e924bdd 100644 --- a/src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionRestoreBuildHandler.cs +++ b/src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionRestoreBuildHandler.cs @@ -3,11 +3,9 @@ using System; using System.ComponentModel.Composition; -using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using Microsoft; -using Microsoft.VisualStudio; using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.Shell.Interop; using Microsoft.VisualStudio.Threading; @@ -40,6 +38,9 @@ public sealed class SolutionRestoreBuildHandler [Import] private Lazy SolutionRestoreWorker { get; set; } + [Import] + private Lazy SolutionRestoreChecker { get; set; } + /// /// The object controlling the update solution events. /// @@ -62,15 +63,17 @@ private SolutionRestoreBuildHandler() public SolutionRestoreBuildHandler( ISettings settings, ISolutionRestoreWorker restoreWorker, - IVsSolutionBuildManager3 buildManager) + IVsSolutionBuildManager3 buildManager, + ISolutionRestoreChecker solutionRestoreChecker) { Assumes.Present(settings); Assumes.Present(restoreWorker); Assumes.Present(buildManager); + Assumes.Present(solutionRestoreChecker); Settings = new Lazy(() => settings); SolutionRestoreWorker = new Lazy(() => restoreWorker); - + SolutionRestoreChecker = new Lazy(() => solutionRestoreChecker); _solutionBuildManager = buildManager; _isMEFInitialized = true; @@ -141,8 +144,9 @@ public async Task RestoreAsync(uint buildAction, CancellationToken token) if ((buildAction & (uint)VSSOLNBUILDUPDATEFLAGS.SBF_OPERATION_CLEAN) != 0 && (buildAction & (uint)VSSOLNBUILDUPDATEFLAGS3.SBF_FLAGS_UPTODATE_CHECK) == 0) { - // Clear the project.json restore cache on clean to ensure that the next build restores again + // Clear the transitive restore cache on clean to ensure that the next build restores again await SolutionRestoreWorker.Value.CleanCacheAsync(); + SolutionRestoreChecker.Value.CleanCache(); } else if ((buildAction & (uint)VSSOLNBUILDUPDATEFLAGS.SBF_OPERATION_BUILD) != 0 && (buildAction & (uint)VSSOLNBUILDUPDATEFLAGS3.SBF_FLAGS_UPTODATE_CHECK) == 0 && diff --git a/src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionRestoreJob.cs b/src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionRestoreJob.cs index 4e539ab6699..b07af7a0f2d 100644 --- a/src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionRestoreJob.cs +++ b/src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionRestoreJob.cs @@ -26,6 +26,7 @@ using NuGet.ProjectManagement.Projects; using NuGet.ProjectModel; using NuGet.Protocol.Core.Types; +using NuGet.Shared; using NuGet.VisualStudio; using NuGet.VisualStudio.Telemetry; using Task = System.Threading.Tasks.Task; @@ -46,6 +47,7 @@ internal sealed class SolutionRestoreJob : ISolutionRestoreJob private readonly ISourceRepositoryProvider _sourceRepositoryProvider; private readonly ISettings _settings; private readonly IRestoreEventsPublisher _restoreEventsPublisher; + private readonly ISolutionRestoreChecker _solutionUpToDateChecker; private RestoreOperationLogger _logger; private INuGetProjectContext _nuGetProjectContext; @@ -54,6 +56,7 @@ internal sealed class SolutionRestoreJob : ISolutionRestoreJob private NuGetOperationStatus _status; private int _packageCount; private int _noOpProjectsCount; + private int _upToDateProjectCount; // relevant to packages.config restore only private int _missingPackagesCount; @@ -65,13 +68,15 @@ public SolutionRestoreJob( IVsSolutionManager solutionManager, ISourceRepositoryProvider sourceRepositoryProvider, IRestoreEventsPublisher restoreEventsPublisher, - ISettings settings) + ISettings settings, + ISolutionRestoreChecker solutionRestoreChecker) : this(AsyncServiceProvider.GlobalProvider, packageRestoreManager, solutionManager, sourceRepositoryProvider, restoreEventsPublisher, - settings + settings, + solutionRestoreChecker ) { } @@ -81,7 +86,8 @@ public SolutionRestoreJob( IVsSolutionManager solutionManager, ISourceRepositoryProvider sourceRepositoryProvider, IRestoreEventsPublisher restoreEventsPublisher, - ISettings settings) + ISettings settings, + ISolutionRestoreChecker solutionRestoreChecker) { Assumes.Present(asyncServiceProvider); Assumes.Present(packageRestoreManager); @@ -89,6 +95,7 @@ public SolutionRestoreJob( Assumes.Present(sourceRepositoryProvider); Assumes.Present(restoreEventsPublisher); Assumes.Present(settings); + Assumes.Present(solutionRestoreChecker); _asyncServiceProvider = asyncServiceProvider; _packageRestoreManager = packageRestoreManager; @@ -97,6 +104,7 @@ public SolutionRestoreJob( _restoreEventsPublisher = restoreEventsPublisher; _settings = settings; _packageRestoreConsent = new PackageRestoreConsent(_settings); + _solutionUpToDateChecker = solutionRestoreChecker; } /// @@ -148,8 +156,6 @@ public async Task ExecuteAsync( return _status == NuGetOperationStatus.NoOp || _status == NuGetOperationStatus.Succeeded; } - - private async Task RestoreAsync(bool forceRestore, RestoreOperationSource restoreSource, CancellationToken token) { var startTime = DateTimeOffset.Now; @@ -279,6 +285,7 @@ private void EmitRestoreTelemetryEvent(IEnumerable projects, startTime, _status, _packageCount, + _upToDateProjectCount, _noOpProjectsCount, DateTimeOffset.Now, duration, @@ -328,6 +335,7 @@ private async Task RestorePackageSpecProjectsAsync( } DependencyGraphCacheContext cacheContext; + DependencyGraphSpec originalDgSpec; DependencyGraphSpec dgSpec; IReadOnlyList additionalMessages; @@ -338,12 +346,40 @@ private async Task RestorePackageSpecProjectsAsync( var pathContext = NuGetPathContext.Create(_settings); // Get full dg spec - (dgSpec, additionalMessages) = await DependencyGraphRestoreUtility.GetSolutionRestoreSpecAndAdditionalMessages(_solutionManager, cacheContext); + (originalDgSpec, additionalMessages) = await DependencyGraphRestoreUtility.GetSolutionRestoreSpecAndAdditionalMessages(_solutionManager, cacheContext); + } + + using (intervalTracker.Start(RestoreTelemetryEvent.SolutionUpToDateCheck)) + { + // Run solution based up to date check. + var projectsNeedingRestore = _solutionUpToDateChecker.PerformUpToDateCheck(originalDgSpec).AsList(); + + dgSpec = originalDgSpec; + // Only use the optimization results if the restore is not force. + // Still run the optimization check anyways to prep the cache for a potential future non-force optimization + if (!forceRestore) + { + // Update the dg spec. + dgSpec = originalDgSpec.WithoutRestores(); + foreach (var uniqueProjectId in projectsNeedingRestore) + { + dgSpec.AddRestore(uniqueProjectId); + } + // loop through all legacy PackageReference projects. We don't know how to replay their warnings & errors yet. + foreach(var project in (await _solutionManager.GetNuGetProjectsAsync()).Where(e => e is LegacyPackageReferenceProject).Select(e => e as LegacyPackageReferenceProject)) + { + dgSpec.AddRestore(project.MSBuildProjectPath); + } + + // recorded the number of up to date projects + _upToDateProjectCount = originalDgSpec.Restore.Count - projectsNeedingRestore.Count; + _noOpProjectsCount = _upToDateProjectCount; + } } using (intervalTracker.Start(RestoreTelemetryEvent.PackageReferenceRestoreDuration)) { - // Avoid restoring solutions with zero potential PackageReference projects. + // Avoid restoring if all the projects are up to date, or the solution does not have build integrated projects. if (DependencyGraphRestoreUtility.IsRestoreRequired(dgSpec)) { // NOTE: During restore for build integrated projects, @@ -379,8 +415,9 @@ await _logger.RunWithProgressAsync( _packageCount += restoreSummaries.Select(summary => summary.InstallCount).Sum(); var isRestoreFailed = restoreSummaries.Any(summary => summary.Success == false); - _noOpProjectsCount = restoreSummaries.Where(summary => summary.NoOpRestore == true).Count(); - + _noOpProjectsCount += restoreSummaries.Where(summary => summary.NoOpRestore == true).Count(); + _solutionUpToDateChecker.ReportStatus(restoreSummaries); + if (isRestoreFailed) { _status = NuGetOperationStatus.Failed; diff --git a/src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionUpToDateChecker.cs b/src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionUpToDateChecker.cs new file mode 100644 index 00000000000..45473ed8a60 --- /dev/null +++ b/src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionUpToDateChecker.cs @@ -0,0 +1,264 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.ComponentModel.Composition; +using System.IO; +using System.Linq; +using NuGet.Commands; +using NuGet.Common; +using NuGet.ProjectModel; + +namespace NuGet.SolutionRestoreManager +{ + [Export(typeof(ISolutionRestoreChecker))] + [PartCreationPolicy(CreationPolicy.Shared)] + public class SolutionUpToDateChecker : ISolutionRestoreChecker + { + private IList _failedProjects = new List(); + private DependencyGraphSpec _cachedDependencyGraphSpec; + private Dictionary _outputWriteTimes = new Dictionary(); + + public void ReportStatus(IReadOnlyList restoreSummaries) + { + _failedProjects.Clear(); + + foreach (var summary in restoreSummaries) + { + if (summary.Success) + { + var packageSpec = _cachedDependencyGraphSpec.GetProjectSpec(summary.InputPath); + GetOutputFilePaths(packageSpec, out string assetsFilePath, out string cacheFilePath, out string targetsFilePath, out string propsFilePath, out string lockFilePath); + + _outputWriteTimes[summary.InputPath] = new RestoreOutputData() + { + _lastAssetsFileWriteTime = GetLastWriteTime(assetsFilePath), + _lastCacheFileWriteTime = GetLastWriteTime(cacheFilePath), + _lastTargetsFileWriteTime = GetLastWriteTime(targetsFilePath), + _lastPropsFileWriteTime = GetLastWriteTime(propsFilePath), + _lastLockFileWriteTime = GetLastWriteTime(lockFilePath), + _globalPackagesFolderCreationTime = GetCreationTime(packageSpec.RestoreMetadata.PackagesPath) + }; + } + else + { + _failedProjects.Add(summary.InputPath); + } + + } + } + + // The algorithm here is a 2 pass. In reality the 2nd pass can do a lot but for huge benefits :) + // Pass #1 + // We check all the specs against the cached ones if any. Any project with a change in the spec is considered dirty. + // If a project had previously been restored and it failed, it is considered dirty. + // Every project that is considered to have a dirty spec will be important in pass #2. + // In the first pass, we also validate the outputs for the projects. Note that these are independent and project specific. Outputs not being up to date it irrelevant for transitivity. + // Pass #2 + // For every project with a dirty spec (the outputs don't matter here), we want to ensure that its parent projects are marked as dirty as well. + // This is a bit more expensive since PackageSpecs do not retain pointers to the projects that reference them as ProjectReference. + // Finally we only update the cache specs if Pass #1 determined that there are projects that are not up to date. + // Result + // Lastly all the projects marked as having dirty specs & dirty outputs are returned. + public IEnumerable PerformUpToDateCheck(DependencyGraphSpec dependencyGraphSpec) + { + if (_cachedDependencyGraphSpec != null) + { + var dirtySpecs = new List(); + var dirtyOutputs = new List(); + bool hasDirtyNonTransitiveSpecs = false; + + // Pass #1. Validate all the data (i/o) + // 1a. Validate the package specs (references & settings) + // 1b. Validate the expected outputs (assets file, nuget.g.*, lock file) + foreach (var project in dependencyGraphSpec.Projects) + { + var projectUniqueName = project.RestoreMetadata.ProjectUniqueName; + var cache = _cachedDependencyGraphSpec.GetProjectSpec(projectUniqueName); + + if (cache == null || !project.Equals(cache)) + { + dirtySpecs.Add(projectUniqueName); + } + + if (project.RestoreMetadata.ProjectStyle == ProjectStyle.PackageReference || + project.RestoreMetadata.ProjectStyle == ProjectStyle.ProjectJson) + { + if (!_failedProjects.Contains(projectUniqueName) && _outputWriteTimes.TryGetValue(projectUniqueName, out RestoreOutputData outputWriteTime)) + { + GetOutputFilePaths(project, out string assetsFilePath, out string cacheFilePath, out string targetsFilePath, out string propsFilePath, out string lockFilePath); + if (!AreOutputsUpToDate(assetsFilePath, cacheFilePath, targetsFilePath, propsFilePath, lockFilePath, project.RestoreMetadata.PackagesPath, outputWriteTime)) + { + dirtyOutputs.Add(projectUniqueName); + } + } + else + { + dirtyOutputs.Add(projectUniqueName); + } + } + else + { + hasDirtyNonTransitiveSpecs = true; + } + } + + // Fast path. Skip Pass #2 + if (dirtySpecs.Count == 0 && dirtyOutputs.Count == 0) + { + return Enumerable.Empty(); + } + // Update the cache before Pass #2 + _cachedDependencyGraphSpec = dependencyGraphSpec; + + // Pass #2 For any dirty specs discrepancies, mark them and their parents as needing restore. + var dirtyProjects = GetParents(dirtySpecs, dependencyGraphSpec); + + // All dirty projects + projects with outputs that need to be restored + // - the projects that are non transitive that never needed restore anyways, hence the insertion with the provider restore projects! + var resultSpecs = dirtyProjects.Union(dirtyOutputs); + if (hasDirtyNonTransitiveSpecs) + { + resultSpecs = dependencyGraphSpec.Restore.Intersect(resultSpecs); + } + return resultSpecs; + } + else + { + _cachedDependencyGraphSpec = dependencyGraphSpec; + + return dependencyGraphSpec.Restore; + } + } + + /// + /// Given a list of project unique names, goes through the dg spec and returns the current projects + all their parents + /// + /// The projects for which we need to find the parents + /// The dependency graph spec contain the projects passed in dirty specs at the minimum. + /// The list of the projects passed in and their parents in the dependencyGraphSpec + internal static IList GetParents(List DirtySpecs, DependencyGraphSpec dependencyGraphSpec) + { + var projectsByUniqueName = dependencyGraphSpec.Projects + .ToDictionary(t => t.RestoreMetadata.ProjectUniqueName, t => t, PathUtility.GetStringComparerBasedOnOS()); + + var DirtyProjects = new HashSet(DirtySpecs, PathUtility.GetStringComparerBasedOnOS()); + + var sortedProjects = DependencyGraphSpec.SortPackagesByDependencyOrder(dependencyGraphSpec.Projects); + + foreach (var project in sortedProjects) + { + if (!DirtyProjects.Contains(project.RestoreMetadata.ProjectUniqueName)) + { + var projectReferences = GetPackageSpecDependencyIds(project); + + foreach (var projectReference in projectReferences) + { + if (DirtyProjects.Contains(projectReference)) + { + DirtyProjects.Add(project.RestoreMetadata.ProjectUniqueName); + } + } + } + } + return DirtyProjects.ToList(); + } + + private static string[] GetPackageSpecDependencyIds(PackageSpec spec) + { + return spec.RestoreMetadata + .TargetFrameworks + .SelectMany(r => r.ProjectReferences) + .Select(r => r.ProjectUniqueName) + .Distinct(PathUtility.GetStringComparerBasedOnOS()) + .ToArray(); + } + + internal static void GetOutputFilePaths(PackageSpec packageSpec, out string assetsFilePath, out string cacheFilePath, out string targetsFilePath, out string propsFilePath, out string lockFilePath) + { + assetsFilePath = GetAssetsFilePath(packageSpec); + cacheFilePath = NoOpRestoreUtilities.GetProjectCacheFilePath(packageSpec.RestoreMetadata.OutputPath); + targetsFilePath = BuildAssetsUtils.GetMSBuildFilePath(packageSpec, BuildAssetsUtils.TargetsExtension); + propsFilePath = BuildAssetsUtils.GetMSBuildFilePath(packageSpec, BuildAssetsUtils.PropsExtension); + lockFilePath = packageSpec.RestoreMetadata.ProjectStyle == ProjectStyle.PackageReference ? + PackagesLockFileUtilities.GetNuGetLockFilePath(packageSpec) : + null; + } + + private static bool AreOutputsUpToDate(string assetsFilePath, string cacheFilePath, string targetsFilePath, string propsFilePath, string lockFilePath, string globalPackagesFolderPath, RestoreOutputData outputWriteTime) + { + DateTime currentAssetsFileWriteTime = GetLastWriteTime(assetsFilePath); + DateTime currentCacheFilePath = GetLastWriteTime(cacheFilePath); + DateTime currentTargetsFilePath = GetLastWriteTime(targetsFilePath); + DateTime currentPropsFilePath = GetLastWriteTime(propsFilePath); + DateTime currentLockFilePath = GetLastWriteTime(lockFilePath); + DateTime globalPackagesFolderCreationTime = GetCreationTime(globalPackagesFolderPath); + + return outputWriteTime._lastAssetsFileWriteTime.Equals(currentAssetsFileWriteTime) && + outputWriteTime._lastCacheFileWriteTime.Equals(currentCacheFilePath) && + outputWriteTime._lastTargetsFileWriteTime.Equals(currentTargetsFilePath) && + outputWriteTime._lastPropsFileWriteTime.Equals(currentPropsFilePath) && + outputWriteTime._lastLockFileWriteTime.Equals(currentLockFilePath) && + outputWriteTime._globalPackagesFolderCreationTime.Equals(globalPackagesFolderCreationTime); + } + + private static DateTime GetLastWriteTime(string assetsFilePath) + { + if (!string.IsNullOrWhiteSpace(assetsFilePath)) + { + var fileInfo = new FileInfo(assetsFilePath); + if (fileInfo.Exists) + { + return fileInfo.LastWriteTimeUtc; + } + } + return default; + } + + private static DateTime GetCreationTime(string assetsFilePath) + { + if (!string.IsNullOrWhiteSpace(assetsFilePath)) + { + var fileInfo = new FileInfo(assetsFilePath); + if (fileInfo.Exists || ((fileInfo.Attributes & FileAttributes.Directory) == FileAttributes.Directory)) + { + return fileInfo.CreationTimeUtc; + } + } + return default; + } + + private static string GetAssetsFilePath(PackageSpec packageSpec) + { + if (packageSpec.RestoreMetadata?.ProjectStyle == ProjectStyle.PackageReference) + { + return Path.Combine( + packageSpec.RestoreMetadata.OutputPath, + LockFileFormat.AssetsFileName); + } + else if (packageSpec.RestoreMetadata?.ProjectStyle == ProjectStyle.ProjectJson) + { + return ProjectJsonPathUtilities.GetLockFilePath(packageSpec.FilePath); + } + return null; + } + + public void CleanCache() + { + _failedProjects.Clear(); + _cachedDependencyGraphSpec = null; + _outputWriteTimes.Clear(); + } + + internal struct RestoreOutputData + { + internal DateTime _lastAssetsFileWriteTime; + internal DateTime _lastCacheFileWriteTime; + internal DateTime _lastTargetsFileWriteTime; + internal DateTime _lastPropsFileWriteTime; + internal DateTime _lastLockFileWriteTime; + internal DateTime _globalPackagesFolderCreationTime; + } + } +} diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Common/GlobalSuppressions.cs b/src/NuGet.Clients/NuGet.VisualStudio.Common/GlobalSuppressions.cs index 312ad16a004..b6c3417f5e8 100644 --- a/src/NuGet.Clients/NuGet.VisualStudio.Common/GlobalSuppressions.cs +++ b/src/NuGet.Clients/NuGet.VisualStudio.Common/GlobalSuppressions.cs @@ -8,10 +8,6 @@ using System.Diagnostics.CodeAnalysis; -[assembly: SuppressMessage("Build", "CA2211:Non-constant fields should not be visible", Justification = "", Scope = "member", Target = "~F:NuGet.VisualStudio.RestoreTelemetryEvent.PackageReferenceRestoreDuration")] -[assembly: SuppressMessage("Build", "CA2211:Non-constant fields should not be visible", Justification = "", Scope = "member", Target = "~F:NuGet.VisualStudio.RestoreTelemetryEvent.PackagesConfigRestore")] -[assembly: SuppressMessage("Build", "CA2211:Non-constant fields should not be visible", Justification = "", Scope = "member", Target = "~F:NuGet.VisualStudio.RestoreTelemetryEvent.RestoreOperationChecks")] -[assembly: SuppressMessage("Build", "CA2211:Non-constant fields should not be visible", Justification = "", Scope = "member", Target = "~F:NuGet.VisualStudio.RestoreTelemetryEvent.SolutionDependencyGraphSpecCreation")] [assembly: SuppressMessage("Build", "CA1802:Field 'EventName' is declared as 'readonly' but is initialized with a constant value. Mark this field as 'const' instead.", Justification = "", Scope = "member", Target = "~F:NuGet.VisualStudio.Telemetry.PackageSourceTelemetry.EventName")] [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'bool PathValidator.IsValidLocalPath(string path)', validate parameter 'path' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "", Scope = "member", Target = "~M:NuGet.PackageManagement.VisualStudio.PathValidator.IsValidLocalPath(System.String)~System.Boolean")] [assembly: SuppressMessage("Build", "CA1062:In externally visible method 'bool PathValidator.IsValidUncPath(string path)', validate parameter 'path' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "", Scope = "member", Target = "~M:NuGet.PackageManagement.VisualStudio.PathValidator.IsValidUncPath(System.String)~System.Boolean")] diff --git a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/RestoreTelemetryEvent.cs b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/RestoreTelemetryEvent.cs index 5d4d26d790f..f97a1cb5dff 100644 --- a/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/RestoreTelemetryEvent.cs +++ b/src/NuGet.Clients/NuGet.VisualStudio.Common/Telemetry/RestoreTelemetryEvent.cs @@ -12,10 +12,12 @@ namespace NuGet.VisualStudio /// public class RestoreTelemetryEvent : ActionEventBase { - public static string RestoreOperationChecks = nameof(RestoreOperationChecks); - public static string PackagesConfigRestore = nameof(PackagesConfigRestore); - public static string SolutionDependencyGraphSpecCreation = nameof(SolutionDependencyGraphSpecCreation); - public static string PackageReferenceRestoreDuration = nameof(PackageReferenceRestoreDuration); + public const string RestoreOperationChecks = nameof(RestoreOperationChecks); + public const string PackagesConfigRestore = nameof(PackagesConfigRestore); + public const string SolutionDependencyGraphSpecCreation = nameof(SolutionDependencyGraphSpecCreation); + public const string PackageReferenceRestoreDuration = nameof(PackageReferenceRestoreDuration); + public const string SolutionUpToDateCheck = nameof(SolutionUpToDateCheck); + private const string UpToDateProjectCount = nameof(UpToDateProjectCount); public RestoreTelemetryEvent( string operationId, @@ -25,12 +27,14 @@ public RestoreTelemetryEvent( NuGetOperationStatus status, int packageCount, int noOpProjectsCount, + int upToDateProjectsCount, DateTimeOffset endTime, double duration, IntervalTracker intervalTimingTracker) : base(RestoreActionEventName, operationId, projectIds, startTime, status, packageCount, endTime, duration) { base[nameof(OperationSource)] = source; base[nameof(NoOpProjectsCount)] = noOpProjectsCount; + base[UpToDateProjectCount] = upToDateProjectsCount; foreach (var (intervalName, intervalDuration) in intervalTimingTracker.GetIntervals()) { diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs index 286c5072162..f8b96799a0a 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/BuildAssetsUtils.cs @@ -363,6 +363,7 @@ public static XDocument ReadExisting(string path, ILogger log) return result; } + [Obsolete("This method looks at the RestoreRequest. It is expected that the PackageSpec contains all the relevant information the RestoreRequest would. Use GetMSBuildFilePath(PackageSpec project, string extension) instead.")] public static string GetMSBuildFilePath(PackageSpec project, RestoreRequest request, string extension) { string path; @@ -382,6 +383,26 @@ public static string GetMSBuildFilePath(PackageSpec project, RestoreRequest requ return path; } + public static string GetMSBuildFilePath(PackageSpec project, string extension) + { + string path; + + if (project.RestoreMetadata?.ProjectStyle == ProjectStyle.PackageReference || project.RestoreMetadata?.ProjectStyle == ProjectStyle.DotnetToolReference) + { + // PackageReference style projects + var projFileName = Path.GetFileName(project.RestoreMetadata.ProjectPath); + path = Path.Combine(project.RestoreMetadata.OutputPath, $"{projFileName}.nuget.g{extension}"); + } + else + { + // Project.json style projects + var dir = Path.GetDirectoryName(project.FilePath); + path = Path.Combine(dir, $"{project.Name}.nuget{extension}"); + } + return path; + + } + public static string GetMSBuildFilePathForPackageReferenceStyleProject(PackageSpec project, string extension) { var projFileName = Path.GetFileName(project.RestoreMetadata.ProjectPath); @@ -399,8 +420,8 @@ public static List GetMSBuildOutputFiles(PackageSpec project, ILogger log) { // Generate file names - var targetsPath = GetMSBuildFilePath(project, request, TargetsExtension); - var propsPath = GetMSBuildFilePath(project, request, PropsExtension); + var targetsPath = GetMSBuildFilePath(project, TargetsExtension); + var propsPath = GetMSBuildFilePath(project, PropsExtension); // Targets files contain a macro for the repository root. If only the user package folder was used // allow a replacement. If fallback folders were used the macro cannot be applied. diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/NoOpRestoreUtilities.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/NoOpRestoreUtilities.cs index 54edc179cd9..d48a20c8089 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/NoOpRestoreUtilities.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/NoOpRestoreUtilities.cs @@ -130,13 +130,13 @@ internal static bool VerifyRestoreOutput(RestoreRequest request, CacheFile cache if (request.ProjectStyle == ProjectStyle.PackageReference || request.ProjectStyle == ProjectStyle.Standalone) { - var targetsFilePath = BuildAssetsUtils.GetMSBuildFilePath(request.Project, request, BuildAssetsUtils.TargetsExtension); + var targetsFilePath = BuildAssetsUtils.GetMSBuildFilePath(request.Project, BuildAssetsUtils.TargetsExtension); if (!File.Exists(targetsFilePath)) { request.Log.LogVerbose(string.Format(CultureInfo.CurrentCulture, Strings.Log_TargetsFileNotOnDisk, request.Project.Name, targetsFilePath)); return false; } - var propsFilePath = BuildAssetsUtils.GetMSBuildFilePath(request.Project, request, BuildAssetsUtils.PropsExtension); + var propsFilePath = BuildAssetsUtils.GetMSBuildFilePath(request.Project, BuildAssetsUtils.PropsExtension); if (!File.Exists(propsFilePath)) { request.Log.LogVerbose(string.Format(CultureInfo.CurrentCulture, Strings.Log_PropsFileNotOnDisk, request.Project.Name, propsFilePath)); diff --git a/test/NuGet.Clients.Tests/NuGet.PackageManagement.VisualStudio.Test/Telemetry/RestoreTelemetryServiceTests.cs b/test/NuGet.Clients.Tests/NuGet.PackageManagement.VisualStudio.Test/Telemetry/RestoreTelemetryServiceTests.cs index bd0ad709bb8..44892ad9635 100644 --- a/test/NuGet.Clients.Tests/NuGet.PackageManagement.VisualStudio.Test/Telemetry/RestoreTelemetryServiceTests.cs +++ b/test/NuGet.Clients.Tests/NuGet.PackageManagement.VisualStudio.Test/Telemetry/RestoreTelemetryServiceTests.cs @@ -46,6 +46,7 @@ public void RestoreTelemetryService_EmitRestoreEvent_OperationSucceed(RestoreOpe status: status, packageCount: 2, noOpProjectsCount: noopProjectsCount, + upToDateProjectsCount: 0, endTime: DateTimeOffset.Now, duration: 2.10, new IntervalTracker("Activity")); @@ -89,6 +90,7 @@ public void RestoreTelemetryService_EmitRestoreEvent_IntervalsAreCaptured() status: NuGetOperationStatus.Succeeded, packageCount: 1, noOpProjectsCount: 0, + upToDateProjectsCount: 0, endTime: DateTimeOffset.Now, duration: 2.10, tracker @@ -102,7 +104,7 @@ public void RestoreTelemetryService_EmitRestoreEvent_IntervalsAreCaptured() Assert.NotNull(lastTelemetryEvent); Assert.Equal(RestoreTelemetryEvent.RestoreActionEventName, lastTelemetryEvent.Name); - Assert.Equal(12, lastTelemetryEvent.Count); + Assert.Equal(13, lastTelemetryEvent.Count); Assert.Equal(restoreTelemetryData.OperationSource.ToString(), lastTelemetryEvent["OperationSource"].ToString()); @@ -115,7 +117,7 @@ private void VerifyTelemetryEventData(string operationId, RestoreTelemetryEvent { Assert.NotNull(actual); Assert.Equal(RestoreTelemetryEvent.RestoreActionEventName, actual.Name); - Assert.Equal(10, actual.Count); + Assert.Equal(11, actual.Count); Assert.Equal(expected.OperationSource.ToString(), actual["OperationSource"].ToString()); diff --git a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/SolutionRestoreBuildHandlerTests.cs b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/SolutionRestoreBuildHandlerTests.cs index 79fae890853..5424dfb88a2 100644 --- a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/SolutionRestoreBuildHandlerTests.cs +++ b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/SolutionRestoreBuildHandlerTests.cs @@ -32,9 +32,10 @@ public async Task QueryDelayBuildAction_CleanBuild() var settings = Mock.Of(); var restoreWorker = Mock.Of(); var buildManager = Mock.Of(); + var restoreChecker = Mock.Of(); var buildAction = (uint)VSSOLNBUILDUPDATEFLAGS.SBF_OPERATION_CLEAN; - using (var handler = new SolutionRestoreBuildHandler(settings, restoreWorker, buildManager)) + using (var handler = new SolutionRestoreBuildHandler(settings, restoreWorker, buildManager, restoreChecker)) { await _jtf.SwitchToMainThreadAsync(); @@ -45,7 +46,9 @@ public async Task QueryDelayBuildAction_CleanBuild() Mock.Get(restoreWorker) .Verify(x => x.CleanCacheAsync(), Times.Once); - + Mock.Get(restoreChecker) + .Verify(x => x.CleanCache(), Times.Once); + Mock.Get(restoreWorker) .Verify(x => x.ScheduleRestoreAsync(It.IsAny(), It.IsAny()), Times.Never); } @@ -56,6 +59,7 @@ public async Task QueryDelayBuildAction_ShouldNotRestoreOnBuild_NoOps() var settings = Mock.Of(); var restoreWorker = Mock.Of(); var buildManager = Mock.Of(); + var restoreChecker = Mock.Of(); var buildAction = (uint)VSSOLNBUILDUPDATEFLAGS.SBF_OPERATION_BUILD; @@ -64,7 +68,7 @@ public async Task QueryDelayBuildAction_ShouldNotRestoreOnBuild_NoOps() .Returns(() => new VirtualSettingSection("packageRestore", new AddItem("automatic", bool.FalseString))); - using (var handler = new SolutionRestoreBuildHandler(settings, restoreWorker, buildManager)) + using (var handler = new SolutionRestoreBuildHandler(settings, restoreWorker, buildManager, restoreChecker)) { await _jtf.SwitchToMainThreadAsync(); @@ -83,6 +87,7 @@ public async Task QueryDelayBuildAction_ShouldNotRestoreOnBuild_ProjectUpToDateM var settings = Mock.Of(); var restoreWorker = Mock.Of(); var buildManager = Mock.Of(); + var restoreChecker = Mock.Of(); var buildAction = (uint)VSSOLNBUILDUPDATEFLAGS.SBF_OPERATION_BUILD + (uint)VSSOLNBUILDUPDATEFLAGS3.SBF_FLAGS_UPTODATE_CHECK; @@ -91,7 +96,7 @@ public async Task QueryDelayBuildAction_ShouldNotRestoreOnBuild_ProjectUpToDateM .Returns(() => new VirtualSettingSection("packageRestore", new AddItem("automatic", bool.TrueString))); - using (var handler = new SolutionRestoreBuildHandler(settings, restoreWorker, buildManager)) + using (var handler = new SolutionRestoreBuildHandler(settings, restoreWorker, buildManager, restoreChecker)) { await _jtf.SwitchToMainThreadAsync(); @@ -110,6 +115,7 @@ public async Task QueryDelayBuildAction_ShouldRestoreOnBuild() var settings = Mock.Of(); var restoreWorker = Mock.Of(); var buildManager = Mock.Of(); + var restoreChecker = Mock.Of(); var buildAction = (uint)VSSOLNBUILDUPDATEFLAGS.SBF_OPERATION_BUILD; @@ -124,7 +130,7 @@ public async Task QueryDelayBuildAction_ShouldRestoreOnBuild() .Setup(x => x.RestoreAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(true); - using (var handler = new SolutionRestoreBuildHandler(settings, restoreWorker, buildManager)) + using (var handler = new SolutionRestoreBuildHandler(settings, restoreWorker, buildManager, restoreChecker)) { await _jtf.SwitchToMainThreadAsync(); diff --git a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/SolutionUpToDateCheckerTests.cs b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/SolutionUpToDateCheckerTests.cs new file mode 100644 index 00000000000..a63ccbd3e7e --- /dev/null +++ b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/SolutionUpToDateCheckerTests.cs @@ -0,0 +1,726 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Collections.ObjectModel; +using System.IO; +using System.Linq; +using FluentAssertions; +using NuGet.Commands; +using NuGet.Commands.Test; +using NuGet.Common; +using NuGet.ProjectModel; +using NuGet.Test.Utility; +using Xunit; + +namespace NuGet.SolutionRestoreManager.Test +{ + public class SolutionUpToDateCheckerTests + { + [Fact] + public void GetOutputFilePaths_GetOutputFilePaths_AllIntermediateOutputsGoToTheOutputFolder() + { + var packageSpec = GetPackageSpec("A"); + packageSpec.RestoreMetadata.RestoreLockProperties = new RestoreLockProperties(restorePackagesWithLockFile: "true", nuGetLockFilePath: null, restoreLockedMode: true); + + SolutionUpToDateChecker.GetOutputFilePaths(packageSpec, out string assetsFilePath, out string cacheFilePath, out string targetsFilePath, out string propsFilePath, out string lockFilePath); + + var expectedIntermediateFolder = packageSpec.RestoreMetadata.OutputPath; + var expectedLockFileFolder = Path.GetDirectoryName(packageSpec.RestoreMetadata.ProjectPath); + + Path.GetDirectoryName(assetsFilePath).Should().Be(expectedIntermediateFolder); + Path.GetDirectoryName(cacheFilePath).Should().Be(expectedIntermediateFolder); + Path.GetDirectoryName(targetsFilePath).Should().Be(expectedIntermediateFolder); + Path.GetDirectoryName(propsFilePath).Should().Be(expectedIntermediateFolder); + Path.GetDirectoryName(lockFilePath).Should().Be(expectedLockFileFolder); + } + + [Fact] + public void GetOutputFilePaths_WorksForProjectJson() + { + var packageSpec = GetProjectJsonPackageSpec("A"); + SolutionUpToDateChecker.GetOutputFilePaths(packageSpec, out string assetsFilePath, out string cacheFilePath, out string targetsFilePath, out string propsFilePath, out string lockFilePath); + + var expectedIntermediateFolder = packageSpec.RestoreMetadata.OutputPath; + var expectedAssetsFolder = Path.GetDirectoryName(packageSpec.FilePath); + + Path.GetDirectoryName(assetsFilePath).Should().Be(expectedAssetsFolder); + Path.GetDirectoryName(cacheFilePath).Should().Be(expectedIntermediateFolder); + Path.GetDirectoryName(targetsFilePath).Should().Be(expectedIntermediateFolder); + Path.GetDirectoryName(propsFilePath).Should().Be(expectedIntermediateFolder); + lockFilePath.Should().BeNull(); + } + + // A => B => C + [Fact] + public void GetParents_WhenDirtySpecsListIsEmpty_ReturnsEmpty() + { + var projectA = GetPackageSpec("A"); + var projectB = GetPackageSpec("B"); + var projectC = GetPackageSpec("C"); + + // A => B => C + projectA = projectA.WithTestProjectReference(projectB); + projectB = projectB.WithTestProjectReference(projectC); + + var dgSpec = new DependencyGraphSpec(); + dgSpec.AddProject(projectA); + dgSpec.AddRestore(projectA.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectB); + dgSpec.AddRestore(projectB.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectC); + dgSpec.AddRestore(projectC.RestoreMetadata.ProjectUniqueName); + + Assert.Empty(SolutionUpToDateChecker.GetParents(new List(), dgSpec)); + } + + // A => B => D + // => C => E + // D & E are dirty + [Fact] + public void GetParents_WhenEveryLeafNodeIsDirty_ReturnsAllProjectsInTheSolution() + { + var projectA = GetPackageSpec("A"); + var projectB = GetPackageSpec("B"); + var projectC = GetPackageSpec("C"); + var projectD = GetPackageSpec("D"); + var projectE = GetPackageSpec("E"); + + // A => B & C + projectA = projectA.WithTestProjectReference(projectB).WithTestProjectReference(projectC); + // B => D + projectB = projectB.WithTestProjectReference(projectD); + // C => E + projectC = projectC.WithTestProjectReference(projectE); + + var dgSpec = new DependencyGraphSpec(); + dgSpec.AddProject(projectA); + dgSpec.AddRestore(projectA.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectB); + dgSpec.AddRestore(projectB.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectC); + dgSpec.AddRestore(projectC.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectD); + dgSpec.AddRestore(projectD.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectE); + dgSpec.AddRestore(projectE.RestoreMetadata.ProjectUniqueName); + + var expected = GetUniqueNames(projectA, projectB, projectC, projectD, projectE); + var actual = SolutionUpToDateChecker.GetParents(GetUniqueNames(projectD, projectE), dgSpec); + + actual.Should().BeEquivalentTo(expected); + } + + // A => B => D + // => C => E + // F => D + [Fact] + public void GetParents_WhenOnlyRootSpecsAreDirty_ReturnsOnlyTheSameDirtyProjects() + { + var projectA = GetPackageSpec("A"); + var projectB = GetPackageSpec("B"); + var projectC = GetPackageSpec("C"); + var projectD = GetPackageSpec("D"); + var projectE = GetPackageSpec("E"); + var projectF = GetPackageSpec("F"); + + // A => B & C + projectA = projectA.WithTestProjectReference(projectB).WithTestProjectReference(projectC); + // B => D + projectB = projectB.WithTestProjectReference(projectD); + // C => E + projectC = projectC.WithTestProjectReference(projectE); + // F => D + projectF = projectF.WithTestProjectReference(projectD); + + var dgSpec = new DependencyGraphSpec(); + dgSpec.AddProject(projectA); + dgSpec.AddRestore(projectA.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectB); + dgSpec.AddRestore(projectB.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectC); + dgSpec.AddRestore(projectC.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectD); + dgSpec.AddRestore(projectD.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectE); + dgSpec.AddRestore(projectE.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectF); + dgSpec.AddRestore(projectF.RestoreMetadata.ProjectUniqueName); + + var expected = GetUniqueNames(projectA, projectF); + var actual = SolutionUpToDateChecker.GetParents(GetUniqueNames(projectA, projectF), dgSpec); + + actual.Should().BeEquivalentTo(expected); + } + + // A => B => D => F + // => C => E + // G => D => F + // H => C => E + // => I => F + // => J => K => L + // M => L + // E & L are dirty + [Fact] + public void GetParents_WithMultiLevelGraph_WhenALeafIsDirty_ReturnsProjectsFromEveryLevelAsDirty() + { + var projectA = GetPackageSpec("A"); + var projectB = GetPackageSpec("B"); + var projectC = GetPackageSpec("C"); + var projectD = GetPackageSpec("D"); + var projectE = GetPackageSpec("E"); + var projectF = GetPackageSpec("F"); + var projectG = GetPackageSpec("G"); + var projectH = GetPackageSpec("H"); + var projectI = GetPackageSpec("I"); + var projectJ = GetPackageSpec("J"); + var projectK = GetPackageSpec("K"); + var projectL = GetPackageSpec("L"); + var projectM = GetPackageSpec("M"); + + // A => B & C + projectA = projectA.WithTestProjectReference(projectB).WithTestProjectReference(projectC); + // B => D + projectB = projectB.WithTestProjectReference(projectD); + // D => F + projectD = projectD.WithTestProjectReference(projectF); + // C => E + projectC = projectC.WithTestProjectReference(projectE); + // G => D + projectG = projectG.WithTestProjectReference(projectD); + // H => C + projectH = projectH.WithTestProjectReference(projectC); + // I => F + projectI = projectI.WithTestProjectReference(projectF); + // H => I + projectH = projectH.WithTestProjectReference(projectI); + // K => L + projectK = projectK.WithTestProjectReference(projectL); + // J => K + projectJ = projectJ.WithTestProjectReference(projectK); + // H => J + projectH = projectH.WithTestProjectReference(projectJ); + // M => L + projectM = projectM.WithTestProjectReference(projectL); + + + var dgSpec = new DependencyGraphSpec(); + dgSpec.AddProject(projectA); + dgSpec.AddRestore(projectA.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectB); + dgSpec.AddRestore(projectB.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectC); + dgSpec.AddRestore(projectC.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectD); + dgSpec.AddRestore(projectD.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectE); + dgSpec.AddRestore(projectE.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectF); + dgSpec.AddRestore(projectF.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectG); + dgSpec.AddRestore(projectG.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectH); + dgSpec.AddRestore(projectH.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectI); + dgSpec.AddRestore(projectI.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectJ); + dgSpec.AddRestore(projectJ.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectK); + dgSpec.AddRestore(projectK.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectL); + dgSpec.AddRestore(projectL.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectM); + dgSpec.AddRestore(projectM.RestoreMetadata.ProjectUniqueName); + + var expected = GetUniqueNames(projectA, projectC, projectE, projectH, projectJ, projectK, projectL, projectM); + var actual = SolutionUpToDateChecker.GetParents(GetUniqueNames(projectE, projectL), dgSpec); + + actual.Should().BeEquivalentTo(expected); + } + + // This behavior is off, but it is consistent with how no-op is handled right now in the RestoreCommand. + // A change in *any* child affects the parents, even if the child is not PR. + // A => B => C + // => D + // => E => F + // F is not a standard project. + [Fact] + public void PerformUpToDateCheck_WhenNonBuildIntegratedProjectIsAParentOfADirtySpec_ReturnsAListWithoutNonBuildIntegratedProjects() + { + var projectA = GetPackageSpec("A"); + var projectB = GetPackageSpec("B"); + var projectC = GetPackageSpec("C"); + var projectD = GetPackageSpec("D"); + var projectE = GetPackageSpec("E"); + var projectF = GetUnknownPackageSpec("F"); + + // A => B & D & E + projectA = projectA.WithTestProjectReference(projectB).WithTestProjectReference(projectD).WithTestProjectReference(projectE); + // B => C + projectB = projectB.WithTestProjectReference(projectC); + // E => F + projectE = projectE.WithTestProjectReference(projectF); + + DependencyGraphSpec dgSpec = ProjectJsonTestHelpers.GetDGSpecFromPackageSpecs(projectA, projectB, projectC, projectD, projectE, projectF); + + var checker = new SolutionUpToDateChecker(); + + var actual = checker.PerformUpToDateCheck(dgSpec); + var expected = GetUniqueNames(projectA, projectB, projectC, projectD, projectE); + actual.Should().BeEquivalentTo(expected); + + // Now we run + var results = RunRestore(failedProjects: new HashSet(), projectA, projectB, projectC, projectD, projectE); + checker.ReportStatus(results); + + // Prepare the new DG Spec: + // Make projectE dirty by setting a random value that's usually not there :) + projectF = projectF.Clone(); + projectF.RestoreMetadata.PackagesPath = @"C:\"; + dgSpec = ProjectJsonTestHelpers.GetDGSpecFromPackageSpecs(projectA, projectB, projectC, projectD, projectE, projectF); + + // Act & Assert. + expected = GetUniqueNames(projectA, projectE); + actual = checker.PerformUpToDateCheck(dgSpec); + actual.Should().BeEquivalentTo(expected); + } + + [Fact] + public void PerformUpToDateCheck_WithNoChanges_ReturnsEmpty() + { + using (var testDirectory = TestDirectory.Create()) + { + var projectA = GetPackageSpec("A", testDirectory.Path); + var projectB = GetPackageSpec("B", testDirectory.Path); + var projectC = GetPackageSpec("C", testDirectory.Path); + + // A => B + projectA = projectA.WithTestProjectReference(projectB); + // B => C + projectB = projectB.WithTestProjectReference(projectC); + + var dgSpec = new DependencyGraphSpec(); + dgSpec.AddProject(projectA); + dgSpec.AddRestore(projectA.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectB); + dgSpec.AddRestore(projectB.RestoreMetadata.ProjectUniqueName); + dgSpec.AddProject(projectC); + dgSpec.AddRestore(projectC.RestoreMetadata.ProjectUniqueName); + + var checker = new SolutionUpToDateChecker(); + + // Preconditions, run 1st check + var actual = checker.PerformUpToDateCheck(dgSpec); + var expected = GetUniqueNames(projectA, projectB, projectC); + actual.Should().BeEquivalentTo(expected); + var results = RunRestore(failedProjects: new HashSet(), projectA, projectB, projectC); + checker.ReportStatus(results); + + // Act & Asset. + actual = checker.PerformUpToDateCheck(dgSpec); + actual.Should().BeEmpty(); + } + } + + // A -> B -> C + // D + // B is dirty when a reference B -> D gets added, A & B are returned. + [Fact] + public void PerformUpToDateCheck_LeafProjectChanges_ReturnsAllItsParents() + { + using (var testDirectory = TestDirectory.Create()) + { + var projectA = GetPackageSpec("A", testDirectory.Path); + var projectB = GetPackageSpec("B", testDirectory.Path); + var projectC = GetPackageSpec("C", testDirectory.Path); + var projectD = GetPackageSpec("D", testDirectory.Path); + + // A => B + projectA = projectA.WithTestProjectReference(projectB); + // B => C + projectB = projectB.WithTestProjectReference(projectC); + DependencyGraphSpec dgSpec = ProjectJsonTestHelpers.GetDGSpecFromPackageSpecs(projectA, projectB, projectC, projectD); + + var checker = new SolutionUpToDateChecker(); + + // Preconditions, run 1st check + var actual = checker.PerformUpToDateCheck(dgSpec); + var expected = GetUniqueNames(projectA, projectB, projectC, projectD); + actual.Should().BeEquivalentTo(expected); + var results = RunRestore(failedProjects: new HashSet(), projectA, projectB, projectC, projectD); + checker.ReportStatus(results); + + // Set-up, B => D + projectB = projectB.WithTestProjectReference(projectD); + dgSpec = ProjectJsonTestHelpers.GetDGSpecFromPackageSpecs(projectA, projectB, projectC, projectD); + + // Act & Assert + actual = checker.PerformUpToDateCheck(dgSpec); + expected = GetUniqueNames(projectA, projectB); + actual.Should().BeEquivalentTo(expected); + } + } + + // A => B => C + // Delete the outputs of C. Forces only that project to restore. + [Fact] + public void PerformUpToDateCheck_WhenALeafProjectHasDirtyOutputs_ReturnsOnlyThatProject() + { + using (var testDirectory = TestDirectory.Create()) + { + var projectA = GetPackageSpec("A", testDirectory.Path); + var projectB = GetPackageSpec("B", testDirectory.Path); + var projectC = GetPackageSpec("C", testDirectory.Path); + + // A => B + projectA = projectA.WithTestProjectReference(projectB); + // B => C + projectB = projectB.WithTestProjectReference(projectC); + DependencyGraphSpec dgSpec = ProjectJsonTestHelpers.GetDGSpecFromPackageSpecs(projectA, projectB, projectC); + + var checker = new SolutionUpToDateChecker(); + + // Preconditions, run 1st check + var actual = checker.PerformUpToDateCheck(dgSpec); + var expected = GetUniqueNames(projectA, projectB, projectC); + actual.Should().BeEquivalentTo(expected); + var results = RunRestore(failedProjects: new HashSet(), projectA, projectB, projectC); + checker.ReportStatus(results); + + // Set-up, delete C's outputs + SolutionUpToDateChecker.GetOutputFilePaths(projectC, out string assetsFilePath, out string _, out string _, out string _, out string _); + File.Delete(assetsFilePath); + + // Act & Assert + actual = checker.PerformUpToDateCheck(dgSpec); + expected = GetUniqueNames(projectC); + actual.Should().BeEquivalentTo(expected); + } + } + + // A => B => C + // Delete the outputs of C. Forces only that project to restore. + [Fact] + public void PerformUpToDateCheck_WhenALeafProjectHasNoCacheFile_ReturnsOnlyThatProject() + { + using (var testDirectory = TestDirectory.Create()) + { + var projectA = GetPackageSpec("A", testDirectory.Path); + var projectB = GetPackageSpec("B", testDirectory.Path); + var projectC = GetPackageSpec("C", testDirectory.Path); + + // A => B + projectA = projectA.WithTestProjectReference(projectB); + // B => C + projectB = projectB.WithTestProjectReference(projectC); + DependencyGraphSpec dgSpec = ProjectJsonTestHelpers.GetDGSpecFromPackageSpecs(projectA, projectB, projectC); + + var checker = new SolutionUpToDateChecker(); + + // Preconditions, run 1st check + var actual = checker.PerformUpToDateCheck(dgSpec); + var expected = GetUniqueNames(projectA, projectB, projectC); + actual.Should().BeEquivalentTo(expected); + var results = RunRestore(failedProjects: new HashSet(), projectA, projectB, projectC); + checker.ReportStatus(results); + + // Set-up, delete C's outputs + SolutionUpToDateChecker.GetOutputFilePaths(projectC, out string _, out string cacheFilePath, out string _, out string _, out string _); + File.Delete(cacheFilePath); + + // Act & Assert + actual = checker.PerformUpToDateCheck(dgSpec); + expected = GetUniqueNames(projectC); + actual.Should().BeEquivalentTo(expected); + } + } + + // A => B + // => C + // D + // + // C & D are project.json, C is dirty, returns A & C. + [Fact] + public void PerformUpToDateCheck_WithProjectJsonProjects_ReturnsOnlyDirtyProjects() + { + using (var testDirectory = TestDirectory.Create()) + { + var projectA = GetPackageSpec("A", testDirectory.Path); + var projectB = GetPackageSpec("B", testDirectory.Path); + var projectC = GetProjectJsonPackageSpec("C", testDirectory.Path); + var projectD = GetProjectJsonPackageSpec("D", testDirectory.Path); + + // A => B & C + projectA = projectA.WithTestProjectReference(projectB).WithTestProjectReference(projectC); + DependencyGraphSpec dgSpec = ProjectJsonTestHelpers.GetDGSpecFromPackageSpecs(projectA, projectB, projectC, projectD); + + var checker = new SolutionUpToDateChecker(); + + // Preconditions, run 1st check + var actual = checker.PerformUpToDateCheck(dgSpec); + var expected = GetUniqueNames(projectA, projectB, projectC, projectD); + actual.Should().BeEquivalentTo(expected); + var results = RunRestore(failedProjects: new HashSet(), projectA, projectB, projectC, projectD); + checker.ReportStatus(results); + + // Set-up, make C dirty. + projectC = projectC.Clone(); + projectC.RestoreMetadata.ConfigFilePaths = new List() { "newFeed" }; + dgSpec = ProjectJsonTestHelpers.GetDGSpecFromPackageSpecs(projectA, projectB, projectC, projectD); + + // Act & Assert + actual = checker.PerformUpToDateCheck(dgSpec); + expected = GetUniqueNames(projectA, projectC); + actual.Should().BeEquivalentTo(expected); + } + } + + [Fact] + public void PerformUpToDateCheck_WithFailedPastRestore_ReturnsADirtyProject() + { + using (var testDirectory = TestDirectory.Create()) + { + var projectA = GetPackageSpec("A", testDirectory.Path); + var projectB = GetPackageSpec("B", testDirectory.Path); + var projectC = GetPackageSpec("C", testDirectory.Path); + + // A => B + projectA = projectA.WithTestProjectReference(projectB); + // B => C + projectB = projectB.WithTestProjectReference(projectC); + DependencyGraphSpec dgSpec = ProjectJsonTestHelpers.GetDGSpecFromPackageSpecs(projectA, projectB, projectC); + + var checker = new SolutionUpToDateChecker(); + + // Preconditions, run 1st check + var actual = checker.PerformUpToDateCheck(dgSpec); + new List() { projectA.RestoreMetadata.ProjectUniqueName, projectB.RestoreMetadata.ProjectUniqueName, projectC.RestoreMetadata.ProjectUniqueName }.Should().BeEquivalentTo(actual); + + // Set-up, ensure the last status for projectC is a failure. + var results = RunRestore(failedProjects: new HashSet() { projectC.RestoreMetadata.ProjectUniqueName }, projectA, projectB, projectC); + checker.ReportStatus(results); + + // Act & Assert + actual = checker.PerformUpToDateCheck(dgSpec); + var expected = new List() { projectC.RestoreMetadata.ProjectUniqueName }; + actual.Should().BeEquivalentTo(expected); + } + } + + // A => B => C + // Delete the outputs of C. Forces only that project to restore. + [Fact] + public void PerformUpToDateCheck_WhenALeafProjectHasNoGlobalPackagesFolder_ReturnsOnlyThatProject() + { + using (var testDirectory = TestDirectory.Create()) + { + var projectA = GetPackageSpec("A", testDirectory.Path); + var projectB = GetPackageSpec("B", testDirectory.Path); + var projectC = GetPackageSpec("C", testDirectory.Path); + projectC.RestoreMetadata.PackagesPath = Path.Combine(testDirectory.Path, "gpf"); + // A => B + projectA = projectA.WithTestProjectReference(projectB); + // B => C + projectB = projectB.WithTestProjectReference(projectC); + DependencyGraphSpec dgSpec = ProjectJsonTestHelpers.GetDGSpecFromPackageSpecs(projectA, projectB, projectC); + + var checker = new SolutionUpToDateChecker(); + + // Preconditions, run 1st check + var actual = checker.PerformUpToDateCheck(dgSpec); + var expected = GetUniqueNames(projectA, projectB, projectC); + actual.Should().BeEquivalentTo(expected); + var results = RunRestore(failedProjects: new HashSet(), projectA, projectB, projectC); + checker.ReportStatus(results); + + // Set-up, delete C's outputs + Directory.Delete(projectC.RestoreMetadata.PackagesPath, recursive: true); + + // Act & Assert + actual = checker.PerformUpToDateCheck(dgSpec); + expected = GetUniqueNames(projectC); + actual.Should().BeEquivalentTo(expected); + } + } + + // A => B => C + // => D + // E + // D is gonna be marked dirty when a project reference to E is added + // The 2nd check should return D & A. + // The 3rd check should return nothing. + [Fact] + public void ReportStatus_WhenPartialResultsAreAvailable_OldStatusIsRetained() + { + using (var testDirectory = TestDirectory.Create()) + { + var projectA = GetPackageSpec("A", testDirectory.Path); + var projectB = GetPackageSpec("B", testDirectory.Path); + var projectC = GetPackageSpec("C", testDirectory.Path); + var projectD = GetPackageSpec("D", testDirectory.Path); + var projectE = GetPackageSpec("E", testDirectory.Path); + + // A => B + projectA = projectA.WithTestProjectReference(projectB); + // B => C + projectB = projectB.WithTestProjectReference(projectC); + // A => D + projectA = projectA.WithTestProjectReference(projectD); + + DependencyGraphSpec dgSpec = ProjectJsonTestHelpers.GetDGSpecFromPackageSpecs(projectA, projectB, projectC, projectD, projectE); + + var checker = new SolutionUpToDateChecker(); + + // Preconditions, run 1st check + var actual = checker.PerformUpToDateCheck(dgSpec); + var expected = GetUniqueNames(projectA, projectB, projectC, projectD, projectE); + actual.Should().BeEquivalentTo(expected); + var results = RunRestore(failedProjects: new HashSet(), projectA, projectB, projectC, projectD, projectE); + checker.ReportStatus(results); + + // D => E + projectD = projectD.WithTestProjectReference(projectE); + + // Set-up dg spec + dgSpec = ProjectJsonTestHelpers.GetDGSpecFromPackageSpecs(projectA, projectB, projectC, projectD, projectE); + // 2nd check + actual = checker.PerformUpToDateCheck(dgSpec); + expected = GetUniqueNames(projectA, projectD); + actual.Should().BeEquivalentTo(expected); + results = RunRestore(failedProjects: new HashSet(), projectA, projectD); + checker.ReportStatus(results); + + // Finally, last check. Run for a 3rd time. Everything should be up to date + actual = checker.PerformUpToDateCheck(dgSpec); + expected = GetUniqueNames(); + actual.Should().BeEquivalentTo(expected); + } + } + + private List GetUniqueNames(params PackageSpec[] packageSpecs) + { + var projects = new List(); + foreach (var package in packageSpecs) + { + projects.Add(package.RestoreMetadata.ProjectUniqueName); + } + return projects; + } + + private static PackageSpec GetUnknownPackageSpec(string projectName) + { + var packageSpec = new PackageSpec(); + var projectPath = Path.Combine(@"C:\", projectName, $"{projectName}.csproj"); + packageSpec.RestoreMetadata = new ProjectRestoreMetadata() + { + ProjectUniqueName = projectPath, + ProjectName = projectPath + }; + packageSpec.FilePath = projectPath; + + return packageSpec; + } + + private static PackageSpec GetPackageSpec(string projectName, string rootPath = @"C:\") + { + const string referenceSpec = @" + { + ""frameworks"": { + ""net5.0"": { + ""dependencies"": { + } + } + } + }"; + return JsonPackageSpecReader.GetPackageSpec(referenceSpec, projectName, Path.Combine(rootPath, projectName, projectName)).WithTestRestoreMetadata(); + } + + private static PackageSpec GetProjectJsonPackageSpec(string projectName, string rootPath = @"C:\") + { + const string referenceSpec = @" + { + ""frameworks"": { + ""net5.0"": { + ""dependencies"": { + } + } + } + }"; + var packageSpec = JsonPackageSpecReader.GetPackageSpec(referenceSpec, projectName, Path.Combine(rootPath, projectName, projectName)); + + var packageSpecFile = new FileInfo(packageSpec.FilePath); + var projectDir = packageSpecFile.Directory.FullName; + var projectPath = Path.Combine(projectDir, packageSpec.Name + ".csproj"); + + packageSpec.RestoreMetadata = new ProjectRestoreMetadata(); + packageSpec.RestoreMetadata.CrossTargeting = packageSpec.TargetFrameworks.Count > 0; + packageSpec.RestoreMetadata.OriginalTargetFrameworks = packageSpec.TargetFrameworks.Select(e => e.FrameworkName.GetShortFolderName()).ToList(); + packageSpec.RestoreMetadata.OutputPath = projectDir; + packageSpec.RestoreMetadata.ProjectStyle = ProjectStyle.ProjectJson; + packageSpec.RestoreMetadata.ProjectName = packageSpec.Name; + packageSpec.RestoreMetadata.ProjectUniqueName = projectPath; + packageSpec.RestoreMetadata.ProjectPath = projectPath; + packageSpec.RestoreMetadata.ConfigFilePaths = new List(); + packageSpec.RestoreMetadata.CentralPackageVersionsEnabled = false; + + foreach (var framework in packageSpec.TargetFrameworks.Select(e => e.FrameworkName)) + { + packageSpec.RestoreMetadata.TargetFrameworks.Add(new ProjectRestoreMetadataFrameworkInfo(framework)); + } + + return packageSpec; + } + + private static IReadOnlyList RunRestore(HashSet failedProjects, params PackageSpec[] packageSpecs) + { + foreach (var spec in packageSpecs) + { + CreateDummyOutputFiles(spec); + } + + return CreateRestoreSummaries(failedProjects, packageSpecs).ToImmutableList(); + } + + private static IEnumerable CreateRestoreSummaries(HashSet failedProjects, params PackageSpec[] packageSpecs) + { + foreach (var spec in packageSpecs) + { + var status = !failedProjects.Contains(spec.RestoreMetadata.ProjectUniqueName); + yield return CreateRestoreSummary(spec, success: status); + } + } + + private static RestoreSummary CreateRestoreSummary(PackageSpec spec, bool success) + { + return new RestoreSummary( + success: success, + inputPath: spec.RestoreMetadata.ProjectUniqueName, + configFiles: new ReadOnlyCollection(new List()), + feedsUsed: new ReadOnlyCollection(new List()), + installCount: 0, + errors: new ReadOnlyCollection(new List())); + } + + internal static void CreateDummyOutputFiles(PackageSpec packageSpec) + { + SolutionUpToDateChecker.GetOutputFilePaths(packageSpec, out string assetsFilePath, out string cacheFilePath, out string targetsFilePath, out string propsFilePath, out string lockFilePath); + var globalPackagesFolderDummyFilePath = packageSpec.RestoreMetadata.PackagesPath != null ? + Path.Combine(packageSpec.RestoreMetadata.PackagesPath, "dummyFile.txt") : + null; + CreateFile(assetsFilePath, cacheFilePath, targetsFilePath, propsFilePath, lockFilePath, globalPackagesFolderDummyFilePath); + } + + private static void CreateFile(params string[] paths) + { + foreach (var path in paths) + { + if (path != null) + { + Directory.CreateDirectory(Path.GetDirectoryName(path)); + File.Create(path).Dispose(); + } + } + } + } +} diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/BuildAssetsUtilsTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/BuildAssetsUtilsTests.cs index 23109506093..137840b55b6 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/BuildAssetsUtilsTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/BuildAssetsUtilsTests.cs @@ -9,7 +9,6 @@ using System.Xml.Linq; using NuGet.Common; using NuGet.Configuration; -using NuGet.Frameworks; using NuGet.LibraryModel; using NuGet.Packaging.Core; using NuGet.ProjectModel; @@ -622,7 +621,6 @@ public void BuildAssetsUtils_VerifyPositionAndSortOrder() public async Task BuildAssetsUtils_GeneratePathProperty() { using (var pathContext = new SimpleTestPathContext()) - using (var randomProjectDirectory = TestDirectory.Create()) { // Arrange var identity = new PackageIdentity("packagea", NuGetVersion.Parse("1.0.0")); @@ -638,17 +636,20 @@ public async Task BuildAssetsUtils_GeneratePathProperty() var logger = new TestLogger(); - var spec = ToolRestoreUtility.GetSpec( - Path.Combine(pathContext.SolutionRoot, "tool", "fake.csproj"), - "a", - VersionRange.Parse("1.0.0"), - NuGetFramework.Parse("netcoreapp1.0"), - pathContext.UserPackagesFolder, - new List() { pathContext.FallbackFolder }, - new List() { new PackageSource(pathContext.PackageSource) }, - projectWideWarningProperties: null); + const string referenceSpec = @" + { + ""frameworks"": { + ""netcoreapp1.0"": { + ""dependencies"": { + } + } + } + }"; + var projectName = "a"; + var rootProjectsPath = pathContext.WorkingDirectory; + var projectDirectory = Path.Combine(rootProjectsPath, projectName); - spec.RestoreMetadata.ProjectStyle = ProjectStyle.PackageReference; + var spec = JsonPackageSpecReader.GetPackageSpec(referenceSpec, projectName, Path.Combine(projectDirectory, projectName)).WithTestRestoreMetadata(); spec.Dependencies.Add(new LibraryDependency { @@ -704,7 +705,7 @@ public async Task BuildAssetsUtils_GeneratePathProperty() ProjectStyle = spec.RestoreMetadata.ProjectStyle }; - var assetsFilePath = Path.Combine(randomProjectDirectory, "obj", "project.assets.json"); + var assetsFilePath = Path.Combine(projectDirectory, "obj", "project.assets.json"); // Act var outputFiles = BuildAssetsUtils.GetMSBuildOutputFiles(spec, lockFile, targetGraphs, repositories, restoreRequest, assetsFilePath, true, logger); @@ -732,7 +733,6 @@ public async Task BuildAssetsUtils_GeneratePathProperty() public async Task BuildAssetsUtils_GeneratePathPropertyForTools(bool hasTools) { using (var pathContext = new SimpleTestPathContext()) - using (var randomProjectDirectory = TestDirectory.Create()) { // Arrange var identity = new PackageIdentity("packagea", NuGetVersion.Parse("1.0.0")); @@ -748,17 +748,20 @@ public async Task BuildAssetsUtils_GeneratePathPropertyForTools(bool hasTools) var logger = new TestLogger(); - var spec = ToolRestoreUtility.GetSpec( - Path.Combine(pathContext.SolutionRoot, "tool", "fake.csproj"), - "a", - VersionRange.Parse("1.0.0"), - NuGetFramework.Parse("netcoreapp1.0"), - pathContext.UserPackagesFolder, - new List() { pathContext.FallbackFolder }, - new List() { new PackageSource(pathContext.PackageSource) }, - projectWideWarningProperties: null); + const string referenceSpec = @" + { + ""frameworks"": { + ""netcoreapp1.0"": { + ""dependencies"": { + } + } + } + }"; + var projectName = "a"; + var rootProjectsPath = pathContext.WorkingDirectory; + var projectDirectory = Path.Combine(rootProjectsPath, projectName); - spec.RestoreMetadata.ProjectStyle = ProjectStyle.PackageReference; + var spec = JsonPackageSpecReader.GetPackageSpec(referenceSpec, projectName, Path.Combine(projectDirectory, projectName)).WithTestRestoreMetadata(); spec.Dependencies.Add(new LibraryDependency { @@ -814,7 +817,7 @@ public async Task BuildAssetsUtils_GeneratePathPropertyForTools(bool hasTools) ProjectStyle = spec.RestoreMetadata.ProjectStyle }; - var assetsFilePath = Path.Combine(randomProjectDirectory, "obj", "project.assets.json"); + var assetsFilePath = Path.Combine(projectDirectory, "obj", "project.assets.json"); // Act var outputFiles = BuildAssetsUtils.GetMSBuildOutputFiles(spec, lockFile, targetGraphs, repositories, restoreRequest, assetsFilePath, true, logger); diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreRunnerTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreRunnerTests.cs index 61102b782bb..909034e548e 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreRunnerTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreRunnerTests.cs @@ -64,7 +64,7 @@ public async Task RestoreRunner_BasicRestoreAsync() spec1.RestoreMetadata.PackagesPath = packagesDir.FullName; var dgSpec = new DependencyGraphSpec(); dgSpec.AddProject(spec1); - dgSpec.AddRestore(projectName); + dgSpec.AddRestore(spec1.RestoreMetadata.ProjectUniqueName); var logger = new TestLogger(); var lockPath = Path.Combine(project1.FullName, "project.assets.json"); @@ -144,7 +144,7 @@ public async Task RestoreRunner_BasicRestore_VerifyFailureWritesFilesAsync() spec1.RestoreMetadata.PackagesPath = packagesDir.FullName; var dgFile = new DependencyGraphSpec(); dgFile.AddProject(spec1); - dgFile.AddRestore("project1"); + dgFile.AddRestore(spec1.RestoreMetadata.ProjectUniqueName); var logger = new TestLogger(); var lockPath = Path.Combine(project1.FullName, "project.assets.json"); @@ -345,7 +345,7 @@ public async Task RestoreRunner_BasicRestoreWithConfigFileAsync() spec1.RestoreMetadata.PackagesPath = packagesDir.FullName; dgFile.AddProject(spec1); - dgFile.AddRestore("project1"); + dgFile.AddRestore(spec1.RestoreMetadata.ProjectUniqueName); var logger = new TestLogger(); var lockPath = Path.Combine(project1.FullName, "project.assets.json"); @@ -485,7 +485,7 @@ public async Task RestoreRunner_RestoreWithExternalFileAsync() foreach (var spec in specs) { - dgFile.AddRestore(spec.RestoreMetadata.ProjectName); + dgFile.AddRestore(spec.RestoreMetadata.ProjectUniqueName); dgFile.AddProject(spec); } @@ -634,7 +634,7 @@ public async Task RestoreRunner_RestoreWithExternalFile_NetCoreOutputAsync() foreach (var spec in specs) { - dgFile.AddRestore(spec.RestoreMetadata.ProjectName); + dgFile.AddRestore(spec.RestoreMetadata.ProjectUniqueName); dgFile.AddProject(spec); } @@ -719,7 +719,7 @@ public async Task RestoreRunner_RestoreWithRuntimeAsync() spec1.RestoreMetadata.PackagesPath = packagesDir.FullName; var dgSpec = new DependencyGraphSpec(); dgSpec.AddProject(spec1); - dgSpec.AddRestore("project1"); + dgSpec.AddRestore(spec1.RestoreMetadata.ProjectUniqueName); var logger = new TestLogger(); var lockPath1 = Path.Combine(project1.FullName, "project.assets.json"); @@ -795,7 +795,7 @@ public async Task RestoreRunner_BasicPackageDownloadRestoreAsync() // set up the dg spec. var dgFile = new DependencyGraphSpec(); dgFile.AddProject(projectSpec); - dgFile.AddRestore(projectSpec.Name); + dgFile.AddRestore(projectSpec.RestoreMetadata.ProjectUniqueName); // set up the packages var packageX = new SimpleTestPackageContext() { @@ -884,7 +884,7 @@ public async Task RestoreRunner_MultiplePackageDownloadRestoreAsync() // set up the dg spec. var dgFile = new DependencyGraphSpec(); dgFile.AddProject(projectSpec); - dgFile.AddRestore(projectSpec.Name); + dgFile.AddRestore(projectSpec.RestoreMetadata.ProjectUniqueName); // set up the packages await SimpleTestPackageUtility.CreateFullPackageAsync(packageSource.FullName, new SimpleTestPackageContext() @@ -983,7 +983,7 @@ public async Task RestoreRunner_PackageDownloadUnresolvedAsync() // set up the dg spec. var dgFile = new DependencyGraphSpec(); dgFile.AddProject(projectSpec); - dgFile.AddRestore(projectSpec.Name); + dgFile.AddRestore(projectSpec.RestoreMetadata.ProjectUniqueName); // set up the packages var packageX = new SimpleTestPackageContext() { @@ -1075,7 +1075,7 @@ public async Task RestoreRunner_PackageReferenceAndPackageDownloadBothLogErrors( // set up the dg spec. var dgFile = new DependencyGraphSpec(); dgFile.AddProject(projectSpec); - dgFile.AddRestore(projectSpec.Name); + dgFile.AddRestore(projectSpec.RestoreMetadata.ProjectUniqueName); // set up the packages var packageX = new SimpleTestPackageContext() { @@ -1181,7 +1181,7 @@ public async Task RestoreRunner_MultiTfmPackageDownloadRestoreAsync() // set up the dg spec. var dgFile = new DependencyGraphSpec(); dgFile.AddProject(projectSpec); - dgFile.AddRestore(projectSpec.Name); + dgFile.AddRestore(projectSpec.RestoreMetadata.ProjectUniqueName); // set up the packages await SimpleTestPackageUtility.CreateFullPackageAsync(packageSource.FullName, new SimpleTestPackageContext() @@ -1286,7 +1286,7 @@ public async Task RestoreRunner_MultiTfmPackageDownloadUnresolved_BothTfmsLogErr // set up the dg spec. var dgFile = new DependencyGraphSpec(); dgFile.AddProject(projectSpec); - dgFile.AddRestore(projectSpec.Name); + dgFile.AddRestore(projectSpec.RestoreMetadata.ProjectUniqueName); // set up the packages await SimpleTestPackageUtility.CreateFullPackageAsync(packageSource.FullName, new SimpleTestPackageContext() @@ -1394,7 +1394,7 @@ public async Task RestoreRunner_MultiTfmPackageDownloadRestore_OnlyMatchingPacka // set up the dg spec. var dgFile = new DependencyGraphSpec(); dgFile.AddProject(projectSpec); - dgFile.AddRestore(projectSpec.Name); + dgFile.AddRestore(projectSpec.RestoreMetadata.ProjectUniqueName); // set up the packages await SimpleTestPackageUtility.CreateFullPackageAsync(packageSource.FullName, new SimpleTestPackageContext() @@ -1504,7 +1504,7 @@ public async Task RestoreRunner_MultiTfmPDandPR_LogsWarningsAsync() // set up the dg spec. var dgFile = new DependencyGraphSpec(); dgFile.AddProject(projectSpec); - dgFile.AddRestore(projectSpec.Name); + dgFile.AddRestore(projectSpec.RestoreMetadata.ProjectUniqueName); // set up the packages await SimpleTestPackageUtility.CreateFullPackageAsync(packageSource.FullName, new SimpleTestPackageContext() { @@ -1595,7 +1595,7 @@ public async Task RestoreRunner_FrameworkReferenceIsWrittenToAssetsFile() // set up the dg spec. var dgFile = new DependencyGraphSpec(); dgFile.AddProject(projectSpec); - dgFile.AddRestore(projectSpec.Name); + dgFile.AddRestore(projectSpec.RestoreMetadata.ProjectUniqueName); // set up the packages var packageX = new SimpleTestPackageContext() { @@ -1673,7 +1673,7 @@ public async Task RestoreRunner_FrameworkReferenceIsProjectToPackageTransitive() // set up the dg spec. var dgFile = new DependencyGraphSpec(); dgFile.AddProject(projectSpec); - dgFile.AddRestore(projectSpec.Name); + dgFile.AddRestore(projectSpec.RestoreMetadata.ProjectUniqueName); // set up the packages var packageX = new SimpleTestPackageContext() { @@ -1806,7 +1806,7 @@ public async Task RestoreRunner_FrameworkReferenceIsProjectToProjectTransitive() .Add(new ProjectRestoreReference() { ProjectPath = projectSpec2.FilePath, - ProjectUniqueName = projectSpec2.Name + ProjectUniqueName = projectSpec2.RestoreMetadata.ProjectUniqueName } ); @@ -1814,8 +1814,8 @@ public async Task RestoreRunner_FrameworkReferenceIsProjectToProjectTransitive() var dgFile = new DependencyGraphSpec(); dgFile.AddProject(projectSpec1); dgFile.AddProject(projectSpec2); - dgFile.AddRestore(projectSpec1.Name); - dgFile.AddRestore(projectSpec2.Name); + dgFile.AddRestore(projectSpec1.RestoreMetadata.ProjectUniqueName); + dgFile.AddRestore(projectSpec2.RestoreMetadata.ProjectUniqueName); // set up the packages var packageX = new SimpleTestPackageContext() diff --git a/test/TestUtilities/Test.Utility/Commands/ProjectJsonTestHelpers.cs b/test/TestUtilities/Test.Utility/Commands/ProjectJsonTestHelpers.cs index 496406db7b0..0a0b433422d 100644 --- a/test/TestUtilities/Test.Utility/Commands/ProjectJsonTestHelpers.cs +++ b/test/TestUtilities/Test.Utility/Commands/ProjectJsonTestHelpers.cs @@ -5,7 +5,7 @@ using System.IO; using System.Linq; using System.Threading.Tasks; -using NuGet.Common; + using NuGet.Frameworks; using NuGet.ProjectModel; @@ -50,6 +50,26 @@ public static DependencyGraphSpec GetDGSpec(params PackageSpec[] projects) return dgSpec; } + /// + /// Creates a dg specs with all PackageReference and project.json projects to be restored. + /// + /// + /// + public static DependencyGraphSpec GetDGSpecFromPackageSpecs(params PackageSpec[] projects) + { + var dgSpec = new DependencyGraphSpec(); + foreach (var project in projects) + { + dgSpec.AddProject(project); + if (project.RestoreMetadata.ProjectStyle == ProjectStyle.PackageReference || + project.RestoreMetadata.ProjectStyle == ProjectStyle.ProjectJson) + { + dgSpec.AddRestore(project.RestoreMetadata.ProjectUniqueName); + } + } + return dgSpec; + } + /// /// Add restore metadata only if not already set. /// Sets the project style to PackageReference. @@ -102,12 +122,12 @@ public static PackageSpec WithTestRestoreMetadata(this PackageSpec spec) updated.FilePath = projectPath; updated.RestoreMetadata = new ProjectRestoreMetadata(); - updated.RestoreMetadata.CrossTargeting = updated.TargetFrameworks.Count > 0; + updated.RestoreMetadata.CrossTargeting = updated.TargetFrameworks.Count > 1; updated.RestoreMetadata.OriginalTargetFrameworks = updated.TargetFrameworks.Select(e => e.FrameworkName.GetShortFolderName()).ToList(); updated.RestoreMetadata.OutputPath = projectDir; updated.RestoreMetadata.ProjectStyle = ProjectStyle.PackageReference; updated.RestoreMetadata.ProjectName = spec.Name; - updated.RestoreMetadata.ProjectUniqueName = spec.Name; + updated.RestoreMetadata.ProjectUniqueName = projectPath; updated.RestoreMetadata.ProjectPath = projectPath; updated.RestoreMetadata.ConfigFilePaths = new List(); updated.RestoreMetadata.CentralPackageVersionsEnabled = spec.RestoreMetadata?.CentralPackageVersionsEnabled ?? false; From 5776c7134301a6d3602244ba632b6f8f99285d3a Mon Sep 17 00:00:00 2001 From: nkolev92 Date: Thu, 4 Jun 2020 09:50:58 -0700 Subject: [PATCH 2/3] address feeedback --- .../NuGet.SolutionRestoreManager/ISolutionRestoreChecker.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/NuGet.Clients/NuGet.SolutionRestoreManager/ISolutionRestoreChecker.cs b/src/NuGet.Clients/NuGet.SolutionRestoreManager/ISolutionRestoreChecker.cs index 33cbfd3e12d..a3a2a95f1d1 100644 --- a/src/NuGet.Clients/NuGet.SolutionRestoreManager/ISolutionRestoreChecker.cs +++ b/src/NuGet.Clients/NuGet.SolutionRestoreManager/ISolutionRestoreChecker.cs @@ -16,7 +16,7 @@ public interface ISolutionRestoreChecker { /// /// Given the current dependency graph spec, perform a fast up to date check and return the dirty projects. - /// The checker itself caches the DependencyGraphSpec it is provided & the last restore status, reported through . + /// The checker itself caches the DependencyGraphSpec it is provided and the last restore status, reported through . /// Accounts for changes in the PackageSpec and marks all the parent projects as dirty as well. /// Additionally also ensures that the expected output files have the same timestamps as the last time a succesful status was reported through . /// @@ -30,7 +30,6 @@ public interface ISolutionRestoreChecker /// /// /// Note that this call is stateful. This method may end up caching the dependency graph spec, so do not invoke multiple times. Ideally call should be followed by a call. - void ReportStatus(IReadOnlyList restoreSummaries); /// From 51dbe50d96bcb3168e726e9e6f32b21327a287d7 Mon Sep 17 00:00:00 2001 From: nkolev92 Date: Thu, 4 Jun 2020 11:45:46 -0700 Subject: [PATCH 3/3] fix up the comments --- .../ISolutionRestoreChecker.cs | 11 ++++++++--- .../SolutionRestoreJob.cs | 6 +++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/NuGet.Clients/NuGet.SolutionRestoreManager/ISolutionRestoreChecker.cs b/src/NuGet.Clients/NuGet.SolutionRestoreManager/ISolutionRestoreChecker.cs index a3a2a95f1d1..bc8c93698d8 100644 --- a/src/NuGet.Clients/NuGet.SolutionRestoreManager/ISolutionRestoreChecker.cs +++ b/src/NuGet.Clients/NuGet.SolutionRestoreManager/ISolutionRestoreChecker.cs @@ -18,18 +18,23 @@ public interface ISolutionRestoreChecker /// Given the current dependency graph spec, perform a fast up to date check and return the dirty projects. /// The checker itself caches the DependencyGraphSpec it is provided and the last restore status, reported through . /// Accounts for changes in the PackageSpec and marks all the parent projects as dirty as well. - /// Additionally also ensures that the expected output files have the same timestamps as the last time a succesful status was reported through . + /// Additionally, ensures that the expected output files have the same timestamps as the last reported status + /// . /// /// The current dependency graph spec. /// Unique ids of the dirty projects - /// Note that this call is stateful. This method may end up caching the dependency graph spec, so do not invoke multiple times. Ideally call should be followed by a call. + /// Note that this call is stateful. This method may end up caching the dependency graph spec, so do not invoke multiple times. + /// Ideally call should be followed by a call. + /// IEnumerable PerformUpToDateCheck(DependencyGraphSpec dependencyGraphSpec); /// /// Report the status of all the projects restored. /// /// - /// Note that this call is stateful. This method may end up caching the dependency graph spec, so do not invoke multiple times. Ideally call should be followed by a call. + /// Note that this call is stateful. This method may end up caching the dependency graph spec, so do not invoke multiple times. + /// Ideally call should be followed by a call. + /// void ReportStatus(IReadOnlyList restoreSummaries); /// diff --git a/src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionRestoreJob.cs b/src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionRestoreJob.cs index b07af7a0f2d..063cd427194 100644 --- a/src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionRestoreJob.cs +++ b/src/NuGet.Clients/NuGet.SolutionRestoreManager/SolutionRestoreJob.cs @@ -355,8 +355,8 @@ private async Task RestorePackageSpecProjectsAsync( var projectsNeedingRestore = _solutionUpToDateChecker.PerformUpToDateCheck(originalDgSpec).AsList(); dgSpec = originalDgSpec; - // Only use the optimization results if the restore is not force. - // Still run the optimization check anyways to prep the cache for a potential future non-force optimization + // Only use the optimization results if the restore is not `force`. + // Still run the optimization check anyways to prep the cache. if (!forceRestore) { // Update the dg spec. @@ -365,7 +365,7 @@ private async Task RestorePackageSpecProjectsAsync( { dgSpec.AddRestore(uniqueProjectId); } - // loop through all legacy PackageReference projects. We don't know how to replay their warnings & errors yet. + // loop through all legacy PackageReference projects. We don't know how to replay their warnings & errors yet. TODO: https://github.com/NuGet/Home/issues/9565 foreach(var project in (await _solutionManager.GetNuGetProjectsAsync()).Where(e => e is LegacyPackageReferenceProject).Select(e => e as LegacyPackageReferenceProject)) { dgSpec.AddRestore(project.MSBuildProjectPath);