Skip to content

How to use libgit2sharp in a .NET 4.5 app? #1663

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

Closed
battlmonstr opened this issue Feb 16, 2019 · 10 comments
Closed

How to use libgit2sharp in a .NET 4.5 app? #1663

battlmonstr opened this issue Feb 16, 2019 · 10 comments

Comments

@battlmonstr
Copy link

From what I understood starting from version v0.25.0+ libgit2sharp doesn't support .NET 4.5,
and a developer needs .NET 4.6 + VS 2017 for it to work from NuGet.

It was mentioned in these issues:
#1557
#1598

What are the reasons that you decided to not include the classic .NET library version in the NuGet package?

I'm sure there are good reasons, so the next question, how much work it is to make a release build from source that would support .NET 4.5?
Do you have release build instructions somewhere to prepare such a build?

Reasoning:
I have a desktop Git client - http://nitrogit.net/ - and for that I consider 4.6 as "too new". According to the stats here 44% of the users are still stuck on Windows 7-8.x. It means that if I support .NET 4.5 those people will not have to install anything extra along with this tiny app. But if I upgrade to .NET 4.6 to use your latest NuGet version, they might have to download and install the framework in addition, and that's not ideal.

P.S. If you have based your decision on some statistics that would be interesting to look at.

@igloo15
Copy link

igloo15 commented Feb 17, 2019

Can't you just use version 0.24.1 of the package that supports .NET 4.5 instead of the latest 0.25.0+.

The reason they don't support .NET 4.5 in the newest releases is because they are targeting netstandard 2.0 and netstandard 2.0 does not support .NET 4.5 Framework. It has nothing to do with statistics.

.NET Standard

@battlmonstr
Copy link
Author

battlmonstr commented Feb 17, 2019

@igloo15 I actually wanted to upgrade to libgit2sharp v0.26, because this bug was fixed there.

I mention statistics, because when you upgrade to a newer technology, it is usually wise to check what your users are on and how it affects them. I'm sure it was considered, but just asking if there's some data about .NET 4.6 adoption the authors could share, and some reasoning for why v2.0 was chosen and not v1.0 of .NET Standard.

@ethomson
Copy link
Member

I mention statistics, because when you upgrade to a newer technology, it is usually wise to check what your users are on and how it affects them.

You can follow the discussions at in #1318 and #1483 which was a multi-year effort.

@igloo15 I actually wanted to upgrade to libgit2sharp v0.26, because this bug was fixed there.

Well, it was actually fixed in the underlying native library (libgit2). Simply upgrading the native dependency in the 0.24 branch is likely to alleviate this concern for you if you're unwilling to upgrade to a newer .NET.

@battlmonstr
Copy link
Author

@ethomson thanks for the links provided. I have read the discussions. I think that is a very good endeavour, and both PRs seem to keep net40 as a target of the main project. I'm all up for expanding onto the new platforms. But it is not clear to me when the net40 support was actually lost in NuGet?

The reasoning for supporting .NET Standard 2.0 and not 1.0 is a bit more clear as explained by @bording that you rely on support for custom marshaling. That's fine, but that doesn't explain why net40 is dumped. It seems that it is still possible to support net40, which had custom marshaling before, isn't it?

I see that now you have 2 folders there: net46 and netstandard2.0, but no net40 or net45.
Would you accept a PR to bring net45 support inside the NuGet package?

Thanks for the workaround, it probably works in the short term. Do you have release build instructions somewhere to prepare a "release" NuGet build by hand?

@ethomson
Copy link
Member

I don't really understand if it's possible for a .NET Standard 2.0 library to support .NET 4.5 or older. I thought that it was explicitly not, but @bording can weigh in here.

Thanks for the workaround, it probably works in the short term. Do you have release build instructions somewhere to prepare a "release" NuGet build by hand?

You can look at the build scripts, but I would encourage you to take the maint branch for 0.24, update the native library dependency, fix any incompatibilities that might have arisen and open an PR. The PR validation build will spit out a nuget package as an artifact and we can upload that to nuget.org.

@battlmonstr
Copy link
Author

The backporting from 4.6 to 4.5 should be doable as long as the code is mostly binary compatible with 4.5. It might require some tinkering with the project files and build scripts.

Thanks for the hint about 0.24 PR, I'll try that.

@bording
Copy link
Member

bording commented Feb 18, 2019

What are the reasons that you decided to not include the classic .NET library version in the NuGet package?

That's fine, but that doesn't explain why net40 is dumped. It seems that it is still possible to support net40, which had custom marshaling before, isn't it?

The primary reason is that .NET Framework 4.0 and 4.5 are no longer supported by Microsoft, and they haven't been for a while:

I don't see much point in continuing to work with unsupported versions of the framework.

There is an argument to be made that we could try and ship an an assembly for .NET Framework 4.5.2, since it is still supported at the moment (https://github.com/Microsoft/dotnet/blob/master/releases/README.md), but at this point, I'm not really sure I see the benefit, since it would increase the complexity of the codebase more than I'd want since we'd likely need to add a third target framework, and be net452, net46, and netstandard2.0.

We originally had intended to just ship a .NET Standard 2.0 assembly, but it turns out that only .NET Framework 4.7.2 supports it without running into issues, primarily around binding redirects. To work around that, we started multi-targeting, and shipped both net461 and netstandard2.0.

However, LibGit2Sharp is used in various Visual Studio extensions, and it turns out that any VS version prior to 2019 really needs all assemblies to reference net46, so that's how we ended up with our current set of assemblies.

It's possible that we could get away with just net452 and netstandard2.0, but I'd really want to get confirmation that it wouldn't cause problems with Visual Studio first, but to be honest I don't really see the benefit in changing things more at this point.

@battlmonstr
Copy link
Author

Thanks for explanation. Makes total sense to me.

Although this article is from 3 years ago, by the stats it seems quite unlikely to me that they drop 4.5.2 soon. And if there were no backpressure they would have probably done it already by this time.

What are the painpoints of having 2 (or 3) targets for you? Do you maintain separate VS projects and have to switch, or is it done differently? Do you work primarily on netstandard2.0 and then have to somehow manually test on net46?

I would like to try to add the net45 target, because if you already support 2 targets, it shouldn't be too hard?
Could you consider accepting such a contribution? I feel that that probably depends on the scope of the change, and the number of sacrifices has to be made in terms of features.
It would be great to know what are the hidden roadblocks before starting this.

The decision about dropping net46 target is also up to you. I can help with testing.
Do you remember exact issues about older Visual Studios with 4.7? Which extensions use libgit2sharp? Is it possible to test them as an outsider?

@bording
Copy link
Member

bording commented Feb 18, 2019

What are the painpoints of having 2 (or 3) targets for you? Do you maintain separate VS projects and have to switch, or is it done differently? Do you work primarily on netstandard2.0 and then have to somehow manually test on net46?

The farther you go back, the more missing APIs you run into, which means instead of having one implementation of something that applies to all of the targets, you have to have conditional compilation directives to have different implementations, and then in some cases that could even result in a different API surface (though I don't think that would apply here).

It also means that it increases testing complexity because you really should be running tests against each different assembly you build, so you might hit the conditional directives in the test code as well, and it increases CI complexity to get the test coverage.

While the actual work to add an additional target might not be that great, there is an ongoing cost in complexity and time that doesn't go away after the PR has been merged.

Based on this, I'm inclined to say that we wouldn't accept a PR to add another target or otherwise support anything earlier than net46. It's already almost 4 years old, and it can run on any supported version of Windows.

@battlmonstr
Copy link
Author

Ok, good to know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants