[wich]

Changed gemspec to include proper directives for the native extension library so
that it can be found on every platform.

Removed the dependency on having libgit2 built in vendor/bundle and instead
provided --with-libgit2-{dir,include,lib} build options with dir_config in
extconf.rb. If libgit2 is installed system wide it will be found in the normal
directories, if it is installde in a non-standard location the location can be
supplide with the new build options. When desired libgit2 can be built
statically in any location, the new build options can then be used to specify
the static version to be preferred over any system version.

The new build options can be supplied to extconf.rb directly, to gem install
with e.g. gem install rugged -- --with-libgit2-include=vendor/libgit2/include
--with-libgit2-lib=vendor/libgit2/build or to bundle config with e.g. bundle
config rugged.build --with-libgit2-dir=/opt/libgit2

[arthurschreiber]

I don't think there are any *.rb files inside vendor/libgit2, so this change is not needed.

[wich]

Oh shoot, wrong line, it was supposed to be the one above to include the extconf.rb as per http://guides.rubygems.org/c-extensions/ Seems that it isn't necessary though as the gem worked fine like this.

[arthurschreiber]

Hey,

first off, thank you for your work!

As libgit2 does still have a pretty unstable API, I don't think these changes really make sense right now. Every libgit2 release usually breaks backwards compatibility, and I don't think that it's a good idea to force users to have an outdated, potentially broken version installed on their system just so some rugged version works correctly.

The current approach of just bundling libgit2 ensures that we always compile against a libgit2 version where we can guarantee that rugged will work in all cases, and updating rugged and the installed libgit2 version can be done independently. As soon as libgit2 reaches 1.0 and gets a stable API, moving to dynamic linking might more sense.

In the end, it's going to be @vmg's decision, tho. 😄

[wich]

Hey Arthur, thanks for the comments.

However I don't think that embedding it and linking to it statically solves the problem here, in that case you will just have various rugged gems in various bundles with outdated, possibly containing security issues, versions of libgit2 linked in statically, with the user probably completely unaware of any such possible security issue existing in some bundle.

If the libgit2 is installed separately the user/package manager can choose to have multiple versions of the dynamic library installed and choose to remove any version which is found to have any security issue, exposing any rugged gem (or other language bindings) that depend on it and need to be updated.

Note that it is also perfectly possibly to simply install libgit2 as a static library and have the rugged gem link to that. That way the installed version of libgit2 can be upgraded without any impact to the installed rugged gems and any newly installed rugged gems will use the updated version of libgit2. Note that of course this recreates the problem of gems remaining with potential security issues in them. But that's a whole other story which belongs in the gems vs package manager discussion and doesn't belong here.

This way however at least the user has a choice in the way they would like to deal with it.

And of course libgit2 should update version number and so name in the correct way with every ABI incompatible change and rugged should depend on the correct so name. Better yet, libgit2 should use proper symbol versioning, but how that would work out on the Windows platform I do not know.

[arthurschreiber]

I'm still not convinced. Even gems like nokogiri come with their dependencies bundled (libxml and all that stuff), unless you specify NOKOGIRI_USE_SYSTEM_LIBRARIES=1 when running gem install.

What we could do is at least honor the --with-* flags when gem installing rugged, and otherwise still go with the current approach.

[arthurschreiber]

What do you think about the change in a750ab8 and 2e64c92? I added them as part of my work on cross compilation support. They allow to specify the libgit2 path on gem installation, but will automatically fallback to the bundled version otherwise.

[vmg]

This is mostly superseded by @arthurschreiber's other PR. In general terms, I'm well aware that linking against the system's version of libgit2 is the "ideal" solution, but in practice this is a massive headache. It's a common convention in the Ruby community to ship gems with a very specific version of their dependencies, to ensure they will never break regardless of system differences. I think the benefits outweigh the issues this approach has.

Thanks for the PR though!

[johnbellone]

Did these build options actually not make it in the release? The largest issue here, which hopefully will be solved with #261 is that we cannot build the embedded gem without checking out a vendored copy ourselves.

[johnbellone]

Ah, I am looking at extconf.rb right now and it seems we can override environment variable? I'll test that out!

[arthurschreiber]

The problem with this PR/idea is that the libgit2 API is pretty unstable right now, and it would be madness to depend on people having some specific revision checked out and and installed on their system. As soon as libgit2 hits 1.0, we might stop vendoring libgit2 in rugged, because the API should be stable for minor releases, and we can depend on people to have libgit2 installed on their systems.

Regarding #261, I'll merge this now. @vmg has been pretty busy lately, and has not yet given his okay there, but it should not introduce any problems, and fix a lot of problems for many people due to the GitHub switch from http to https for all repos.

[johnbellone]

Awesome @arthurschreiber thanks much!