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

Remove runtime dependency on git package #9807

Open
4 tasks
roberth opened this issue Jan 19, 2024 · 13 comments
Open
4 tasks

Remove runtime dependency on git package #9807

roberth opened this issue Jan 19, 2024 · 13 comments
Labels
feature Feature request or proposal fetching Networking with the outside (non-Nix) world, input locking

Comments

@roberth
Copy link
Member

roberth commented Jan 19, 2024

Is your feature request related to a problem? Please describe.

  • Having a dependency on external commands is not great when you're in a constrained environment. E.g. iirc macOS ships a git that's a trap door, asking the user to install xcode.
  • Library performance tends to be better than external process performance.
  • Libraries may be more secure, as we skip argument parsing logic, which may be flawed and subject to dangerous changes.
  • Closure size perhaps.

Describe the solution you'd like

Use libgit2 for everything.
Do not call the Git CLI from Nix.
We'll still need it in the functional test suite, but that's ok.
Do not regress in functionality.

Additional context

Priorities

Personally, I just opened this for tracking purposes.

Add 👍 to issues you find important.

@roberth roberth added feature Feature request or proposal fetching Networking with the outside (non-Nix) world, input locking labels Jan 19, 2024
@roberth roberth mentioned this issue Jan 19, 2024
2 tasks
@lf-
Copy link
Member

lf- commented Jan 19, 2024

You should be aware that libgit2 has severe performance problems, which would be rather unfortunate given that git is already not the fastest thing in the world. It seems much more sensible to deliberately put a fallback git into the closure of Nix, so it will consistently be available, and be of the better implementation.

@roberth
Copy link
Member Author

roberth commented Jan 19, 2024

You should be aware that libgit2 has severe performance problems

We've seen some performance regressions that we expect to be offset by the ability to operate on subtrees of repositories, and not adding everything to the store. I haven't seen anything that I'd have to call severe.
@lf- Could you elaborate on that? Do you happen to know why this might be?

Note also that use libgit2 to sidestep not entirely reproducible behaviors in the git checkout and archive logic, such as smudge filters and export-ignore. This is solved simply and reliably by accessing the tree objects directly.

deliberately put a fallback git [command] into the closure

There's no doubt that we keep it as long as we need it. Performance can be a valid reason to keep it.

@abathur
Copy link
Member

abathur commented Jan 19, 2024

I've used a very teeny slice of libgit2 indirectly through python and rust bindings and my experience was that it's a mix when it comes to performance.

In particular, I found diffing was quite a bit slower (especially on big repos) via libgit2 than by shelling out to git. (Some other stuff was quite a bit faster, though.)

I think it just merits benchmarking each implementation against a good mix of repo sizes. Maybe an easy-to-run ~benchmarking suite pays off over time by warding off problems and encouraging people to experiment with optimizations.

Edit: there's also the rust gitoxide project. It's an alternative to the rust binding, but not a drop-in replacement. It also has a nascent gix cli, but IIRC it is likewise not a drop-in replacement. If there are any obvious performance hotspots, it might be worth exploring whether an equivalent implementation can use gitoxide or gix to see how they perform. The maintainer's pretty responsive with respect to tracking/prioritizing missing features to help onboard projects.

@lf-
Copy link
Member

lf- commented Jan 19, 2024

Here's an example of the pain it's caused with cargo: rust-lang/cargo#11567 (comment)

It's plausible that nix's use case of pulling objects out of the store for a vfs would be faster on libgit2 and yet also that clone would be unacceptably slow without instead shelling out to git. I can believe that various use cases end up with very different perf results.

@roberth
Copy link
Member Author

roberth commented Jan 21, 2024

Regarding libgit2 performance, romkatv has made optimizations that may be interesting to upstream.
libgit2/libgit2#5044 attempted to upstream a number of them. Perhaps we could cherry-pick ones that are relevant to us and turn them into smaller PRs that are easier for upstream to merge?
Their motivating use case is a highly optimized git status: https://github.com/romkatv/gitstatus/blob/master/README.md#why-fast

@abathur
Copy link
Member

abathur commented Jan 21, 2024

Their motivating use case is a highly optimized git status

Interesting. Hadn't spotted that before. Amusingly, my motivating case is more or less the same (I implemented lilgit as a more minimal alternative to gitstatus to see if i could claw back more time) :)

@lorenzleutgeb
Copy link
Member

Related to:

@lorenzleutgeb
Copy link
Member

lorenzleutgeb commented May 21, 2024

I filed #10567 which goes in the opposite direction of this issue. Great input by @fricklerhandwerk makes me think that removing git as a runtime dependency is the way to go. However, on an IMO more basic level, there should be an interface for plugging fetchers. See also my #10567 (comment). Commenting this here for reach regarding "pluggable fetchers".

@roberth
Copy link
Member Author

roberth commented May 21, 2024

When libgit2 supports remote helpers, #10567 and removing the git command dependency won't be in opposition.

I've added "Do not regress in functionality" to the issue description, because I'm certainly not advocating for breaking people's stuff for some minor technical reason.

@lorenzleutgeb
Copy link
Member

lorenzleutgeb commented May 21, 2024

When libgit2 supports remote helpers, #10567 and removing the git command dependency won't be in opposition.

Right. But do you have any indication that libgit2 would consider adding this, or even better someone working on it? I searched their repo but couldn't find anything. Don't get me wrong, I think it'd be very cool if it would support remote helpers, I am just genuinely asking.

@roberth
Copy link
Member Author

roberth commented May 21, 2024

I haven't talked to them about these features; I've only opened this issue to track what we'd need, because it came up in review and discussions. I don't think this issue is strategically important for Nix, but any efforts will be appreciated.

@matthewbauer
Copy link
Member

It looks like libgit2 is used pretty extensively. I wonder what is remaining that calls git?

@roberth
Copy link
Member Author

roberth commented Jun 25, 2024

Most significantly it uses clone/fetch for performance and functionality; see Additional context for blockers.
Also a signature check, and git add in the flakes CLI.
Currently you can find the call sites with

git grep -E 'runProgram\("git"|.program = "git"'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature request or proposal fetching Networking with the outside (non-Nix) world, input locking
Projects
None yet
Development

No branches or pull requests

5 participants