-
Notifications
You must be signed in to change notification settings - Fork 520
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
Resolving .net standard depedencies #1883
Comments
Yep that is a bug: The condition for the dependency itself are correct:
However for its transitive dependencies they are not (note that the full framework is missing):
It seems like netstandard dependencies are filtered out somewhere for other frameworks... Probably a leftover from the early days when we filtered everything containing netstandard. |
OK I'm not sure if this is going to work properly but just for testing purposes I released a alpha version with a potential fix. Could you please test it? |
Doesn't seem to make any difference: .paket/paket.bootstrapper.exe prerelease
.paket/paket.exe install --force
grep -A 6 -B 3 "System.Data" src/vscode/vscode.fsproj <Choose>
<When Condition="$(TargetFrameworkIdentifier) == '.NETStandard' And ($(TargetFrameworkVersion) == 'v1.3' Or $(TargetFrameworkVersion) == 'v1.4' Or $(TargetFrameworkVersion) == 'v1.5' Or $(TargetFrameworkVersion) == 'v1.6')">
<ItemGroup>
<Reference Include="System.Data.Common">
<HintPath>..\..\packages\System.Data.Common\ref\netstandard1.2\System.Data.Common.dll</HintPath>
<Private>True</Private>
<Paket>True</Paket>
</Reference>
</ItemGroup>
</When>
</Choose> |
|
Seems like I'm getting the same result
|
you guys need to run update since it influences the lock file as well |
Hm okay, I could swear that I actually deleted the lockfile but whatever it works after update :) |
question is: does it makes sense? |
will take a look later. But at least the sample now works ;) |
Oh yeah sweet that works. Only thing that might be confusing is I get this message now:
|
yeah that message is annoying. I start to believe we should just nuke it |
I think it makes only sense for direct deps... |
I'm really worried about this change. |
now we basically reference most of that new .netstandard crap even in older projects |
@TheAngryByrd could you please try with latest alpha? the message should now only be shown for direct packages. @matthid we need massive testing for this change. |
One thing that worries me is that paket update on itself is now broken because of fsharp.core issue. I don't think it's correct |
Just making sure the steps i should take is:
Seems to work still on windows with .net 4.6.1 without the warning messages. However on OSX/Mono 4.4.2 it seems to have the issue with not finding certain dlls.
|
@TheAngryByrd The project file looks broken. Is Fake by any chance restoring the latest stable again? I mean it shouldn't actually matter, because Fake is only restoring (and shouldn't touch project files). @forki Can you tell me how Paket itself is broken For me the diff in the lockfile looks like this and no fsproj file is touched/changed (needless to say that everything is working for me)... Am I doing something wrong (testing with diff --git a/paket.lock b/paket.lock
index 1984dd6..c04c28b 100644
--- a/paket.lock
b/paket.lock
@@ -4,44 4,44 @@ NUGET
Argu (3.2)
Chessie (0.6)
FSharp.Core (4.0.0.1) - redirects: force
- Mono.Cecil (0.9.6.3)
Mono.Cecil (0.9.6.4)
Newtonsoft.Json (9.0.1) - redirects: force
GITHUB
remote: fsharp/FAKE
- src/app/FakeLib/Globbing/Globbing.fs (7f9fe8f546f6adec66febeec31e50821b60814d6)
src/app/FakeLib/Globbing/Globbing.fs (e5870cb85f9b500246e02c6daa94e5131dad7a6e)
remote: fsprojects/FSharp.TypeProviders.StarterPack
- src/AssemblyReader.fs (dfbca9b83fb70067e85abddcb8b89332aae4c28d)
src/AssemblyReader.fs (8f647a4cde71bdeda10e7f5b534ec3c879601570)
GROUP Build
NUGET
remote: https://www.nuget.org/api/v2
- FAKE (4.31.1)
FAKE (4.39)
FSharp.Compiler.Service (2.0.0.6)
FSharp.Formatting (2.14.4)
FSharp.Compiler.Service (2.0.0.6)
FSharpVSPowerTools.Core (>= 2.3 < 2.4)
FSharpVSPowerTools.Core (2.3)
FSharp.Compiler.Service (>= 2.0.0.3)
- ILRepack (2.0.10)
ILRepack (2.0.11)
Microsoft.Bcl (1.1.10) - framework: net10, net11, net20, net30, net35, net40, net40-full
Microsoft.Bcl.Build (>= 1.0.14)
Microsoft.Bcl.Build (1.0.21) - import_targets: false, framework: net10, net11, net20, net30, net35, net40, net40-full
Microsoft.Net.Http (2.2.29) - framework: net10, net11, net20, net30, net35, net40, net40-full
Microsoft.Bcl (>= 1.1.10)
Microsoft.Bcl.Build (>= 1.0.14)
- Octokit (0.20)
Octokit (0.21.1)
Microsoft.Net.Http - framework: net10, net11, net20, net30, net35, net40, net40-full
GITHUB
remote: fsharp/FAKE
- modules/Octokit/Octokit.fsx (7f9fe8f546f6adec66febeec31e50821b60814d6)
- Octokit
modules/Octokit/Octokit.fsx (e5870cb85f9b500246e02c6daa94e5131dad7a6e)
Octokit (>= 0.20)
GROUP Test
NUGET
remote: https://www.nuget.org/api/v2
Castle.Core (3.3.3) - framework: >= net45
- FsCheck (2.5)
FsCheck (2.6)
FSharp.Core (>= 3.1.2.5)
FSharp.Core (4.0.0.1)
- Moq (4.5.16)
Moq (4.5.21)
Castle.Core (>= 3.3.3) - framework: >= net45
NUnit (3.4.1)
NUnit.Console (3.4.1)
|
did you update -f? you will then see:
which leads to paket installing FSharp.Core (4.0.1.7-alpha) |
ok that Chessie thing is fixed. |
Since when do we need |
you usually don't need that, but if you don't use it then paket uses cached package details from before we changed that logic and that doesn't run restriction optimizer again |
In other words: I should invalidate the cache by increasing the internal cache version number. |
At first it's surprising that paket chooses an alpha version for a stable dependency (doesn't nuget even prevent uploading a stable with deps to unstable?). But its the only package which fullfills all requirements of Chessi. Now I'm wondering what would happen if we specify
I think paket would stop with an error and we would need to specify a framework restriction. One thing we could do here is tell paket to rather drop frameworks than to introduce alpha packages (ie keep things stable)? |
the problem would be the same if 4.0.1.7 would be stable... |
Because you are arguing that the problem is
And I think the problem here is that an alpha package was pulled. One thing we could do is make the restrictions more powerful. IE add blacklisting support like Long term we will be unable to prevent the pulling of netstandard packages, because more and more libraries will only provide a netstandard binary... |
@matthid the problem was: paket had that restriction on >= net45 and this intermediate alpha version still pulled in the .net standard stuff |
Hm, maybe a packaging bug of the alpha package. Shouldn't it contain an empty group for net40 because there actually is a "net40" binary included? Edit: not package i meant binary |
maybe. |
Another question is where is that |
Hm restricting to exactly |
Scratch that it doesn't work for |
ok released new version with fix for mspec. please give it a try |
I think there is still something wrong: It seems some netstandard dependencies are added to net461, which I wouldn't expect: NETStandard.Library (1.6) - framework: netstandard16
System.Globalization.Calendars (>= 4.0.1) - framework: net461, netstandard16 It is added to the project file ( <When Condition="$(TargetFrameworkIdentifier) == '.NETFramework' And $(TargetFrameworkVersion) == 'v4.6.1'">
<ItemGroup>
<Reference Include="System.Globalization.Calendars">
<HintPath>..\..\..\..\..\packages\System.Globalization.Calendars\ref\net46\System.Globalization.Calendars.dll</HintPath>
<Private>True</Private>
<Paket>True</Paket>
</Reference>
<Reference Include="mscorlib">
<Paket>True</Paket>
</Reference>
</ItemGroup>
</When> When I change to - <When Condition="$(TargetFrameworkIdentifier) == '.NETFramework' And $(TargetFrameworkVersion) == 'v4.6.1'">
- <ItemGroup>
- <Reference Include="System.Globalization.Calendars">
- <HintPath>..\..\..\..\..\packages\System.Globalization.Calendars\ref\net46\System.Globalization.Calendars.dll</HintPath>
- <Private>True</Private>
- <Paket>True</Paket>
- </Reference>
- <Reference Include="mscorlib">
- <Paket>True</Paket>
- </Reference>
- </ItemGroup>
- </When> I would expect paket to generate always the same references for the same framework.... |
https://www.nuget.org/packages/RabbitMQ.Client/ <- also fucked up |
The package itself looks fine to me. I think paket is not delegating the framework restrictions properly. I think I found the culprit:
|
@matthid I started over again with an Ionide console template. Changed the TargetFrameworkVersion in the fsproj to 4.6.1. Made sure the build.(cmd/sh) used prelease for the paket.bootstrapper. Installed Marten, still same problem. This is as of 3.17.0-alpha008. If I go back to 3.17.0-alpha003 it seems to work. Maybe I'm doing something stupidly and wrong? |
@TheAngryByrd Yeah the test @forki added is failing again. I would say fixing this is complicated, therefore it could take a while to get this right... |
Yeah, it looks complex from the outside. Can't imagine what kind of hell it is to code it. |
please try again |
@forki Paket is still adding additional/too many references to net461 for FAKE (for the Machine.Specifications package). But at least references seem to be consistent again. I might be able to take a look at this at the weekend... |
ok @TheAngryByrd please retry with latest. only to make sure I didn't break you again. |
Sorry, I was just able to get to this. Yeah it looks like it's broken again for me =\ Back to this silliness:
|
on that demo project? with update -f? |
Yes. However on mono I get slightly different results:
|
ok will retry tomorrow. weird stuff |
Note that it should work when you use the latest alpha of Marten (add "pre" to paket.dependencies). With nuget you are using that version as well... @forki I'm trying to find the reason and create another test for it. I think paket still gets a bit confused with when to apply which framework restrictions... |
Might have been the failing test :) |
This case is sent directly by the devil. What paket does seems to be perfectly valid. I think the root cause is that marten doesn't specify its dependencies correctly. Its binary directly depends on "System.Data.Common". Introducing such dependencies might be very very subtle (ie. if you call a method returning a class from a transitive dependency). Of course Paket is using the net46 version of Npgsql which doesn't have the "System.Data.Common" dependency. But Marten still has it (at least the compiler needs the reference assembly, it might not even be a runtime dependency). I think one way to solve this would be for paket to always use the lowest framework version a dependency introduces... I'll try to write a test case for this :) |
To clarify: This is at install time not at resolution time. |
matthid@28efb12 But we can only do one way or the other. I think there is no right or wrong here. What paket does right now is correct IMHO. |
@matthid can you try to send PR to marten then? |
Why? The latest alpha should be working already... |
Yep. The alphas of Marten seem to work. |
Description
I'm experiencing the same problem as @baronfel from the gitter chat:
Repro steps
Please provide the steps required to reproduce the problem
Here is a repo demonstrating the issue: https://github.com/TheAngryByrd/paketnetstandard
paket folder uses paket (VSCode Ionide console template) while nuget folder uses nuget (VS console template).
Expected behavior
Should be able to compile and put correct dlls in bin
Actual behavior
Does not compile.
Known workarounds
Use nuget 😿
Related information
Build error from FAKE:
The text was updated successfully, but these errors were encountered: