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

Losing version-range when 'dotnet pack' #2883

Open
MiloszKrajewski opened this issue Nov 2, 2017 · 16 comments
Open

Losing version-range when 'dotnet pack' #2883

MiloszKrajewski opened this issue Nov 2, 2017 · 16 comments

Comments

@MiloszKrajewski
Copy link
Contributor

Description

Yet again, it might not be paket's problem, but maybe there is something you can do about it.

Guys from 'NLog' team made a mess and rolled back version making making 4.5 newer than 5.0.

My library, let's call it libA requires version 4.5 so paket.dependencies contains:

nuget NLog ~> 4 beta

and it is getting 4.5. So far, so good.

Now, my other project, let's call it appB, which uses libA does not know or care about NLog. The only dependency it has is libA:

nuget libA

But... when I do paket install it resolves NLog version 5.0.

Apparently, dotnet pack for libA puts the restriction >= 4.5 instead of >= 4.5 && < 5.

Long story short, my very precise restrictions about versions are lost when crossing dotnet pack boundary.

Repro steps

nlfu.zip

Run setup.cmd. Notice NLog 4 is referenced in libA (correctly) but appB references NLog 5 (wrongly).

Expected behavior

Both libA and appB reference version 4 of NLog.

Actual behavior

Restrictions are "forgotten", and appB references "newest" version of "NLog"

Known workarounds

Explicitly put restrictions in appB again (even if assembly is not used directly).

@matthid matthid changed the title Losing restrictions when 'dotnet pack' Losing version-range when 'dotnet pack' May 27, 2018
@matthid
Copy link
Member

matthid commented May 27, 2018

We can make this work by modifying fix-nuspec which is called in the dotnet pack process.
We have to not only remove all transitives but also fix the range accoding to paket.dependencies (and ideally paket.template). Help is welcome.

@matthid
Copy link
Member

matthid commented May 27, 2018

@baronfel
Copy link
Contributor

the code in question to be changed should be

let rec traverse (parent:XmlNode) =
let nodesToRemove = ResizeArray()
for node in parent.ChildNodes do
if node.Name = "dependency" then
let packageName =
match node.Attributes.["id"] with null -> "" | x -> x.InnerText
let packName = PackageName packageName
// Ignore unknown packages, see https://github.com/fsprojects/Paket/issues/2694
// TODO: Add some version sanity check here.
// Assert that the version we remove it not newer than what we have in our resolution!
if known.Contains packName && not (directDeps.Contains (PackageName packageName)) then
nodesToRemove.Add node |> ignore
, from what I'm seeing

@forki
Copy link
Member

forki commented Apr 24, 2020 via email

@baronfel
Copy link
Contributor

I'm working on it right now, as a matter of fact :)

@MiloszKrajewski
Copy link
Contributor Author

MiloszKrajewski commented Apr 27, 2020

Seems like it broke something. While 5.243.0 is working fine 0.244.0 is failing on pack with:

dotnet "pack" "xyz" --configuration Release --output xyz --no-build --no-restore
Microsoft (R) Build Engine version 16.2.32702+c4012a063 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.
  Paket version 5.244.0
  Performance:
   - Runtime: 702 milliseconds
  Paket failed with
  -> Could not find version range for package Microsoft.Extensions.DependencyInjection
xyz\.paket\Paket.Restore.targets(309,5): error MSB3073: The command ""xyz\.paket\paket.exe" fix-nuspecs files "obj\Release\xyz.nuspec" project-file "xyz.csproj" " exited with code 1.

I know this is not very helpful, I'll try to give you better example, but I'm 2 minutes from sprint planning ;-)

For now: forcing 5.243.0 fixes problem.

@forki
Copy link
Member

forki commented Apr 27, 2020 via email

@baronfel
Copy link
Contributor

@MiloszKrajewski are you able to give an example project file/paket.references/paket.dependencies/paket.lock? It would be lovely to have as a test case.

@MiloszKrajewski
Copy link
Contributor Author

@baronfel I will start working on it now (I need to extract the "essence of the problem" from quite large commercial codebase).

@MiloszKrajewski
Copy link
Contributor Author

MiloszKrajewski commented Apr 28, 2020

  1. Unpack
  2. paket restore
  3. dotnet pack src/lib (FAIL)
  4. edit paket.dependencies and set version to 5.243.0
  5. dotnet pack src/lib (SUCCESS)

p244.zip
p244

@baronfel
Copy link
Contributor

Thank you! I'll dig into this over the next couple days.

@baronfel
Copy link
Contributor

One thing I note is that your paket.references file for the lib project has an explicit reference to Microsoft.Extensions.DependencyInjection, but this package is not a direct, first-level dependency in your paket.dependencies. I expect this is the cause of the exception that you saw, but we do have enough information available to handle this case:

➜  before git:(2883-fixup-version-ranges-in-dotnet-sdk) ✗ dotnet ../../../../src/Paket.preview3/bin/Release/netcoreapp2.1/paket.dll fix-nuspecs files lib.1.0.0.nuspec project-file lib.csproj
Paket version 5.243.0
Checking dependency status for package Some(core), version Some(>= 1.0)
Package 'core' is not part of the explicit dependency tree in /Users/chethusk/oss/Paket/integrationtests/scenarios/i002883-fixup-nuspecs/before/paket.references and so will be skipped
Checking dependency status for package Some(Microsoft.CSharp), version Some(>= 4.5)
Package 'Microsoft.CSharp' is a direct dependency and requires no version patching
Checking dependency status for package Some(Microsoft.Extensions.DependencyInjection), version Some(>= 2.2)
Couldn't find a version range for package 'Microsoft.Extensions.DependencyInjection' in group 'Main', is this package in your paket.dependencies file?
Checking dependency status for package Some(Microsoft.Extensions.Logging.Abstractions), version Some(>= 2.2)
Package 'Microsoft.Extensions.Logging.Abstractions' is a direct dependency and requires no version patching
Checking dependency status for package Some(System.ValueTuple), version Some(>= 4.5)
Package 'System.ValueTuple' is a direct dependency and requires no version patching
Performance:
 - Runtime: 987 milliseconds

@MiloszKrajewski
Copy link
Contributor Author

MiloszKrajewski commented Apr 29, 2020

@baronfel I didn't notice that, especially wasn't looking for anything like that as many times I saw something like "paket.references mentions X but X is not in paket.dependencies" so I assumed it is checked by paket itself.
I understand you are saying that as long as something is a transitive dependency of something else it can be mentioned in .references without explicitly adding it .dependencies?

@MiloszKrajewski
Copy link
Contributor Author

@baronfel after fixing the reference it packs fine. Thank you!

It is still a bug in my opinion though as it wasn't found while building (It was referencing assembly which was not in paket.dependencies), but I understand it is outside the scope of this ticket.

Is it possible to have more descriptive error message?

@baronfel
Copy link
Contributor

I'm still working on the fix/rewrite of this feature in #3838, but I do intend to emit a warning when we encounter this corner case of 'referenced package is not a direct dependency'. I'm currently printing the message:

Couldn't find a version range for package 'Microsoft.Extensions.DependencyInjection' in group 'Main', is this package in your paket.dependencies file?

but that could very easily be reworded to something like

Package '<package name>' in group '<group name>' is referenced in <path to references file>, but is not a direct dependency. We recommend adding all explicitly-referenced packages to your paket.dependencies file for ease of tracking.

Do you have any suggestions here as to what would be the most clear?

@MiloszKrajewski
Copy link
Contributor Author

MiloszKrajewski commented Apr 29, 2020

I think the problem was a little bit different: I am stupid.
I didn't pay attention to Could not find version range for package Microsoft.Extensions.DependencyInjection because.... it wasn't in red (or yellow).
Definitely saying why ("because it is not in paket.dependencies") would help, but really... color was primary reason.

I behaved like an idiot and I feel really ashamed about that.

(I would still expect this to be warning during build - not only pack, but that's minor)

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

No branches or pull requests

5 participants