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

Update libgit2 to latest master. #440

Merged
merged 2 commits into from
Jul 15, 2019
Merged

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Jul 13, 2019

No description provided.

ehuss added 2 commits July 13, 2019 10:58
This switches to the CollisionDetection SHA1 implementation for all platforms
regardless if the `https` feature is enabled. This is the default behavior
of libgit2 now. Also, the windows sha1 implementation doesn't seem to compile
anymore.
@ehuss
Copy link
Contributor Author

ehuss commented Jul 13, 2019

Let me know if you have any questions, I have some notes on some of the changes.

I switched the SHA1 implementation because the Windows HTTPS version seems to be broken. "CollisionDetection" is the default now on libgit2, so I imagine it's probably good enough?

Let me know if there is a better way to handle the settings for the PCRE stuff. I couldn't find a way to tell the cc crate to build a set of files with different settings. It seems to be all or nothing?

// Ideally these defines would be specific to the pcre files (or placed in
// a config.h), but since libgit2 already has a config.h used for other
// reasons, just define on the command-line for everything. Perhaps there
// is some way with cc to have different instructions per-file?
Copy link
Member

Choose a reason for hiding this comment

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

Typically the way to go for this is to just use multiple cc::Build instances, but it's probably fine to do the same config for all of libgit2

@alexcrichton
Copy link
Member

Following upstream for defaults sounds reasonable to me! Did the regex backend get dropped in favor of a pcre2 dependency? It'd be sort of neat if we could patch in the regex crate instead of having libgit2 use its own backend, but that's probably a more long-term feature dependency. I'm also not entirely sure what it's even used for :)

@ehuss
Copy link
Contributor Author

ehuss commented Jul 15, 2019

Did the regex backend get dropped in favor of a pcre2 dependency?

Yes, here's the PR: libgit2/libgit2#4935 It looks like they were having some issues with the old glibc implementation.

It'd be sort of neat if we could patch in the regex crate instead of having libgit2 use its own backend

It's kind hard for me to wrap my head around how linking would work for that. Presumably you'd have something that had extern "C" definitions for the API that libgit2 expects that would translate to the equivalent Rust code? I'm not really sure how that would get injected into the right time and place during linking, and deal with all the rust side of things like libstd.

I'm also not entirely sure what it's even used for :)

A few places:

  • Config stuff, like searching through config values.
  • Used in the diff engine.
  • "rev parse" supports patterns.

@alexcrichton
Copy link
Member

Ok that all sounds good to me! Thanks again for taking this on!

As for how it'd use the regex crate itself, that's mostly just an idle musing. It's probably invalid anyway with different syntaxes, but I'd hope for something like we intialize libgit2 with a set of function pointers that it can call back into to do regex operations. Anyway that's pie-in-the-sky and not at all related to this, so I'll merge!`

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