choco icon indicating copy to clipboard operation
choco copied to clipboard

NuGet library uplift testing

Open TheCakeIsNaOH opened this issue 3 years ago • 17 comments

Description Of Changes

TODO

Motivation and Context

TODO

Testing

TODO

Things to test list

  • [ ] Test proxy functionality. If stuff is broken in NuGet.Client, see if work can be upstreamed.
  • [x] Test stuff when running on Linux, including uninstalling uppercase packages. (Everything seems to be working as far as I can tell).
  • [ ] Test if new (to Chocolatey) nuspec elements break anything locally, in licensed, or on CCR. Then potentially add validation to pack (and/or push) to ensure the new .nuspec element are not used if needed, or warn if they would be ignored currently?
  • [ ] Test and add tests for semver 2.0 prerelease strings
  • [ ] Test against v3 feeds in more scenarios
  • [ ] Test to see if #1648 is fixed
  • [ ] See if I can get a reproduction for #1408 and test if it is fixed
  • [ ] Test if #1092 is fixed by this.
  • [ ] Test leading zeros, breaking change? #1174
  • [ ] Investigate changes to pack token substitution, this issue seems to indicate that there may have been changes upstream: https://github.com/chocolatey/choco/issues/2100
  • [ ] Investigate changes to pack encoding filenames, due to potential upstream changes: https://github.com/chocolatey/choco/issues/1628
  • [ ] See if proxy support for push comes along for the ride with supporting proxies for other calls: https://github.com/chocolatey/choco/issues/1271
  • [ ] Test other push authentication, https://github.com/chocolatey/choco/issues/2026
  • [ ] See if https://github.com/chocolatey/choco/issues/1822 is fixed

Tests to add:

  • [ ] Add test(s) to ensure that the NuGet version is still correct after ilmerging the libraries. Potentially would throw errors about needing to upgrade NuGet if it is not working.
  • [ ] Add tests around new semantic versioning prerelease strings, check that pack, install, etc all work.
  • [ ] Add tests to ensure that #2166 does not reoccur (perhaps upstream the fix for it?)
  • [ ] Update the fork to port forward the commit and then add tests to ensure that #2233 does not reoccur
  • [ ] Add tests for install/search/outdated/upgrade/etc with relative local path
  • [ ] Add tests around logging for all install messages
  • [ ] Add tests around more side by side stuff
  • [ ] Add more pin scenarios, and/or mock out pending pin scenarios (should be more doable with new libraries hopefully) and enable the tests
  • [ ] Add tests to ensure that package and install arguments are not passed to dependent packages unless argument(s) specified to do so
  • [ ] Add scenarios for pinned dependencies for install/upgrade/uninstall
    • Install that requires upgrade/downgrade of pinned dependency
    • Upgrade that requires upgrade/downgrade of pinned dependency
    • Upgrade that requires upgrade of pinned parent
    • Uninstall (with force dependencies) that tries to uninstall pinned dependency
  • [ ] Add tests for chocolateyPackageVersionPrerelease and chocolateyPackageVersionPackageRelease to ensure that they are set correctly, e.g. normalized, etc
  • [ ] Add test for uninstalling package with package ID that includes uppercase letter(s)
  • [ ] Add test for running beforemodify script in dependency

Known issues

  • [ ] Logging from NuGet is at a more verbose level than needed. This can be adjusted either in the NuGet.Client fork, or in the ChocolateyNugetLogger class. What to do should be determined by a review of the messages from libraries once they used classes/methods are more settled on. This can be seen when running choco install and choco search by the network requests information logged to info by NuGet.
  • [ ] Pushing packages to CCR does not work (and does not error, where it should work or error). This can be seen by pushing a valid package to CCR with a valid apikey, and having it say that it was ok, but no package was actually uploaded.
  • [ ] When push has errors, they are not always caught correctly. I have not always been able to reproduce this, but pushing to nexus with a bad apikey, or where the server is not available are two situations that should trigger it. Once pushing to CCR is working, then testing the error handling if the package already exists would be appropriate.
  • [ ] Using a relative local path as a source is not working. When using a dot . as the source like .\choco.exe list -s ., it erors with The path '.' for the selected source could not be resolved.. However, installs do seem to be working?
  • [ ] There are quirks with list/search. Not sure what all is broken. Test matrix should include listing/searching from a local folder, from v2 APIs like CCR and nexus, and from V3 APIs like NuGet.org and Nexus, then should have a search with no search term (so listing all package), listing with a search term, listing with a search term with --exact and/or --all-versions
  • [ ] A couple of the list/search command line options are not properly functional. --page, --page-size=, --by-tags-only are all likely broken in some cases.
  • [ ] Download progress is not available. Previously, the PackageDownloader class was overridden, but the class no longer actually downloads the packages inside it.
  • [ ] The code to set a source bypass a proxy has not been ported to NuGet.Client, so it won't work. Proxy functionality should be tested first.
  • [ ] Search and outdated commands use way more API calls than previously in some cases. Try running search with and without a search term, --exact and --all-versions, then compare to NuGet.Core quantity of calls. It may also be worth looking at install/upgrade when --version is not passed, and for packages that have a large dependency chain.
  • [ ] Source priority is not respected. It might just so happen to work, but there is no longer any code that explicitly enforces it.

Features to add?

Moved to comment: https://github.com/chocolatey/choco/pull/2740#issuecomment-1204458618

Completed

  • [x] Fork NuGet.Client
  • [x] Create a list of commits added to nuget-chocolatey that should be ported/upstreamed/ignored/investigated
  • [x] Investigation of various things relating to the licensed extension. Overriding nuget download service?

Change Types Made

  • [ ] Bug fix (non-breaking change)
  • [ ] Feature / Enhancement (non-breaking change)
  • [x] Breaking change (fix or feature that could cause existing functionality to change)

Related Issue

Preparing for #508

Change Checklist

  • [ ] Requires a change to the documentation
  • [ ] Documentation has been updated
  • [ ] Tests to cover my changes, have been added
  • [ ] All new and existing tests passed.
  • [ ] PowerShell v2 compatibility checked.

TheCakeIsNaOH avatar Jun 16 '22 19:06 TheCakeIsNaOH

Some initial thoughts:

  • Semver stuff is now available in NuGet.Versioning, and appears to be fairly compatible, at least from a compilation point of view.
  • There is a ILogger available in NuGet.Common, against somewhat compatible with the current interface.
  • Metadata, information about packages, and IPackageDownloader are available in NuGet.Packaging
  • IPackage is no longer available, and it is heavily used in the Chocolatey codebase.
  • I'm completely ignoring the unit/integration tests for the moment, some do have NuGet references.
  • There are NuGet references to IPackage and similar across the codebase, but as to calling actual methods, that is mostly limited to stuff in infrastructure.app.nuget, in the NugetService, and in the templates/pins commands (to check for locally installed packages, these probably could be abstracted into something in the NugetService or whatever).

TheCakeIsNaOH avatar Jun 16 '22 20:06 TheCakeIsNaOH

I went for the low hanging fruit and got push working to my local nexus repository. There are still a number of things that are broken, but it is kinda working:

  • The logging levels need adjustment to be less verbose. I'm not sure if that is adjustment in ChocolateyNugetLogger, something that can be set via a method in NuGet, or something that needs to be changed in the NuGet code.
  • When attempting to push to Nexus with an invalid apikey, it instead asks for username/password, although I can't reproduce it anymore. Either way, it is something to possibly investigate if it pops up: https://github.com/NuGet/Home/issues/3391
  • The user-agent header is being partially set, but it always includes the version of the NuGet library, and there is no way to set that. So, I've set it to do this, Chocolatey Command Line/<version> via NuGet Client/<nuget version>. There is an upstream issue open here: https://github.com/NuGet/Home/issues/7464
  • Pushing to CCR (via https://push.chocolatey.org) is not working. If it is a valid package and apikey, then nuget logs ok and that the package was pushed, but the package was definitely not pushed. Same thing happens with a pre-existing immutable package version, with both a valid and invalid API key.
  • I suspect that different exceptions would be thrown than with NuGet.Core , so the exception handling will need to be checked, once things are working better.

Other notes:

  • I traced the codepath that nuget.exe uses, and replicated the approiate method calls, at about the same abstraction level as push via NuGet.Core had.
  • Detection of the push being a v2 vs v3 feed seems to happen automatically, and so this should theoretically "just work" for both.
  • Appending of /api/v2/packages to the push source was disabled by noServiceEndpoint, as Chocolatey has push URLs that are specified explicitly.
  • Reminder to self, test "pushing" packages to a unc location. Edit: this is working for me. It does spit out an error about skipping duplicates not being supported, but that can be dealt with later.

TheCakeIsNaOH avatar Jun 17 '22 20:06 TheCakeIsNaOH

So, I've got pack working as well. It works with NuGet compatible .nupecs, so using the older schema and not using any of the elements added to nuget-chocolatey.

  • This is one area that has not changed much from older library versions. It just required tweaking a couple of things like switching from implementing an interface to passing a function, but overall, not super difficult. Yay for low-hanging fruit.
  • Since there are not a lot of changes, it is looking like it is still not easy to add additional elements, but on the upside, it looks like it would be reasonably easy to bring forward the commit(s) from nuget-chocolatey to the newer libraries. It is possible everything would be contained within NuGet.Packaging, but it would actually have to be ported to ensure that.
  • File filtering, and .nuspec metadata property replacement are working as far as I can tell
  • It also looks like it would be fairly simple to bring in the command line specification of file exclude filters in from NuGet, the code would be almost copy/paste for the filtering.

TheCakeIsNaOH avatar Jun 17 '22 22:06 TheCakeIsNaOH

A couple of things I noted today in research:

  • The protocol for pushing is the same between v2 and v3, so there is not a new/separate push URL for nuget.org or for Nexus as they both would have the same push URL between v2 and v3 APIs. https://github.com/NuGet/docs.microsoft.com-nuget/issues/964 and https://docs.microsoft.com/en-us/nuget/api/package-publish-resource#base-url
  • There is another way that could be used to get a PackageUpdateResource documented here: https://docs.microsoft.com/en-us/nuget/reference/nuget-client-sdk#push-a-package as compared to the method used when running the nuget push command: https://github.com/NuGet/NuGet.Client/blob/beb3bd51b1d4af1a1c17cbed5a9cc4b190a10a30/src/NuGet.Core/NuGet.Commands/CommandRunners/PushRunner.cs#L34-L42
  • After looking at the metadata code and confirming with Gary, it looks like it will be a requirement to fork the NuGet.Client repository to allow Chocolatey packages to be packed, instead of only NuGet compatible packages. A custom version of the NuGet.Packaging package would be required to add the Chocolatey .nuspec schema. Potentially other packages would also have to be forked, but that is still up in the air.
  • Fortunately, it seems like everything required is in the one repository. There are submodules currently, but they mostly gone in the dev branch: https://github.com/NuGet/NuGet.Client/pull/4536
  • Also fortunately, it looks like that area of the codebase has not changed as much as some other areas, so hopefully it should be fairly smooth to port the relevant commit(s) from nuget-chocolatey.
  • As to my todo from Friday for testing pushing to a UNC path, that is working for me.

TheCakeIsNaOH avatar Jun 21 '22 00:06 TheCakeIsNaOH

In nuget-chocolatey, IPackage inherits from IPackageMetadata (which provides the metadata available in .nupec) and from IServerPackageMetadata (which provides the other metadata not in .nuspec schema but available from v2 API feeds). Therefore,

In NuGet.Client, IPackage has been removed. IPackageMetadata still exists, in almost unchanged form. However, IServerPackageMetadata is not available, with IPackageSearchMetadata taking it's place. IPackageSearchMetadata has all of the metadata from a source, not just the metadata not available in a .nuspec. Therefore, we can't just re-add IPackage, as there would be name conflicts/ambiguity between the two now, for example they both have title.

Instead, I'm thinking of using IPackageMetadata and IPackageSearchMetadata directly, and replacing usages of IPackage with the appropriate one as required. Say for example, with PackageResult, instead of using instance.Package, use instance.PackageMetadata/instance.SearchMetadata (or whatever they are called).

This would require changes in lots of files, but theoretically the changes mostly should just be updating the names of things without too many larger changes. I talked with @gep13 about this, and it seems like a reasonable option to explore.

TheCakeIsNaOH avatar Jun 24 '22 21:06 TheCakeIsNaOH

While looking into getting list/search/find working, there are a couple of fun things I discovered:

  • There is no longer a concept of an AggregateRepository that I could find to have a single repository object with multiple sources. Instead, commands like nuget list and nuget search have to keep a list of sources, and iterate through them to initialize the repository, get the resource, get the package(s), and print out the results. This means that Chocolatey will have to take ownership of that, although that is an upside is that version resolution behavior can be adjusted as required without having to dive into nuget code (see #283 for example).
  • I do not think there is currently a concept of a source priority in the NuGet.Client codebase, at least nothing close to the way Chocolatey does. It will have to be investigated further.
  • There are a number of nice resources to do some search things, like finding all versions of a package (FindPackageByIdResource), or getting information about a specific package (PackageMetadataResource), or searching for a package (PackageSearchResource). Some of these fit very neatly into a specific use (i.e. specific switch combination) of the list/search/find command, but not all uses.
  • There is also a ListResource, for listing all packages on a source, but unfortunately, that is only available for V2 feeds and local packages. There is no direct equivalent for the V3 api at the moment, https://github.com/NuGet/NuGetGallery/issues/3878
  • The only reason the nuget list command works for the nuget v3 source is because that source has a link that says "here, go look at the v2 feed to list packages", see this https://github.com/NuGet/Samples/pull/41#issuecomment-703971272 I discovered this when nuget list refused to list against my local Nexus instance.
  • The NuGet catalog also exists as a suggested alternative to using the v2 search API, but it is only implemented for some repositories (so nuget.org has it, but my Nexus instance does not). Additionally, there is no library in NuGet.Client that I am aware of that will process this, so Chocolatey would either have to fork NuGet.Jobs to use NuGet.Protocol.Catalog from their gallery jobs implementation or download and process the Json manually.
  • On the upside, the catalog is a good candidate for downloading to use as a local package index, since it is an append only log of additions/modifications/deletions of packages on the repository. And since there is no longer an aggregate repository, then package indices could be stored per source without issue and results combined inside the Chocolatey code as needed.
  • Since not all repositories Chocolatey would use would have one of these options to use for a listing of all packages, then a fallback to the PackageSearchResource with an empty search term would be required to list all packages. The fun part of using this is that it would mean that it is pagination time. Joy. Though it could mean getting results out to the screen quicker instead of waiting for everything, and being able to work around Nexus pagination issues if needed.

TheCakeIsNaOH avatar Jun 24 '22 23:06 TheCakeIsNaOH

Update on handling async:

  • Quite a few of the NuGet library methods are async, but Chocolatey itself is synchronous throughout the codebase.
  • Therefore, await can't be used when calling the NuGet methods.
  • The proper way to fix this is to switch Chocolatey to be async, but this would require quite a few changes, and major breaking change for chocolatey.lib.
  • There are workarounds to be able to run async methods synchronously:
    • https://stackoverflow.com/questions/5095183/how-would-i-run-an-async-taskt-method-synchronously
    • https://stackoverflow.com/questions/9343594/how-to-call-asynchronous-method-from-synchronous-method-in-c
    • https://docs.microsoft.com/en-us/archive/msdn-magazine/2015/july/async-programming-brownfield-async-development
  • The .GetAwaiter().GetResult() workaround seems to be the best option for usage within choco.exe, however as linked above, it could be more susceptible to deadlocks with asp.net or a GUI application.
  • Another option would be to use the Nito.AsyncEx library, which provides AsyncContext.Run(). This seems to work slightly better than the previous option, and is transitively used in other Chocolatey product(s).
  • However, the Nito.AsyncEx library is not strongname signed (and the library author is strongly against strongnaming), so usage would require strong naming it.
  • So sticking with .GetAwaiter().GetResult() for the moment seems like the best option. Using Nito.AsyncEx or switching Chocolatey to async can be investigated later.

After checking in with Gary, I've setup strongnaming during build and switched over to using Nito.AsyncEx, and things seem to be working.

TheCakeIsNaOH avatar Jul 11 '22 23:07 TheCakeIsNaOH

The PackageDownloader was changed in nuget-chocolatey to be overridable. As far as I can tell, this was for the exclusive purpose of being able to add a download progress bar while downloading .nupkgs.

Given the changes, a similar approach may or may not be ideal, but it depends on what is settled on for usage as a packge installation routine, specifically whether to use NuGetPackageManager or not.

Another possibility would be to add the capability to report download progress it to upstream NuGet, or wait for them to add it. Related issues: https://github.com/NuGet/Home/issues/10430 https://github.com/NuGet/Home/issues/4346 https://github.com/NuGet/Home/issues/11142

TheCakeIsNaOH avatar Jul 14 '22 00:07 TheCakeIsNaOH

Commit Hash Link to Commit Commit Date Author Commit Title Status Notes
057ac237 https://github.com/chocolatey/nuget-chocolatey/commit/057ac237acf09b8bdf7bac6e3a265aacd4b44e54 2021-10-15 Gary Ewan Park (maint) Make nuspec more specific 🟦 Not relevant
4cfc25ae https://github.com/chocolatey/nuget-chocolatey/commit/4cfc25ae1fe05cfc37c4d72e31c0a94bab05ebed 2021-10-15 Gary Ewan Park Add generated file to gitignore 🟦 Not relevant
8fc6f5d8 https://github.com/chocolatey/nuget-chocolatey/commit/8fc6f5d827883f5d54ebbec7ec2b54af6ad50f60 2021-10-14 Gary Ewan Park (build) Update release notes 🟦 Not relevant
a4bdeb1d https://github.com/chocolatey/nuget-chocolatey/commit/a4bdeb1d305e6497e4ddabf73d6f7b7b2da7badb 2021-10-14 Gary Ewan Park Merge pull request chocolatey/nuget-chocolatey#28 from TheCakeIsNaOH/xml-escape 🟦 Not relevant
1e3b6339 https://github.com/chocolatey/nuget-chocolatey/commit/1e3b633974a2d53333ba283c9f465616d9d945bd 2021-09-29 TheCakeIsNaOH (chocolatey/nuget-chocolatey#27) Escape PackageProperties elements 🟦 Not relevant Tested and no longer an issue
f4714091 https://github.com/chocolatey/nuget-chocolatey/commit/f4714091788588eb84ba5f106682c569ff70a49d 2021-10-14 Gary Ewan Park Merge pull request chocolatey/nuget-chocolatey#18 from jsyeo/jsyeo-mutex-name 🟦 Not relevant
0703b6bd https://github.com/chocolatey/nuget-chocolatey/commit/0703b6bd61870c6375ace8d1ac12fc3d8e8751c9 2019-04-30 Jason Yeo (chocolatey/nuget-chocolatey#17) Remove slashes from mutex name 🟦 Not relevant NuGet.Client appears to no longer use Mutexes, or at least not use them directly
4c6bbc9d https://github.com/chocolatey/nuget-chocolatey/commit/4c6bbc9db181769d05d7c0925f9e00184614c544 2021-05-06 Gary Ewan Park (build) Update release notes 🟦 Not relevant
54179574 https://github.com/chocolatey/nuget-chocolatey/commit/5417957453419052fc07f689733261b8adae901c 2021-05-06 Gary Ewan Park Merge pull request #24 from gep13/issue-23 🟦 Not relevant
a211f197 https://github.com/chocolatey/nuget-chocolatey/commit/a211f1975a04c440497ec5a2a3e2f4810bf5cf41 2021-05-05 Gary Ewan Park (chocolatey/nuget-chocolatey#23) Prevent creation of empty NuGet.Config file 🟩 Ported Need to add tests for this.
f873e608 https://github.com/chocolatey/nuget-chocolatey/commit/f873e608faa86ef79832bbbe400f5496350844cb 2021-05-05 Gary Ewan Park (chocolatey/nuget-chocolatey#25) Add basic nuspec file for generating package 🟦 Not relevant Libraries already packaged into .nupkgs
7e8d6860 https://github.com/chocolatey/nuget-chocolatey/commit/7e8d68608f604ff2523789f7b778af3424d0af0b 2021-05-05 Gary Ewan Park (chocolatey/nuget-chocolatey#25) Add strongname script and key 🟩 Ported Ported the key, they already have strongnaming setup in msbuild.
6556972b https://github.com/chocolatey/nuget-chocolatey/commit/6556972b30718ab560f229fe9d5b0bfe533502e1 2021-05-05 Gary Ewan Park (chocolatey/nuget-chocolatey#25) Add ILMerge files into repository 🟦 Not relevant
07a5350d https://github.com/chocolatey/nuget-chocolatey/commit/07a5350defbf4465bd6ade41bd41c82c561a9501 2021-05-04 Gary Ewan Park Merge pull request chocolatey/nuget-chocolatey#21 from TheCakeIsNaOH/empty-files-fix 🟦 Not relevant
afcef12b https://github.com/chocolatey/nuget-chocolatey/commit/afcef12bfdfd974ed3e2876567a54ca48fa13d09 2021-05-04 Gary Ewan Park (maint) Remove unnecessary whitespace 🟦 Not relevant
4a7603a3 https://github.com/chocolatey/nuget-chocolatey/commit/4a7603a34e5c57a491c72e3b78486a2c099b242e 2021-04-21 TheCakeIsNaOH (#2258) Pack all files if no files element in nuspec 🟦 Not relevant Tested and it is working in upstream
a8e34324 https://github.com/chocolatey/nuget-chocolatey/commit/a8e3432444ae48e54ef32c007729e15e0cfdcd7f 2021-05-04 Gary Ewan Park Merge branch 'TheCakeIsNaOH/2.11_adds' into 2.11_adds 🟦 Not relevant
979a47d7 https://github.com/chocolatey/nuget-chocolatey/commit/979a47d78d3fc82e93d195db7df6432f77aef9c1 2020-07-03 TheCakeIsNaOH (#502) Fix path separator in nuspec file element 🟩 Ported Ported to keep chocolatey/choco#2166 fixed, but chocolatey/choco#502 is not an issue. Perhaps upstream
bedcd19c https://github.com/chocolatey/nuget-chocolatey/commit/bedcd19c0a78deae565d6c9bbc4d27e659fc14c7 2019-03-02 Rob Reynolds (maint)(log) surround source value with apostrophes 🟦 Not relevant
a40709a6 https://github.com/chocolatey/nuget-chocolatey/commit/a40709a67a46a2c168b89ec8c4886f565398ba25 2019-03-02 Rob Reynolds (chocolatey/nuget-chocolatey#14) Do not fail grabbing source string 🟦 Not relevant The dataservicerepository is no longer a thing
3fd4ff1b https://github.com/chocolatey/nuget-chocolatey/commit/3fd4ff1b64b4a28b5426cfcd5d0cae6b2e7013fd 2019-03-02 Rob Reynolds (chocolatey/nuget-chocolatey#14) Provide override to set IgnoreFailingRepositories 🟦 Not relevant The aggregaterepository is no longer a thing
8ce0c247 https://github.com/chocolatey/nuget-chocolatey/commit/8ce0c247d6f8664375e22710d415c6630116534c 2017-07-14 Rob Reynolds (maint) fix null reference possibilities 🟦 Not relevant Neither class still exists
77cf33c6 https://github.com/chocolatey/nuget-chocolatey/commit/77cf33c64cfa3182448993f495730444f79e8a2b 2017-03-30 Rob Reynolds (chocolatey/nuget-chocolatey#11) move release version to end 🟪 Action Later Ignoring unless it is potentially upstreamed
f5ff6208 https://github.com/chocolatey/nuget-chocolatey/commit/f5ff6208ab560da8d2782d07f05c3846ce62f265 2017-03-29 Rob Reynolds (chocolatey/nuget-chocolatey#14) Warn on failing repositories 🟦 Not relevant Aggregate repository is not longer available, and repositories are instead individually accessed
0028e41e https://github.com/chocolatey/nuget-chocolatey/commit/0028e41ee9741bb941b011a91f4a4fc13f37bdb8 2017-03-29 Rob Reynolds (chocolatey/nuget-chocolatey#10) ensure source uris bypass proxy 🟥 Needs investigation Potentially upstream
ea24fe77 https://github.com/chocolatey/nuget-chocolatey/commit/ea24fe77cfc5b946770b00ee6708448ebc731b48 2017-03-22 Rob Reynolds (maint) log unable to delete directory as debug 🟦 Not relevant Class no longer exists, instead system.io is used directly
144c49ce https://github.com/chocolatey/nuget-chocolatey/commit/144c49ceedfb7980e00aaa8e504c7ca3fabc1997 2017-03-21 Rob Reynolds (chocolatey/nuget-chocolatey#13) override original version 🟩 Ported Ported into Chocolatey, into the ChocolateyPackageMetadata class
099581db https://github.com/chocolatey/nuget-chocolatey/commit/099581db62fb998c21be40c0003b35551a3d1c48 2017-03-21 Rob Reynolds (maint) formatting 🟦 Not relevant
b9c4bc53 https://github.com/chocolatey/nuget-chocolatey/commit/b9c4bc532fba01a5310c16cdb3c80ef6c4b62d8c 2017-03-21 Rob Reynolds (chocolatey/nuget-chocolatey#12) add software display name / version 🟩 Ported
6bd10884 https://github.com/chocolatey/nuget-chocolatey/commit/6bd10884d41d7f41ea1d9b0989574b20b879b2ed 2017-03-14 Rob Reynolds (maint) notes on possible upload progress 🟪 Action Later Download Progress Related
1d72f45e https://github.com/chocolatey/nuget-chocolatey/commit/1d72f45eebb71e5c2a65640965053ee7081ce88d 2017-03-14 Rob Reynolds (chocolatey/nuget-chocolatey#11) SemanticVersion - Add package release version 🟪 Action Later Ignoring unless it is potentially upstreamed
6adec652 https://github.com/chocolatey/nuget-chocolatey/commit/6adec65238479589b50804982820402dd060dfcb 2017-03-03 Rob Reynolds (chocolatey/nuget-chocolatey#10) bypass proxy on interface 🟥 Needs investigation Potentially upstream
da4fb5b1 https://github.com/chocolatey/nuget-chocolatey/commit/da4fb5b1689a08274a56cab286a8a51b0c46b95f 2017-03-03 Rob Reynolds (chocolatey/nuget-chocolatey#5) ensure download is virtual 🟦 Not relevant Download Progress Related, but not relevant to current codebase
2e8101da https://github.com/chocolatey/nuget-chocolatey/commit/2e8101da8b33fd8031e84b8610afe5052c7864ae 2017-03-01 Rob Reynolds Merge branch 'pr9' into 2.11_adds 🟦 Not relevant
26559a4a https://github.com/chocolatey/nuget-chocolatey/commit/26559a4a96b0388442a2933ee3fa0ab683e11c26 2017-01-22 Jakub Cislo (chocolatey/nuget-chocolatey#8) Accept null as proxy override 🟩 Ported Potentially upstream
7e9da862 https://github.com/chocolatey/nuget-chocolatey/commit/7e9da862f94ac2a9682f9154d8fe5c200dd72493 2017-03-01 Rob Reynolds (chocolatey/nuget-chocolatey#10) Allow source to bypass proxy 🟥 Needs investigation Potentially upstream
ee5f731e https://github.com/chocolatey/nuget-chocolatey/commit/ee5f731e27500cac1c66fb25ffae3740bd04b3b6 2017-03-01 Rob Reynolds (chocolatey/nuget-chocolatey#5) Adjust to use IPackageDownloader 🟩 Ported Download Progress Related
b3096c6a https://github.com/chocolatey/nuget-chocolatey/commit/b3096c6a4b05841617d3a7778e581a36335cf382 2017-03-01 Rob Reynolds (chocolatey/nuget-chocolatey#4) adjust specs for new logging types 🟪 Action Later Logging Related, to do after we figure out what is excessive logging
19986e7 https://github.com/chocolatey/nuget-chocolatey/commit/19986e79e573ff2bcd34f0ed9ce758f3282576f6 2016-12-18 Rob Reynolds (chocolatey/nuget-chocolatey#5) Interface for PackageDownloader 🟩 Ported Download Progress Related
a48c742c https://github.com/chocolatey/nuget-chocolatey/commit/a48c742c5d3581e3b72cba54aa311e5dca439eae 2016-12-18 Rob Reynolds (chocolatey/nuget-chocolatey#4) move most logging items to verbose 🟪 Action Later Logging Related, to do after we figure out what is excessive logging
cf748a77 https://github.com/chocolatey/nuget-chocolatey/commit/cf748a77e18b444ac4f1471135e1980728a9f3fb 2016-12-18 Rob Reynolds (chocolatey/nuget-chocolatey#4) Add more logging types 🟪 Action Later Logging Related, to do after we figure out what is excessive logging
d344d472 https://github.com/chocolatey/nuget-chocolatey/commit/d344d4721e8d7aaf1b705ad3000007d50ee5bf49 2016-12-15 Rob Reynolds remove empty entries 🟩 Ported Potentially upstream
77c3b12d https://github.com/chocolatey/nuget-chocolatey/commit/77c3b12dc10759ff1d061fa5cddf574a441e3653 2016-12-15 Rob Reynolds (maint) formatting 🟦 Not relevant
4e28997f https://github.com/chocolatey/nuget-chocolatey/commit/4e28997f92d57e3a278b9e5547eedc1eabec20b7 2016-02-04 Rob Reynolds (#607) Pack strips out Choco specific metadata 🟩 Ported
fa884f8b https://github.com/chocolatey/nuget-chocolatey/commit/fa884f8b47c9778f504e46b201a5a31a54b5a10a 2016-02-02 Rob Reynolds Data Services Package must be simple primitives 🟦 Not relevant The upstream NuGet.Server is still on NuGet.Core, with no plans to update to v3+ libraries. There are other more complex data types in use now, so if NuGet.Server ever was ported (and hence ChocolateySimpleServer), it would be a non-issue
133552a0 https://github.com/chocolatey/nuget-chocolatey/commit/133552a05d061bc01d6bf76ef9ee770bcb4a7f3e 2016-01-20 Rob Reynolds Download Cache information 🟩 Ported
3866411a https://github.com/chocolatey/nuget-chocolatey/commit/3866411adaf37867c0be41a15cae6285e9bfc185 2016-01-20 Rob Reynolds (#403) Provide server related information 🟩 Ported
f303bba2 https://github.com/chocolatey/nuget-chocolatey/commit/f303bba243491755c58ecf2906dddb13ee56f3eb 2016-01-04 Rob Reynolds version bump to 2.11.0 🟦 Not relevant
ef32173d https://github.com/chocolatey/nuget-chocolatey/commit/ef32173d0fcfddc487072022fa310267958e8e14 2015-11-17 eugene.tolmachev (#399) Client certificate provider 🟥 Needs investigation
4b206ed6 https://github.com/chocolatey/nuget-chocolatey/commit/4b206ed6bc550dbff1518c71c5446c9040dca30f 2015-09-22 Rob Reynolds #243 Allow explicit proxy override 🟩 Ported Potentially upstream
17ec714d https://github.com/chocolatey/nuget-chocolatey/commit/17ec714dafd9ee19ee4e5c6ae958d19243bb729d 2015-09-21 Rob Reynolds comment out this bit - tests pass again 🟦 Not relevant This is "reverting" part of the previous commit
e364bb8b https://github.com/chocolatey/nuget-chocolatey/commit/e364bb8b0435a836e79332d90865f938e5811da5 2015-09-21 Rob Reynolds XDT changes 🟦 Not relevant Classes are public, and the null check is "reverted" by commenting out in next commit.
e22839d2 https://github.com/chocolatey/nuget-chocolatey/commit/e22839d2ec84f678d49c9ec93cc3989ee34a59f1 2015-06-12 Rob Reynolds (maint) release build by default 🟦 Not relevant
9b703a89 https://github.com/chocolatey/nuget-chocolatey/commit/9b703a89ffdffed488f04c4daa10eb961470e945 2015-06-12 Rob Reynolds Add Package Information Fields 🟩 Ported
27671d27 https://github.com/chocolatey/nuget-chocolatey/commit/27671d2792ebac02f61a8e314a1b5538379b3212 2015-06-12 Rob Reynolds Allow IPackageMetadata to be partial interface 🟩 Ported
2a0aa362 https://github.com/chocolatey/nuget-chocolatey/commit/2a0aa362b782f9e7fd9c044c97155780a10960f5 2015-06-11 Rob Reynolds Set the version specifically for ILMerging 🟩 Ported
8acde68e https://github.com/chocolatey/nuget-chocolatey/commit/8acde68e219932859a26e394e5e036cda25406e4 2015-06-11 Rob Reynolds Remove the content folder restriction 🟥 Needs investigation
740a034f https://github.com/chocolatey/nuget-chocolatey/commit/740a034ff5f480c3a6a9fce7a5e5cdfa1dab7d1d 2015-06-11 Rob Reynolds build working 🟦 Not relevant

TheCakeIsNaOH avatar Jul 14 '22 17:07 TheCakeIsNaOH

The AsyncContext.Run seems to have created some deadlocks, so I've reverted that change for the moment, and instead will work on it in a seperate branch: https://github.com/TheCakeIsNaOH/choco/tree/nuget-v3-nito-asyncex

TheCakeIsNaOH avatar Jul 15 '22 17:07 TheCakeIsNaOH

Coverage Status

Coverage decreased (-1.5%) to 27.08% when pulling a499a40d5abf158471571087d45e2a61db702f21 on TheCakeIsNaOH:nuget-v3-devel into 2864c01d72bd2fda1fa6833acdfcea2e51be16f5 on chocolatey:develop.

coveralls avatar Jul 15 '22 18:07 coveralls

Previously, PackageDownloader was used to download .nupkgs with NuGet.HttpClient to setup the connection. Now instead there are various implementations of the DownloadResource class that are used. These implementations use HttpSource setup the connection and open a stream, and then copy that stream to a local file.

The HttpSource can be switched to an interface and allowed to be overridden as needed for connection purposes.

However, there is no longer any easy way to add in download progress as far as I can see. For example, when downloading from a v2 feed, Stream.CopyToAsync is used in GetDownloadResultUtility.DirectDownloadAsync, and that does not have a way to monitor progress. So the previous method used with nuget-chocolatey will not work for download progress.

TheCakeIsNaOH avatar Jul 19 '22 16:07 TheCakeIsNaOH

With Chocolatey CLI versions that use NuGet.Core, when upgrading a package that has a "parent" package, it will upgrade the parent package to the lowest version that will work with the dependency package being installed, while with the NuGet SDK, it is the highest version. A "parent" package means a package that depends on the dependency package that is being upgraded.

Here is the integration test example. It has a starting situation of hasdependency 1.0.0 and isdependency 1.0.0 being installed, where hasdependency 1.0.0 depends on isdependency [1.0.0, 2.0.0). Then upgrade is run for hasdependency, with the latest version of 2.1.0, which is not compatible with the installed version of hasdependency. Therefore, hasdependency is upgraded to a version that is compatible with isdependency 2.1.0. The source has multiple versions of hasdependency that are compatible (with isdependency 2.1.0) ranging from version 1.0.1 to 2.1.0.

With NuGet.Core (where PackageManager is set to DependencyVersion.Highest), then it will choose version 1.0.1 of hasdependency, i.e. the lowest compatible version available. With NuGet SDK package resolver (Set to DependencyVersion.Highest), then it will choose version 2.1.0 of hasdependency, i.e. the highest compatible version available.

This does seem to be a breaking change, as it is behavior that is integration tested for, and the tests do fail when using NuGet SDK. However the NuGet SDK behavior where it installs the highest compatible version seems to be the correct behavior, given that it is set to DependencyVersion.Highest.

I don't think there is a way to tweak this behavior without changing DependencyVersion.Highest to something else, which would break other things. Or of course, changing code in the NuGet.Resolver library, which I am not touching if at all possible.

So, the question becomes, are there any operational cases where this breaking change would be a problem? If so, this needs to be investigated further to see if there is a way to mitigate or it that case. If not, then it should be documented during release, and the tests changed to reflect the new behavior

I'll assume it is the second option for the moment, and modify the tests to assume the highest compatible version of parent package(s) will be installed, instead of the lowest.

TheCakeIsNaOH avatar Jul 26 '22 20:07 TheCakeIsNaOH

The NuGet.Versioning library has both a SemanticVersion class and a NuGetVersion class. The SemanticVersion class is a strict semver 2.0 system, while the NuGetVersion class has the support for the fourth revision element in the version. See documentation here: https://docs.microsoft.com/en-us/nuget/concepts/package-versioning#where-nugetversion-diverges-from-semantic-versioning

Therefore, usages of SemanticVersion from NuGet.Core have been switched to use NuGetVersion, and same for any further uses unless there is some reason to use strict semver 2 in a specific area.

Another change from NuGet.Core is that the Constants class has been renamed to NuGetConstants, but some things are now in PackagingConstants. All of the ones that Chocolatey uses (like .nupkg, .nuspec) are still available, so it is simply a matter of a search+replace.

For reading .nupkg files, NuGet.Core had OptimizedZipPackage. The equivalent in NuGet SDK is PackageArchiveReader, which offers similar functionality. It does have to be disposed of though, so using statements need to be added in, but fortunately, this does not translate to huge amounts of indentation changes since most times it is being used in Chocolatey, it is just a line or two of getting the metadata from the .nuspec in the .nupkg

TheCakeIsNaOH avatar Aug 02 '22 19:08 TheCakeIsNaOH

Proposal for a branching strategy for the NuGet.Client fork:

  • Upstream tracks development in the dev branch as the main branch PRs target. Then for a release a release-A.B.x branch is created (where A.B is the major.minor version like 6.3), and any fixes for A.B are pull into that branch (and/or picked from the dev branch).
  • From what I understand, the intention is for Chocolatey to create custom builds with changes, but that follow upstream fairly closely. So the intent is for the changes to be more of a patch set that is applied on top of the latest code, instead of a major fork that changes lots of things and diverges from upstream.
  • So, assuming we want to use git to add the patch set as commits, instead of having a separate repository of .patch files or something, that leaves two options: merging changes, or rebasing on top of changes.
  • Given that rebasing the patch commits on top of new upstream commits (in the same branch) means rewriting history, that is a no-go for a number of reasons.
  • But merging could work. So base the fork on the dev branch, and then upstream changes get brought in with merge commits. The downside of this is that it makes the history very messy, and it could be hard to get a sense of what the actual patch set on top of upstream is, because there are the patches mixed in on the commit history.
  • So, instead I propose basing the fork on the release-A.B.x branch. Then once upstream creates the release-A.B+1 branch, the required commits can be cherry-picked onto the release-A.B+1 branch (and cleaned up if needed to create a streamlined patch set). If there are any upstream commits on the release- branch, then they can be merged in. This allows for a much shorter history that has to be picked through to separate out upstream work from work on the fork. The downside is that it means that the active development branch of the fork is changing frequently.

To keep the fork up to date, it may be a good idea to utilize a bot to create PRs to merge in changes on the release-* branch from upstream, and to create new release-* branches as they become available.

  • One option for merging in changes is pull: https://github.com/wei/pull
  • An option for doing both would be to build something using probot: https://github.com/probot/probot
    • The Nextcloud organization has one that uses probot to backport code to previous release branches: https://github.com/nextcloud/backportbot

TheCakeIsNaOH avatar Aug 02 '22 21:08 TheCakeIsNaOH

The NuGet PackageResolver class does not have proper documentation, but fortunately, with the method parameter comments and reading through how NuGetPackageManager uses it, it is possible to set up. The Resolve() method takes a PackageResolverContext, and the parameters passed into that are where the real action is in regard to using the resolver.

There is not a distinction between upgrades and installs for the dependency resolver, so more or less the same setup can be used for installs and upgrades.

Here is my explanation of what to pass to the PackageResolverContext to use it with Chocolatey:

  • dependencyBehavior: This is set to DependencyBehavior.Highest, same a used with the NuGet.Core package manager. Given what Chocolatey's intended use case, having everything be the newest version that work for everything is ideal.
  • targetIds: A IEnumerable of strings, with just one entry with the ID of the package being installed.
  • requiredPackageIds: An IEnumerable of strings. This should have the package IDs of all packages locally installed, minus the ID of the package being installed (in the case that it was already installed, and it is being forced or a different version is being installed).
  • packagesConfig: An IEnumerable of PackageReference (a representation of a package ID, version, and allowed framework). Same as above, should include all packages installed locally, minus any versions of the package to be installed.
  • preferredVersions: An IEnumerable of PackageIdentity (a representation of a package ID and version). Same as the two above, should include all packages installed locally, minus any versions of the package to be installed.
  • availablePackages: An IEnumerable of SourcePackageDependencyInfo (A partial set of metadata about a package). This should include the dependency infos of all locally installed packages (again minus any versions of the package that is to be installed), joined with the dependency info of the package version to be installed, and the dependency infos of all available versions all direct and child dependencies of the package version to be installed.
  • packageSources: Pass in the PackageSource from the enabled sources
  • log: Pass in a ILogger like ChocolateyNuGetLogger

These have worked for all situations in the tests, and everything I have played with manually. There could be situations, say for example with multiple versions of packages installed, and complex dependency trees, that it does not work for, but nothing has popped out yet.

As for uninstalls, instead of using PackageResolver, use NuGet.PackageManagement.UninstallResolver.GetPackagesToBeUninstalled.

TheCakeIsNaOH avatar Aug 02 '22 22:08 TheCakeIsNaOH

Features to possibly add

Investigate adding support for v3 catalog reading/parsing

This would be for the purpose of adding a local index of packages: https://github.com/chocolatey/choco/issues/820

Some NuGet v3 feeds support the catalog resource: https://docs.microsoft.com/en-us/nuget/api/catalog-resource

It is a append-only resource that records all package additions/changes/unlistings in a json format. So it could be paged through and saved locally, and updated via a new update command to update the index. Then the data can be read locally and parsed as needed.

There are a few samples and projects that use it, but it is not something available through NuGet.Client as far as I am aware, so it would have to be implemented within Chocolatey.

https://github.com/NuGet/NuGet.Jobs/tree/main/src/NuGet.Protocol.Catalog https://github.com/emgarten/NuGet.CatalogReader https://github.com/NuGet/Samples/tree/main/CatalogReaderExample

Figure out version ranges, potentially get version range working for --version and/or pin

Feature requests from these two issues: https://github.com/chocolatey/choco/issues/1278, https://github.com/chocolatey/choco/issues/800

The NuGet SDK provides a nice class called VersionRange that allows representing a range of allowed versions. It should be possible to be able to use this to parse and represent a version range passed in.

The version range can be saved within the pin file, if the pin file is empty, then it falls back to the previous behavior of pinning the current version.

Then during upgrade, the metadata for all package versions (for the package being upgraded) can be gotten, and filtered to versions allowed in the version range, and then the max of those be used for upgrade.

Add and test validation of package signing

Feature request here: https://github.com/chocolatey/choco/issues/504

This may already be working, but has not been tested yet. It should be a matter of setting the ClientPolicyContext correctly in the ChocolateyNuGetProjectContext implementation of INuGetProjectContext. Which then gets passed into FolderNuGetProject.InstallPackageAsync which has signature validation, depending on what ClientPolicyContext is set to.

Add in per-source confirmation?

Feature request here: https://github.com/chocolatey/choco/issues/179

Given the manual package installation routine, it would be possible to toggle confirmation on and off without too much issue for each package.

This may or may not require changes to NuGet.Client, similar to what is required for https://github.com/chocolatey/choco/issues/1486 to make sure that the source is available inside Chocolatey to check if it require confirmation.

Add in checksum validation of downloaded .nupkgs?

Feature request: https://github.com/chocolatey/choco/issues/1144

It may be a good idea to investigate adding checksum validation at the same time as validating the .nupkg code signature. Checksums are provided from some repositories, and exposed via IPackageSearchMetadata

Improve the NuGet user-agent?

The user-agent header is being partially set by Chocolatey, but it always includes the version of the NuGet library, and there is no way to currently change that.

So, I've set it to do this, Chocolatey Command Line/<version> via NuGet Client/<nuget version>. This method is fine, but potentially it may want to be changed.

There is an upstream issue open here to allow complete control over the user-agent string: https://github.com/NuGet/Home/issues/7464

Install a (locally available) non-packed/extracted .nupkg?

Feature request #903, may require #624

This should be a case of coping the required files, and then moving right along to the continue action to run the package automation scripts.

Don't save the .nupkg, only keep the .nuspec

Feature request #624

This could be as simple as flipping the current PackageSaveMode.Nupkg | PackageSaveMode.Nuspec | PackageSaveMode.Files to PackageSaveMode.Nuspec | PackageSaveMode.Files in the package extraction context in the ChocolateyNuGetProjectContext implementation of INuGetProjectContext

It would breaks lots of tests, and potentially anything else in Chocolatey that relies on the .nupkg being there, but I think those all could be worked around by just reading the info from the .nuspec directly, and if a list of files is required, reading the .files file for the package. It also should be looked at if any of the list -lo things get broken, but again, that should be reasonable to work around.

Allow dependencies from alternate sources?

Feature request #879

Probably requires changes to the .nuspec schema to add an extra element. Due to the manual installation routine, any dependencies from alternate sources can be removed before passing things to the dependency resolver, then it should be a case of just calling the alternate source for the install of the dependency.

TheCakeIsNaOH avatar Aug 03 '22 20:08 TheCakeIsNaOH

As side by side is going to be removed, I'm updating this to not include side by side support. The code with side by side support is backed up in this branch: https://github.com/TheCakeIsNaOH/choco/tree/nuget-v3-sidebyside

And here is the branch with the previous version of the NuGet.Client code: https://github.com/TheCakeIsNaOH/NuGet.Client/tree/sidebyside-backup

TheCakeIsNaOH avatar Sep 25 '22 02:09 TheCakeIsNaOH

To be able to rebase this without a huge amount of hassle, I had to squash a bunch of commits, since otherwise changes would not merge smoothly in. As noted in the last comment, the previous version of this branch is still available here: https://github.com/TheCakeIsNaOH/choco/tree/nuget-v3-sidebyside

TheCakeIsNaOH avatar Nov 08 '22 03:11 TheCakeIsNaOH

Failures on GitHub action here is expected, we have not made the NuGet.Client libraries we use public yet.

AdmiringWorm avatar Nov 23 '22 17:11 AdmiringWorm