Changes for .NET Core

[bording]

This makes some changes to the native binaries packages to take advantage of the changes in https://github.com/libgit2/libgit2sharp/pull/1318.

The embedded files that LibGit2Sharp itself needs are now embedded via contentFiles instead of being referenced in the .props file.

The .props file is only ever needed for full .NET Framework and mono, so it's been scoped to net40 only.

The result of these changes is that when LibGit2Sharp references the package directly via PackageReference, it gets the files embedded as needed, but projects that consume LibGit2Sharp won't get those files embedded as well.

This also means that projects targeting .NET Core/Standard won't get the lib folder needlessly copied into the output folder.

@ethomson You mentioned wanting to do a LibGit2Sharp release before merging https://github.com/libgit2/libgit2sharp/pull/1318, right? Were you thinking you'd need an updated native binaries package for that as well? If so, you should build that first before this PR gets merged, because these changes assume LibGit2Sharp will be consuming the package via PackageReference and not packages.config.

CC: @AArnott

[AArnott]

these changes assume LibGit2Sharp will be consuming the package via PackageReference and not packages.config.

Are you sure you're comfortable with that? I believe VS2017 still supports packages.config, even with the latest nuget version. And if the latest nuget version doesn't support it, you could add a MinClientVersion property to your nuspec file to help inform users.

[bording]

Are you sure you're comfortable with that? I believe VS2017 still supports packages.config, even with the latest nuget version. And if the latest nuget version doesn't support it, you could add a MinClientVersion property to your nuspec file to help inform users.

I'm not sure I'm following you. We know the LibGit2Sharp project will require VS 2017 once your PR lands, and it's consuming the native package via PackageReference. That is what this change is assuming.

With this change in place, that means LibGit2Sharp gets the two text files embedded in both the net40and netstandard1.3 targets, so nothing changes as far that LibGit2Sharp is concerned, aside from the netstandard1.3, not getting the lib folder and the LibGit2Sharp.dll.config file copied into the output folder, neither of which it needs anyway.

The big difference/improvement here will be for people consuming LibGit2Sharp.

If they pull the package into their project via packages.config, then that means they're doing something with .NET Framwork or mono, and the net40 targeted .props file will continue to copy over the lib folder and the LibGit2Sharp.dll.config file, which are still needed for net40. What they won't get any more is the two text files embedded in their assembly, which they don't need.

If the package is consumed via PackageReference instead, then they are either targeting .NET Core, or multi-targeting to .NET Framework also. In that case, because we're now opting into the transitive restore, that means the contentFiles in the native package could show up, depending on the setting in LibGit2Sharp, but I don't want thecontentFiles in this case, which is why I pushed up the commit to your PR to make them PrivateAssets.

This means that for PackageReferences where the native binary package is referenced transitively via LibGit2Sharp, .NET Core will have access to the native libraries via the runtimes, and the .props file will copy them to a .NET Framework target.

So, these changes aren't removing options for consumers of LibGit2Sharp, just improving things for all scenarios.

[AArnott]

We know the LibGit2Sharp project will require VS 2017 once your PR lands, and it's consuming the native package via PackageReference.

I don't know that. The fact that we use PackageReference to build the package is an implementation detail. The nuspec is all that matters and that expresses the dependency the same way it did before. Also, VS 2017 vs. VS 2015 support is different than the point I was making, which is that packages.config is still supported in VS 2017. So I just tested VS2015 with a project that uses packages.config with libgit2sharp (as of 8ca086eb) and it still works. Not only does nuget do the right thing, but the (net46) app runs. I also tested on VS2017 on a .NET console app with packages.config and that also works (today).

To be clear, ridding two embedded files that our consumers don't need is great. But I think we should take care to not break packages.config scenarios if we can avoid it since most of your current customers are on it.

[bording]

@AArnott I'm still not really following your point. I specifically tested these changes for both packages.config and PackageReference scenarios, and everything works exactly as it should. These changes should also even work for the old project.json tooling as well.

[AArnott]

OK, if you've tested and it works, that's fine. My concerns were that:

these changes assume LibGit2Sharp will be consuming the package via PackageReference and not packages.config.

made it sound like you were cutting support for packages.config. And your more recent comment:

We know the LibGit2Sharp project will require VS 2017 once your PR lands

Just sounds wrong, because AFAIK we don't require VS 2017 when my PR lands. Why do you think it does?

[bording]

Ah, I see the confusion now!

Your PR has moved the LibGit2Sharp project to the new SDK-style csproj format, which is not supported by earlier versions of Visual Studio, therefore we know it will be used with VS 2017. My statements about PackageReference and not packages.config are specifically referring to the LibGit2Sharp project itself, not consumers of LibGit2Sharp.

Perhaps that would have been better phrased as "We know the LibGit2Sharp project will be consuming NuGet packages in a way that opts into the transitive restore process, so we know that it will use the contentFiles section."

That means we can put files into contentFiles that are specific to being consumed by LibGit2Sharp itself, but no downstream consumers of LibGit2Sharp itself will care about. That's why I made the change to make contentFiles PrivateAssets on your PR.

The .props file now only has stuff in it that is relevant to the net40 target, so regardless if this package is consumed directly via PackageReference (like LibGit2Sharp does), indirectly via transitive reference through PackageReference (consumers of LibGit2Sharp using SDK-style csproj), or directly via packages.config (consumers of LibGit2Sharp using original csproj) everything continues to work.

[AArnott]

Got it. Thanks for the clarification.

[bording]

@ethomson I'll get these conflicts resolved, and this should be ready to go now that 0.24.0 is out.

[bording]

@ethomson This is rebased and ready to go now.