Replace git2 with gix
#129
Conversation
Byron
force-pushed
the
replace-git2-with-gitoxide
branch
9 times, most recently
from
2fbbbde to
b75ccf7
Compare
Byron
force-pushed
the
replace-git2-with-gitoxide
branch
from
9c7c910 to
a054b0d
Compare
|
For the review, I recommend using If I can be of any help otherwise, please let me know :). |
Byron
force-pushed
the
replace-git2-with-gitoxide
branch
3 times, most recently
from
45453c5 to
52526a1
Compare
The idea is to have it side-by-side until all `git2` usages are replaced. Note that we also switch to the new `dep:` style with the `gix` dependency.
* Let most tests share the same state, and avoid altering the users cargo registry. Make sure that shared registry is cloned only once and teach initialization to wait for it on first clone. On CI, we use the freshly cloned crates index that was needed to build the binary in the first place, or put it there in case the sparse index was used for additional testing. * make `mem` check operational again, and adjust expectation so it passes on MacOS (maybe unrelated, it didnt' run for a while probably) * make `bare_iterator()` test much faster * reduce startup time of `test_can_parse_all()` by using a shared index
Note that this commit also adds the infrastructure to support `gix` side-by-side. This is done by renaming the soon-to-be-replaced types and fields with `git2_<name>`. Additionally, CI is adjusted to be able to deliver best possible performance for some of the long-running tests.
When cloning the index at the same time, as is done by tests, it's possible to discover a newly initialized repository that isn't quite ready yet, causing failures lateron. To prevent that, we have to be sure there is no opening happening while we are cloning.
Byron
force-pushed
the
replace-git2-with-gitoxide
branch
from
9f09f47 to
11a7522
Compare
And undo some of it to better fit into the overall styling of this crate.
|
After noticing this awesome new |
Byron
force-pushed
the
replace-git2-with-gitoxide
branch
from
412a442 to
f50308f
Compare
…g the index. This example can also help to do some real-world testing registries that are less than optimal. This is particularly important now that the unit tests are sandboxed.
Previously they could get stuck in an outdated position if fast-forwarding them wasn't possible. This is a problem in case index squashes happen inbetween. Now the refs will be forcefully overwritten, so we retain the most recent state that we just fetched, which is vital for consistency.
We really want to be sure we find the most recent commit, and put extra safe-guards in place to assure this happens.
This way it's easier to add new errors in future, and less boilerplate. It's already part of the dependency tree now as `gix` uses it, so it's basically free.
|
And it's ready for review again, with improvements for finding the most recent commit to be more resilient towards the various kinds of indices it may encounter. I want to note that I ran into performance problems with version 1.0 due to a particular case where my index was out-of-date, and on each All this is fixed in this branch as well, along with I assume significant performance improvements. |
This means `git2` is now fully removed from the tools used by `gitoxide`. Note that this also means that the `vendored-ssl` feature has been removed as there is no equivalent. It might be worth to add feature toggles that change to another backend though.
This means `git2` is now fully removed from the tools used by `gitoxide`. Note that this also means that the `vendored-ssl` feature has been removed as there is no equivalent. It might be worth to add feature toggles that change to another backend though.
|
just in case so you know #132 btw: |
|
Oh, I wasn't aware, and was sorry to read this :/!
Thank you Even though I probably shouldn't review my own PR, I am happy to become a maintainer of Due to my work on |
This means `git2` is now fully removed from the tools used by `gitoxide`. Note that this also means that the `vendored-ssl` feature has been removed as there is no equivalent. It might be worth to add feature toggles that change to another backend though.
|
@frewsxcv sry for the ping... but i wanted to make sure that you have seen Byron's application:
|
Apparently cargo, when cloning via git2, never sets a remote, so the logic for fetching updates basically never worked. I will add a test for this later... Shamelessly stolen from frewsxcv/rust-crates-index#129
This PR makes a big change to how the RemoteGitIndex performs fetches (and clones) and subsequently reads blobs, to resolve issues discovered while doing obi1kenobi/cargo-semver-checks#506 Previously when performing a fetch, we also updated references so that HEAD pointed at the same commit as the remote HEAD. This works fine in local operation and in tests, but had a drawback, namely ``` Error: Failed to update references to their new position to match their remote locations Caused by: 0: The reflog could not be created or updated 1: reflog messages need a committer which isn't set ``` can occur in CI environments. My attempts to fix this...failed (configuring a committer name + email before editing references). Then I noticed frewsxcv/rust-crates-index#129 which....is just way better than that previous approach since we don't need to edit references any longer just to be able to use `repo.head_commit` for reading blobs, but rather just store the remote HEAD when opening/fetching and retrieving the tree to lookup blobs from that, sidestepping the whole issue with updating references. There is a big difference between that PR though, in that this PR also adds a function (`crate::utils::git::write_fetch_head`) to actually write a FETCH_HEAD _similarly_ to how cargo, via git or libgit2 writes FETCH_HEAD when performing a fetch on an index. This was done for 2 reasons: 1. Performing a fetch of an index via this crate will now update the index repository the same as cargo would, which makes this play nicer with cargo and thus the wider ecosystem 2. I had already done (a slightly worse) version of writing a FETCH_HEAD for [cargo-deny](https://github.com/EmbarkStudios/cargo-deny/blob/main/src/advisories/helpers/db.rs#L451-L457), because, AFAIK, retrieving the timestamp of the FETCH_HEAD file is by far the [most/only reliable](https://stackoverflow.com/questions/2993902/how-do-i-check-the-date-and-time-of-the-latest-git-pull-that-was-executed) way to determine when a fetch was last performed on a repository. The reason this was important for cargo deny is that it fetches advisory databases, but can decide _not_ to fetch them if the user doesn't want to perform network calls, however if they _never_ fetch from the remote advisory db they run the risk of mistakenly thinking everything is fine even if they have crate dependencies that have active advisories that were added/modified since the last time they fetched. So until `gix` adds support for writing FETCH_HEAD (which I think is planned, but couldn't find it), making this function public allows cargo-deny or others to also have a (simple, definitely not git/libgit2 compliant) way to write a FETCH_HEAD with gix.
Thanks for offering! Just sent you an invite to join the repo and crates.io crate |
|
Thank you :), I feel honoured to be allowed to maintain this crate, as I have been using it on and off for many years now! I will be working on a 2.0 release that will incorporate this PR, and be sure to communicate the one breaking change properly. |
This means `git2` is now fully removed from the tools used by `gitoxide`. Note that this also means that the `vendored-ssl` feature has been removed as there is no equivalent. It might be worth to add feature toggles that change to another backend though.
This PR replaces
git2withgix. It does so trying to make no breaking changes to the public API.Please note that this is an ongoing effort, this PR will change a lot.
Why now?
With the sparse index slowly becoming the new default, users of this crate will probably be less succeptible
to changes in the
gitimplementation which might come with compatibility issues on windows whensshis used.Furthermore,
gixnow has a robust implementation of the negotiation protocol which allows it to work forcrates.io proxies as well, which is important for users in China.
Making this changes has high value as it can solve a list of issues which have been open for a long time.
Tasks 📝
git2_andGit2to find leftover references in docs to old names and fix themgixfor smoother API and refer to new release heremasterchangesfeature.gixrelease with latest changes to adapt APIFETCH_HEADhandling work (the only way to see what was actually fetched) (noticed while dog-fooding tooling built with this version)Breaking Changes ❗️
vendored-opensslfeature toggle was removed as there is no equivalent, additive feature ingitoxide. It's replaced with documentation on how to getrustlsinstead.Highlights 🔦
mem usagetests seems to be a little bit faster with ~5s forgixand ~6s forgit2. (it's definitely not as fast as I would have hoped though, ODB is the bottleneck)changes()takes 1.2s onmasterbut only 335ms withgix.Differences to
git2implementation ≠git.git. This works particularly well with private registries.sshoptions are handled by thesshprogram, just likegit. ssh-relatedgitconfiguration is respected as well.I am leaving it with the defaults as I think they improve compatibility and make it easier to connect to a variety of remotes.
Potential Shortcomings⚠️
sshimplementation.gitoxidedoesn't have that yet, and uses thesshprogram, just like git.gitoxidecan't read theFETCH_HEAD, nor does it write it. If that's an issue, this could be implemented.Questions 🙋♂️
httpsfeature, one has to make a choice. Usingcurlprovides the best capabilities, but one would have to chose thereqwestbased backend to solve the OpenSSL related issues. Ideally, this could be a choice, but that doesn't work withgixfeature toggles as these backend features are mutually exclusive.gixaccepts that--all-featurebuilds won't compile, but this crate might not.fslockinstead ofgix::lock?lib.rsmachine? @kornelskimem_usagetests also shows that bothpeakandtotalmemory usage are up, but I have a feeling it counts virtual memory differently maybe. It's best to just try it and also compare performance on smaller machines.Related Issues 🐞
gitCLI typically stems from authentication not working correctly.gixdoesn't have such issues and will generally authenticate remote just likegitdoes, using its configuration automatically.git2creates, and now we usegixfile locks to protect against concurrent access and block instead. The test-suite also makes use of this capability for better performance.gixuses thesshprogram just likegitdoes, sosshprotocol related incompatibilities should go away.gixaims to behave just likegitrustlsinstead of OpenSSL #88httpsbackend to use a pure Rust implementation, which in turn usesrustls.crates-indexGitoxideLabs/gitoxide#574 (tracking ticket ingitoxide)