Tracking Issue for `-Zgitoxide`

[Byron]

Summary

RFC: None
Documentation: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#gitoxide
Issues: Z-gitoxide Nightly: gitoxide integration
Original issue:

• shallow clones Allow specifying shallow clones in .config #1171
• slow on file:// Cargo-update: exceedingly slow progress on update when local registry/index is used and git-fetch-with-cli is false #9167
• slow on fetch Resolving deltas takes long time  #11014
• high CPU usage High CPU usage when cargo updates its index #9215
• high memory usage Cargo eating all RAM during aarch64 build under Docker on x86-64 #10583
• publish failure due to git-ignored .DS_Store cargo-package: list of dirty files contains ignored files #8407

With the ‘gitoxide’ unstable feature, all of the specified git operations will be performed by the gitoxide crate instead of git2. This allows git2 to be replaced in full.

Steps

Unstable feature sub-categories:

• fetch: all fetches are done with gitxide (gitoxide integration: fetch  #11448)
• shallow_index: the crates index will be a shallow clone (support for shallow clones and fetches with gitoxide #11840)
• shallow_deps: git dependencies will be a shallow clone (support for shallow clones and fetches with gitoxide #11840)
• checkout: plain checkouts with gitoxide (including submodule handling)
• package: status checks, untracked files, excluded files

Further tasks

• short-hash handling can be done by gix_hash::Prefix.
• optimize dependencies to only include those that cargo actually uses.
• When specifying a dependency with rev but nothing found, cargo with shallow fetch should fetch full history as a fallback.

Unresolved Issues

• work with https://rsproxy.cn (the proxy seems to work different making naive negotiations fail due to multi-round negotiation)
• all features as listed in this gitoxide tracking issue to be able to replace git2 in full
• support for the file:// protocol without relying on the git binary
• support of the ssh:// protocol without relying on the ssh binary
• double-check and validate that authentication works similarly to git2 or if unfeasible, assure users have a migration path. How to deal with this correctly is still to be determined.
• transitive dependency tempfile is pinned to ~3.3.0 temporarily (see gix-tempfile@4.1.1)
• A-git Area: anything dealing with git might contain more potential blockers
  • take a look at all of these and either fix them, resolve them, or store them in gitoxide itself

Future Extensions

Some features not in libgit2 are good candidates for future extensions

• sparse checkout Cargo build crash in project with git Split Index #10150
• split index Add support for sparse git checkouts #11165
• git-lfs integration Git dependency with LFS objects #9692

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Implementation history

• gitoxide integration: fetch  #11448
• gitoxide progress bar fixes #11800
• Disable flaky auth tests when gitoxide runs them #11830
• [BLOCKED] upgrade gix to v0.41 to fix flaky auth tests. #11831
• support for shallow clones and fetches with gitoxide #11840

[epage]

transitive dependency tempfile is pinned to ~3.3.0 temporarily (see gix-tempfile@4.1.1)

This surprises me and the links don't quite make the motivation for this clear.

From my experience

• Using anything but ^ for general dependencies like tempfile can be ecosystem-breaking
• Generally workarounds like this should be handled in Cargo.lock

[weihanglo]

Hmm… agreed! gix-tempile previously depends on 3.4.0, so alternatively it could just depend on ^3.3.0 and we handle it in rust-lang/rust. I cannot guarantee but believe bootstrap team will fix it in the near future :).

[ehuss]

I would agree that using ^3.3.0 would be the more appropriate requirement. I would generally avoid ~ in most situations.

I cannot guarantee but believe bootstrap team will fix it in the near future

I do not think anyone is looking at fixing this particular issue. The fix from rust-lang/rust#106610 didn't seem to work for me, though I'm not sure why.

If cargo moves to a separate workspace, then this particular problem might not be an issue.

[Byron]

I don't know if there is a better solution to this problem than the one we opted for, but by sharing the status quo maybe we will come up with something.

• gix-tempfile v4.1.0 refers tempfile ^v3.4
• gix-tempfile v4.1.1 refers tempfile ~v3.3 and is the latest version that everybody gets right now.

When publishing gix-tempfile v4.1.1 my CI broke (due to cargo deny) and for some reason I didn't even notice it. Had I known that there is a RUSTSEC advisory out aganst tempfile 3.3 I would not have pinned to it so lightheartedly.

Since then, I have removed the pin of tempfile in such a way that the next release of gix-tempfile will be a major version change to fix my CI. gix-tempfile v4.1.1 will still cause other peoples CI to fail for the same reason it failed for me, and it's pulled in by default until it is yanked by all available versions of gix.

Soon I hope to be able to publish a new major version of gix which then depends on gix-tempfile v5.0, providing an upgrade path out of this situation.

Thus far, doing this was quite some churn and already caused breakage in other people's CI, and there is probably some fallout that I don't know about. The main culprit here was me being unaware of the security advisory which is sure way to break people who run cargo deny. I am sure there are plenty of other ways as well even without that.

It might be worth investigating a better process to handle such issues as I have a feeling the probability for this to happen is quite high given the large amount of additional dependencies that gitoxide (still) pulls in combined with the large amount of platforms cargo is being compiled for.

[epage]

When publishing gix-tempfile v4.1.1 my CI broke (due to cargo deny) and for some reason I didn't even notice it. Had I known that there is a RUSTSEC advisory out aganst tempfile 3.3 I would not have pinned to it so lightheartedly.

This is not the only reason to avoid pinning in version requirements. See my previous link where another crate author pinned things and caused problems.

Since then, I have removed the pin of tempfile in such a way that the next release of gix-tempfile will be a major version change to fix my CI.

Why did removing the ~ require a major version change? It seems like a ^3.3 could be in a 4.1.2

[Byron]

This is not the only reason to avoid pinning in version requirements. See my previous link where another crate author pinned things and caused problems.

Even though I didn't acknowledge it I don't think I made the claim that what happened here is all that can happen. Am I missing another point?

Why did removing the ~ require a major version change? It seems like a ^3.3 could be in a 4.1.2

As of now, cargo seems to require the dependency fix/hack to function. I made my CI fix (removing the pin) breaking to assure that the next release will be a major one that cargo update will not automatically update to, which would undo the fix. Now, gix up to v0.40 will use the pinned version, and gix v0.41 which I am prepping now will put things back to normal.

Funnily enough I am also preparing a PR that upgrades cargo to gix v0.41 to get a fix for this issue, so I guess that one will be blocked until the underlying cargo issue is fixed. 'Kerfuffle' is my favorite word for this, right after 'conundrum' 😅.

[epage]

Even though I didn't acknowledge it I don't think I made the claim that what happened here is all that can happen. Am I missing another point?

I was just wanting to confirm since I had read your comment as the RUSTSEC issue being the only thing that would have had you re-evaluate.

As of now, cargo seems to require the dependency fix/hack to function. I made my CI fix (removing the pin) breaking to assure that the next release will be a major one that cargo update will not automatically update to, which would undo the fix

Maybe others feel different but I would not consider this to be a part of your project's compatibility guarantees and personally feel making it a breaking change exacerbates the problem as the max version (default for the resolver) in the 4.x.y series does not play well with other crates.

Funnily enough I am also preparing a PR that upgrades cargo to gix v0.41 to get a fix for #11821, so I guess that one will be blocked until the underlying cargo issue is fixed. 'Kerfuffle' is my favorite word for this, right after 'conundrum' 😅.

Why is this necessarily blocked? Couldn't gix-tempfile use ^3.3 instead of ^3.4? Then cargo could use 3.3 and everyone else can avoid the tainted version and use 3.4.

[Byron]

I was just wanting to confirm since I had read your comment as the RUSTSEC issue being the only thing that would have had you re-evaluate.

Right! Even though I ran into issues like the one in your link myself and know that pinning is usually going to bite, in this case I didn't think much about the possible repercussions as fixing cargo seemed like an overwhelming priority along with a lack of alternatives.

Maybe others feel different but I would not consider this to be a part of your project's compatibility guarantees and personally feel making it a breaking change exacerbates the problem as the max version (default for the resolver) in the 4.x.y series does not play well with other crates.

Definitely, it breaks the stability guarantee of the project as per STABILITY.md (even though it doesn't actually break anybody), and the fix for the v4.x. line of gix-tempfile will happen by yanking v4.1.1 once cargo is in the clear. It is a hack to protect another hack which seemed inevitable.

Why is this necessarily blocked? Couldn't gix-tempfile use ^3.3 instead of ^3.4? Then cargo could use 3.3 and everyone else can avoid the tainted version and use 3.4.

How would cargo be able to force itself to use gix-tempfile v4.1.1? Refer to the crate and pin it maybe using ~4.1.1? Right now the assumption is that it is served whatever gix comes with, and that will currently top out at 4.1.1 by normal resolution rules.

Probably I was missing the obvious solution of pinning gix-tempfile in cargo, as now what happened is said exacerbation of the problem. gix v0.41 now refers gix-tempfile v5.0 which is good technically but wrong from cargo, which now cannot pin a lower version anymore without conflicts (or at least, it wouldn't force the whole dependency tree to use the same version). But if that were true, couldn't cargo have pinned tempfile all by itself in the first place?

So it looks like all that can be done to fix this issue is to have one PR which disables the tests in question, and another that re-enables them and upgrades to gix v0.41 that contains the actual fix and removes the pinned tempfile crate. And then we hope cargo and tempfile v3.4 will soon work again so the hacks can be removed as good as possible.

[epage]

Definitely, it breaks the stability guarantee of the project as per STABILITY.md (even though it doesn't actually break anybody

I did not see anything that said you are responsible for bugs in your dependencies. tempfile didn't see a reason to make it a breaking version, so what isn't clear to me is why it needs to be one for gix-tempfile.

Also, unsure if we've already had this conversation but I would recommend reconsidering MSRV changes being breaking changes. I know that was a topic for a while during the early days of MSRV discussions but I feel like things have mostly settled towards not doing that. Take the recent libc MSRV discussion. That had a lot of voices with a lot of different (strong) opinions but only 1 person advocated for a general policy. The only other mention of it I found was Josh bringing it as part of brainsxtorming for breaking the gordian knot for libc. Updating MSRV is like rustc updating the kernel, glibc, or android NDK.

semver is a tool to help and if it is used to cover too much ground it can make it difficult to make any progress or can cause extra churn from too many breaking releases and then hurts more than helps. I originally wrote this in talking about MSRV but I suspect it ties into our tempfile conversation

How would cargo be able to force itself to use gix-tempfile v4.1.1? Refer to the crate and pin it maybe using ~4.1.1? Right now the assumption is that it is served whatever gix comes with, and that will currently top out at 4.1.1 by normal resolution rules.

Unsure if this is what you are getting at but something I had overlooked is the challenge of (some) monorepo development: intra-repo version requirements. I'm assuming on every release all dependents get their version requirement updated to latest. This makes development in the repo easier but increases churn for end-users and offers users fewer workarounds, like in this case. I say this not to say what is the right approach for gix but to acknowledge it. gix is of the scale it could be hard to manage. In some of my projects, I do similar though in ones like clap where its smaller and the pieces are less entangled, I don't. One future possibility to help with this is my idea to stablize the #[stable] attribute and have clippy or rustc produce warnings when you use an API item from newer than the minimal version of your version requirement. While it wouldn't guarantee that version reqs within the repo are always valid, it helps.

. But if that were true, couldn't cargo have pinned tempfile all by itself in the first place?

Maybe? I don't have the context for what precipitated this. The tracking issue links to a tag which links to a comment from Chris Denton that says nothing about tempfile and I've expanded commetns around it and I don't see anything. I did a search for tempfile on the page and only found this comment from ehuss.

[Byron]

semver is a tool to help and if it is used to cover too much ground it can make it difficult to make any progress or can cause extra churn from too many breaking releases and then hurts more than helps. I originally wrote this in talking about MSRV but I suspect it ties into our tempfile conversation

Making MSRV changes non-breaking definitely can be discussed - while gix is in pre-release mode I find it easy to declare it a breaking change and prevent anybody downstream from being broken by a patch release. For stable releases, I'd love this to change as well even though I see value in the downstream being able to protect itself from breakage without having to pin their gix dependency down to the patch level. gitoxide's MSRV is effectively dictated by the windows dependency, and that makes it even harder to 'do the right thing'.
Wasn't there an RFC that would allow cargo to choose only versions that have a compatible MSRV? With that, the problem would be solved I think.

This is not to discard the suggestion of making MSRV changes non-breaking, it's just to explain who one could choose one over the other to prevent downstream breakage. To me, that is still paramount, and ideally there is a better way of doing things once gix goes 1.0 to maintain that major version number for 6 months or more.

[..] One future possibility to help with this is my idea to stablize the #[stable] attribute and have clippy or rustc produce warnings when you use an API item from newer than the minimal version of your version requirement. While it wouldn't guarantee that version reqs within the repo are always valid, it helps.

On the question on managing complex workspaces with plenty of interconnected crates, having #[stable] would definitely be another tool in the belt. It would probably be particularly useful for stable crates (i.e. X.Y with X>0).

Maybe? I don't have the context for what precipitated this. The tracking issue links to a tag which links to a comment from Chris Denton that says nothing about tempfile and I've expanded commetns around it and I don't see anything. I did a search for tempfile on the page and only found rust-lang/rust#108665 (comment).

Seeing this makes me fear I made all this up due to some huge misunderstanding 😅. The issue feels quite illusive, too, and I guess it's time for some sharing my take-aways:

• don't ever hack dependencies, even if it would fix cargo
• to avoid the previous point, try to find other solutions and involve more people if you have to

This whole topic did costs me a couple of hours and certainly had higher costs throughout the ecosystem due to gix-tempfile being tainted for a day without alternative. I will wait for the tempfile and cargo issues to be resolved (in the hopes of being pinged when that happens) to yank the tainted version and allow upgrades to more recent gix versions (> v0.40) to happen.

[epage]

Wasn't there an RFC that would allow cargo to choose only versions that have a compatible MSRV? With that, the problem would be solved I think.

It was discussed as a possibility as part of package.rust-version. We still need to work out the UX once we have package.rust-version in the index. This will help a lot, along with a list of other ideas for improving the experience summarized in a post I made in the libc thread that is now in the hidden section, making it difficult to get to...

Seeing this makes me fear I made all this up due to some huge misunderstanding 😅. The issue feels quite illusive, too, and I guess it's time for some sharing my take-aways:

Seems like we are on the same page now. Unsure what to do about whats been done but hopefully we won't hit this again. I'll go ahead and close these comments out as resolved.