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

lowest_matching no longer respected from v5.125.3 and onwards #3032

Closed
Allon-Guralnek opened this issue Feb 8, 2018 · 14 comments · Fixed by #3033
Closed

lowest_matching no longer respected from v5.125.3 and onwards #3032

Allon-Guralnek opened this issue Feb 8, 2018 · 14 comments · Fixed by #3033

Comments

@Allon-Guralnek
Copy link

Description

It seems that lowest_matching was respected until 5.124.1 but starting from 5.125.3 and up to the latest version (5.133.0) it doesn't work anymore (for us).

Repro steps

We have the following line in our solution:

nuget Newtonsoft.Json ~> 10.0 >= 9 lowest_matching: true

This should resolve to Newtonsoft.Json 9.0.1 but any NuGets created in that solution should declare a NuGet dependency of [9.0.1, 11.0) (i.e. both 9.x and 10.x). Starting from Paket 5.125.3, it resolves to Newtonsoft.Json 10.0.3.

Expected behavior

The supplied dependency should resolve to package Newtonsoft.Json 9.0.1

Actual behavior

The supplied dependency resolves to package Newtonsoft.Json 10.0.3

Known workarounds

Using Paket version 5.124.1

@inosik
Copy link
Contributor

inosik commented Feb 9, 2018

Can you try changing the version requirement to >= 9 < 11? I use lowest_matching in a lot of projects at work and didn't notice anything wrong with it.

@inosik
Copy link
Contributor

inosik commented Feb 9, 2018

I just tried with this paket.dependencies:

source https://api.nuget.org/v3/index.json
nuget Newtonsoft.Json ~> 10.0 >= 9 lowest_matching: true

group Two

source https://api.nuget.org/v3/index.json
nuget Newtonsoft.Json >= 9 < 11 lowest_matching: true

Latest Paket (5.133.0) resolves both to 9.0.1.

@forki
Copy link
Member

forki commented Feb 9, 2018

resolves to 9.0.1 for me. But I seriously wonder what ~> 10.0 >= 9 actually means?!

/cc @matthid @agross

@agross
Copy link
Contributor

agross commented Feb 9, 2018

~> 10.0 >= 9 == ~> 10.0 as ~> 10.0 requires the version to be >= 9

@forki
Copy link
Member

forki commented Feb 9, 2018

for me that string looks like bug in parser. we should have rejected it.

regarding version: @Allon-Guralnek expect 9.0.1 which it resolves to for me.

@matthid
Copy link
Member

matthid commented Feb 9, 2018

~> 10.0 >= 9 == ~> 10.0 as ~> 10.0 requires the version to be >= 9

The question is: Is that valid syntax or an issue with the parser that we accept it (and do something random with it).

@agross
Copy link
Contributor

agross commented Feb 9, 2018

we should have rejected it

👍

@agross
Copy link
Contributor

agross commented Feb 9, 2018

Is that valid syntax or an issue with the parser that we accept it

The question is what the parser thinks the constraint is. If it is equal to ~> 10.0 then it's not an issue, but we should at least warn the user that they specified something they probably did not intend to.

@forki
Copy link
Member

forki commented Feb 9, 2018

@matthid
Copy link
Member

matthid commented Feb 9, 2018

@Allon-Guralnek To get to the root of this issue: Maybe it's because of some other dependency that paket upgrades to 10? Can you post your full dependencies file or check with paket why Newtonsoft.Json (or post its output)?

@Allon-Guralnek
Copy link
Author

Allon-Guralnek commented Feb 9, 2018

Its seems I have wandered into quite a fundamental issue as to what the dependency syntax should be. I too found that line to be rather odd. But let's get back to the issue at hand. I switched to the following more accepted syntax, but the same bug persists:

nuget Newtonsoft.Json >= 9 < 11 lowest_matching: true

Since it works for all of you as a standalone dependency, I must assume it has to do with other dependencies in our paket.dependency located here (paket.lock for v5.124.1 is here and for v5.134.0 is here). Feel free to clone the repository (on the develop branch) and reproduce the issue yourself, but since you have better things to do I've included a snippet of the outputs from two versions of Paket, to show the difference.

Paket version 5.124.1
Resolving packages for group Main:
 - Newtonsoft.Json 9.0.1
 - Castle.Core 4.2.1
 - Nito.AsyncEx 4.0.1
 - NLog 4.4.12
 - System.ComponentModel.Annotations 4.4.1
 - System.Threading.Tasks.Dataflow 4.8.0
 - ZooKeeperNetEx 3.4.10.1
 - Ninject 3.3.4
 - Ninject.Extensions.Conventions 3.3.0
 - Ninject.Extensions.Factory 3.3.2
 - NUnit 3.9.0
 - DataAnnotationsValidator 2.1.0
 - System.Collections.Immutable 1.4.0
 - Metrics.NET 0.5.3
 - Microsoft.CodeAnalysis.Common is pinned to 1.3.2
 - Microsoft.CodeAnalysis.CSharp is pinned to 1.3.2
 - Microsoft.Orleans.Core is pinned to 1.3.1
 - Microsoft.Orleans.OrleansCodeGenerator is pinned to 1.3.1
[...]
Paket version 5.134.0
Resolving packages for group Main:
 - Microsoft.CodeAnalysis.CSharp is pinned to 1.3.2
 - Microsoft.CodeAnalysis.Common is pinned to 1.3.2
 - Microsoft.Orleans.Core is pinned to 1.3.1
 - Microsoft.Orleans.OrleansRuntime is pinned to 1.3.1
 - Microsoft.Orleans.OrleansProviders is pinned to 1.3.1
 - Microsoft.Orleans.OrleansSqlUtils is pinned to 1.3.1
 - Microsoft.Orleans.OrleansZooKeeperUtils is pinned to 1.3.1
 - Microsoft.Orleans.OrleansCodeGenerator is pinned to 1.3.1
 - Microsoft.Orleans.OrleansCodeGenerator.Build is pinned to 1.3.1
 - Microsoft.Orleans.TestingHost is pinned to 1.3.1
 - Microsoft.Extensions.DependencyInjection.Abstractions is pinned to 1.1.1
 - Microsoft.Extensions.DependencyInjection is pinned to 1.1.1
 - Newtonsoft.Json 10.0.3
 - Nito.AsyncEx 4.0.1
 - System.Threading.Tasks.Dataflow 4.8.0
 - System.ComponentModel.Annotations 4.4.1
 - Castle.Core 4.2.1
 - NLog 4.4.12
 - ZooKeeperNetEx 3.4.10.1
[...]

As you can see, the package resolution order is a bit different, and different version of Newtonsoft.Json are chosen.

Both versions produce the exact same paket why Newtonsoft.Json --details output (apart from the version header):

Paket version 5.134.0
NuGet Newtonsoft.Json - 9.0.1 is a direct (paket.dependencies) dependency.
It is part of following dependency chains:

-> Microsoft.Orleans.OrleansCodeGenerator - 1.3.1
  -> Microsoft.Orleans.Core - 1.3.1
    -> Newtonsoft.Json - 9.0.1 - (>= 7.0.1)

-> Microsoft.Orleans.OrleansCodeGenerator.Build - 1.3.1
  -> Microsoft.Orleans.Core - 1.3.1
    -> Newtonsoft.Json - 9.0.1 - (>= 7.0.1)

-> Microsoft.Orleans.OrleansSqlUtils - 1.3.1
  -> Newtonsoft.Json - 9.0.1 - (>= 7.0.1)

-> Microsoft.Orleans.OrleansSqlUtils - 1.3.1
  -> Microsoft.Orleans.Core - 1.3.1
    -> Newtonsoft.Json - 9.0.1 - (>= 7.0.1)

-> Microsoft.Orleans.OrleansZooKeeperUtils - 1.3.1
  -> Microsoft.Orleans.Core - 1.3.1
    -> Newtonsoft.Json - 9.0.1 - (>= 7.0.1)

-> Microsoft.Orleans.OrleansZooKeeperUtils - 1.3.1
  -> Microsoft.Orleans.OrleansRuntime - 1.3.1
    -> Microsoft.Orleans.Core - 1.3.1
      -> Newtonsoft.Json - 9.0.1 - (>= 7.0.1)

-> Microsoft.Orleans.TestingHost - 1.3.1
  -> Microsoft.Orleans.Core - 1.3.1
    -> Newtonsoft.Json - 9.0.1 - (>= 7.0.1)

-> Microsoft.Orleans.TestingHost - 1.3.1
  -> Microsoft.Orleans.OrleansProviders - 1.3.1
    -> Microsoft.Orleans.Core - 1.3.1
      -> Newtonsoft.Json - 9.0.1 - (>= 7.0.1)

-> Microsoft.Orleans.TestingHost - 1.3.1
  -> Microsoft.Orleans.OrleansRuntime - 1.3.1
    -> Microsoft.Orleans.Core - 1.3.1
      -> Newtonsoft.Json - 9.0.1 - (>= 7.0.1)

Performance:
 - Runtime: 1 second

BTW, since when did you have that awesome paket why feature and why didn't you put an ad spot for it on the Super Bowl so everyone would know about it?!?

@matthid
Copy link
Member

matthid commented Feb 9, 2018

So paket why said 9.0.1 but in the lockfile we have 10.*? But yeah it looks like you found some kind of bug.

@forki
Copy link
Member

forki commented Feb 9, 2018

yes confirmed. it's a bug. possible fix in #3033

@Allon-Guralnek
Copy link
Author

Allon-Guralnek commented Feb 10, 2018

I can confirm the issue with lowest_matching was resolved in v1.135.0. Thanks for the quick response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants