Multiple installed versions of the same package are not handled correctly

Question

Multiple installed versions of the same package are not handled correctly

laurihelkkula opened this issue 3 years ago · 3 comments

laurihelkkula commented 3 years ago

🐛 Bug Report

Nukeeper does find all packages to update, if multiple versions of the same package are in use.

Expected behavior

All package updates are found.

Reproduction steps

In an empty folder, create two projects with a single package reference for different versions of the same package:

dotnet new console -o a
dotnet new console -o b
dotnet add .\a\a.csproj package xunit -v 2.4.0
dotnet add .\b\b.csproj package xunit

Running nuget inspect shows:

Found 2 packages
Found 2 packages in use, 1 distinct, in 2 projects.
xunit
Found 0 possible updates
Found no package updates

I expect it to find the update xunit 2.4.0 -> xunit 2.4.1 in project a. The update command has the same problem.

Configuration

Version: 0.35.0

Platform if applicable:

Preliminary debugging

I think the method BulkPackageLookup.FindVersionUpdates does not handle this situation correctly.

In the example case, the packages parameter contains xunit.2.4.0 and xunit.2.4.1, as I would expect. The statement starting from line 32 checks updates for only the highest version of each package.

The code probably intended to call FindVersionUpdate only once per package. Reducing the amount of lookups is good, but at the same time the implementation ignores the non-highest installed versions of the same package.

Some variations in behavior:
If installed version are 2.3.0 and 2.4.0: One update is shown (2.4.0 -> 2.4.1). Expected two updates.
If installed versions are 1.9.0 and 2.4.1: No updates are shown. Expected one update.
If installed versions are 1.9.0 and 2.4.1, and allowed change is minor: Two updates are shown, as expected.

Workaround: running nuget inspect for only single project, instead of multiple projects, will show the update as expected.

There is a test for this situation. I think the assertions are not correct.

Answer 1 · 2021-10-01T12:12:47.000Z

If you agree that the current behavior is incorrect, I can create a PR.

A quick example of a fix

diff --git a/NuKeeper.Inspection/NuGetApi/BulkPackageLookup.cs b/NuKeeper.Inspection/NuGetApi/BulkPackageLookup.cs
index 8f1f0a4..0f937ce 100644
--- a/NuKeeper.Inspection/NuGetApi/BulkPackageLookup.cs
+++ b/NuKeeper.Inspection/NuGetApi/BulkPackageLookup.cs
@@ -32,8 +32,15 @@ namespace NuKeeper.Inspection.NuGetApi
             var lookupTasks = packages
                 .Distinct()
                 .GroupBy(pi => (pi.Id, MaxVersion: GetMaxVersion(pi, allowedChange)))
-                .Select(HighestVersion)
-                .Select(id => new { Package = id, Update = _packageLookup.FindVersionUpdate(id, sources, allowedChange, usePrerelease) })
+                .Select(groupedByAllowedChange => new
+                {
+                    HighestVersion = HighestVersion(groupedByAllowedChange),
+                    All = groupedByAllowedChange
+                })
+                .Select(versions => new {
+                    Packages = versions.All,
+                    Update = _packageLookup.FindVersionUpdate(versions.HighestVersion, sources, allowedChange, usePrerelease)
+                })
                 .ToList();

             await Task.WhenAll(lookupTasks.Select(l => l.Update));
@@ -42,7 +49,10 @@ namespace NuKeeper.Inspection.NuGetApi

             foreach (var lookupTask in lookupTasks)
             {
-                ProcessLookupResult(lookupTask.Package, lookupTask.Update.Result, result);
+                foreach (var package in lookupTask.Packages)
+                {
+                    ProcessLookupResult(package, lookupTask.Update.Result, result);
+                }
             }

             return result;

And the unit test:

diff --git a/NuKeeper.Inspection.Tests/NuGetApi/BulkPackageLookupTests.cs b/NuKeeper.Inspection.Tests/NuGetApi/BulkPackageLookupTests.cs
index acc32d3..ac1af87 100644
--- a/NuKeeper.Inspection.Tests/NuGetApi/BulkPackageLookupTests.cs
+++ b/NuKeeper.Inspection.Tests/NuGetApi/BulkPackageLookupTests.cs
@@ -165,8 +165,9 @@ namespace NuKeeper.Inspection.Tests.NuGetApi
                 Arg.Any<NuGetSources>(), Arg.Any<VersionChange>(), Arg.Any<UsePrerelease>());

             var packages = results.Select(kvp => kvp.Key);
-            Assert.That(results.Count, Is.EqualTo(1));
-            Assert.That(packages, Has.Some.Matches<PackageIdentity>(pi => pi.Id == "foo"));
+            Assert.That(results.Count, Is.EqualTo(2));
+            Assert.That(packages, Has.Some.Matches<PackageIdentity>(pi => pi.Id == "foo" && pi.Version == new NuGetVersion(1, 2, 3)));
+            Assert.That(packages, Has.Some.Matches<PackageIdentity>(pi => pi.Id == "foo" && pi.Version == new NuGetVersion(1, 3, 4)));
             Assert.That(packages, Has.None.Matches<PackageIdentity>(pi => pi.Id == "bar"));
         }

Answer 2 · 2021-10-02T06:15:45.000Z

Hi I have the same issue. Will be nice to have this.
The actual problem is that in our team we use local nuget feeds for packages. We have two sections for our own packages - one for build on a build server and one for local builds. Like this


   <ItemGroup Condition="'$(BUILD_SERVER_ENV)'=='true'">
        <PackageReference Include="SomeAssembly" Version="1.3.321" />
        .......
    </ItemGroup>

    <ItemGroup Condition="'$(BUILD_SERVER_ENV)' == ''">
        <PackageReference Include="SomeAssembly" />
        .......
    </ItemGroup>

Answer 3 · 2021-12-31T12:18:53.000Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.