From 059e4e94f024a8f56c56ec7f20eeccf7f355406d Mon Sep 17 00:00:00 2001 From: Joel Verhagen Date: Wed, 9 Mar 2016 17:04:24 -0800 Subject: [PATCH] Read the existing lock file for the tool Don't rewrite the same tool lock file (just like existing project lock file logic) Do not observe the "locked" property in tool restore --- .../RestoreCommand/IRestoreResult.cs | 34 ++ .../RestoreCommand/ProjectRestoreCommand.cs | 8 +- .../RestoreCommand/RestoreCommand.cs | 156 +++++--- .../RestoreCommand/RestoreResult.cs | 118 ++++-- .../RestoreCommand/ToolRestoreResult.cs | 11 +- src/NuGet.Core/NuGet.ProjectModel/LockFile.cs | 44 ++- .../NuGet.ProjectModel/LockFileFormat.cs | 13 +- .../compiler/resources/uwpBlankAppV2.json | 4 +- .../RestoreCommandTests.cs | 128 ++++++- .../NuGet.Commands.Test/RestoreResultTests.cs | 48 ++- .../LockFileFormatTests.cs | 155 ++++++-- .../NuGet.ProjectModel.Test/LockFileTests.cs | 348 ++++++++++++++++++ 12 files changed, 940 insertions(+), 127 deletions(-) create mode 100644 src/NuGet.Core/NuGet.Commands/RestoreCommand/IRestoreResult.cs create mode 100644 test/NuGet.Core.Tests/NuGet.ProjectModel.Test/LockFileTests.cs diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/IRestoreResult.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/IRestoreResult.cs new file mode 100644 index 000000000..f9e0adafa --- /dev/null +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/IRestoreResult.cs @@ -0,0 +1,34 @@ +// 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 NuGet.ProjectModel; + +namespace NuGet.Commands +{ + public interface IRestoreResult + { + bool Success { get; } + + /// + /// Gets the path that the lock file will be written to. + /// + string LockFilePath { get; } + + /// + /// Gets a boolean indicating if the lock file will be re-written on + /// because the file needs to be re-locked. + /// + bool RelockFile { get; } + + /// + /// Gets the lock file that was generated during the restore or, in the case of a locked lock file, + /// was used to determine the packages to install during the restore. + /// + LockFile LockFile { get; } + + /// + /// The existing lock file. This is null if no lock file was provided on the . + /// + LockFile PreviousLockFile { get; } + } +} \ No newline at end of file diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreCommand.cs index 11e5a49f6..f9ee569c3 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/ProjectRestoreCommand.cs @@ -316,10 +316,14 @@ namespace NuGet.Commands foreach (var dependency in dependencies) { // Ignore floating or version-less (project) dependencies - if (dependency.LibraryRange.VersionRange != null && !dependency.LibraryRange.VersionRange.IsFloating) + // Avoid warnings for non-packages + if (dependency.LibraryRange.TypeConstraintAllows(LibraryDependencyTarget.Package) + && dependency.LibraryRange.VersionRange != null && !dependency.LibraryRange.VersionRange.IsFloating) { var match = result.Flattened.FirstOrDefault(g => g.Key.Name.Equals(dependency.LibraryRange.Name)); - if (match != null && match.Key.Version > dependency.LibraryRange.VersionRange.MinVersion) + if (match != null + && LibraryTypes.Package == match.Key.Type + && match.Key.Version > dependency.LibraryRange.VersionRange.MinVersion) { _logger.LogWarning(string.Format(CultureInfo.CurrentCulture, Strings.Log_DependencyBumpedUp, dependency.LibraryRange.Name, diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs index a25626b03..ac8aa8860 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs @@ -8,7 +8,6 @@ using System.Diagnostics; using System.Globalization; using System.IO; using System.Linq; -using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using Newtonsoft.Json.Linq; @@ -72,16 +71,7 @@ namespace NuGet.Commands Path.Combine(_request.Project.BaseDirectory, LockFileFormat.LockFileName) : _request.LockFilePath; - bool relockFile = false; - if (_request.ExistingLockFile != null - && _request.ExistingLockFile.IsLocked - && !_request.ExistingLockFile.IsValidForPackageSpec(_request.Project, _request.LockFileVersion)) - { - // The lock file was locked, but the project.json is out of date - relockFile = true; - _request.ExistingLockFile.IsLocked = false; - _logger.LogMinimal(Strings.Log_LockFileOutOfDate); - } + var relockFile = ShouldRelockFile(_request.ExistingLockFile, _request.Project); var contextForProject = CreateRemoteWalkContext(_request); @@ -95,27 +85,14 @@ namespace NuGet.Commands toolRestoreResults = await ExecuteToolRestoresAsync(localRepository, token); } - // Build the lock file - LockFile lockFile; - if (_request.ExistingLockFile != null && _request.ExistingLockFile.IsLocked) - { - // No lock file to write! - lockFile = _request.ExistingLockFile; - } - else - { - lockFile = new LockFileBuilder(_request.LockFileVersion, _logger, _includeFlagGraphs) - .CreateLockFile( - _request.ExistingLockFile, - _request.Project, - graphs, - localRepository, - contextForProject, - toolRestoreResults); - - // If the lock file was locked originally but we are re-locking it, well... re-lock it :) - lockFile.IsLocked = relockFile; - } + var lockFile = BuildLockFile( + _request.ExistingLockFile, + _request.Project, + graphs, + localRepository, + contextForProject, + toolRestoreResults, + relockFile); if(!ValidateRestoreGraphs(graphs, _logger)) { @@ -167,6 +144,55 @@ namespace NuGet.Commands toolRestoreResults); } + private LockFile BuildLockFile( + LockFile existingLockFile, + PackageSpec project, + IEnumerable graphs, + NuGetv3LocalRepository localRepository, + RemoteWalkContext contextForProject, + IEnumerable toolRestoreResults, + bool relockFile) + { + // Build the lock file + LockFile lockFile; + if (existingLockFile != null && existingLockFile.IsLocked) + { + // No lock file to write! + lockFile = existingLockFile; + } + else + { + lockFile = new LockFileBuilder(_request.LockFileVersion, _logger, _includeFlagGraphs) + .CreateLockFile( + existingLockFile, + project, + graphs, + localRepository, + contextForProject, + toolRestoreResults); + + // If the lock file was locked originally but we are re-locking it, well... re-lock it :) + lockFile.IsLocked = relockFile; + } + + return lockFile; + } + + private bool ShouldRelockFile(LockFile existingLockFile, PackageSpec project) + { + bool relockFile = false; + if (existingLockFile != null + && existingLockFile.IsLocked + && !existingLockFile.IsValidForPackageSpec(project, _request.LockFileVersion)) + { + // The lock file was locked, but the project.json is out of date + relockFile = true; + existingLockFile.IsLocked = false; + _logger.LogMinimal(Strings.Log_LockFileOutOfDate); + } + return relockFile; + } + private static bool ValidateRestoreGraphs(IEnumerable graphs, ILogger logger) { foreach (var g in graphs) @@ -216,7 +242,6 @@ namespace NuGet.Commands CancellationToken token) { var toolPathResolver = new ToolPathResolver(_request.PackagesDirectory); - var lockFileBuilder = new LockFileBuilder(_request.LockFileVersion, _logger, _includeFlagGraphs); var results = new List(); foreach (var tool in _request.Project.Tools) @@ -251,6 +276,39 @@ namespace NuGet.Commands } }; + // Try to find the existing lock file. Since the existing lock file is pathed under + // a folder that includes the resolved tool's version, this is a bit of a chicken + // and egg problem. That is, we need to run the restore operation in order to resolve + // a tool version, but we need the tool version to find the existing project.lock.json + // file which is required before executing the restore! Fortunately, this is solved by + // looking at the tool's consuming project's lock file to see if the tool has been + // restored before. + LockFile existingToolLockFile = null; + if (_request.ExistingLockFile != null) + { + var existingTarget = _request + .ExistingLockFile + .Tools + .Where(t => t.RuntimeIdentifier == null) + .Where(t => t.TargetFramework.Equals(LockFile.ToolFramework)) + .FirstOrDefault(); + + var existingLibrary = existingTarget?.Libraries + .Where(l => StringComparer.OrdinalIgnoreCase.Equals(l.Name, tool.LibraryRange.Name)) + .Where(l => tool.LibraryRange.VersionRange.Satisfies(l.Version)) + .FirstOrDefault(); + + if (existingLibrary != null) + { + var existingLockFilePath = toolPathResolver.GetLockFilePath( + existingLibrary.Name, + existingLibrary.Version, + existingTarget.TargetFramework); + + existingToolLockFile = LockFileUtilities.GetLockFile(existingLockFilePath, _logger); + } + } + // Execute the restore. var toolSuccess = true; // success for this individual tool restore var runtimeIds = new HashSet(); @@ -261,7 +319,7 @@ namespace NuGet.Commands var projectRestoreRequest = new ProjectRestoreRequest( _request, toolPackageSpec, - null, + existingToolLockFile, new Dictionary(), _runtimeGraphCacheByPackage); var projectRestoreCommand = new ProjectRestoreCommand(_logger, projectRestoreRequest); @@ -283,24 +341,25 @@ namespace NuGet.Commands } // Create the lock file (in memory). - var lockFile = lockFileBuilder.CreateLockFile( - previousLockFile: null, - project: toolPackageSpec, - targetGraphs: graphs, - repository: localRepository, - context: contextForTool, - toolRestoreResults: Enumerable.Empty()); + var toolLockFile = BuildLockFile( + existingToolLockFile, + toolPackageSpec, + graphs, + localRepository, + contextForTool, + Enumerable.Empty(), + false); // Build the path based off of the resolved tool. For now, we assume there is only // ever one target. - var target = lockFile.Targets.Single(); + var target = toolLockFile.Targets.Single(); var fileTargetLibrary = target .Libraries .FirstOrDefault(l => StringComparer.OrdinalIgnoreCase.Equals(tool.LibraryRange.Name, l.Name)); - string lockFilePath = null; + string toolLockFilePath = null; if (fileTargetLibrary != null) { - lockFilePath = toolPathResolver.GetLockFilePath( + toolLockFilePath = toolPathResolver.GetLockFilePath( fileTargetLibrary.Name, fileTargetLibrary.Version, target.TargetFramework); @@ -318,8 +377,9 @@ namespace NuGet.Commands toolSuccess, target, fileTargetLibrary, - lockFilePath, - lockFile)); + toolLockFilePath, + toolLockFile, + existingToolLockFile)); } return results; @@ -651,6 +711,10 @@ namespace NuGet.Commands // Null out all target types, these did not exist in v1 library.Type = null; } + + // Remove tools + lockFile.Tools = null; + lockFile.ProjectFileToolGroups = null; } } } diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreResult.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreResult.cs index 396a8795f..d74d28326 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreResult.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreResult.cs @@ -13,7 +13,7 @@ using NuGet.ProjectModel; namespace NuGet.Commands { - public class RestoreResult + public class RestoreResult : IRestoreResult { public bool Success { get; } @@ -111,54 +111,102 @@ namespace NuGet.Commands /// /// If and are identical /// the file will not be written to disk. - /// The logger. /// Write out the lock file even if no changes exist. - /// The cancellation token. public async Task CommitAsync(ILogger log, bool forceWrite, CancellationToken token) { // Write the lock file var lockFileFormat = new LockFileFormat(); - // Don't write the lock file if it is Locked AND we're not re-locking the file - if (!LockFile.IsLocked || RelockFile) - { - // Avoid writing out the lock file if it is the same to avoid triggering an intellisense - // update on a restore with no actual changes. - if (forceWrite - || PreviousLockFile == null - || !PreviousLockFile.Equals(LockFile)) - { - log.LogMinimal($"Writing lock file to disk. Path: {LockFilePath}"); + await CommitAsync( + lockFileFormat, + result: this, + log: log, + forceWrite: forceWrite, + toolCommit: false, + token: token); - lockFileFormat.Write(LockFilePath, LockFile); - } - else - { - log.LogMinimal($"Lock file has not changed. Skipping lock file write. Path: {LockFilePath}"); - } - } - - // Always write the tool lock files foreach (var toolRestoreResult in ToolRestoreResults) { - if (toolRestoreResult.LockFilePath != null) + if (toolRestoreResult.LockFilePath != null && toolRestoreResult.LockFile != null) { - log.LogDebug($"Writing tool lock file to disk. Path: {toolRestoreResult.LockFilePath}"); - - await ConcurrencyUtilities.ExecuteWithFileLockedAsync( - toolRestoreResult.LockFilePath, - lockedToken => - { - var lockFileDirectory = Path.GetDirectoryName(toolRestoreResult.LockFilePath); - Directory.CreateDirectory(lockFileDirectory); - lockFileFormat.Write(toolRestoreResult.LockFilePath, toolRestoreResult.LockFile); - return Task.FromResult((object)null); - }, - token); + await CommitAsync( + lockFileFormat, + result: toolRestoreResult, + log: log, + forceWrite: forceWrite, + toolCommit: true, + token: token); } } MSBuild.Commit(log); } + + private static async Task CommitAsync( + LockFileFormat lockFileFormat, + IRestoreResult result, + ILogger log, + bool forceWrite, + bool toolCommit, + CancellationToken token) + { + // Don't write the lock file if it is Locked AND we're not re-locking the file + if (!result.LockFile.IsLocked || result.RelockFile || toolCommit) + { + // Avoid writing out the lock file if it is the same to avoid triggering an intellisense + // update on a restore with no actual changes. + if (forceWrite + || result.PreviousLockFile == null + || !result.PreviousLockFile.Equals(result.LockFile)) + { + if (toolCommit) + { + log.LogDebug($"Writing tool lock file to disk. Path: {result.LockFilePath}"); + + await ConcurrencyUtilities.ExecuteWithFileLockedAsync( + result.LockFilePath, + lockedToken => + { + var lockFileDirectory = Path.GetDirectoryName(result.LockFilePath); + Directory.CreateDirectory(lockFileDirectory); + + lockFileFormat.Write(result.LockFilePath, result.LockFile); + + return Task.FromResult(0); + }, + token); + } + else + { + log.LogMinimal($"Writing lock file to disk. Path: {result.LockFilePath}"); + + lockFileFormat.Write(result.LockFilePath, result.LockFile); + } + } + else + { + if (toolCommit) + { + log.LogDebug($"Tool lock file has not changed. Skipping lock file write. Path: {result.LockFilePath}"); + } + else + { + log.LogMinimal($"Lock file has not changed. Skipping lock file write. Path: {result.LockFilePath}"); + } + } + } + } + + private static void WriteLockFile( + LockFileFormat lockFileFormat, + IRestoreResult result, + bool createDirectory) + { + if (createDirectory) + { + } + + lockFileFormat.Write(result.LockFilePath, result.LockFile); + } } } diff --git a/src/NuGet.Core/NuGet.Commands/RestoreCommand/ToolRestoreResult.cs b/src/NuGet.Core/NuGet.Commands/RestoreCommand/ToolRestoreResult.cs index 822d7224f..70edb174e 100644 --- a/src/NuGet.Core/NuGet.Commands/RestoreCommand/ToolRestoreResult.cs +++ b/src/NuGet.Core/NuGet.Commands/RestoreCommand/ToolRestoreResult.cs @@ -5,7 +5,7 @@ using NuGet.ProjectModel; namespace NuGet.Commands { - public class ToolRestoreResult + public class ToolRestoreResult : IRestoreResult { public ToolRestoreResult( string toolName, @@ -13,7 +13,8 @@ namespace NuGet.Commands LockFileTarget lockFileTarget, LockFileTargetLibrary fileTargetLibrary, string lockFilePath, - LockFile lockFile) + LockFile lockFile, + LockFile previousLockFile) { ToolName = toolName; Success = success; @@ -21,6 +22,10 @@ namespace NuGet.Commands FileTargetLibrary = fileTargetLibrary; LockFilePath = lockFilePath; LockFile = lockFile; + PreviousLockFile = previousLockFile; + + // "locked" property is not supported on tools + RelockFile = false; } public string ToolName { get; } @@ -29,5 +34,7 @@ namespace NuGet.Commands public LockFileTargetLibrary FileTargetLibrary { get; } public string LockFilePath { get; } public LockFile LockFile { get; } + public LockFile PreviousLockFile { get; } + public bool RelockFile { get; } } } \ No newline at end of file diff --git a/src/NuGet.Core/NuGet.ProjectModel/LockFile.cs b/src/NuGet.Core/NuGet.ProjectModel/LockFile.cs index 69cb335d1..e288ca701 100644 --- a/src/NuGet.Core/NuGet.ProjectModel/LockFile.cs +++ b/src/NuGet.Core/NuGet.ProjectModel/LockFile.cs @@ -72,9 +72,11 @@ namespace NuGet.ProjectModel } else { - var framework = actualTargetFrameworks - .FirstOrDefault(f => - string.Equals(f.FrameworkName.ToString(), @group.FrameworkName, StringComparison.OrdinalIgnoreCase)); + var framework = actualTargetFrameworks.FirstOrDefault(f => string.Equals( + f.FrameworkName.DotNetFrameworkName, + @group.FrameworkName, + StringComparison.OrdinalIgnoreCase)); + if (framework == null) { return false; @@ -166,7 +168,11 @@ namespace NuGet.ProjectModel group => group.FrameworkName, StringComparer.OrdinalIgnoreCase)) && Libraries.OrderBy(library => library.Name, StringComparer.OrdinalIgnoreCase) .SequenceEqual(other.Libraries.OrderBy(library => library.Name, StringComparer.OrdinalIgnoreCase)) - && Targets.OrderBy(target => target.Name).SequenceEqual(other.Targets.OrderBy(target => target.Name)); + && Targets.OrderBy(target => target.Name).SequenceEqual(other.Targets.OrderBy(target => target.Name)) + && ProjectFileToolGroups.OrderBy(group => group.FrameworkName, StringComparer.OrdinalIgnoreCase) + .SequenceEqual(other.ProjectFileToolGroups.OrderBy( + group => group.FrameworkName, StringComparer.OrdinalIgnoreCase)) + && Tools.OrderBy(target => target.Name).SequenceEqual(other.Tools.OrderBy(target => target.Name)); } public override bool Equals(object obj) @@ -181,23 +187,35 @@ namespace NuGet.ProjectModel combiner.AddObject(IsLocked); combiner.AddObject(Version); - foreach (var item in ProjectFileDependencyGroups.OrderBy( - group => group.FrameworkName, StringComparer.OrdinalIgnoreCase)) - { - combiner.AddObject(item); - } + HashProjectFileDependencyGroups(combiner, ProjectFileDependencyGroups); foreach (var item in Libraries.OrderBy(library => library.Name, StringComparer.OrdinalIgnoreCase)) { combiner.AddObject(item); } - foreach (var item in Targets.OrderBy(target => target.Name, StringComparer.OrdinalIgnoreCase)) - { - combiner.AddObject(item); - } + HashLockFileTargets(combiner, Targets); + HashProjectFileDependencyGroups(combiner, ProjectFileToolGroups); + HashLockFileTargets(combiner, Tools); return combiner.CombinedHash; } + + private static void HashLockFileTargets(HashCodeCombiner combiner, IList targets) + { + foreach (var item in targets.OrderBy(target => target.Name, StringComparer.OrdinalIgnoreCase)) + { + combiner.AddObject(item); + } + } + + private static void HashProjectFileDependencyGroups(HashCodeCombiner combiner, IList groups) + { + foreach (var item in groups.OrderBy( + group => @group.FrameworkName, StringComparer.OrdinalIgnoreCase)) + { + combiner.AddObject(item); + } + } } } diff --git a/src/NuGet.Core/NuGet.ProjectModel/LockFileFormat.cs b/src/NuGet.Core/NuGet.ProjectModel/LockFileFormat.cs index f51f5f527..2d312d878 100644 --- a/src/NuGet.Core/NuGet.ProjectModel/LockFileFormat.cs +++ b/src/NuGet.Core/NuGet.ProjectModel/LockFileFormat.cs @@ -183,8 +183,17 @@ namespace NuGet.ProjectModel json[TargetsProperty] = WriteObject(lockFile.Targets, WriteTarget); json[LibrariesProperty] = WriteObject(lockFile.Libraries, WriteLibrary); json[ProjectFileDependencyGroupsProperty] = WriteObject(lockFile.ProjectFileDependencyGroups, WriteProjectFileDependencyGroup); - json[ToolsProperty] = WriteObject(lockFile.Tools, WriteTarget); - json[ProjectFileToolGroupsProperty] = WriteObject(lockFile.ProjectFileToolGroups, WriteProjectFileDependencyGroup); + + if (lockFile.Tools != null) + { + json[ToolsProperty] = WriteObject(lockFile.Tools, WriteTarget); + } + + if (lockFile.ProjectFileToolGroups != null) + { + json[ProjectFileToolGroupsProperty] = WriteObject(lockFile.ProjectFileToolGroups, WriteProjectFileDependencyGroup); + } + return json; } diff --git a/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/compiler/resources/uwpBlankAppV2.json b/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/compiler/resources/uwpBlankAppV2.json index 573b1dbac..616885ab0 100644 --- a/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/compiler/resources/uwpBlankAppV2.json +++ b/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/compiler/resources/uwpBlankAppV2.json @@ -15379,5 +15379,7 @@ "Microsoft.NETCore.UniversalWindowsPlatform >= 5.0.0" ], "UAP,Version=v10.0": [] - } + }, + "tools": {}, + "projectFileToolGroups": {} } \ No newline at end of file diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests.cs index a274ba6f2..4eb854456 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreCommandTests.cs @@ -6,6 +6,7 @@ using System.Threading; using System.Threading.Tasks; using NuGet.Configuration; using NuGet.Frameworks; +using NuGet.Logging; using NuGet.ProjectModel; using NuGet.Test.Utility; using Xunit; @@ -247,6 +248,124 @@ namespace NuGet.Commands.Test } } + [Fact] + public async Task RestoreCommand_DoesNotRedoRestoreToolsWithValidLockFile() + { + // Arrange + using (var workingDir = TestFileSystemUtility.CreateRandomTestFolder()) + { + var tc = new ToolTestContext(workingDir) + { + ProjectJson = @" + { + ""frameworks"": { + ""net45"": { } + }, + ""tools"": { + ""packageA"": ""*"" + } + }", + }; + + var packageA = new SimpleTestPackageContext("packageA"); + packageA.AddFile("lib/netstandard1.3/a.dll"); + + SimpleTestPackageUtility.CreatePackages(tc.PackageSource.FullName, packageA); + + tc.Initialize(); + + // the first restore + await (await tc.Command.ExecuteAsync()).CommitAsync(tc.Logger, CancellationToken.None); + + // reset + tc.Logger = new TestLogger(); + tc.Request.Log = tc.Logger; + tc.Initialize(); + tc.Request.ExistingLockFile = LockFileUtilities.GetLockFile(tc.Request.LockFilePath, tc.Logger); + + // Act + var result = await tc.Command.ExecuteAsync(); + await result.CommitAsync(tc.Logger, CancellationToken.None); + + // Assert + Assert.True( + result.Success, + "The command did not succeed. Error messages: " + + Environment.NewLine + tc.Logger.ShowErrors()); + Assert.Equal(1, result.ToolRestoreResults.Count()); + + Assert.Contains( + $"Lock file has not changed. Skipping lock file write. Path: {result.LockFilePath}", + tc.Logger.Messages); + + var toolResult = result.ToolRestoreResults.First(); + Assert.Contains( + $"Tool lock file has not changed. Skipping lock file write. Path: {toolResult.LockFilePath}", + tc.Logger.Messages); + } + } + + [Fact] + public async Task RestoreCommand_ObservesLockedFilesOnlyInProject() + { + // Arrange + using (var workingDir = TestFileSystemUtility.CreateRandomTestFolder()) + { + var tc = new ToolTestContext(workingDir) + { + ProjectJson = @" + { + ""frameworks"": { + ""net45"": { } + }, + ""tools"": { + ""packageA"": ""*"" + } + }", + }; + + var packageA = new SimpleTestPackageContext("packageA"); + packageA.AddFile("lib/netstandard1.3/a.dll"); + + SimpleTestPackageUtility.CreatePackages(tc.PackageSource.FullName, packageA); + + tc.Initialize(); + + // the first restore + var initialResult = await tc.Command.ExecuteAsync(); + await initialResult.CommitAsync(tc.Logger, CancellationToken.None); + + tc.SetIsLocked(initialResult.LockFilePath, true, tc.Logger); + tc.SetIsLocked(initialResult.ToolRestoreResults.First().LockFilePath, true, tc.Logger); + + // reset + tc.Logger = new TestLogger(); + tc.Request.Log = tc.Logger; + tc.Initialize(); + tc.Request.ExistingLockFile = LockFileUtilities.GetLockFile(tc.Request.LockFilePath, tc.Logger); + + // Act + var result = await tc.Command.ExecuteAsync(); + await result.CommitAsync(tc.Logger, CancellationToken.None); + + // Assert + Assert.True( + result.Success, + "The command did not succeed. Error messages: " + + Environment.NewLine + tc.Logger.ShowErrors()); + Assert.Equal(1, result.ToolRestoreResults.Count()); + + // Since the files are locked and valid, the commit is completely skipped. + Assert.DoesNotContain(result.LockFilePath, tc.Logger.ShowMessages()); + + // Locking a tool lock file is not supported. + var toolResult = result.ToolRestoreResults.First(); + Assert.Contains( + $"Tool lock file has not changed. Skipping lock file write. Path: {toolResult.LockFilePath}", + tc.Logger.Messages); + } + } + [Fact] public async Task RestoreCommand_FailsCommandWhenToolRestoreFails() { @@ -379,7 +498,7 @@ namespace NuGet.Commands.Test } public DirectoryInfo PackageSource { get; } - public TestLogger Logger { get; } + public TestLogger Logger { get; set; } public string ProjectJson { private get; set; } public RestoreRequest Request { get; set; } public RestoreCommand Command { get; private set; } @@ -399,6 +518,13 @@ namespace NuGet.Commands.Test Request.LockFilePath = Path.Combine(Project.FullName, "project.lock.json"); Command = new RestoreCommand(Request); } + + public void SetIsLocked(string path, bool isLocked, ILogger logger) + { + var lockFile = LockFileUtilities.GetLockFile(path, logger); + lockFile.IsLocked = isLocked; + new LockFileFormat().Write(path, lockFile); + } } } } diff --git a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreResultTests.cs b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreResultTests.cs index 3d7fabc4d..9f84b5ec1 100644 --- a/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreResultTests.cs +++ b/test/NuGet.Core.Tests/NuGet.Commands.Test/RestoreResultTests.cs @@ -78,13 +78,16 @@ namespace NuGet.Commands.Test // Arrange using (var td = TestFileSystemUtility.CreateRandomTestFolder()) { - var path = Path.Combine(td, "project.lock.json"); + var path = Path.Combine(td, ".tools", "project.lock.json"); var logger = new TestLogger(); var toolResult = new ToolRestoreResult( + toolName: null, success: true, - restoreGraphs: null, + lockFileTarget: null, + fileTargetLibrary: null, lockFilePath: path, - lockFile: new LockFile()); + lockFile: new LockFile(), + previousLockFile: null); var result = new RestoreResult( success: true, restoreGraphs: null, @@ -104,7 +107,44 @@ namespace NuGet.Commands.Test logger.DebugMessages); Assert.True(File.Exists(path), $"The tool lock file should have been written: {path}"); Assert.Equal(1, logger.DebugMessages.Count); - Assert.Equal(1, logger.MinimalMessages.Count); + } + } + + [Fact] + public async Task RestoreResult_WritesToolSkipCommitToDebug() + { + // Arrange + using (var td = TestFileSystemUtility.CreateRandomTestFolder()) + { + var path = Path.Combine(td, "tools", "project.lock.json"); + var logger = new TestLogger(); + var toolResult = new ToolRestoreResult( + toolName: null, + success: true, + lockFileTarget: null, + fileTargetLibrary: null, + lockFilePath: path, + lockFile: new LockFile(), + previousLockFile: new LockFile()); // same lock file + var result = new RestoreResult( + success: true, + restoreGraphs: null, + compatibilityCheckResults: null, + lockFile: new LockFile(), + previousLockFile: new LockFile(), // same lock file + lockFilePath: null, + msbuild: new MSBuildRestoreResult("project", td, true), + toolRestoreResults: new[] { toolResult }); + + // Act + await result.CommitAsync(logger, CancellationToken.None); + + // Assert + Assert.Contains( + $"Tool lock file has not changed. Skipping lock file write. Path: {path}", + logger.DebugMessages); + Assert.False(File.Exists(path), $"The tool lock file should not have been written: {path}"); + Assert.Equal(1, logger.DebugMessages.Count); } } } diff --git a/test/NuGet.Core.Tests/NuGet.ProjectModel.Test/LockFileFormatTests.cs b/test/NuGet.Core.Tests/NuGet.ProjectModel.Test/LockFileFormatTests.cs index 92208ed05..369dfe1c9 100644 --- a/test/NuGet.Core.Tests/NuGet.ProjectModel.Test/LockFileFormatTests.cs +++ b/test/NuGet.Core.Tests/NuGet.ProjectModel.Test/LockFileFormatTests.cs @@ -12,10 +12,9 @@ namespace NuGet.ProjectModel.Test public class LockFileFormatTests { [Fact] - public void ReadBasicLockFile() + public void LockFileFormat_ReadsLockFileWithNoTools() { - const string lockFileContent = @" -{ + string lockFileContent = @"{ ""locked"": true, ""version"": 1, ""targets"": { @@ -90,44 +89,94 @@ namespace NuGet.ProjectModel.Test var netPlatDepGroup = lockFile.ProjectFileDependencyGroups.Last(); Assert.Equal(NuGetFramework.Parse("dotnet").DotNetFrameworkName, netPlatDepGroup.FrameworkName); Assert.Empty(netPlatDepGroup.Dependencies); + + Assert.Equal(0, lockFile.ProjectFileToolGroups.Count); + Assert.Equal(0, lockFile.Tools.Count); } [Fact] - public void WriteBasicLockFile() + public void LockFileFormat_ReadsLockFileWithTools() + { + string lockFileContent = @"{ + ""tools"": { + "".NETStandard,Version=v1.2"": { + ""System.Runtime/4.0.20-beta-22927"": { + ""dependencies"": { + ""Frob"": ""[4.0.20, )"" + }, + ""compile"": { + ""ref/dotnet/System.Runtime.dll"": {} + } + } + } + }, + ""projectFileToolGroups"": { + "".NETFramework,Version=v4.5.1"": [], + "".NETStandard,Version=v1.5"": [ + ""System.Runtime [4.0.10-beta-*, )"" + ] + } +}"; + var lockFileFormat = new LockFileFormat(); + var lockFile = lockFileFormat.Parse(lockFileContent, "In Memory"); + + var tool = lockFile.Tools.Single(); + Assert.Equal(FrameworkConstants.CommonFrameworks.NetStandard12, tool.TargetFramework); + + var runtimeTargetLibrary = tool.Libraries.Single(); + Assert.Equal("System.Runtime", runtimeTargetLibrary.Name); + Assert.Equal(NuGetVersion.Parse("4.0.20-beta-22927"), runtimeTargetLibrary.Version); + Assert.Equal(0, runtimeTargetLibrary.NativeLibraries.Count); + Assert.Equal(0, runtimeTargetLibrary.ResourceAssemblies.Count); + Assert.Equal(0, runtimeTargetLibrary.FrameworkAssemblies.Count); + Assert.Equal(0, runtimeTargetLibrary.RuntimeAssemblies.Count); + Assert.Equal("ref/dotnet/System.Runtime.dll", runtimeTargetLibrary.CompileTimeAssemblies.Single().Path); + + var net451Group = lockFile.ProjectFileToolGroups.First(); + Assert.Equal(FrameworkConstants.CommonFrameworks.Net451, NuGetFramework.Parse(net451Group.FrameworkName)); + + var netStandardGroup = lockFile.ProjectFileToolGroups.Last(); + Assert.Equal(FrameworkConstants.CommonFrameworks.NetStandard15, NuGetFramework.Parse(netStandardGroup.FrameworkName)); + Assert.Equal("System.Runtime [4.0.10-beta-*, )", netStandardGroup.Dependencies.Single()); + } + + [Fact] + public void LockFileFormat_WritesLockFileWithNoTools() { // Arrange - const string lockFileContent = @"{ + string lockFileContent = @"{ ""locked"": true, ""version"": 2, ""targets"": { - "".NETPlatform,Version=v5.0"": { - ""System.Runtime/4.0.20-beta-22927"": { - ""type"": ""package"", - ""dependencies"": { - ""Frob"": ""4.0.20"" - }, + "".NETPlatform,Version=v5.0"": { + ""System.Runtime/4.0.20-beta-22927"": { + ""type"": ""package"", + ""dependencies"": { + ""Frob"": ""4.0.20"" + }, ""compile"": { - ""ref/dotnet/System.Runtime.dll"": { - } - } - } - } - }, + ""ref/dotnet/System.Runtime.dll"": {} + } + } + } + }, ""libraries"": { - ""System.Runtime/4.0.20-beta-22927"": { - ""sha512"": ""sup3rs3cur3"", + ""System.Runtime/4.0.20-beta-22927"": { + ""sha512"": ""sup3rs3cur3"", ""type"": ""package"", ""files"": [ ""System.Runtime.nuspec"" ] } -}, + }, ""projectFileDependencyGroups"": { """": [ ""System.Runtime [4.0.10-beta-*, )"" ], "".NETPlatform,Version=v5.0"": [] - } + }, + ""tools"": {}, + ""projectFileToolGroups"": {} }"; var lockFile = new LockFile(); lockFile.IsLocked = true; @@ -172,5 +221,69 @@ namespace NuGet.ProjectModel.Test // Assert Assert.Equal(expected.ToString(), output.ToString()); } + + [Fact] + public void LockFileFormat_WritesLockFileWithTools() + { + // Arrange + string lockFileContent = @"{ + ""locked"": true, + ""version"": 2, + ""targets"": {}, + ""libraries"": {}, + ""projectFileDependencyGroups"": {}, + ""tools"": { + "".NETPlatform,Version=v5.0"": { + ""System.Runtime/4.0.20-beta-22927"": { + ""type"": ""package"", + ""dependencies"": { + ""Frob"": ""4.0.20"" + }, + ""compile"": { + ""ref/dotnet/System.Runtime.dll"": {} + } + } + } + }, + ""projectFileToolGroups"": { + """": [ + ""System.Runtime [4.0.10-beta-*, )"" + ], + "".NETPlatform,Version=v5.0"": [] + } +}"; + var lockFile = new LockFile(); + lockFile.IsLocked = true; + lockFile.Version = 2; + + var target = new LockFileTarget() + { + TargetFramework = FrameworkConstants.CommonFrameworks.DotNet + }; + var targetLib = new LockFileTargetLibrary() + { + Name = "System.Runtime", + Version = NuGetVersion.Parse("4.0.20-beta-22927"), + Type = LibraryTypes.Package + }; + targetLib.Dependencies.Add(new NuGet.Packaging.Core.PackageDependency("Frob", + new VersionRange(NuGetVersion.Parse("4.0.20")))); + targetLib.CompileTimeAssemblies.Add(new LockFileItem("ref/dotnet/System.Runtime.dll")); + target.Libraries.Add(targetLib); + lockFile.Tools.Add(target); + + lockFile.ProjectFileToolGroups.Add( + new ProjectFileDependencyGroup("", new string[] { "System.Runtime [4.0.10-beta-*, )" })); + lockFile.ProjectFileToolGroups.Add( + new ProjectFileDependencyGroup(FrameworkConstants.CommonFrameworks.DotNet.DotNetFrameworkName, new string[0])); + + // Act + var lockFileFormat = new LockFileFormat(); + var output = JObject.Parse(lockFileFormat.Render(lockFile)); + var expected = JObject.Parse(lockFileContent); + + // Assert + Assert.Equal(expected.ToString(), output.ToString()); + } } } diff --git a/test/NuGet.Core.Tests/NuGet.ProjectModel.Test/LockFileTests.cs b/test/NuGet.Core.Tests/NuGet.ProjectModel.Test/LockFileTests.cs new file mode 100644 index 000000000..bd3ac5b76 --- /dev/null +++ b/test/NuGet.Core.Tests/NuGet.ProjectModel.Test/LockFileTests.cs @@ -0,0 +1,348 @@ +using System.Collections.Generic; +using Newtonsoft.Json.Linq; +using NuGet.Frameworks; +using NuGet.LibraryModel; +using NuGet.Versioning; +using Xunit; + +namespace NuGet.ProjectModel.Test +{ + public class LockFileTests + { + [Fact] + public void LockFile_ComparesEqualTools() + { + // Arrange + var lockFileA = new LockFile + { + Tools = + { + new LockFileTarget + { + TargetFramework = FrameworkConstants.CommonFrameworks.NetStandard12, + Libraries = { new LockFileTargetLibrary { Name = "SomeLibrary" } } + } + } + }; + + // same thing + var lockFileB = new LockFile + { + Tools = + { + new LockFileTarget + { + TargetFramework = FrameworkConstants.CommonFrameworks.NetStandard12, + Libraries = { new LockFileTargetLibrary { Name = "SomeLibrary" } } + } + } + }; + + // Act & Assert + Assert.True(lockFileA.Equals(lockFileB), "The two lock files should be equal."); + } + + [Fact] + public void LockFile_ComparesDifferentTools() + { + // Arrange + var lockFileA = new LockFile + { + Tools = + { + new LockFileTarget + { + TargetFramework = FrameworkConstants.CommonFrameworks.NetStandard12, + Libraries = { new LockFileTargetLibrary { Name = "SomeLibrary" } } + } + } + }; + + // different thing + var lockFileB = new LockFile + { + Tools = + { + new LockFileTarget + { + TargetFramework = FrameworkConstants.CommonFrameworks.NetStandard12, + Libraries = { new LockFileTargetLibrary { Name = "SomeLibrary" } } + }, + new LockFileTarget + { + TargetFramework = FrameworkConstants.CommonFrameworks.NetStandard12, + Libraries = { new LockFileTargetLibrary { Name = "OtherLibrary" } } + } + } + }; + + // Act & Assert + Assert.False(lockFileA.Equals(lockFileB), "The two lock files should not be equal."); + } + + [Fact] + public void LockFile_ComparesEqualProjectFileToolGroups() + { + // Arrange + var lockFileA = new LockFile + { + ProjectFileToolGroups = + { + new ProjectFileDependencyGroup("FrameworkA", new [] { "DependencyA" }) + } + }; + + // same thing + var lockFileB = new LockFile + { + ProjectFileToolGroups = + { + new ProjectFileDependencyGroup("FrameworkA", new [] { "DependencyA" }) + } + }; + + // Act & Assert + Assert.True(lockFileA.Equals(lockFileB), "The two lock files should be equal."); + } + + [Fact] + public void LockFile_ComparesDifferentProjectFileToolGroups() + { + // Arrange + var lockFileA = new LockFile + { + ProjectFileToolGroups = + { + new ProjectFileDependencyGroup("FrameworkA", new [] { "DependencyA" }) + } + }; + + // different thing + var lockFileB = new LockFile + { + ProjectFileToolGroups = + { + new ProjectFileDependencyGroup("FrameworkA", new [] { "DependencyA" }), + new ProjectFileDependencyGroup("FrameworkB", new [] { "DependencyB" }) + } + }; + + // Act & Assert + Assert.False(lockFileA.Equals(lockFileB), "The two lock files should not be equal."); + } + + [Fact] + public void LockFile_IsValidForPackageSpec_DifferentToolVersions() + { + // Arrange + var lockFile = new LockFile + { + ProjectFileDependencyGroups = + { + new ProjectFileDependencyGroup("", new string[0]) + }, + ProjectFileToolGroups = + { + new ProjectFileDependencyGroup(LockFile.ToolFramework.ToString(), new [] { "DependencyA >= 2.0.0" }) + } + }; + + var packageSpec = new PackageSpec(new JObject()) + { + Dependencies = new List(), + Tools = new List + { + new ToolDependency + { + LibraryRange = new LibraryRange + { + Name = "DependencyA", + TypeConstraint = LibraryDependencyTarget.Package, + VersionRange = VersionRange.Parse("1.0.0") + } + } + } + }; + + // Act & Assert + Assert.False(lockFile.IsValidForPackageSpec(packageSpec)); + } + + [Fact] + public void LockFile_IsValidForPackageSpec_MissingTools() + { + // Arrange + var lockFile = new LockFile + { + ProjectFileDependencyGroups = + { + new ProjectFileDependencyGroup("", new string[0]) + }, + ProjectFileToolGroups = + { + new ProjectFileDependencyGroup(LockFile.ToolFramework.ToString(), new [] { "DependencyA >= 2.0.0" }) + } + }; + + var packageSpec = new PackageSpec(new JObject()) + { + Dependencies = new List(), + Tools = new List + { + new ToolDependency + { + LibraryRange = new LibraryRange + { + Name = "DependencyA", + TypeConstraint = LibraryDependencyTarget.Package, + VersionRange = VersionRange.Parse("1.0.0") + } + }, + new ToolDependency + { + LibraryRange = new LibraryRange + { + Name = "DependencyB", + TypeConstraint = LibraryDependencyTarget.Package, + VersionRange = VersionRange.Parse("2.0.0") + } + } + } + }; + + // Act & Assert + Assert.False(lockFile.IsValidForPackageSpec(packageSpec)); + } + + [Fact] + public void LockFile_IsValidForPackageSpec_ExtraTools() + { + // Arrange + var lockFile = new LockFile + { + ProjectFileDependencyGroups = + { + new ProjectFileDependencyGroup("", new string[0]) + }, + ProjectFileToolGroups = + { + new ProjectFileDependencyGroup(LockFile.ToolFramework.ToString(), new [] + { + "DependencyA >= 1.0.0", + "DependencyB >= 2.0.0", + }) + } + }; + + var packageSpec = new PackageSpec(new JObject()) + { + Dependencies = new List(), + Tools = new List + { + new ToolDependency + { + LibraryRange = new LibraryRange + { + Name = "DependencyA", + TypeConstraint = LibraryDependencyTarget.Package, + VersionRange = VersionRange.Parse("1.0.0") + } + } + } + }; + + // Act & Assert + Assert.False(lockFile.IsValidForPackageSpec(packageSpec)); + } + + [Fact] + public void LockFile_IsValidForPackageSpec_InvalidToolFramework() + { + // Arrange + var lockFile = new LockFile + { + ProjectFileDependencyGroups = + { + new ProjectFileDependencyGroup("", new string[0]) + }, + ProjectFileToolGroups = + { + new ProjectFileDependencyGroup("FrameworkA", new [] + { + "DependencyA >= 1.0.0" + }) + } + }; + + var packageSpec = new PackageSpec(new JObject()) + { + Dependencies = new List(), + Tools = new List + { + new ToolDependency + { + LibraryRange = new LibraryRange + { + Name = "DependencyA", + TypeConstraint = LibraryDependencyTarget.Package, + VersionRange = VersionRange.Parse("1.0.0") + } + } + } + }; + + // Act & Assert + Assert.False(lockFile.IsValidForPackageSpec(packageSpec)); + } + + [Fact] + public void LockFile_IsValidForPackageSpec_MatchingTools() + { + // Arrange + var lockFile = new LockFile + { + ProjectFileDependencyGroups = + { + new ProjectFileDependencyGroup("", new string[0]) + }, + ProjectFileToolGroups = + { + new ProjectFileDependencyGroup(LockFile.ToolFramework.ToString(), new [] + { + "DependencyA >= 1.0.0", + "DependencyB >= 2.0.0", + }) + } + }; + + var packageSpec = new PackageSpec(new JObject()) + { + Dependencies = new List(), + Tools = new List + { + new ToolDependency + { + LibraryRange = new LibraryRange + { + Name = "DependencyB", + TypeConstraint = LibraryDependencyTarget.Package, + VersionRange = VersionRange.Parse("2.0.0") + } + }, + new ToolDependency + { + LibraryRange = new LibraryRange + { + Name = "DependencyA", + TypeConstraint = LibraryDependencyTarget.Package, + VersionRange = VersionRange.Parse("1.0.0") + } + } + } + }; + + // Act & Assert + Assert.True(lockFile.IsValidForPackageSpec(packageSpec)); + } + } +}