libgit2 / libgit2sharp.nativebinaries

MIT License
28 stars 62 forks source link

Use docker to build for multiple distros #51

Closed bording closed 6 years ago

bording commented 7 years ago

@JakeGinnivan I wanted to bring this to your attention, since you gave me the idea for this in https://github.com/libgit2/libgit2sharp/issues/1391.

Once https://github.com/libgit2/libgit2sharp/pull/1318 is merged, which should be happening soon, we'll be able to provide better support for different distributions when running on .NET Core.

Mono can also benefit, but the LibGit2Sharp.dll.config file will have to be manually updated to point to the correct distro folder. There's no good way to automatically select the correct one like there is in .NET Core.

JakeGinnivan commented 7 years ago

Awesome, I actually have started using docker to fix the issue for myself. See https://hub.docker.com/r/gittools/gitversion

But having it more portable with .net core is a better option.

bording commented 7 years ago

I've updated this PR to fix the merge conflicts.

I've also been doing research and creating a lot of VMs to test the linux distros listed here to figure out what exactly we'd need to ship to support all of them, and I've run into some problems.

I've raised https://github.com/dotnet/corefx/issues/23699, so let's see what the response to that is.

bording commented 6 years ago

To update anyone watching this, https://github.com/dotnet/corefx/pull/26439 was merged, and it was backported to 2.0.x in https://github.com/dotnet/corefx/pull/26577, so at this point I'm just waiting for a 2.0.x patch release that includes those changes to arrive so I can finish this up and include enough native binaries to let every distro .NET Core 2.0 supports to work out of the box.

bording commented 6 years ago

And with https://github.com/dotnet/core/blob/master/release-notes/2.0/2.0.7.md, it looks like the additional RIDs are available now! 🎉

Now to finish this up and get an updated package out.

bording commented 6 years ago

@ethomson This should be ready to go now. Based on my research/testing, with these changes, we be able to run on every platform list here: https://github.com/dotnet/core/blob/master/release-notes/2.0/2.0-supported-os.md.

Once we get a version of LibGit2Sharp that uses these changes, I'll verify that things are working as expected.

ethomson commented 6 years ago

What about Alpine linux? I'm surprised that's not in the supported OS list from .net core - isn't that what their containers are based on? Do we work there? It's not called out explicitly.

chlowell commented 6 years ago

I tried Alpine 3.7 a couple weeks ago, LibGit2Sharp.Core.NativeMethods throws (with libgit2 installed). I didn't dig into it any further.

bording commented 6 years ago

AFAIK Alpine hasn't been a supported distro for .NET Core until the almost-released 2.1. I believe their containers have been debian-based so far.

Given that 2.1 is adding Alpine, that would a good candidate to add as a followup to this PR.

bording commented 6 years ago

I'm also thinking it might be a good idea to move building even the ubuntu/linux-x64 library into docker instead of building directly against whatever Travis is providing. That would leave osx being the only one we build outside of a docker container.

ethomson commented 6 years ago

I'm also thinking it might be a good idea to move building even the ubuntu/linux-x64 library into docker instead of building directly against whatever Travis is providing.

I agree with that.

ethomson commented 6 years ago

So overall this seems pretty sane to me. Couple of questions:

  1. Do we really need RHEL and Fedora both? RHEL and Fedora are very similar, I would expect the same binary to work on both. (Same with Debian and Ubuntu.) Does the RID have any sort of mechanism to combine (say) RHEL and Fedora? I assume that there's no mechanism to support symbolic links in NuGet packages, either. If there's a way to shrink our binary package, that would be 👍 .

  2. How do the RIDs work here, where you have three explicit ones, then "linux". I assume that "linux" is the fallback if none of the more specific RIDs match? Is it Ubuntu by convention? Or are we just doing that because it's generic and matches whatever Travis is generically using?

bording commented 6 years ago

Do we really need RHEL and Fedora both? RHEL and Fedora are very similar, I would expect the same binary to work on both. (Same with Debian and Ubuntu.) Does the RID have any sort of mechanism to combine (say) RHEL and Fedora? I assume that there's no mechanism to support symbolic links in NuGet packages, either. If there's a way to shrink our binary package, that would be 👍 .

Unfortunately, if you look at the RID graph that Microsoft has made, there is no relationship between RHEL and Fedora, so there's no way to share binaries between them, other than at the top level linux-x64 RID. :cry:

And no, there's no way to use symbolic links that I'm aware of.

How do the RIDs work here, where you have three explicit ones, then "linux". I assume that "linux" is the fallback if none of the more specific RIDs match? Is it Ubuntu by convention? Or are we just doing that because it's generic and matches whatever Travis is generically using?

If you look at the RID graph, linux-x64 is near the top of the graph for all of the other RIDs, so it is a fallback if you don't include a more specific asset.

I came up with the current RID scenario by examing the RID graph and actually installing all of the relevant distros to verify things like the version/soname of the openssl lib installed.

Using linux-x64 to be the "ubuntu" library resulted in fewer separate binaries needed. I also examined the scenario of using linux-x64 to be the RHEL binary instead (which would have been able to have RHEL and fedora share), but that actually results in increasing the number of binaries required.

So, for the set of OSes supported by .NET Core 2.0, this is the minimal set of binaries I could get given the constraints of the RID graph.

Once this is merged (and we can get a preview of LibGit2Sharp that uses it on nuget.org), I want to do a round of testing to verify that my research was correct and that this results in a usable package for each of the supported distros. Based on that, another round of tweaking might be required.

Also, .NET Core 2.1 is out now, so we do have some additional distros to consider from that as well.

ethomson commented 6 years ago

Thanks for the details; I assumed that was the state of #1.

Thanks for all the hard work and I'm sorry that it took so long to get around to reviewing it.

bording commented 6 years ago

@ethomson I just noticed that this PR still had references to the 6311e88 version of the library, and not the new b0d9952.

We'll need to run the script to update the checked in files.