Fix empty combobox when package is not present in project file by martinrrm · Pull Request #4844 · NuGet/NuGet.Client

martinrrm · 2022-10-05T07:50:40Z

Bug

Fixes: NuGet/Home#12057

Regression? Last working version:

Description

For some type of projects there are installed packages that are not listed in the project file (.csproj). This was not considered when developing the combobox version and breaks when trying to get the OriginalVersion.

Before:

After:

PR Checklist

nkolev92 · 2022-10-05T18:07:55Z

src/NuGet.Clients/NuGet.PackageManagement.UI/Models/PackageDetailControlModel.cs


            // Add installed version if the project is PackageReference
-            if (_nugetProjects.Any() && installedDependency != null && installedDependency.VersionRange != null && _nugetProjects.First().ProjectStyle.Equals(ProjectModel.ProjectStyle.PackageReference))
+            if (_nugetProjects.Any() && installedDependency != null && installedDependency.VersionRange.OriginalString != null && _nugetProjects.First().ProjectStyle.Equals(ProjectModel.ProjectStyle.PackageReference))


I'm trying to understand why original string would be null when a package is not in the csproj file.

Is the package not being in the csproj file the problem, or is it the fact that it's autoreferenced?

I'm not sure if it is because is auto referenced but it could also be the problem. I was taking the .csproj as the only source for packages. When creating the versions from the project file we create the object from a string using VersionRange.Parse(string), this is the only way the attribute OriginalString is populated, so I think because this package is autoreferenced, im not sure how but probably using new VersionRange(Version)

I'd find it surprising if we're generating the Version Range differently for autoreferenced package.

At least in the nomination workflow. Maybe there's something we do differently in the PM UI side?

I'll double check this behavior

I found that when the package is autoreferenced we use new VersionRange() and this doesnt populates the OriginalString attribute. https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.PackageManagement/BuildIntegratedPackageReference.cs#L91

I see.
A few ideas:

We can consider making sure that we have an original version there no? Just doing that is probably not the right fix, but it's a valid consideration.

If OriginalString isn't an option should we show the normalized version instead?

Please note that for an auto referenced version you should not be able to update the version through the PM UI.

OriginalString cannot be set, only by using VersionRange.Parse()

The change in L170 is not affecting functionality, right we are displaying the version that the user wrote in the .csproj at the top and then all the versions, with this fix, if the OriginalString is null then we will display the latest version and then all the versions, like before but with filtering

I tried considering using the normalized version when the OriginalString was null, but this created more issues when adding the blocking versions later in the code and would need more changes.

martinrrm · 2022-10-06T00:35:54Z

src/NuGet.Clients/NuGet.PackageManagement.UI/Models/DetailControlModel.cs

            foreach (var version in blockedVersions)
            {
-                _versions.Add(new DisplayVersion(version, string.Empty, isValidVersion: false));
+                _versions.Add(new DisplayVersion(version, additionalInfo: null, isValidVersion: false));


I'm doing this to keep the same format like in https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Clients/NuGet.PackageManagement.UI/Models/PackageDetailControlModel.cs#L218

martinrrm · 2022-10-06T00:36:14Z

src/NuGet.Clients/NuGet.PackageManagement.UI/Models/PackageDetailControlModel.cs

            get
            {
-                return SelectedVersion != null && !IsSelectedVersionInstalled;
+                return SelectedVersion != null && !IsSelectedVersionInstalled && !InstalledVersionIsAutoReferenced;


If the package is autoreferenced then block the Update button

nkolev92 · 2022-10-06T02:05:20Z

Manually debugging the fix appears to work.
There are some null ref exceptions being raised. I'll investigate whether the fix has anything to do with that.

nkolev92 · 2022-10-06T02:10:36Z

Initial look it doesn't seem related. I'll merge when the build passes.

#4848) Co-authored-by: Martin Ruiz <martin.ruiz.mares@gmail.com>

Insert 6.4.0-rc.123 into rel/d17.4 on 11/07/2022 23:47:12 * tag '6.4.0.123': (60 commits) fix a logic error that caused AbandonedMutexException while executing migrations (release-6.4.x) (NuGet#4895) unblock source build failing due to fatal: transport 'file' not allowed error (NuGet#4867) (NuGet#4874) Signing: update to August 2022 CTL (NuGet#4791) (NuGet#4850) Merged PR 422933: Prefer BCL Directory create API over helper class (7.0.1xx-rc2) Fix empty combobox when package is not present in project file (NuGet#4844) (NuGet#4848) Fix component detection alert for microsoft.owin package (NuGet#4841) (NuGet#4845) Make release label RC, move to escrow mode Adds special case to include transitive origins in GetInstalledAndTransitivePackagesAsync API (NuGet#4824) Add longPathAware manifest to NuGet.Build.Tasks.Console (NuGet#4830) VsPackageInstallerServices should not post ProjectNotNominatedException faults (NuGet#4814) Skip test GetOrCreateAsync_WithUnhandledExceptionInPlugin_Throws (NuGet#4831) Improve OptProf pipeline job run names (NuGet#4825) Increase HttpClientHandler.MaxConnectionsPerServer to 64 to improve PM UI performance in Visual Studio (NuGet#4798) Suppress CA2213 warnings to unblock dev branch (NuGet#4823) Ensure IsVsOfflineFeed is calculated correctly on 64-bit machines (NuGet#4817) Add better handling of AggregateExceptions in static graph-based restore (NuGet#4809) Add Component Detection task into each pipeline (NuGet#4813) Localizes nuget.exe with default, embedded resource assembly lookup (NuGet#4773) Removes BrowseObjectBase class in NuGet Solution Explorer (NuGet#4807) Improve TryCreateContext (NuGet#4762) ...

fix empty combobox when package is not present in project file

97d5cc8

martinrrm requested a review from a team as a code owner October 5, 2022 07:50

remove unnecesary guard

49ecb35

jeffkl previously approved these changes Oct 5, 2022

View reviewed changes

erdembayar previously approved these changes Oct 5, 2022

View reviewed changes

nkolev92 reviewed Oct 5, 2022

View reviewed changes

fix filtering and disable button

d0a0ab0

martinrrm dismissed stale reviews from erdembayar and jeffkl via d0a0ab0 October 6, 2022 00:23

martinrrm commented Oct 6, 2022

View reviewed changes

nkolev92 approved these changes Oct 6, 2022

View reviewed changes

nkolev92 merged commit d8a2220 into dev Oct 6, 2022

nkolev92 deleted the dev-martinrrm-fix-combobox-package-not-in-projectfile branch October 6, 2022 16:47

nkolev92 pushed a commit that referenced this pull request Oct 7, 2022

Fix empty combobox when package is not present in project file (#4844)

3a95760

dominoFire mentioned this pull request Oct 7, 2022

Details for Installed MAUI packages are missing NuGet Project PM UI NuGet/Home#12130

Closed

nkolev92 added a commit that referenced this pull request Oct 7, 2022

Fix empty combobox when package is not present in project file (#4844) (

445850b

#4848) Co-authored-by: Martin Ruiz <martin.ruiz.mares@gmail.com>

Conversation

martinrrm commented Oct 5, 2022

Bug

Description

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nkolev92 Oct 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nkolev92 commented Oct 6, 2022

Uh oh!

nkolev92 commented Oct 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nkolev92 Oct 5, 2022 •

edited

Loading