Update Package Management to Latest Stable NuGet (4.x)

[ilexp]

Summary

As a prerequisite of issue #573, a NuGet update will be required. Regardless of this requirement, updating to the latest NuGet for package management is overdue, considering that Duality is still using 2.12, while 4.x has been out for a while.

Analysis

• Check out the new NuGet client package.
• The package manager classes and tests need to be updated. This will likely be the biggest work item.
  • Address API and behavior changes.
  • Make sure it can consume both Duality packages and regular NuGet library packages.
  • Deal with platform-dependent dependencies and files.
  • Consider writing some more unit tests for this.
  • Consider moving it to a separate project (still part of the Duality editor package in deployment though) to keep direct editor dependencies clean.
• Investigate whether the nightly build needs to be updated.
  • It uses command line nuget, probably the system installed version.
  • Check out which version is used on the CI build and locally on the dev machine.
  • Might be okay as-is, but double-check the used nuget version for publishing packages, as it will soon be required to use the newest one.
• All .nuspec files might need to be updated - or they might not. Investigate this.
  • Keep the changes minimal, only do what is required to make and keep them consumable when switching to netstandard1.1 later on.
  • Create a test project that lets VS 2017 export a netstandard1.1 nuget package and see what the spec says. Especially the framework paths for files and dependencies should be interesting.
  • Make sure they're still as compatible as possible. Don't use features that aren't required.
  • If changes are required, will need to update dependencies as well, including OpenTK, FarseerDuality, AdamsLair.Winforms, etc.
• Backwards compatibility is key. Don't require a new NuGet for building a package that doesn't need its features.
• Due to the big new set of NuGet-related dependencies, update the Publish Game script copy blacklist, as well as potentially existing .gitignore files and nightly builder binary package config.
• Consider adding an updated .gitignore file to the binary release package by default.

[ilexp]

Seems like NuGet v3 adopted a micro-dependency model and the former NuGet.Core is no longer updated and locked to v2.x. This could get really ugly - according to the following links, there might be between 24 and 40 dependencies required to get this up an running.

• https://daveaglick.com/posts/exploring-the-nuget-v3-libraries-part-1
• https://daveaglick.com/posts/exploring-the-nuget-v3-libraries-part-2
• https://daveaglick.com/posts/exploring-the-nuget-v3-libraries-part-3

Investigate further. If it turns out that there will actually be that many different assemblies required in the working directory, consider adding a prerequisite issue to find out if there are ways to keep the main directory clean.

[ilexp]

Implementation related to the above blog: https://github.com/Wyamio/Wyam/tree/1897149192a0cb1efe6378aa6185fbaab1eadf9d/src/core/Wyam.Configuration/NuGet

Latest version: https://github.com/Wyamio/Wyam/tree/develop/src/core/Wyam.Configuration/NuGet

[ilexp]

Did a quick test and found that there will be at least 15 individual assemblies required, plus 15 optional XML docs files. This is definitely too much to put into the main directory. Created issue #578 as a dependency for this.

[ilexp]

Closed issue #578 unresolved due to running into more problems than expected while implementing it, skewing its cost / gain ratio for the worse. Could be re-opened in isolation at some later point, but should no longer be considered a dependency of this issue.

[ilexp]

Immediate ToDo

• Create a separate test project and prototype package discovery, install and potentially updates, should they differ a lot from installs.
  • Set up a test repository of packages, both Duality and others
• Consider moving Duality package management into a separate editor library.
• Update the NuGet dependency to 4.X.
  • Gradually fix all the broken things until it compiles.
  • Gradually fix all the failing tests until it's completely green again.
  • Deal with target platforms and platform-dependent dependencies.
  • Make sure it can consume both Duality packages and regular NuGet library packages.
• Investigate whether the nightly build needs to be updated.
• Investigate whether the nuspec files need to be updated, including dependencies outside the main repo.
• Due to the big new set of NuGet-related dependencies, update the Publish Game script copy blacklist, as well as potentially existing .gitignore files and nightly builder binary package config.
• Consider adding an updated .gitignore file to the binary release package by default.

[ilexp]

Progress

• Started experimenting with a standalone test project to get a feel for how to use the new NuGet libraries.
  • Metadata discovery works.
  • Simple package install works.
  • Found that the FolderNuGetProject doesn't seem to keep track of installed packages or select target frameworks at all. Need to implement a custom one resembling MSBuildNuGetProject or WyamFolderNuGetProject?
• Created an issue on the NuGet repository to double-check which of the inconveniences so far are actually by design, which are potentially better in the future, and which are just due to me not understanding NuGet.

Immediate ToDo

• Create a separate test project to figure out how to use the new NuGet and how to integrate it properly.
  • Implement a custom NuGetProject.
  • Find the best matching package contents based on the current target framework.
  • Discover packages by tag.
  • Sandbox NuGet, i.e. not depend on any settings outside the project, especially not package sources.
  • Consider implementing a custom ISettings class based on Settings default values and without external input.
  • This would also fix the problem of the auto-created nuget config file.
  • Set up a test repository of packages, both Duality and others.
  • Do some test installs and dependency resolves.
• Consider moving Duality package management into a separate editor library.
• Update the NuGet dependency to 4.X.
  • Gradually fix all the broken things until it compiles.
  • Gradually fix all the failing tests until it's completely green again.
  • Deal with target platforms and platform-dependent dependencies.
  • Make sure it can consume both Duality packages and regular NuGet library packages.
• Investigate whether the nightly build needs to be updated.
• Investigate whether the nuspec files need to be updated, including dependencies outside the main repo.
• Due to the big new set of NuGet-related dependencies, update the Publish Game script copy blacklist, as well as potentially existing .gitignore files and nightly builder binary package config.
• Consider adding an updated .gitignore file to the binary release package by default.

[ilexp]

Progress

• Cleaned up test project a bit for continued development.
• Continued discussion on the NuGet repo.
• Investigated ISettings and Settings a bit more.
• Created a custom, readonly settings implementation that does not rely on a config file and emulates the required values. Seems to work so far.

Immediate ToDo

• Create a separate test project to figure out how to use the new NuGet and how to integrate it properly.
  • Implement a custom NuGetProject.
  • Keep track of installed packages. Maybe just use folder contents?
  • Find the best matching package contents based on the current target framework.
  • Investigate why NuGet queries seem to take so long.
  • Discover packages by tag.
  • Set up a test repository of packages, both Duality and others.
  • Do some test installs and dependency resolves.
• Consider moving Duality package management into a separate editor library.
• Update the NuGet dependency to 4.X.
  • Gradually fix all the broken things until it compiles.
  • Gradually fix all the failing tests until it's completely green again.
  • Deal with target platforms and platform-dependent dependencies.
  • Make sure it can consume both Duality packages and regular NuGet library packages.
• Investigate whether the nightly build needs to be updated.
• Investigate whether the nuspec files need to be updated, including dependencies outside the main repo.
• Due to the big new set of NuGet-related dependencies, update the Publish Game script copy blacklist, as well as potentially existing .gitignore files and nightly builder binary package config.
• Consider adding an updated .gitignore file to the binary release package by default.

[ilexp]

Side note about NuGet packing support for NetStandard packages: http://blog.tdwright.co.uk/2017/11/20/the-perils-of-publishing-a-net-standard-library-to-nuget/ See also: https://github.com/NuGet/docs.microsoft.com-nuget/blob/master/docs/Guides/Create-NET-Standard-Packages-VS2015.md

[ilexp]

Progress

• Implemented GetInstalledPackages method for custom nuget project based on its installation folder contents.
• Experimented with a first install-update-uninstall test cycle, but didn't get the update part to actually uninstall an old package. Currently inquiring on the NuGet issue about this.

Immediate ToDo

• Create a separate test project to figure out how to use the new NuGet and how to integrate it properly.
  • Implement a custom NuGetProject.
  • Find the best matching package contents based on the current target framework.
  • Investigate why NuGet queries seem to take so long.
  • Discover packages by tag.
  • Set up a test repository of packages, both Duality and others.
  • Do some test installs and dependency resolves.
• Consider moving Duality package management into a separate editor library.
• Update the NuGet dependency to 4.X.
  • Gradually fix all the broken things until it compiles.
  • Gradually fix all the failing tests until it's completely green again.
  • Deal with target platforms and platform-dependent dependencies.
  • Make sure it can consume both Duality packages and regular NuGet library packages.
• Investigate whether the nightly build needs to be updated.
• Investigate whether the nuspec files need to be updated, including dependencies outside the main repo.
• Due to the big new set of NuGet-related dependencies, update the Publish Game script copy blacklist, as well as potentially existing .gitignore files and nightly builder binary package config.
• Consider adding an updated .gitignore file to the binary release package by default.

[ilexp]

Putting this up for grabs to indicate that I'd appreciate help on this issue. Its main work will also be nicely self-contained and currently takes place building a prototype in a separate repo, making collaboration a lot easier and less constrained. So if anyone's up for joining in, let me know 👍

[Barsonax]

I have a netstandard branch for pathfindax here. I havent published it to nuget yet though (and probably would have to fix some appveyor related stuff to do so). If you want to test with a real plugin I could publish it to nuget as a prerelease package but as it is now duality does not support prerelease packages. Might be a good moment to add this functionality (I mean this sounds like really simple to add right?).

[Barsonax]

Is there any branch I can base my new branch on? Also I see alot of nuget nuget packages on nuget but not really sure what is old and what is the v4 version. Is this the correct version?https://www.nuget.org/packages/NuGet.Common

Looking at the dependencies that one depends on NETStandard.Library (>= 1.6.0) which means we need to use the duality branch that uses netstandard I believe.

EDIT: checked this and I get this error when I try to install:

[ilexp]

If you're going to look into using NuGet 4.X in Duality, I'd try out the basics in a standalone project first. Check out this one that I started earlier, it's a regular .Net Framework project. From what I could gather, the main required package is NuGet.PackageManagement, which brings in a lot of required dependencies automatically. You can then use a NuGetPackageManager to preview and execute the most common operations, though I haven't managed to get all of them running 100% yet in a test scenario - updating seems to provide the wrong list of steps for example.

[Barsonax]

Ah seems after nuget v4.5 they dropped support for net45 which was the reason I couldnt install it.

Looking at your test project it seems updating doesnt remove the old package?

[ilexp]

Looking at your test project it seems updating doesnt remove the old package?

Yep, not sure why exactly. If that's fixed, it should be possible to recreate common operations and use cases in the test project more closely to what the Duality package manager does, and if that works well we might as well start to work on it in a v3 branch.

Ah seems after nuget v4.5 they dropped support for net45 which was the reason I couldnt install it.

Do you know what their new minimum supported .Net Framework version is, or do you have a link with more background info on this?

[Barsonax]

Not sure either why it doesn't remove the old package. Maybe you are supposed to do this yourself? Might be worth it to ask them why and how you should fix it before resorting to possible hacks.

Net 4.6 is their next minimum framework I believe. The nuget packages themselves have that info in them so its easy to check.

[Barsonax]

Noticed that GetInstalledPackagesInDependencyOrder returns 0 installed packages. Since updating a nuget package also calls a similar method internally here I suspect this is the reason the old package is not removed.

EDIT: strangely when I copied all the neccesary code for updating a package of the NuGetPackageManager.cs I linked earlier (of which the commit should match v4.4?) in the test project it did generate a uninstall action as shown below here:

EDIT2: made a new .NET4.6 project and tested with nuget v4.6 but this still happens.

[ilexp]

I think you're onto something with GetInstalledPackagesInDependencyOrder returning zero. Maybe it has something to do with how the project or repository is defined. Any idea why exactly we get the zero result here?

EDIT: strangely when I copied all the neccesary code for updating a package of the NuGetPackageManager.cs I linked earlier (of which the commit should match v4.4?) in the test project it did generate a uninstall action as shown below here:

Weird. What happens when you copy the implementation of the installed packages method - does it produce a non-zero result as well?

[Barsonax]

Ye when you do that it returns the installed packages correctly. I could give you the modified test code, maybe you will notice something I didn't.

Except that the source code is somehow different I have no idea how this can happen.

[Barsonax]

Solved the package updating issue. The manager was using a different path than the project which caused it to not find the installed packages.

When you change them both packagesPath and targetPath to use the ProjectRoot\Packages folder the uninstall action is generated succesfully. Not sure why its even possible to have a 2 different packages folders in the nugetproject and the packagemanager at the same time when using the API.

That still leaves the InvalidOperationException: SolutionManagerNotAvailableForUninstall which seems to be caused because the manager requires a instance of ISolutionManager which is not present in the test project.

EDIT: implemented a untested solutionmanager and deletemanager but installing, updating and uninstalling a package seems to work now. I have uploaded the code below: NuGet4xTest.zip

As a side note since from nuget v4.6 on you need .NET 4.6 so maybe its time to start considering updating duality to this version?

[ilexp]

EDIT: implemented a untested solutionmanager and deletemanager but installing, updating and uninstalling a package seems to work now. I have uploaded the code below:

I can add you as a collaborator for the NuGet4X test project so you get push access to the repo. Let me know if that makes sense for you.

As a side note since from nuget v4.6 on you need .NET 4.6 so maybe its time to start considering updating duality to this version?

Agreed. Let's consider updating editor related projects, but also keep the launcher as-is to ensure a low profile and ability to "just run" on whatever the user has installed.

[Barsonax]

I don't think you even want to update the launcher to .net4.6 but to .net core eventually so fully agree with that.

If you give me access I will push the necessary changes to the repo.

[ilexp]

You should have an invite link 👍

[Barsonax]

Fixed the master branch so now updating and uninstalling should work.

Found a bug in the CustomNugetProject.GetInstalledPackagesAsync. It seems to wrongly parse the version number for Pathfindax since my versioning is slightly different (the last number is the build number).

Also I made a new branch 'Barsonax' where I cleaned up some of the nuget code in the Program.cs and moved it into its own class.

EDIT: noticed that nuget might be using some hardcoded /packages folder somewhere. When I change the targetPath to be different than packages things do not work properly anymore...

EDIT2: it seems when you provide a ISolutionManager it creates a SEPARATE FolderNuGetProject here (and there is no way to pass our project) instance and pulls the folder from the ISettings instance. If its not found there it defaults to packages. Who designed this mess?....Sure you can workaround all this but it will never be clean....

[ilexp]

Seems like this is really making progress now, nice work 👍 Roadmap-wise, here's a list of items that we might benefit from looking into before starting to upgrade the package manager itself:

• Check out the basic package operations and verify they work as expected
  • Install packages
  • Uninstall packages
  • Update packages
• Check out the basics of package discovery
  • List all online packages that have a certain tag
  • List all online packages that have a certain ID
  • Retrieve all package metadata for an online package, given ID and version
  • List all locally installed packages
• Investigate why NuGet queries seem to take so long - this was from my early experiments, is this still an issue in the current version?
• Dependency resolve and target frameworks
  • Given ".NET 4.5" / ".NET 4.6" target framework, check if package dependencies resolve correctly and we don't end up with lots of reference assemblies.
  • Given an installed packages metadata, find a way to list package contents that match the current target framework most closely. Find the closest match, without it having to be exact.
• Take a final look at whether there's any way to avoid having to add a 10+ new .dll files to Duality just for this update.

Given that you've already solved some of them, first integration experiments might not be that far off, depending on what results we get for the remaining items or what unexpected roadblocks there might be.

Also I made a new branch 'Barsonax' where I cleaned up some of the nuget code in the Program.cs and moved it into its own class.

Sounds good!

[Barsonax]

Added a way to discover duality plugins. I dont see a way to search using just tags as you have to search on the id (you can further filter it client side though). So what iam doing now is finding all packages with 'Duality' in them and then filtering this further on the client.

Once you have found a package (which is returned as a IPackageSearchMetadata) you can get all versions for that package with GetVersionsAsync(). Getting the latest version is easier as you can just use the Identity property.

Currently there is no way yet of tracking installed packages (other than searching through the package folder). Just like currently in duality there needs to be some PackageConfig.xml that keeps track of all this. For now it just scans the packages folder though.

Another issue I found is because of the async keyword. Duality currently uses C#4 which does not support this. Raising the C# version to C# 5 wont break any compatibility I believe as all VS from 2012 and up support C# 5. If you go lower then you wont have .NET 4.5 anymore so it wouldnt work anyway. Also its fine to use a library written in a higher C# version and reference it in a project thats still on a older C# version.

Aside from that from nuget v4.6 on only .NET 4.6 is supported so if you want to keep up with the versions this would have to be updated as well. Obviously the latter would mean you wont be able to use duality in a .NET 4.5 environment anymore.

Performance seems to be good? Not seeing anything strange here.

If you want to avoid adding 10+ dlls to duality just to update nuget we could look into just referencing Nuget.Protocol. This would mean we would have implement alot ourselves though and even with just this package thats still like 9 packages in total.

[ilexp]

Added a way to discover duality plugins.

👍

I dont see a way to search using just tags as you have to search on the id (you can further filter it client side though). So what iam doing now is finding all packages with 'Duality' in them and then filtering this further on the client.

We can't expect all Duality packages to carry Duality in their ID or title. If there's no direct "search by tag", maybe there's a general search that includes tags, which can be used as a first pass, filtering results afterwards?

The NuGet API allows queries over various package fields - could be worth checking if there's any equivalent in the NuGet lib API:

• https://docs.microsoft.com/en-us/nuget/api/search-query-service-resource
• https://docs.microsoft.com/en-us/nuget/consume-packages/finding-and-choosing-packages#search-syntax

Currently there is no way yet of tracking installed packages (other than searching through the package folder). Just like currently in duality there needs to be some PackageConfig.xml that keeps track of all this. For now it just scans the packages folder though.

We need something like this beyond the manual management though, as installed package metadata is retrieved that way. I'm 100% sure NuGet has the info somewhere internally, as it needs to query this when installing, updating or uninstalling, so there should be a way to query this information somehow.

Another issue I found is because of the async keyword. Duality currently uses C#4 which does not support this. Raising the C# version to C# 5 wont break any compatibility I believe as all VS from 2012 and up support C# 5. If you go lower then you wont have .NET 4.5 anymore so it wouldnt work anyway. Also its fine to use a library written in a higher C# version and reference it in a project thats still on a older C# version.

Aside from that from nuget v4.6 on only .NET 4.6 is supported so if you want to keep up with the versions this would have to be updated as well. Obviously the latter would mean you wont be able to use duality in a .NET 4.5 environment anymore.

Good point. I think it's fine if we bump the editor to .NET 4.6 and C# 5.0 when upgrading. Core and launcher are unaffected by this, so we don't lose a lot of compatibility where it's critical.

Performance seems to be good? Not seeing anything strange here.

Sounds good, maybe it was just a hiccup on my test setup.

If you want to avoid adding 10+ dlls to duality just to update nuget we could look into just referencing Nuget.Protocol. This would mean we would have implement alot ourselves though and even with just this package thats still like 9 packages in total.

Let's keep it then. 9 is better than 10+, but probably not as much to weigh out re-inventing them locally.

[Barsonax]

We can't expect all Duality packages to carry Duality in their ID or title. If there's no direct "search by tag", maybe there's a general search that includes tags, which can be used as a first pass, filtering results afterwards?

Maybe there is some domain specific syntax you can use in the searchterm parameter but would have to dig in the nuget source yet again to confirm this. Documentation for the whole nuget lib doesnt seem to exist...

EDIT: turns out the searchTerm works the same as you linked so the following will find all packages with both the Duality and Plugin tag: tags:Duality, Plugin. Fixed this in the code so now its no longer needed to do any client side filtering

We need something like this beyond the manual management though, as installed package metadata is retrieved that way. I'm 100% sure NuGet has the info somewhere internally, as it needs to query this when installing, updating or uninstalling, so there should be a way to query this information somehow.

It currently uses the installed packages in the packages folder as package metadata (which is a bit of a hack really). Thats it there is not some other place where this info is kept.

Good point. I think it's fine if we bump the editor to .NET 4.6 and C# 5.0 when upgrading. Core and launcher are unaffected by this, so we don't lose a lot of compatibility where it's critical.

Consider upgrading to C# 7.2 (which should enable us to do #600 as well) while we are at it anyway if we are going to .NET 4.6. I dont think you will lose any compatibity. Any version from VS2012 and up should support this if the correct frameworks and compiler are installed which is in case of VS2017 is by default. Having said this why do we still use C# 4 in the rest of duality? Is VS2010 still so populair for duality development? :P (though this is a different issue I think).

[Barsonax]

The following is not directly related to the development of the new packagemanager but handy for testing purposes:

As I managed to fix all the (local) bugs in the process of porting pathfindax to netstandard I can now push a new (prerelease) version to nuget. Hopefully since iam doing a dual build that targets both .NET4.5 and netstandard2.0 (this is currently the minimum version I can target because of the dependency on duality) it is compatible with the current and future duality packagemanager. Regardless of the outcome it will be good to know how duality is going to handle such a package.

Ofcourse since the current packagemanager does not support prerelease packages the code would have to be modified a bit. I suppose this wouldnt be hard to change just for this test.

EDIT: pushed it to nuget, now waiting for indexing/validation EDIT2: the duality packagemanager picks the netstandard version when installing. Everything seems to work though which is good news. Still this makes my dual target build configuration kinda redundant.

[ilexp]

As I managed to fix all the (local) bugs in the process of porting pathfindax to netstandard I can now push a new (prerelease) version to nuget. Hopefully since iam doing a dual build that targets both .NET4.5 and netstandard2.0 (this is currently the minimum version I can target because of the dependency on duality) it is compatible with the current and future duality packagemanager. Regardless of the outcome it will be good to know how duality is going to handle such a package.

Ofcourse since the current packagemanager does not support prerelease packages the code would have to be modified a bit. I suppose this wouldnt be hard to change just for this test.

EDIT: pushed it to nuget, now waiting for indexing/validation EDIT2: the duality packagemanager picks the netstandard version when installing. Everything seems to work though which is good news. Still this makes my dual target build configuration kinda redundant.

Super valuable info, thx for sharing. And pleasantly surprised the .NET Standard package just works with the regular build.

Upgrading the current package manager to distinguish between platform specific content in packages even before upgrading to the new NuGet is on the ToDo for issue #573 as far as I recall, and definitely something to investigate regardless of any other advances.

[Barsonax]

Just discovered you can reference duality in a netstandard 1.1 project. See #696.

This is good news is this means you can reference a netstandard library in a pcl and vice versa. So this shouldn't break existing packages.

While this works fine in visual studio it still breaks with the duality package manager. This might very well be because the nuget version is too old.

So gonna check if it works correctly with a newer nuget version. If it works all we have to do is upgrade nuget to support netstandard plugins.

[Barsonax]

This is the error you currently get when installing Singularity 0.1.3.68 (which is Netstandard1.1) with the duality package manager.

[ilexp]

Hmm, this is actually inside NuGet, probably because the outdated core lib can't handle what's in the package manifest. Can you post a link to your .nuspec and .nupkg that causes this?

In any case, we should revisit the NuGet update sometime soon.

[Barsonax]

You can actually download the package from nuget if you go there with the browser.

Thats how I compared the duality packages to others.

Did some work in the weekend to the nuget test repo. Since we will basically be rewriting alot of code we might also consider making DualityEditor less dependend on nuget. I think we should aim to keep the surface area smaller than it is now. Maybe even put a interface and plugin inbetween so there is no coupling anymore.

Also everything in nuget v4 is async and duality code base currently is not. If we adapt it it will spread fast and we need to bump up the C# version.

Besides why are we still on C#4? Better to switch that to 7.3. When we switch to async and net standard we would lose compatibility with prehistoric VS versions anyway. Better to take advantage of the latest C#.

[ilexp]

Also everything in nuget v4 is async and duality code base currently is not. If we adapt it it will spread fast and we need to bump up the C# version.

Besides why are we still on C#4? Better to switch that to 7.3. When we switch to async and net standard we would lose compatibility with prehistoric VS versions anyway. Better to take advantage of the latest C#.

As far as I know there's no need to bump the version for consuming async methods from a library, only when writing async methods. I'm not a fan of viral code changes where you touch one place and suddenly need to change half the project either, so I'd like to approach this topic in a more "minimal invasive" way, including the use of async code.

That doesn't mean we can't expand later - but as far as the NuGet update is concerned, it ideally shouldn't change the PackageManager API surface at all and remain transparent to the rest of the codebase. Embracing modern features like async is something that can still happen independently and gradually where it makes sense.

I think we should aim to keep the surface area smaller than it is now. Maybe even put a interface and plugin inbetween so there is no coupling anymore.

That layer already exists, it's all the DualityEditor PackageManager code! Besides this, there is no NuGet reference in the codebase.

[Barsonax]

As far as I know there's no need to bump the version for consuming async methods from a library, only when writing async methods. I'm not a fan of viral code changes where you touch one place and suddenly need to change half the project either, so I'd like to approach this topic in a more "minimal invasive" way, including the use of async code.

We need to await the async methods. The alternative is to use .Wait() (or for nicer exceptions with .GetAwaiter().GetResult()). I tested this in a winforms application and it deadlocks (due to the synchronization context winforms uses). Async and synchronious code just don't mix very well.

Ofcourse there ways around it though... You can wrap up the async code in a separate task (which doesnt have that synchronization context) and call .Wait() on that task. It isnt going to be pretty.

That layer already exists, it's all the DualityEditor PackageManager code! Besides this, there is no NuGet reference in the codebase.

That class is 1k lines long and seems to be in dire need of some refactoring. This seems to be because both duality and nuget concerns are being mixed in the same class. I don't think PackageManager.cs should depend on nuget at all. All it needs is something that installs packages.

So what I propose is to define a interface that contain the basic operations (install, uninstall, getmetadata etc, basically like the DualityPackageManager.cs in the Nuget4XTest repo). Then we write our duality specific code around that interface. This will make it way easier to handle any future nuget API changes and also makes the code easier to read and test.

[ilexp]

We need to await the async methods.

Possibly, but not necessarily - all I'm saying is, it doesn't hurt to look for alternatives first, rather than forcing a codebase-global C# version change because of an implementation detail somewhere.

.Wait is a way to get back to synchronous territory with existing API, but, if choosing to adjust the existing API, it would also be possible to use ContinueWith internally where required, and pass the continuation task outside as a return value. There may be other ways, I don't think this has to be a black or white issue.

I don't think PackageManager.cs should depend on nuget at all. All it needs is something that installs packages.

This sounds like unnecessary abstraction to me. NuGet is something that installs packages, and I don't see we're moving away from NuGet as a delivery mechanism anytime soon - so why put in an extra layer that ends up essentially unused, or worse, even obfuscating what's happening?

An interface / plugin solution would make perfect sense when multiple variants are to be expected, or when system A is actually replaced with system B, to harden the design for future occasions, but neither of this is the case right now. Hiding NuGet as a dependency to user code is totally reasonable as well, but that already happens, so nothing to gain here either.

[Barsonax]

Possibly, but not necessarily - all I'm saying is, it doesn't hurt to look for alternatives first, rather than forcing a codebase-global C# version change because of an implementation detail somewhere.

I don't see the point of not using what the newer C# versions provide. It is a hassle and it will only get worse. Nobody will use VS2010 to contribute to the duality repo. We are not just missing async but alot of other features as well.

.Wait is a way to get back to synchronous territory with existing API, but, if choosing to adjust the existing API, it would also be possible to use ContinueWith internally where required, and pass the continuation task outside as a return value. There may be other ways, I don't think this has to be a black or white issue.

No just .Wait() will deadlock. Worse this will only happens in a winforms context so unit test won't catch it. You would have to use a different thread to really get around this as this problem is linked to the SynchronizationContext.

This sounds like unnecessary abstraction to me. NuGet is something that installs packages, and I don't see we're moving away from NuGet as a delivery mechanism anytime soon - so why put in an extra layer that ends up essentially unused, or worse, even obfuscating what's happening?

Its not that I see us moving away from nuget but the current code is very entangled with nuget. It uses nuget types all over the place. Since nuget changed their API so much ALL that code is now broken. If we only had a ~150 lines class implementing purely the stuff we need it would be much easier to fix just that class. We don't control the nuget libraries so its best to keep that surface area as small as possible and not mix duality and nuget concerns. Using a interface could enforce that a bit more (also its easier to test) but is ofcourse not strictly necessary.

[ilexp]

I don't see the point of not using what the newer C# versions provide. It is a hassle and it will only get worse. Nobody will use VS2010 to contribute to the duality repo. We are not just missing async but alot of other features as well.

We're not losing anything by not updating, we're just not gaining anything either. It's not a situation that demands immediate action, and while there may be good reasons to update, they should be discussed and evaluated separately. Let's move this to a different issue.

Its not that I see us moving away from nuget but the current code is very entangled with nuget. It uses nuget types all over the place.

Yeah, but that kind of makes sense. It's entangled with NuGet because it uses it. If you move that into a different class, then that one will be entangled with NuGet instead. The bridge needs to be built somewhere, and better close than far.

You make a good point with reducing surface area to NuGet though, especially with the breaking changes they introduce in the big update. I don't think we should go all the way here with interfaces and plugins, but instead extract a ~150 lines class wrapping basic NuGet access that you proposed. With the extracted class governing internal "low level" access to packages while the existing PackageManager keeps its "high level" functionality, we'd have a good chance to end up with easier to understand code as well.

[Barsonax]

You make a good point with reducing surface area to NuGet though, especially with the breaking changes they introduce in the big update. I don't think we should go all the way here with interfaces and plugins, but instead extract a ~150 lines class wrapping basic NuGet access that you proposed. With the extracted class governing internal "low level" access to packages while the existing PackageManager keeps its "high level" functionality, we'd have a good chance to end up with easier to understand code as well.

We can always do the interface later if theres a need for it. Especially when you already have a clean basic operations class its just a small step.

So next step then is figuring out how to turn all those async calls to normal calls. Its not going to be very pretty but I think we can wrap all the calls to the nuget api in Task.Run() calls and wait on that. This should prevent the deadlocks you get if you just call .Wait() (or any other blocking call) on the nuget api directly. Just putting .ConfgureAwait(false) won't fix this as I think nuget didnt do this in their library.

The other task is preventing any nuget types from leaving our abstraction layer and only work with duality types outside of our abstraction layer.

[ilexp]

The other task is preventing any nuget types from leaving our abstraction layer and only work with duality types outside of our abstraction layer.

Since it's internal only, I'd make this a "soft goal" only, as in: If it takes too many wrappers or too much code to achieve encapsulation, a leaky abstraction should be enough. It should prevent rewriting big parts of PackageManager for a NuGet update, but that doesn't mean every single NuGet type needs to be wrapped. We should keep a reasonable balance.

[Barsonax]

Going to try getting the v4 nuget package manager working in duality here: https://github.com/Barsonax/duality/tree/feature/nuget_package_manager_update_v4

But first some further changes to the Nuget4XTest repo as preparation for this.

Steps:

1. Get the current Nuget4XTest working without async await. Set the C# version to 4.0 to force this. DONE
2. Try get the method signatures in Nuget4XTest to match the nuget v2 packagemanager so we have less code to change
3. Replace the nuget v2 package manager with our own v4 packagemanager
4. Fix any remaining compile errors
5. Get all unit tests working

[Barsonax]

The new package manager works completely different btw. These are the differences:

• First of all by default nuget now installs to the global nuget package folder. When calling the api ourselves this doesn't happen because we can provide our own path. Still I think its actually better to install to the global packages folder and then copy from that global packages folder.
• Each targetframework has its own subfolder. There doesn't seem to be a way to go around this. This seems to be tied to how the packages themselves are structured. So there still is a possibility we have to deal with a 'old' style package.
• The package manager doesn't seem to update the config file when installing/upgrading/uninstalling packages. No idea if this is even intended or just a bug somewhere.

So maybe we should change our approach here. Instead of directly installing the nuget packages to the duality plugins folder I think we should let nuget do its thing and then copy the packages + dependencies to the plugin folder, taking the target framework into account.

The same process happens when you build a csproj. It copies all the needed dll's to the output folder (well unless you turned that off ofcourse).

This does mean we will have to write some code to do this ourselves but that is still probably alot easier than trying to hack around the nuget library. If we use this approach nuget just has to provide us with a path where to find the packages which is much simpler to deal with.

EDIT: I just can't get that horrible nuget library to work. Either it doesn't update the packages.config or it doesn't update a package or some other weird thing goes wrong. I seriously doubt this library is even intended to be used like we want it to. Maybe we are better off just calling the REST api ourselves.

[ilexp]

So maybe we should change our approach here. Instead of directly installing the nuget packages to the duality plugins folder I think we should let nuget do its thing and then copy the packages + dependencies to the plugin folder, taking the target framework into account.

This is already how things work, so we should be in a good starting point here:

1. Duality invokes NuGet API and lets it do its thing - in NuGet 2.x, this meant installing into some folder, which is Source/Packages in Duality.
2. Duality uses NuGet API to find out what it installed and where all the files are.
3. Selective copy of those files to their destinations within Duality, i.e. Plugins, Data, Source/Code and the main directory.

What changes is where NuGet puts stuff, and probably how that stuff is structured.

One thing we do have to look out for is how we determine which packages we need to copy or remove: Previously, we could rely on NuGet events for install and uninstall, because they were only called in the context of the local project, not globally. If a global package folder means that those events now work in a global scope, they will no be a reliable indicator on the required steps to install / uninstall / update a package.

[Barsonax]

If #710 is merged and if we also update Nuget.Core to 2.12 or later (which is easy as there are no breaking changes from what I have seen) then I dont think we need to update to nuget 4.x.

Even though the issue says duality is using Nuget.Core 2.12 I see the actual nuget package we use is 2.8.3. Netstandard support was added in 2.12 so without upgrading it wont understand netstandard.

[ilexp]

Some links for picking up this issue again at some point:

• Samples Repo
• NuGet.Client Implementation

No need right now due to our recent progress and NuGet.Core update, just keeping the links around.

[ilexp]

Since we've run into trouble with .NET Standard only packages via NuGet 2.14 in issue #573, I'm bumping this issue again with some fresh thought and brief research.

• Reference to NuGet/8312 where we asked to clarify porting and API docs (thx to @Barsonax for bringing it up)
• NuGet Client SDK stub page on the Microsoft docs. Nothing much to gain here, but at least we now know what exact term to google for when researching this.
• Revisiting the NuGet Client Libraries, a blog post going into detail on approaches to use the client SDK in custom projects.

After letting this topic sink in for a while, and also moving forward with NuGet.Core integration in #703, #710 and #715, I think it makes sense to be more open towards lower level implementations here, as @Barsonax suggested before, and ditch the high level NuGet.PackageManagement stuff. Our use case is not necessarily always the same as intended by the NuGet / Visual Studio folks, package management is among the best unit-tested areas in Duality, and we've had to do a custom implementation of the 2.14 PackageManager anyway to get target frameworks working. Another pro argument would be reducing the number of dependencies and being .NET Standard compatible, as mentioned in the above blog posting.

[Barsonax]

Yup upgrading to Nuget 2.14 only solved some errors but didn't actually made nuget understand how to resolve netstandard properly.

What currently works is a package that targets both netstandard and netframework (before nuget 2.14 this resulted in a exception I believe). What does not work is a package targeting only netstandard. It will install the package but it will think there are no dependencies and thus it won't install any dependencies.

So once more into the breach.

[Barsonax]

Started working on a nuget v5.1 implementation for duality (yup thats the latest version currently) based on findings in https://martinbjorkstrom.com/posts/2018-09-19-revisiting-nuget-client-libraries

The good part is that it correctly resolves dependencies as can be seen here: .

The branch can be found here https://github.com/ilexp/NuGet4xTest/tree/Barsonax

Gonna take some work to get all needed functionality though

[ilexp]

Awesome 🙂 Already looks better than the previous approach, more commands and work, less interface plumbing and fake project implementations. How are the API docs on all those classes?