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

Flurl switches from PCL reference to .NET Standard reference after upgrading to .NET 4.6.2 #176

Closed
mwelsh1118 opened this issue Apr 10, 2017 · 29 comments
Assignees
Labels

Comments

@mwelsh1118
Copy link

I have a large application targeting .NET 4.5.2 that is referencing Flurl.Http.Signed (and therefore Flurl.Signed) via NuGet.

When I change the target to .NET 4.6.2, I run into MissingMethodExceptions around System.Net.Http. It seems Flurl is expecting System.Net.Http 4.1. The rest of the application is expecting System.Net.Http 4.0.

In the .NET 4.5.2 case, I end up with the PCL version of Flurl (lib\portable40-net40+sl5...). In the 4.6.2 case, I end up with the .NET Standard version of Flurl (lib\netstandard1.4).

I noticed Flurl.Http.Signed has lib\net45, which seems like what I really want in either case, but Flurl.Signed does not.

Is there a way to make Flurl work while targeting .NET 4.6.2?

@tmenier
Copy link
Owner

tmenier commented Apr 11, 2017

Try completely uninstalling both Flurl(.Signed) and Flurl.Http(.Signed) and re-install just Http. If that doesn't work, try this.

Let me know your findings. A couple things are in the works that may address this as well:

  1. The next major versions of Flurl (and Http and Signed etc), will tentatively move away from PCLs entirely and go with just .NET Standard.

  2. In the near term, I plan to update the System.Net.Http references to 4.3. I don't know if that will resolve this issue, but I seem to recall there being some binding quirks people were running into associated with 4.0/4.1 that were resolved with 4.3, and it'd be good for Flurl to have the latest anyway.

@mwelsh1118
Copy link
Author

Unfortunately, neither worked. I tried removing Flurl.Http.Signed from the project.json where it is referenced, ran nuget restore, added the reference back to the project.json, and ran nuget restore again (note: I never referenced Flurl.Signed explicitly -- it was always implicitly referenced through Flurl.Http.Signed). I got the same MissingMethodException in my unit tests. I also tried adding a reference to Microsoft.Net.Http 2.2.29. Same MissingMethodExceptions.

@kroniak
Copy link
Collaborator

kroniak commented Apr 11, 2017

@mwelsh1118 please give me a link to the repo which reproduces this problem.

@mwelsh1118
Copy link
Author

Here you go:
https://github.com/mwelsh1118/FlurlRepro

The real problem exists in a large proprietary application, but I replicated the symptom in the above repo. I'm referencing Flurl via a project.json reference, but I saw the same problem if I switch to PackageReference tags in the csproj file. This repro uses VS 2017, but the problem exists with NuGet 3.4 & VS 2015 as well.

@kroniak
Copy link
Collaborator

kroniak commented Apr 18, 2017

@mwelsh1118 I dont have any problems with your repo. I see packages and no build errors. I have vs2017 sp1.

{
  "runtimes": {
    "win": {}
  },
  "frameworks": {
    "net462": {}
  },
  "dependencies": {
    "Flurl.Signed": "2.3.0"
  }
}

@mwelsh1118
Copy link
Author

@kroniak Please check out the readme for the repo. It builds fine, but it pulls the netstandard version of Flurl.dll which causes various problems.

@kroniak
Copy link
Collaborator

kroniak commented Apr 18, 2017

@mwelsh1118 I dont know what to do in this case. NET STD 1.4 include NET462 target. Nuget installer installs this version correctly. I can add a NET462 target and export dlls to package, but we can't do that with all targets 452, 461, 462, 470.

@mwelsh1118
Copy link
Author

@kroniak Flurl.Http has a net45 folder (which gets used when targeting 4.5+) but Flurl itself does not. I think doing the same for Flurl would fix the problem.

@kroniak
Copy link
Collaborator

kroniak commented Apr 18, 2017

@mwelsh1118 It does not fix this problem. Nuget will installs NETSTD dll for net 451+. NET45 dll uses only for net 45 which not supported by NETSTD.

@mwelsh1118
Copy link
Author

@kroniak That's not what I'm seeing, at least with Flurl.Http. The project.lock.json file points to lib/net45/Flurl.Http.dll when I'm targeting .NET 4.6.2.

@kroniak
Copy link
Collaborator

kroniak commented Apr 18, 2017

@mwelsh1118 I will look this case at this week.

@adrianiftode
Copy link

I had the same issue. I created a fresh new .Net 4.6.2 library project, installed Flurl from nuget, created another test project (same 4.6.2) and got this error while running some test. I downgraded the library project to .Net 4.5 and worked ok. With .Net 4.6.2 it installed lots of dependant packages (System.*)

@mwelsh1118
Copy link
Author

I worked around it for the time being by creating my own NuGet packages with a net45 folder that mirrored the portable one. That did indeed fix the problem.

@kroniak
Copy link
Collaborator

kroniak commented Apr 19, 2017

@mwelsh1118 @adrianiftode I am working on this problem now on fix-176.

@kroniak
Copy link
Collaborator

kroniak commented Apr 21, 2017

NuGet/Home#5075

@tmenier
Copy link
Owner

tmenier commented Jul 17, 2017

@kroniak With my last 2 commits I am able to build in VS 2017 and CLI and all tests are passing. However, AppVeyor is not happy. I thought that might be the case with my last commit. dotnet build can't resolve that MSBuild property it seems, so I resorted to hard-coding absolute paths. Undo that commit if you want, but it proves that CLI build is otherwise working.

Possibly related issues:
https://github.com/dotnet/cli/issues/5504
dotnet/project-system#1369

Might help but I'm not sure:
https://github.com/onovotny/MSBuildSdkExtras

@tmenier
Copy link
Owner

tmenier commented Jul 17, 2017

This is looking good, we'll try to get a prerelease out in the next couple days. I want to be sure and test what @adrianiftode saw (I have noticed this too): Currently when Flurl.Http is pulled into .NET 4.6 projects, it pulls with it about a dozen additional libraries it shouldn't need, presumably because it's using the PCL version. The PackageTesters confirm this. When the prerelease is live, let's just remember to clear all NuGet packages from PackageTester.NET45 and PackageTester.NET461, install the prerelease, and confirm that the only installed packages are Flurl, Flurl.Http, and Newtonsoft.Json.

@kroniak
Copy link
Collaborator

kroniak commented Jul 17, 2017

@tmenier prereleasing versions are avalible in

<add key="appveyor feed" value="https://ci.appveyor.com/nuget/flurl" />	

Nuget pulls additional packages from NETSTD dependicies for net4.6, not PCL, I see.

@tmenier
Copy link
Owner

tmenier commented Jul 17, 2017

@kroniak I removed all nuget packages from the NET45 and NET461 package testers, then reinstalled Flurl.Http 1.2 (from appveyor feed) in both. That worked perfectly - both contain just the 3 packages. However, the NET461 version will not build. It's saying it needs a reference to System.Net.Http 4.0. If I add that to the project it will probably work, but I shouldn't need to do that, right? I also noticed the NET461 version is pointed at net40/Flurl.dll and net45/Flurl.Http.dll. Shouldn't it be pointing to the netstandard1.3 versions of those?

I can commit and push this if you want to take a look.

@kroniak
Copy link
Collaborator

kroniak commented Jul 17, 2017

@tmenier

3 scenario:

  1. Update from old Flurl.
    All is fine, but many dependencies will be deleted manually.

  2. New project net45 \ net461 \ net462 .
    New version of nuget client in VS2017 is using net45+ lib. You are seeing only 3 dependencies and System.Net.Http;

  3. Drop all package and reinstall.
    In this case you have manually drop System.Net.Http package and remove system reference to System.Net.Http. After reinstall system reference must be added manually. It is bug?? May be.

But in case 2, system reference was added fine.

@tmenier
Copy link
Owner

tmenier commented Jul 17, 2017

When I installed in a .NET 4.6.x project (new or existing), net45 and net40 are preferred over netstandard1.3. Does that seem correct to you? I am guessing that's just how NuGet's algorithm works. It isn't documented very well, at least from what I can find.

@kroniak
Copy link
Collaborator

kroniak commented Jul 17, 2017

@tmenier As for me, that is more correct than target netstandard1.3 libs.

@tmenier
Copy link
Owner

tmenier commented Jul 17, 2017

Since netstandard1.3 is compatible with 4.6 and not 4.5/4.0, it could contain newer implementations of things so I would have hoped it would be preferred. But, probably just how NuGet works. I have asked here.

Are you still investigating anything here or would you say this is ready for release?

@kroniak
Copy link
Collaborator

kroniak commented Jul 17, 2017

@tmenier No, I am not. I want to hear a feedback from issues creators before a release.

@tmenier
Copy link
Owner

tmenier commented Jul 17, 2017

Answered already, and by Oren Novotny, who I'd say is an authority on the topic:

That is by design. Specific TFM's always win over netstandard. If you want newer functionality, you can use the SDK-style projects to multi-target to a newer net version. You could also put the netstandard1.3 version in a net461 directory too. If you do that, make sure to also include a net461 dependency group that pulls in the right netstandard.library version too.

I'd rather that the consumer got the latest & greatest bits available for the platform they're targeting, but maybe it doesn't matter. We might want to discuss this more for the next major versions.

@tmenier
Copy link
Owner

tmenier commented Jul 17, 2017

Sounds good. I'd still like to publish a -pre release on nuget.org. Easier for people to discover and consume for their own testing. (The AppVeyor feed is still great for quick smoke testing with the package testers though.)

@tmenier
Copy link
Owner

tmenier commented Jul 18, 2017

@mwelsh1118 @adrianiftode Fix for this in pre-release. Please test and report your findings.

PM> Install-Package Flurl.Http -Pre

@adrianiftode
Copy link

adrianiftode commented Jul 18, 2017

On my .Net 4.6.2 project, the outcome of the following command
Update-Package Flurl.Http
is
capture
After your fix, it works as expected
capture2

@tmenier
Copy link
Owner

tmenier commented Jul 21, 2017

Thx all, full release is live so we can finally put this one to bed.

@tmenier tmenier closed this as completed Jul 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants