Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH2578: Support version ranges for referenced nuget packages #2576

Open
wants to merge 3 commits into
base: develop
from

Conversation

@msioen
Copy link

commented Jul 17, 2019

Extended the version pinning support of the nuget package installer to also support version ranges.

Would support formats like the following to indicate you want to stay on major version 1 but ok to get minor/fix updates.
#addin nuget:?package=Cake.Foo&version=[1.0.0,2.0.0)

Our usecase is for a set of shared buildscripts which will be used across several projects. We'd like to version pin on major changes but still allow bugfixes to go through without having to change all the projects. This isn't currently possible.

Note: this code isn't really covered by any unit test - I'm also not really sure on how to add one to specifically test this.

@devlead
Copy link
Member

left a comment

Hi,

Thanks for your contribution! ♥

  • Is there an GitHub issue in accordance with with our contribution guidelines?
  • Could you please add some unit tests for this change and preferably also some integration tests.
@msioen

This comment has been minimized.

Copy link
Author

commented Jul 17, 2019

Must have glossed over the required issue while reading the guidelines, sorry. Just created one with some additional background: #2578

I'll have a look at tests tomorrow. Can you give me a hint how to start on this? I might be missing something but there doesn't seem to be a test I can really start from.

  • the 'uri parsing' logic could be made public and unit tested separately: clear input uri and output nuget version with expected data => this does change public interface
  • a separate testfixture could be made for the Install/NuGetPackageInstaller - currently only the mocked nuget installer is used in tests => not sure if this will work properly
  • maybe something else?

I'd like to avoid writing the tests in a way that aren't desired

@devlead devlead changed the title Support version ranges for referenced nuget packages GH2578: Support version ranges for referenced nuget packages Jul 19, 2019

@devlead

This comment has been minimized.

Copy link
Member

commented Jul 19, 2019

Regarding tests we could start with integration tests ( https://github.com/cake-build/cake/blob/develop/tests/integration/Cake.Core/Scripting/AddinDirective.cake )

I.e. load an nuget package an with an in-between version number and check assembly version on a type in that assembly.

@msioen

This comment has been minimized.

Copy link
Author

commented Jul 20, 2019

Pushed some tests. Note that my initial example wasn't correct according to expected nuget versioning behaviour.
'normal' version ranges will favor the lowest version possible within the range ie [1.0.0,2.0.0) will return 1.0.0 if both 1.0.0 and 1.1.0 exist.
wildcard version ranges do offer the functionality I proposed which is part of the same nuget versioning class so also implemented. ie [1.*,2.0.0) will favor the highest version possible within 1->2 major range

@megakid

This comment has been minimized.

Copy link

commented Aug 27, 2019

Is there anything I can help out here? This is a great feature that'd we're looking forward to putting to good use.

@msioen

This comment has been minimized.

Copy link
Author

commented Aug 27, 2019

Kind of lost track of this due to holidays. The code and unit tests work on mac and linux, no idea why the tests fail on Windows.

@megakid (or someone else)
I currently don't have a dev machine setup with windows so if anyone can have a look on Windows what's going wrong there that'd be great.

@megakid

This comment has been minimized.

Copy link

commented Aug 27, 2019

Ok so locally on my windows box, the only test that fails is Tool_settings_collection_properties_must_be_initialized. However, when I check out the develop branch, the same test fails so unsure if that's the test failing above.

Unfortunately the build history has already been cleared - can we trigger another build somehow? a new commit to this branch perhaps?

@megakid

This comment has been minimized.

Copy link

commented Aug 27, 2019

Actually that test only failed within Visual Studio.

Using build.ps1, everything worked. I can't find the Azure DevOps pipeline definitions so unable to confidentally fully reproduce what's happening there. Perhaps we can rebase/update this branch to current develop to retrigger a build and see if it works.

@msioen msioen force-pushed the msioen:feature/nuget-versionranges branch from 5a21c15 to 89850e9 Sep 11, 2019

@msioen

This comment has been minimized.

Copy link
Author

commented Sep 11, 2019

I had a chance to test on Windows today as well. The integration tests seem to work ok locally. Rebased develop and pushed to see if the automated builds go through or not.

@megakid

This comment has been minimized.

Copy link

commented Sep 11, 2019

I can see the failure now but haven't looked at what's causing it:

[xUnit.net 00:00:00.8333158]     HWApp.Tests.GreeterTests+TheGreaterMethod.Should_Not_Fail_Test [FAIL]
  X HWApp.Tests.GreeterTests+TheGreaterMethod.Should_Not_Fail_Test [12ms]
  Error Message:
   Assert.NotEqual() Failure
Expected: Not "true"
Actual:   "true"
  Stack Trace:
     at HWApp.Tests.GreeterTests.TheGreaterMethod.Should_Not_Fail_Test() in D:\a\1\s\tests\integration\temp\Cake.Common\Tools\DotNetCore\hwapp.tests\Tests.cs:line 14

Test Run Failed.
Total tests: 3
     Passed: 2
     Failed: 1
 Total time: 2.5399 Seconds
Shutting down MSBuild server...
Shutting down VB/C# compiler server...
MSBuild server shut down successfully.
VB/C# compiler server shut down successfully.
Package Source with Name: TestSource added successfully.
Package source with Name: TestSource removed successfully.
Error: Failed to install addin 'Serilog'.
An error occurred when executing task 'Cake.Nuget.Install.NugetPackageInstaller.VersionPinFull'.
Error: One or more errors occurred. (.NET Core CLI: Process returned an error (exit code 1).)
	.NET Core CLI: Process returned an error (exit code 1).
An error occurred when executing task 'Run-Integration-Tests'.```
@megakid

This comment has been minimized.

Copy link

commented Sep 12, 2019

With debug logging:

Cake.Nuget.Install.NugetPackageInstaller.VersionPinFull
========================================
Executing task: Cake.Nuget.Install.NugetPackageInstaller.VersionPinFull
Executing: "C:/hostedtoolcache/windows/dotnet/dotnet.exe" "D:/a/1/s/tests/integration/tools/Cake.CoreCLR/Cake.dll" "C:/Users/VSSADM~1/AppData/Local/Temp/91353fbb-c031-4cb6-a5e6-0d83654bd9d8.cake" -verbosity=Diagnostic
Module directory does not exist.
NuGet Config not specified. Will use NuGet default mechanism for resolving it.
Analyzing build script...
Analyzing C:/Users/VssAdministrator/AppData/Local/Temp/91353fbb-c031-4cb6-a5e6-0d83654bd9d8.cake...
Processing build script...
Installing addins...
Error: Cake.Core.CakeException: Failed to install addin 'Serilog'.
   at Cake.Core.Scripting.ScriptProcessor.InstallAddins(IReadOnlyCollection`1 addins, DirectoryPath installPath) in D:\a\1\s\src\Cake.Core\Scripting\ScriptProcessor.cs:line 114
   at Cake.Core.Scripting.ScriptRunner.Run(IScriptHost host, FilePath scriptPath, IDictionary`2 arguments) in D:\a\1\s\src\Cake.Core\Scripting\ScriptRunner.cs:line 152
   at Cake.Commands.BuildCommand.Execute(CakeOptions options) in D:\a\1\s\src\Cake\Commands\BuildCommand.cs:line 41
   at Cake.CakeApplication.Run(CakeOptions options) in D:\a\1\s\src\Cake\CakeApplication.cs:line 45
   at Cake.Program.Main() in D:\a\1\s\src\Cake\Program.cs:line 73
An error occurred when executing task 'Cake.Nuget.Install.NugetPackageInstaller.VersionPinFull'.```

@msioen

This comment has been minimized.

Copy link
Author

commented Sep 12, 2019

Most relevant trace log is the first test run:

========================================
Cake.Nuget.Install.NugetPackageInstaller.VersionPinFull
========================================
Executing task: Cake.Nuget.Install.NugetPackageInstaller.VersionPinFull
Executing: "C:/hostedtoolcache/windows/dotnet/dotnet.exe" "D:/a/1/s/tests/integration/tools/Cake.CoreCLR/Cake.dll" "C:/Users/VSSADM~1/AppData/Local/Temp/3f198b3b-aebd-4ada-8583-4c537b7e1942.cake" -verbosity=Diagnostic
Module directory does not exist.
NuGet Config not specified. Will use NuGet default mechanism for resolving it.
Analyzing build script...
Analyzing C:/Users/VssAdministrator/AppData/Local/Temp/3f198b3b-aebd-4ada-8583-4c537b7e1942.cake...
Processing build script...
Installing addins...
Missing C:\Users\VssAdministrator\.nuget\packages\serilog\2.7.1\serilog.2.7.1.nupkg
  GET https://api.nuget.org/v3/registration3-gz-semver2/serilog/index.json
  OK https://api.nuget.org/v3/registration3-gz-semver2/serilog/index.json 245ms
  GET https://api.nuget.org/v3/registration3-gz-semver2/serilog/page/2.5.0-dev-00817/2.8.1-dev-01052.json
  OK https://api.nuget.org/v3/registration3-gz-semver2/serilog/page/2.5.0-dev-00817/2.8.1-dev-01052.json 68ms
  GET https://api.nuget.org/v3-flatcontainer/serilog/2.7.1/serilog.2.7.1.nupkg
  OK https://api.nuget.org/v3-flatcontainer/serilog/2.7.1/serilog.2.7.1.nupkg 9ms
Acquiring lock for the installation of Serilog 2.7.1
Acquired lock for the installation of Serilog 2.7.1
Installing Serilog 2.7.1.
Completed installation of Serilog 2.7.1
Error: Cake.Core.CakeException: Failed to install addin 'Serilog'.
   at Cake.Core.Scripting.ScriptProcessor.InstallAddins(IReadOnlyCollection`1 addins, DirectoryPath installPath) in D:\a\1\s\src\Cake.Core\Scripting\ScriptProcessor.cs:line 114
   at Cake.Core.Scripting.ScriptRunner.Run(IScriptHost host, FilePath scriptPath, IDictionary`2 arguments) in D:\a\1\s\src\Cake.Core\Scripting\ScriptRunner.cs:line 152
   at Cake.Commands.BuildCommand.Execute(CakeOptions options) in D:\a\1\s\src\Cake\Commands\BuildCommand.cs:line 41
   at Cake.CakeApplication.Run(CakeOptions options) in D:\a\1\s\src\Cake\CakeApplication.cs:line 45
   at Cake.Program.Main() in D:\a\1\s\src\Cake\Program.cs:line 73
An error occurred when executing task 'Cake.Nuget.Install.NugetPackageInstaller.VersionPinFull'.

This clearly shows that nuget works properly and according to the nuget logs that the install of the package is succesful. The ScriptProcessor checks the installed packages for dll files - for some reason it can't find any dll files and concludes that addin install failed.
I tried some setup changes locally but wasn't able to reproduce. Maybe I can add some additional logging in the ScriptProcessor / NugetPackageInstaller to find out some more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.