Repository navigation
[Feature]: Use reference to System.Net.Http using framework Reference instead of PackageReference #2120
Description
Background and motivation
I hope this is the correct place to raise this.
Currently for .net47 target you are adding reference to System.Net.Http as package dependency:
<ItemGroup Condition="'$(TargetFramework)' == 'net47'">
<PackageReference Include="System.Net.Http" Version="4.3.4" />
</ItemGroup>
According to this issue dot net team advise adding System.Net.Http standard framework reference instead. Just quoting them:
if you're building a .NET Framework 4.7.2 class library, then you could just add a normal Framework reference to System.Net.Http.
In most cases, we don't advise people use the separate System.Net.Http NuGet package anymore. See dotnet/runtime#18280 and dotnet/runtime#20777.
The main reason why I am suggesting this change is that VisualStudio resolve 4.7.2 project dependency to System.Net.Http and all its System dependencies by adding references to .net framework installed on local machine instead of project nuget. I think this issue has been raise in multiple place e.g. Broken System.Net.Http 4.1.1-4.3.0 post-mortem . It can also be problematic when you have reference to same assembly with both a single name reference and a package reference. For more details please take a look on Single-name references are removed by the SDK when targeting .NET Framework 4.7.2 and referencing a package with the same assembly
Alternative Concerns
Could you consider changing above to something like:
<ItemGroup Condition=" '$(TargetFramework)' == 'net47' ">
<Reference Include="System.Net.Http" />
</ItemGroup>
It looks like another few other projects decide to follow this advise e.g. IdentityModel, Microsoft Graph
Activity
eNeRGy164 commentedon Feb 2, 2023
Is this related to #1806?
jacekmlynek commentedon Feb 3, 2023
@eNeRGy164 if you ask me, I can say that changes from PR will solve my problem, but #1805, was more about linux runtime and framework 4.8. Definitely final solution mention in last comment will not works for me.
jnyrup commentedon Feb 3, 2023
Just to get a more complete understanding.
Is this causing a real problem for you or are you pointing out the pitfalls?
jacekmlynek commentedon Feb 3, 2023
@jnyrup yes that cause real problem for some of my projects that based on 4.7.2.
jnyrup commentedon Feb 3, 2023
Alright 👍
Can you provide a complete example which reproduces a problem?
dennisdoomen commentedon Feb 4, 2023
Just read through all the references, and it seems to be a valid problem. Even Claire Novotny was pulled in to help the IdentityModel folks understand this. My only concern is how this affects existing code bases that would upgrade.
dennisdoomen commentedon Feb 4, 2023
Is this a coincidence? #2121
dennisdoomen commentedon Feb 4, 2023
I am wondering why some of the .NET 4.7.2 projects I've been involved with didn't have that problem.
dennisdoomen commentedon Feb 4, 2023
Could you try this NuGet package (it's wrapped in an
artifacts.zip)?dennisdoomen commentedon Feb 5, 2023
Instead, @AArnott produced an even better PR. So can you try this NuGet package?
AArnott commentedon Feb 5, 2023
@dennisdoomen This is an amazing coincidence.
AArnott commentedon Feb 5, 2023
I had to pull in one of Microsoft's MSBuild team engineers to understand it. :) (and I used to be on the MSBuild team myself!)
jacekmlynek commentedon Feb 6, 2023
sorry, I wasn't able to answer during the weekend.
@dennisdoomen @AArnott I have take a look at #2122 - looks sensible to me and what is more import, the attached pre-release version solved my issues.
To be honest I do not fully understand why msbuild resolved
System.Net.Httpreference in this way and why in some projects behaviour is different The only thing that I was able to find suggest that this may be related to bindings in configI agree that it is insane coincidence :)
IT-VBFK commentedon Feb 10, 2023
Fixed with 97cf195?