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

Revert 86721d1 #859

Closed
wants to merge 1 commit into from
Closed

Revert 86721d1 #859

wants to merge 1 commit into from

Conversation

Teutates
Copy link

PR #817 (and commit 86721d1) are based on the false premise that ABI changes == API changes. This commit did not solve any issues, because the only reason git2-rs would be incompatible to compile with a new version of the system libgit2 would be if there was an API change. While libgit2 does have an unstable ABI when it comes to major releases, they do not have an unstable API. This means that system versions above what the library is "currently compatible with" can take advantage of the newer library version with minimal changes, there will just need to be a recompile.

Again, all this commit did was artificially limit the newer versions of libgit2 you can use with this library and not didn't solve anything. Programs will still need a recompile, and git2-rs will need a version bump simply to increase this number.

PR rust-lang#817 (and commit 86721d1) are based on the false premise that ABI changes == API changes. This commit did not solve any issues, because the only reason git2-rs would be incompatible to compile with a new version of the system libgit2 would be if there was an *API* change. While libgit2 does have an unstable ABI when it comes to major releases, they do not have an unstable API. This means that system versions above what the library is "currently compatible with" can take advantage of the newer library version with minimal changes, there will just need to be a recompile.

Again, all this commit did was artificially limit the newer versions of libgit2 you can use with this library and not didn't solve anything. Programs will still need a recompile, and git2-rs will need a version bump simply to increase this number.
@Teutates Teutates mentioned this pull request Jul 18, 2022
@ehuss
Copy link
Contributor

ehuss commented Jul 18, 2022

I'm not sure I follow what this PR is trying to say. There were concrete problems where changes between minor versions would cause breakage. For example, trying to build a Rust project with git2 0.13 (which used atleast_version("1.3.0")) with a system libgit2 version 1.4 or 1.5 would cause crashes due to changes in the structures on the C side. I think #813 explains one of the issues pretty well. The ABI changed between 1.3 and 1.4, and libgit2-sys expected the old ABI, as the layout of a structure changed.

@joshtriplett
Copy link
Member

As @ehuss said, this addresses real problems, and without it we were experiencing widespread reports of breakage with newer libgit2. That breakage was getting reported to git2-rs, to the libgit2 project upstream, and to projects built on git2-rs.

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.

3 participants