Move DependencyModel to libraries

[eerhardt]

Moving DependencyModel to the same folder and infrastructure as the rest of our libraries, and out of the installer folder.

I also dropped support for building live anything below netstandard2.0 so we don't need to reference Newtonsoft. The lower TFMs are harvested from the previously shipped package.

Contributes to #3470
Fix #3425

[Dotnet-GitSync-Bot]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[eerhardt]

error:

C:\Users\eerhardt\.nuget\packages\microsoft.dotnet.genfacades\5.0.0-beta.20171.1\build\Microsoft.DotNet.GenPartialFacadeSource.targets(30,5):
error : The type 'System.Collections.Generic.CollectionExtensions' is defined in multiple seed assemblies.
The multiple assemblies are Microsoft.Extensions.DependencyModel.dll, System.Collections.dll. If this is intentional, specify the alias for this type and project reference 
[F:\git\runtime\src\libraries\shims\generated\netstandard.csproj]

This is unfortunate that we shipped this API already. Would this be OK to make a breaking change in 5.0 for this assembly? We are already making other breaking changes -

• dropping support for net451, netstandard1.x
• In 3.0, we accidentally shipped an unintentional public type in this assembly:

runtime/src/installer/managed/Microsoft.Extensions.DependencyModel/Microsoft.Extensions.DependencyModel.csproj

Line 11 in 335ffb0

<Compile Include="..\Microsoft.DotNet.PlatformAbstractions\HashCodeCombiner.cs" />

However, this would be a binary and potentially source breaking change to change CollectionExtensions, but I think it might be worth it going forward.

Thoughts - @ericstj @joperezr @safern ?

[ericstj]

We found a similar bug in other extensions libraries #33998. Since its just extension methods that folks aren't going to directly reference it's not a terrible type conflict. I'd just suppress the error for now and file a bug. If folks have a scenario for using the two types and notice the clash we can address it.

runtime/src/libraries/pkg/test/packageTest.targets

Lines 24 to 25 in 562232c

	<!-- Type duplicated: https://github.com/dotnet/runtime/issues/33998 -->
	<IgnoredTypes Include="Microsoft.Extensions.Logging.LoggingBuilderExtensions" />

HashCodeCombiner

What do you suggest? Removing is breaking, we could obsolete it and/or implement on top of System.HashCode?

[joperezr]

@safern do you think that the versions of these packages should live in Versions.props?

[eerhardt]

Both Microsoft.DotNet.InternalAbstractions and Microsoft.DotNet.ProjectModel are obsolete packages (think project.json and .xproj time frame). The issue is our unit tests are written relying on these old packages, and we never took the time to rewrite the tests. But we don't want to lose the test coverage.

These versions will never be updated, so I didn't think we would want them in a global/central place.

FluentAssertions is different - this version could get updated in the future. However since no other test in src\libraries appears to use this package, my hope is we can remove this dependency in the future.

[safern]

I think it is fine to keep them here. Maybe FluentAssertions as you mention might be good to move it, but since you intend to remove it, I don't have an strong opinion.

[eerhardt]

Do we intend to use FluentAssertions more in the repo? If we wanted to use it more, then I'd be OK with moving it. But I was thinking we didn't want to use it (since no one uses it today).

[danmoseley]

My impression is that we don't want to - as it's inconsistent with what we've been doing (amongst some other reasons). if we want to, we should have a discussion first and achieve consensus.

[joperezr]

Left a few comments but this Looks good in general.

[eerhardt]

Since its just extension methods that folks aren't going to directly reference it's not a terrible type conflict. I'd just suppress the error for now and file a bug. If folks have a scenario for using the two types and notice the clash we can address it.

Done - #34420

HashCodeCombiner

What do you suggest? Removing is breaking, we could obsolete it and/or implement on top of System.HashCode?

My suggestion is to make the breaking change in 5.0 (which I've done in this PR). It was just added in 3.0 (mistakenly since it was a shared source file, and the type is public), and if anyone is using it, they shouldn't be. Also, if they were using it - it would collide with the same type in PlatformAbstractions. So I think we can make this small break, so we don't have to worry about it going forward.

[joperezr]

NIT: technically since you are not shipping the ref in the package and the API surface is not changing between netstandard2.0 and net461, it should be fine to only keep the netstandard2.0 configuration in this project if you wanted.

[joperezr]

Shouldn't we be able to just prefer one of the seed types so that we can still generate the shims using this dependency? I'm actually not too familiar with this so if @ericstj suggested this that's fine.

[eerhardt]

I'm not aware of a better fix here. If there is one, I'd be happy to change this.

Honestly, I'm not sure why we are passing Microsoft.Extensions.* libraries in here at all.

[joperezr]

We pass in the whole RefPath (line 17 above) if you think it would make sense to remove all of these that would be fine. Basically the idea of this shims are so that if some library tries to load types that have been moved into these Microsoft.Extensions* packages for core, that we redirect the types correctly.

[ericstj]

You could, however I'm not sure how much value that has. We don't intent for any Micrsoft.Extensions assemblies to satisfy .NETFramework nor .NETStandard surface area, so I am fine omitting them.

[joperezr]

Left a couple of small questions, but this looks good to me otherwise. Thanks @eerhardt