Skip to content
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

LibGit2Sharp update to 0.26.0 #1659

Closed
wants to merge 7 commits into from

Conversation

arturcic
Copy link
Member

@arturcic arturcic commented Apr 18, 2019

This PR is meant to update GitVersion to the latest LibGit2Sharp version 0.26.0.

This means the command line tools will not require anymore to have the workaround (you can see the Dockerfiles, the libgit2-devel.x86_64 dependency is not installed).

As for the GitVersionTask, I had to change the way we package the nuget artifact, and inspired from https://github.com/dotnet/sourcelink I have created a separate package called GitVersion.MsBuild.

GitVersion.MsBuild is meant to replace GitVersionTask as it is using the latest LibGit2Sharp and has the mechanism of loading the native libraries in place. In order to do that I had to create a custom AssemblyLoadContext that for .net core knows how to load the native dependencies.

For now GitVersion.MsBuild package was tested on .net full framework on windows and .net core on window and linux (ubuntu 16.04) with single TargetFramework.

I'm planning to add multi TargetFrameworks support and also some docker files for all supported linux OSes Libgit2sharp supports to validate the GitVersion.MsBuild package.

Copy link
Member

@asbjornu asbjornu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like a lot of files are copied from GitVersionTask? Would it be possible to reuse the same files with linking or moving logic into the core?

@asbjornu
Copy link
Member

asbjornu commented Apr 18, 2019

This is fantastic, @arturcic! Thank you so much for the work you’ve put into figuring this out. 🙏 🎉 ❤️

@arturcic
Copy link
Member Author

This is fantastic, @arturcic! Thank you so much for the work you’ve put into figuring this out. 🙏 🎉 :love:

I took me a while to understand how all of these works, had to understand how the dependencies are loaded, and I had good inspiration.

If everything goes well I'll move the Libgit2Sharp specific code that loads native dependencies into a separate nuget package as suggested here

@arturcic arturcic force-pushed the feature/GitVersion.MsBuild branch 3 times, most recently from cea4d12 to b1d0240 Compare April 18, 2019 13:45
@arturcic
Copy link
Member Author

@asbjornu I have reworked the PR, the GitVersionTask transforms into GitVersion.MsBuild keeping the history on the ways as discussed. Can you have a new review?

Copy link
Member

@asbjornu asbjornu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides a few files not being detected as moved, this is looking really good!

@arturcic arturcic added this to the 5.0.0 milestone Apr 19, 2019
@arturcic
Copy link
Member Author

Looks like the 0.26.0 version of mono is not properly working with mono dotnet/sourcelink#155.

I guess for now till we have a version that does work, we can move forward with .net core (all supported oses) + windows full framework. I'll follow the progress on that thread and update with mono support.

This PR makes sense to be merged as it adds support for a lot of OSes, and it does not have dependency on libcurl

@asbjornu ideas?

@asbjornu
Copy link
Member

This PR is the only sensible step forward, imho. :shipit:

@dazinator
Copy link
Member

Is the intention to drop UtilPack and handle the dependency resolution ourselves in GitVersionTask? If so, better let @stazz know so he doesn't waste any further time on PR's related to this area!

@dazinator
Copy link
Member

One other scenario we should try and test is with Omnisharp (i.e build a csproj that has the msbuild task installed from vscode)

@arturcic
Copy link
Member Author

One other scenario we should try and test is with Omnisharp (i.e build a csproj that has the msbuild task installed from vscode)

I will give it a try

@arturcic
Copy link
Member Author

Is the intention to drop UtilPack and handle the dependency resolution ourselves in GitVersionTask? If so, better let @stazz know so he doesn't waste any further time on PR's related to this area!

To be honest I know about that PR and I also was playing with the artifacts from that PR and still for .net core on linux it was not working even with the latest Libgit2Sharp version. I started the work as a proof of concept. That's why initially I have created a separate nuget package GitVersion.MsBuild. I ended up with the current code that does work for .net core windows and linux, but it's not working on mono as it seems like the latest Libgit2Sharp has some issues with it.

@arturcic arturcic force-pushed the feature/GitVersion.MsBuild branch 4 times, most recently from 7c5ed20 to 8fe03e2 Compare April 26, 2019 14:58
@twsl
Copy link

twsl commented Apr 29, 2019

Is anything currently blocking this from getting merged?

@bonesoul
Copy link

bonesoul commented May 8, 2019

any updates?

@arturcic
Copy link
Member Author

arturcic commented May 8, 2019

One other scenario we should try and test is with Omnisharp (i.e build a csproj that has the msbuild task installed from vscode)

I will give it a try

I have tried that with VSCode on windows with a test repo I have and it was working, on macOS I could not point VSCode to use local nuget package, so probably I could test it with published nuget package

@arturcic
Copy link
Member Author

arturcic commented May 8, 2019

Is anything currently blocking this from getting merged?

The GitVersionTask in this PR is not able to run on mono as there are issues running Libgit2Sharp on mono. The other blocking issue is it's not fully tested on some of the linux distributions Libgit2Sharp supports.

@arturcic arturcic force-pushed the feature/GitVersion.MsBuild branch from 84516b5 to ac27ceb Compare May 21, 2019 11:52
@seanamosw
Copy link

Want to confirm that using the GitVersion.CommandLine.DotNetCore.5.0.0-PullRequest1659-79.nupkg artifact from AppVeyor hosted on our internal nuget resolves the issue on our Ubuntu 18 dev machines.

Would be great if we could get this in preview/stable on nuget.org.

@arturcic
Copy link
Member Author

@seanamosw thanks for the feedback. Unfortunately this PR is not compatible with the recent changes on master. So I will need to re-implement the PR, but that will be challenging.

@arturcic
Copy link
Member Author

Closing in favor of #1713

@arturcic arturcic closed this Jun 21, 2019
@arturcic arturcic removed this from the 5.0.0 milestone Jun 21, 2019
@arturcic arturcic deleted the feature/GitVersion.MsBuild branch July 5, 2019 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants