Improve NuGet CLI behaviour when running as global tool (#402)

* Log error instead of throwing when NuGet cli cannot be found. This returns to the old behaviour, that was still expected by the consumers of this class. * Do not run global nuget restore when dealing with a dotnetcore-only project. * Include NuGet.exe in packaged tool * Expanded diagnostic output when probing for NuGet.exe location * NuGetFileRestoreCommand created by IoC * NuGetUpdatePackageCommand created by IoC * DotNetUpdateCommand created by IoC * UpdateProjectImportsCommand created by IoC * UpdateNuspecCommand created by IoC * Extract helper method to improve readability * Cache the path to NuGet.exe when found * Delay evaluation of NuGetPath so that it happens after logger is configured
NuKeeperDotNet · Aug 23, 2018 · d0610ad · d0610ad
1 parent 809cdcb
commit d0610ad
Show file tree

Hide file tree

Showing 18 changed files with 133 additions and 42 deletions.
diff --git a/NuKeeper.Integration.Tests/NuGet/Process/DotNetUpdatePackageCommandTests.cs b/NuKeeper.Integration.Tests/NuGet/Process/DotNetUpdatePackageCommandTests.cs
@@ -9,6 +9,7 @@
 using NuKeeper.Inspection.RepositoryInspection;
 using NuKeeper.Inspection.Sources;
 using NuKeeper.Update.Process;
+using NuKeeper.Update.ProcessRunner;
 using NUnit.Framework;
 
 namespace NuKeeper.Integration.Tests.NuGet.Process
@@ -116,7 +117,7 @@ private async Task ExecuteValidUpdateTest(
             var projectPath = Path.Combine(workDirectory, testProject);
             await File.WriteAllTextAsync(projectPath, projectContents);
 
-            var command = new DotNetUpdatePackageCommand(Substitute.For<INuKeeperLogger>());
+            var command = new DotNetUpdatePackageCommand(new ExternalProcess(Substitute.For<INuKeeperLogger>()));
 
             var packageToUpdate = new PackageInProject("Microsoft.AspNet.WebApi.Client", oldPackageVersion,
                 new PackagePath(workDirectory, testProject, packageReferenceType));

diff --git a/NuKeeper.Integration.Tests/NuGet/Process/NuGetPathTests.cs b/NuKeeper.Integration.Tests/NuGet/Process/NuGetPathTests.cs
@@ -1,3 +1,5 @@
+using NSubstitute;
+using NuKeeper.Inspection.Logging;
 using NuKeeper.Update.Process;
 using NUnit.Framework;
 
@@ -9,7 +11,7 @@ public class NuGetPathTests
         [Test]
         public void HasNugetPath()
         {
-            var nugetPath = NuGetPath.FindExecutable();
+            var nugetPath = new NuGetPath(Substitute.For<INuKeeperLogger>()).Executable;
 
             Assert.That(nugetPath, Is.Not.Empty);
             FileAssert.Exists(nugetPath);

diff --git a/NuKeeper.Integration.Tests/NuGet/Process/NuGetUpdatePackageCommandTests.cs b/NuKeeper.Integration.Tests/NuGet/Process/NuGetUpdatePackageCommandTests.cs
@@ -8,6 +8,8 @@
 using NuKeeper.Inspection.Logging;
 using NuKeeper.Inspection.RepositoryInspection;
 using NuKeeper.Inspection.Sources;
+using NuKeeper.Update.Process;
+using NuKeeper.Update.ProcessRunner;
 
 namespace NuKeeper.Integration.Tests.NuGet.Process
 {
@@ -57,7 +59,8 @@ public async Task ShouldUpdateDotnetClassicProject()
 
             await File.WriteAllTextAsync(Path.Combine(workDirectory, "nuget.config"), _nugetConfig);
 
-            var command = new Update.Process.NuGetUpdatePackageCommand(Substitute.For<INuKeeperLogger>());
+            var logger = Substitute.For<INuKeeperLogger>();
+            var command = new NuGetUpdatePackageCommand(logger, new NuGetPath(logger), new ExternalProcess(logger));
 
             var packageToUpdate = new PackageInProject("Microsoft.AspNet.WebApi.Client", oldPackageVersion,
                     new PackagePath(workDirectory, testProject, PackageReferenceType.PackagesConfig));

diff --git a/NuKeeper.Update/Process/DotNetUpdatePackageCommand.cs b/NuKeeper.Update/Process/DotNetUpdatePackageCommand.cs
@@ -1,24 +1,19 @@
 using System.Threading.Tasks;
 using NuGet.Configuration;
 using NuGet.Versioning;
-using NuKeeper.Inspection.Logging;
 using NuKeeper.Inspection.RepositoryInspection;
 using NuKeeper.Inspection.Sources;
 using NuKeeper.Update.ProcessRunner;
 
 namespace NuKeeper.Update.Process
 {
-    public class DotNetUpdatePackageCommand : IPackageCommand
+    public class DotNetUpdatePackageCommand : IDotNetUpdatePackageCommand
     {
         private readonly IExternalProcess _externalProcess;
-        private readonly INuKeeperLogger _logger;
 
-        public DotNetUpdatePackageCommand(
-            INuKeeperLogger logger,
-            IExternalProcess externalProcess = null)
+        public DotNetUpdatePackageCommand(IExternalProcess externalProcess)
         {
-            _logger = logger;
-            _externalProcess = externalProcess ?? new ExternalProcess(logger);
+            _externalProcess = externalProcess;
         }
 
         public async Task Invoke(PackageInProject currentPackage,

diff --git a/NuKeeper.Update/Process/IDotNetUpdatePackageCommand.cs b/NuKeeper.Update/Process/IDotNetUpdatePackageCommand.cs
@@ -0,0 +1,6 @@
+namespace NuKeeper.Update.Process
+{
+    public interface IDotNetUpdatePackageCommand : IPackageCommand
+    {
+    }
+}
diff --git a/NuKeeper.Update/Process/INuGetPath.cs b/NuKeeper.Update/Process/INuGetPath.cs
@@ -0,0 +1,7 @@
+namespace NuKeeper.Update.Process
+{
+    public interface INuGetPath
+    {
+        string Executable { get; }
+    }
+}
diff --git a/NuKeeper.Update/Process/INuGetUpdatePackageCommand.cs b/NuKeeper.Update/Process/INuGetUpdatePackageCommand.cs
@@ -0,0 +1,6 @@
+namespace NuKeeper.Update.Process
+{
+    public interface INuGetUpdatePackageCommand : IPackageCommand
+    {
+    }
+}
diff --git a/NuKeeper.Update/Process/IUpdateNuspecCommand.cs b/NuKeeper.Update/Process/IUpdateNuspecCommand.cs
@@ -0,0 +1,6 @@
+namespace NuKeeper.Update.Process
+{
+    public interface IUpdateNuspecCommand : IPackageCommand
+    {
+    }
+}
diff --git a/NuKeeper.Update/Process/IUpdateProjectImportsCommand.cs b/NuKeeper.Update/Process/IUpdateProjectImportsCommand.cs
@@ -0,0 +1,6 @@
+namespace NuKeeper.Update.Process
+{
+    public interface IUpdateProjectImportsCommand : IPackageCommand
+    {
+    }
+}
diff --git a/NuKeeper.Update/Process/NuGetFileRestoreCommand.cs b/NuKeeper.Update/Process/NuGetFileRestoreCommand.cs
@@ -13,14 +13,17 @@ namespace NuKeeper.Update.Process
     public class NuGetFileRestoreCommand : IFileRestoreCommand
     {
         private readonly INuKeeperLogger _logger;
+        private readonly INuGetPath _nuGetPath;
         private readonly IExternalProcess _externalProcess;
 
         public NuGetFileRestoreCommand(
             INuKeeperLogger logger,
-            IExternalProcess externalProcess = null)
+            INuGetPath nuGetPath,
+            IExternalProcess externalProcess)
         {
             _logger = logger;
-            _externalProcess = externalProcess ?? new ExternalProcess(logger);
+            _nuGetPath = nuGetPath;
+            _externalProcess = externalProcess;
         }
 
         public async Task Invoke(FileInfo file, NuGetSources sources)
@@ -33,7 +36,7 @@ public async Task Invoke(FileInfo file, NuGetSources sources)
                 return;
             }
 
-            var nuget = NuGetPath.FindExecutable();
+            var nuget = _nuGetPath.Executable;
 
             if (string.IsNullOrWhiteSpace(nuget))
             {

diff --git a/NuKeeper.Update/Process/NuGetPath.cs b/NuKeeper.Update/Process/NuGetPath.cs
@@ -1,12 +1,24 @@
 using System;
 using System.IO;
 using System.Linq;
+using NuKeeper.Inspection.Logging;
 
 namespace NuKeeper.Update.Process
 {
-    public static class NuGetPath
+    public class NuGetPath : INuGetPath
     {
-        public static string FindExecutable()
+        private readonly INuKeeperLogger _logger;
+        private readonly Lazy<string> _executablePath;
+
+        public NuGetPath(INuKeeperLogger logger)
+        {
+            _logger = logger;
+            this._executablePath = new Lazy<string>(FindExecutable);
+        }
+
+        public string Executable => _executablePath.Value;
+
+        private string FindExecutable()
         {
             var localNugetPath = FindLocalNuget();
 
@@ -15,36 +27,39 @@ public static string FindExecutable()
                 return localNugetPath;
             }
 
-
             return FindNugetInPackagesUnderProfile();
         }
 
-        private static string FindLocalNuget()
+        private string FindLocalNuget()
         {
             var appDir = AppDomain.CurrentDomain.BaseDirectory;
             var fullPath = Path.Combine(appDir, "NuGet.exe");
             if (File.Exists(fullPath))
             {
+                _logger.Detailed("Found NuGet.exe: " + fullPath);
                 return fullPath;
             }
 
             return string.Empty;
         }
 
-        private static string FindNugetInPackagesUnderProfile()
+        private string FindNugetInPackagesUnderProfile()
         {
             var profile = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
 
             if (string.IsNullOrWhiteSpace(profile))
             {
-                throw new Exception("Could not find user profile path");
+                _logger.Error("Could not find user profile path");
+                return string.Empty;
             }
 
             var commandlinePackageDir = Path.Combine(profile, ".nuget", "packages", "nuget.commandline");
+            _logger.Detailed("Checking for NuGet.exe in packages directory: " + commandlinePackageDir);
 
             if (!Directory.Exists(commandlinePackageDir))
             {
-                throw new Exception("Could not find nuget commandline path: " + commandlinePackageDir);
+                _logger.Error("Could not find nuget commandline path: " + commandlinePackageDir);
+                return string.Empty;
             }
 
             var highestVersion = Directory.GetDirectories(commandlinePackageDir)
@@ -53,11 +68,15 @@ private static string FindNugetInPackagesUnderProfile()
 
             if (string.IsNullOrWhiteSpace(highestVersion))
             {
-                throw new Exception("Could not find a version of nuget.commandline");
+                _logger.Error("Could not find a version of nuget.commandline");
+                return string.Empty;
             }
 
             var nugetProgramPath = Path.Combine(highestVersion, "tools", "NuGet.exe");
-            return Path.GetFullPath(nugetProgramPath);
+            var fullPath = Path.GetFullPath(nugetProgramPath);
+            _logger.Detailed("Found NuGet.exe: " + fullPath);
+
+            return fullPath;
         }
     }
 }
diff --git a/NuKeeper.Update/Process/NuGetUpdatePackageCommand.cs b/NuKeeper.Update/Process/NuGetUpdatePackageCommand.cs
@@ -9,17 +9,20 @@
 
 namespace NuKeeper.Update.Process
 {
-    public class NuGetUpdatePackageCommand : IPackageCommand
+    public class NuGetUpdatePackageCommand : INuGetUpdatePackageCommand
     {
         private readonly IExternalProcess _externalProcess;
         private readonly INuKeeperLogger _logger;
+        private readonly INuGetPath _nuGetPath;
 
         public NuGetUpdatePackageCommand(
             INuKeeperLogger logger,
-            IExternalProcess externalProcess = null)
+            INuGetPath nuGetPath,
+            IExternalProcess externalProcess)
         {
             _logger = logger;
-            _externalProcess = externalProcess ?? new ExternalProcess(logger);
+            _nuGetPath = nuGetPath;
+            _externalProcess = externalProcess;
         }
 
         public async Task Invoke(PackageInProject currentPackage,
@@ -33,7 +36,7 @@ public async Task Invoke(PackageInProject currentPackage,
 
             var projectPath = currentPackage.Path.Info.DirectoryName;
 
-            var nuget = NuGetPath.FindExecutable();
+            var nuget = _nuGetPath.Executable;
             if (string.IsNullOrWhiteSpace(nuget))
             {
                 _logger.Normal("Cannot find NuGet exe for package update");

diff --git a/NuKeeper.Update/Process/UpdateNuspecCommand.cs b/NuKeeper.Update/Process/UpdateNuspecCommand.cs
@@ -11,7 +11,7 @@
 
 namespace NuKeeper.Update.Process
 {
-    public class UpdateNuspecCommand : IPackageCommand
+    public class UpdateNuspecCommand : IUpdateNuspecCommand
     {
         private readonly INuKeeperLogger _logger;
 

diff --git a/NuKeeper.Update/Process/UpdateProjectImportsCommand.cs b/NuKeeper.Update/Process/UpdateProjectImportsCommand.cs
@@ -11,7 +11,7 @@
 
 namespace NuKeeper.Update.Process
 {
-    public class UpdateProjectImportsCommand : IPackageCommand
+    public class UpdateProjectImportsCommand : IUpdateProjectImportsCommand
     {
         public async Task Invoke(PackageInProject currentPackage,
             NuGetVersion newVersion, PackageSource packageSource, NuGetSources allSources)

diff --git a/NuKeeper.Update/UpdateRunner.cs b/NuKeeper.Update/UpdateRunner.cs
@@ -11,10 +11,26 @@ namespace NuKeeper.Update
     public class UpdateRunner : IUpdateRunner
     {
         private readonly INuKeeperLogger _logger;
+        private readonly IFileRestoreCommand _fileRestoreCommand;
+        private readonly INuGetUpdatePackageCommand _nuGetUpdatePackageCommand;
+        private readonly IDotNetUpdatePackageCommand _dotNetUpdatePackageCommand;
+        private readonly IUpdateProjectImportsCommand _updateProjectImportsCommand;
+        private readonly IUpdateNuspecCommand _updateNuspecCommand;
 
-        public UpdateRunner(INuKeeperLogger logger)
+        public UpdateRunner(
+            INuKeeperLogger logger,
+            IFileRestoreCommand fileRestoreCommand,
+            INuGetUpdatePackageCommand nuGetUpdatePackageCommand,
+            IDotNetUpdatePackageCommand dotNetUpdatePackageCommand,
+            IUpdateProjectImportsCommand updateProjectImportsCommand,
+            IUpdateNuspecCommand updateNuspecCommand)
         {
             _logger = logger;
+            _fileRestoreCommand = fileRestoreCommand;
+            _nuGetUpdatePackageCommand = nuGetUpdatePackageCommand;
+            _dotNetUpdatePackageCommand = dotNetUpdatePackageCommand;
+            _updateProjectImportsCommand = updateProjectImportsCommand;
+            _updateNuspecCommand = updateNuspecCommand;
         }
 
         public async Task Update(PackageUpdateSet updateSet, NuGetSources sources)
@@ -37,23 +53,23 @@ private IReadOnlyCollection<IPackageCommand> GetUpdateCommands(
                 case PackageReferenceType.PackagesConfig:
                     return new IPackageCommand[]
                     {
-                        new NuGetFileRestoreCommand(_logger),
-                        new NuGetUpdatePackageCommand(_logger)
+                        _fileRestoreCommand,
+                        _nuGetUpdatePackageCommand
                     };
 
                 case PackageReferenceType.ProjectFileOldStyle:
                     return new IPackageCommand[]
                     {
-                        new UpdateProjectImportsCommand(),
-                        new NuGetFileRestoreCommand(_logger),
-                        new DotNetUpdatePackageCommand(_logger)
+                        _updateProjectImportsCommand,
+                        _fileRestoreCommand,
+                        _dotNetUpdatePackageCommand
                     };
 
                 case PackageReferenceType.ProjectFile:
-                    return new[] {new DotNetUpdatePackageCommand(_logger) };
+                    return new[] { _dotNetUpdatePackageCommand };
 
                 case PackageReferenceType.Nuspec:
-                    return new[] { new UpdateNuspecCommand(_logger) };
+                    return new[] { _updateNuspecCommand };
 
                 default:
                     throw new ArgumentOutOfRangeException(nameof(packageReferenceType));

diff --git a/NuKeeper/ContainerUpdateRegistration.cs b/NuKeeper/ContainerUpdateRegistration.cs
@@ -10,8 +10,13 @@ public static class ContainerUpdateRegistration
         public static void Register(Container container)
         {
             container.Register<IFileRestoreCommand, NuGetFileRestoreCommand>();
+            container.Register<INuGetUpdatePackageCommand, NuGetUpdatePackageCommand>();
+            container.Register<IDotNetUpdatePackageCommand, DotNetUpdatePackageCommand>();
+            container.Register<IUpdateProjectImportsCommand, UpdateProjectImportsCommand>();
+            container.Register<IUpdateNuspecCommand, UpdateNuspecCommand>();
             container.Register<IExternalProcess, ExternalProcess>();
             container.Register<IUpdateRunner, UpdateRunner>();
+            container.Register<INuGetPath, NuGetPath>();
         }
     }
 }
diff --git a/NuKeeper/Engine/RepositoryUpdater.cs b/NuKeeper/Engine/RepositoryUpdater.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Threading.Tasks;
 using NuKeeper.Configuration;
 using NuKeeper.Engine.Packages;
@@ -97,7 +98,10 @@ private async Task<int> DoTargetUpdates(
                 return 0;
             }
 
-            await _solutionsRestore.Restore(git.WorkingFolder, sources);
+            if (AnyProjectRequiresNuGetRestore(targetUpdates))
+            {
+                await _solutionsRestore.Restore(git.WorkingFolder, sources);
+            }
 
             var updatesDone = await UpdateAllTargets(git, repository, targetUpdates, sources);
 
@@ -113,6 +117,12 @@ private async Task<int> DoTargetUpdates(
             return updatesDone;
         }
 
+        private static bool AnyProjectRequiresNuGetRestore(IEnumerable<PackageUpdateSet> targetUpdates)
+        {
+            return targetUpdates.SelectMany(u => u.CurrentPackages)
+                .Any(p => p.Path.PackageReferenceType != PackageReferenceType.ProjectFile);
+        }
+
         private static void GitInit(IGitDriver git, RepositoryData repository)
         {
             git.Clone(repository.Pull.Uri);