Skip to content

Include all platform binaries #13

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

Merged
merged 1 commit into from
Jun 29, 2015
Merged

Conversation

bording
Copy link
Member

@bording bording commented Jun 28, 2015

This change to the props file will make the native binaries for all platforms
be copied into the NativeBinaries folder regardless of which platform you are
building on.

The new LibGit2Sharp.dll.config will ensure that mono can find the linux and
osx binaries when the are in the NativeBinaries folder.

This addresses the primary issue discussed in #9

This change to the props file will make the native binaries for all platforms
be copied into the NativeBinaries folder regardless of which platform you are
building on.

The new LibGit2Sharp.dll.config will ensure that mono can find the linux and
osx binaries when the are in the NativeBinaries folder.
@bording
Copy link
Member Author

bording commented Jun 28, 2015

@nulltoken An interesting side effect of this change is that LibGit2Sharp's build.libgit2sharp.sh no longer needs LD_LIBRARY_PATH set at all. Once a version of the nuget package with these changes is being used in the main repo, I'll fix that up too.

@bording
Copy link
Member Author

bording commented Jun 28, 2015

One other thing I wanted to point out, I didn't change the path of windows binaries at all with this.

This does have the unfortunate side effect of making the NativeBinaries folder have the follwowing subfolders:

  • amd64
  • linux
  • osx
  • x86

It would be nice to move those into a windows subfolder, but I know that would require changes in the LibGit2Sharp code, so I'm not sure it's worth bothering with.

nulltoken added a commit that referenced this pull request Jun 29, 2015
@nulltoken nulltoken merged commit c355228 into master Jun 29, 2015
@nulltoken nulltoken deleted the bording/includeAllBinaries branch June 29, 2015 20:34
@nulltoken
Copy link
Member

😍 ‼️

@nulltoken
Copy link
Member

@bording The only drawback, so far, I can think of is that now the build fails when it cannot finds the Mac OS X or Linux binaries. This is actually quite cumbersome when testing a new libgit2 upgrade.

Previously my flow was

  • Upgrade libgit2.nativebinaries && open a pull request
  • Download the produced NuGet package && run the tests locally
  • If everything's green, merge the pull request in native.binaries, publish the NuGet package
  • Ensure everything's still green from the LibGit2sharp CI servers standpoint

Thoughts?

@bording
Copy link
Member Author

bording commented Jun 29, 2015

Ah, yeah I didn't think about it from the testing perspective when you have an incomplete, windows-only test package.

I think I can add some exists conditions around the includes, so it will only try if the files exist.

If for some reason that doesn't work, then perhaps some sort of "skipunix" msbuild property you could set when using the test package?

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

Successfully merging this pull request may close these issues.

2 participants