Improvements for nuget.exe install #1634

emgarten · 2017-08-10T08:45:18Z

Improvements for nuget.exe install

Allow updating packages when -ExcludeVersion is used
Improved detection of already installed packages when -ExcludeVersion is used
Adding -Framework switch to allow setting the target framework used when resolving dependencies.
Remove NU1000 code from packages.config restore errors.
Avoid unneeded downloads when a version is not given and the package is cached.
Disable parallel for mono
Display errors and return a non-zero exit when install on packages.config fails
Remove old files during upgrades with -ExcludeVersion
Adding tests for nuget.exe install -ExcludeVersion scenarios and failure cases.
Improving current nuget.exe install tests to avoid packages getting shared through the global packages folder.
Removing old skipped install command tests that no longer apply
Re-enabling caching test for install

Fixes NuGet/Home#5743
Fixes NuGet/Home#5737
Fixes NuGet/Home#5736
Fixes NuGet/Home#5741
Fixes NuGet/Home#5017
Fixes NuGet/Home#3957
Fixes NuGet/Home#2405
Partial fix for NuGet/Home#3244

mishra14 · 2017-08-10T17:20:54Z

@emgarten not so great if it doesn't build!! :P

nkolev92 · 2017-08-10T17:52:38Z

@mishra14 I assume that's why it's a WIP :P

emgarten · 2017-08-11T08:37:07Z

Tests are passing on teamcity

zhili1208 · 2017-08-11T21:54:14Z

src/NuGet.Clients/NuGet.CommandLine/Commands/InstallCommand.cs

                        CultureInfo.CurrentCulture,
                        LocalizedResourceManager.GetString("RestoreCommandPackageRestoreOptOutMessage"),
                        NuGetResources.PackageRestoreConsentCheckBoxText.Replace("&", ""));
                    Console.WriteLine(message);
                }

-                return PerformV2Restore(configFilePath, installPath);
+                return PerformV2RestoreAsync(configFilePath, installPath);


why not await here?

it returns the task and is awaited higher up

zhili1208 · 2017-08-11T21:54:24Z

src/NuGet.Clients/NuGet.CommandLine/Commands/InstallCommand.cs

            }
            else
            {
                var packageId = Arguments[0];
                var version = Version != null ? new NuGetVersion(Version) : null;
-                return InstallPackage(packageId, version, installPath);
+                return InstallPackageAsync(packageId, version, installPath);


also returning the task

zhili1208 · 2017-08-11T22:02:22Z

src/NuGet.Clients/NuGet.CommandLine/Commands/DownloadCommandBase.cs

@@ -45,6 +45,11 @@ public ICollection<string> Source
        [Option(typeof(NuGetCommand), "CommandPackageSaveMode")]
        public string PackageSaveMode { get; set; }

+        /// <summary>
+        /// If true the global packages folder will be added as a source.


If true the global packages folder will not be added as a source

zhili1208 · 2017-08-11T22:05:07Z

src/NuGet.Clients/NuGet.CommandLine/Commands/DownloadCommandBase.cs

@@ -117,7 +122,7 @@ protected IReadOnlyCollection<Configuration.PackageSource> GetPackageSources(Con
            var availableSources = SourceProvider.LoadPackageSources().Where(source => source.IsEnabled);
            var packageSources = new List<Configuration.PackageSource>();

-            if (!NoCache)
+            if (!NoCache && !ExcludeCacheAsSource)


Is "NoCache" also used in other place? why can't just use "NoCache" to exclude cache

ExcludeCacheAsSource is a subset of NoCache

zhili1208 · 2017-08-11T22:08:41Z

src/NuGet.Clients/NuGet.CommandLine/Common/InstallCommandProject.cs

+            var packageReference = installedPackagesList.Where(p => p.PackageIdentity.Equals(packageIdentity)).FirstOrDefault();
+            if (packageReference == null)
+            {
+                // Package does not exit


mishra14

how will people realize how ExcludeCacheAsSource work?

mishra14 · 2017-08-11T22:15:57Z

src/NuGet.Clients/NuGet.CommandLine/Commands/InstallCommand.cs

@@ -198,6 +207,17 @@ internal string ResolveInstallPath()
                {
                    GetDownloadResultUtility.CleanUpDirectDownloads(downloadContext);
                }
+
+                if (!result.Restored || failedEvents.Count > 0)


mishra14 · 2017-08-11T22:26:46Z

src/NuGet.Clients/NuGet.CommandLine/Commands/InstallCommand.cs

+                // Avoid searching for the highest version in the global packages folder,
+                // it needs to come from the feeds instead. Once found it may come from
+                // the global packages folder unless NoCache is true.
+                ExcludeCacheAsSource = true;


this seems a bit arbitrary. how will people realize what is the resolution logic?

Discussed offline. This is fine as we set ExcludeCacheAsSource only if we see floating versions in the dependency list.

mishra14 · 2017-08-11T22:52:11Z

src/NuGet.Clients/NuGet.CommandLine/Commands/RestoreCommand.cs

@@ -333,7 +333,7 @@ private void ReadSettings(PackageRestoreInputs packageRestoreInputs)
                    Settings.Priority.Select(x => Path.Combine(x.Root, x.FileName)),
                    packageSources.Select(x => x.Source),
                    installCount,
-                    collectorLogger.Errors.Concat(failedEvents.Select(e => new RestoreLogMessage(LogLevel.Error, e.Exception.Message))));
+                    collectorLogger.Errors.Concat(failedEvents.Select(e => new RestoreLogMessage(LogLevel.Error, NuGetLogCode.Undefined, e.Exception.Message))));


mishra14 · 2017-08-11T23:00:34Z

src/NuGet.Core/NuGet.PackageManagement/Projects/FolderNuGetProject.cs

+        /// <exception cref="ArgumentNullException">Thrown if <paramref name="targetFramework" /> is <c>null</c>.</exception>
+        public FolderNuGetProject(string root, PackagePathResolver packagePathResolver, NuGetFramework targetFramework)
+        {
+            if (targetFramework == null)


nit: simplify these just as root and packagePathResolver

mishra14

Discussed offline. Looks good

* Allow updating packages when -ExcludeVersion is used * Improved detection of already installed packages when -ExcludeVersion is used * Adding -Framework switch to allow setting the target framework used when resolving dependencies. * Remove NU1000 code from packages.config restore errors. * Avoid unneeded downloads when a version is not given and the package is cached. * Disable parallel for mono * Display errors and return a non-zero exit when install on packages.config fails * Remove old files during upgrades with -ExcludeVersion Fixes NuGet/Home#5743 Fixes NuGet/Home#5737 Fixes NuGet/Home#5736 Fixes NuGet/Home#5741 Fixes NuGet/Home#5017 Fixes NuGet/Home#3957 Fixes NuGet/Home#2405

NuGet/NuGet.Client#1634 introduced a Framework option for the install command.

dnfclas added the cla-already-signed label Aug 10, 2017

emgarten force-pushed the dev-emgarten-cmdInstall branch 2 times, most recently from e434394 to 6d5bed7 Compare August 11, 2017 06:49

emgarten mentioned this pull request Aug 11, 2017

Inconsistent -NoCache behavior NuGet/Home#3244

Closed

emgarten force-pushed the dev-emgarten-cmdInstall branch from 6d5bed7 to 406dc88 Compare August 11, 2017 07:30

emgarten changed the title ~~[WIP] the great nuget.exe install clean up~~ Improvements for nuget.exe install Aug 11, 2017

emgarten requested review from joelverhagen, rohit21agrawal, PatoBeltran, nkolev92, zhili1208, alpaix, mishra14 and jainaashish and removed request for joelverhagen August 11, 2017 08:34

zhili1208 reviewed Aug 11, 2017

View reviewed changes

mishra14 suggested changes Aug 11, 2017

View reviewed changes

mishra14 approved these changes Aug 11, 2017

View reviewed changes

emgarten force-pushed the dev-emgarten-cmdInstall branch from 406dc88 to 8614255 Compare August 12, 2017 00:59

emgarten merged commit 5b6edbb into dev Aug 12, 2017

emgarten deleted the dev-emgarten-cmdInstall branch August 12, 2017 02:50

hickford added a commit to hickford/docs.microsoft.com-nuget that referenced this pull request Nov 3, 2017

Document Framework option for install command

eff0d37

NuGet/NuGet.Client#1634 introduced a Framework option for the install command.

hickford mentioned this pull request Nov 3, 2017

Document Framework option for install command NuGet/docs.microsoft.com-nuget#552

Merged

nkolev92 mentioned this pull request Nov 6, 2017

Regression in preview nuget.exe v4.4.0 install command NuGet/Home#6144

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements for nuget.exe install #1634

Improvements for nuget.exe install #1634

emgarten commented Aug 10, 2017 •

edited

Loading

mishra14 commented Aug 10, 2017 •

edited

Loading

nkolev92 commented Aug 10, 2017

emgarten commented Aug 11, 2017

zhili1208 Aug 11, 2017

emgarten Aug 11, 2017

zhili1208 Aug 11, 2017

emgarten Aug 11, 2017

zhili1208 Aug 11, 2017

zhili1208 Aug 11, 2017

emgarten Aug 11, 2017

zhili1208 Aug 11, 2017

mishra14 left a comment

mishra14 Aug 11, 2017

mishra14 Aug 11, 2017

mishra14 Aug 11, 2017

mishra14 Aug 11, 2017

mishra14 Aug 11, 2017

mishra14 left a comment

Improvements for nuget.exe install #1634

Improvements for nuget.exe install #1634

Conversation

emgarten commented Aug 10, 2017 • edited Loading

mishra14 commented Aug 10, 2017 • edited Loading

nkolev92 commented Aug 10, 2017

emgarten commented Aug 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mishra14 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mishra14 left a comment

Choose a reason for hiding this comment

emgarten commented Aug 10, 2017 •

edited

Loading

mishra14 commented Aug 10, 2017 •

edited

Loading