Byron / gitoxide

An idiomatic, lean, fast & safe pure Rust implementation of Git
Apache License 2.0
8.38k stars 274 forks source link

Oxidize `starship` #438

Open Byron opened 2 years ago

Byron commented 2 years ago

There is a transition ongoing in this PR.

https://github.com/starship/starship

Byron commented 2 years ago

@davidkna I keep track of the features required to support starship here, please feel free to make me aware of anything you need to track its completion here.

davidkna commented 2 years ago

I think the requirements have been covered now, so I would appreciate a new release. For upcoming feature development in starship, it would be nice to have the repo-local git config available in git-repository, but that can be worked around until git-config handling is more mature. Beyond that, in the future I'd also like to look into replacing git-cli usage in starship (git status, ahead/behind info, stashed count, number of added and deleted lines).

Byron commented 2 years ago

I think the requirements have been covered now, so I would appreciate a new release.

Great to hear, I am excited! The release is done now, git-repository is at v0.19.

For upcoming feature development in starship, it would be nice to have the repo-local git config available in git-repository

I assume you'd expect it to be complete as well, as a repository-local (i.e. .git/config) may not provide the complete picture depending on the value being looked up. In any case, #331 is the tracking ticket for getting git-config ready for prime time.

Beyond that, in the future I'd also like to look into replacing git-cli usage in starship (git status, ahead/behind info, stashed count, number of added and deleted lines).

I'd expect this to happen but depending my workload maybe not even this year. However, if you are interested in contributing some, please feel free to start a discussion or WIP PR so I can at least guide the implementation.

davidkna commented 2 years ago

Great to hear, I am excited! The release is done now, git-repository is at v0.19.

Thanks!

I assume you'd expect it to be complete as well, as a repository-local (i.e. .git/config) may not provide the complete picture depending on the value being looked up. In any case, #331 is the tracking ticket for getting git-config ready for prime time.

The things that are currently in the works only require the repository-local config or wouldn't benefit much from a complete config view. Now that I think about, support for secure.directories would also be very nice to have to make git-sec less restrictive.

I'd expect this to happen but depending my workload maybe not even this year. However, if you are interested in contributing some, please feel free to start a discussion or WIP PR so I can at least guide the implementation.

I'll try to contribute again in the future.

Byron commented 2 years ago

The things that are currently in the works only require the repository-local config or wouldn't benefit much from a complete config view.

Since that is already working and actively used, it should be no problem to find a suitable API to expose it. However, I am also thinking that you'd probably need it to be auto-updating or to be fresh each time you access its data, so it needs some consideration on how it's exposed in gitoxide. In any case, it's nothing that should take forever and if you say you'd like this functionality sooner, I will prioritize it.

Now that I think about, support for secure.directories would also be very nice to have to make git-sec less restrictive.

I saw you are using git::sec::Trust yourself maybe even in anticipation of what git would do when invoked. I'd hope that what will happen first is full support for git-config so you can read these values yourself and anticipate what git would do correctly, or you could invoke git to see if it barks (I think the exit code is quite distinct then). Maybe doing either will work for now.

As a final state when gitoxide supports all features so you don't need to call git anymore, git::sec::Trust and safe directories will be less important as gitoxide has much more fine-grained controls and won't bail out for read-only operation. Instead, it will know what configuration and which files are trusted and behave accordingly.

davidkna commented 2 years ago

However, I am also thinking that you'd probably need it to be auto-updating or to be fresh each time you access its data, so it needs some consideration on how it's exposed in gitoxide.

For the starship case, the binary runs once per prompt draw for a short time, so staleness of data is no concern.

Byron commented 2 years ago

Perfect, this makes providing configuration so much easier. I have some ideas too and hope it won't take too long. If you could point me to some section of code that would benefit from it, that would be greatly appreciated.

Byron commented 1 year ago

@davidkna I think today we had a break-through as it appeared clear what features gitoxide needs to provide to fully replace (and accelerate) previous git invocations while providing support for all known repository configurations out there.

The check-list has been updated, maybe you can have a look to see if anything is missing or should be fleshed out more. Please note that I'd expect everything to be resolved in the first half of 2023.

davidkna commented 1 year ago

@Byron Thanks for the update and heads-up! The list looks fine to me for now.

Byron commented 3 months ago

@davidkna Another update, just 1.5 years later 😁. I am closing in on the status portion with some excitement as the git_metrics part will very soon be available. The git_status portions also incorporates changes between the HEAD and the index, which won't be too hard to implement as it's basically already present as tree-vs-tree diff, including rename tracking.

The great news here is that both operations are very similar and their separation makes no sense with gitoxide anymore, which means that the time to obtain all required information will basically be cut in half, at least. In practice, it will be faster as gitoxide parallelises more.

My intuition here is that it's best to wait until the git_status part is also available, to then be able to unify the data acquisition among both of these modules.

As an example of what I think is possible here I looked at WebKit. Here is the baseline costs for getting a status.

WebKit ( main) via △
❯ hyperfine -N -w1 'git status --no-renames --untracked-files=all --ignored=no --ignore-submodules=all' 'gix status'
Benchmark 1: git status --no-renames --untracked-files=all --ignored=no --ignore-submodules=all
  Time (mean ± σ):      5.072 s ±  0.042 s    [User: 0.474 s, System: 23.604 s]
  Range (min … max):    4.990 s …  5.116 s    10 runs

Benchmark 2: gix status
  Time (mean ± σ):      3.441 s ±  0.049 s    [User: 0.612 s, System: 6.508 s]
  Range (min … max):    3.381 s …  3.543 s    10 runs

Summary
  gix status ran
    1.47 ± 0.02 times faster than git status --no-renames --untracked-files=all --ignored=no --ignore-submodules=all

gitoxide is already 50% faster here for the same amount of work (note that submodules will be included, it's just disabled in this version and doesn't play a role).

Currently, we have these timings:

❯ starship timings

 Here are the timings of modules in your prompt (>=1ms or output):
 git_status   -  4981ms  -   "[»!?] "
 git_metrics  -  3779ms  -   "+2 "
 directory    -     1ms  -   "WebKit "
 pulumi       -     1ms  -   ""
 git_branch   -    <1ms  -   "( main) "
 cmake        -    <1ms  -   "via △ "
 line_break   -    <1ms  -   "\n"
 character    -    <1ms  -   "❯ "

WebKit ( main) +2 [»!?] via △ took 8s

In future, I'd expect that both git_status and git_metrics cost only 3.5 seconds, together, instead of ~8.7s like right now, a 2.5x improvement. This of course is an extreme and I had to crank up command_timeout to painful numbers, yet I think that the 2.5x improvement is something we can expect to see.

This also means that repositories that take 2.5x more time will now be in range for metrics, without being aborted 🎉. Note that the whole operation of course is interruptible as well, so it should integrate nicely.

Please let me know if waiting-until-its-complete-before-integrating is also what you would do.

davidkna commented 3 months ago

Thanks, these numbers look great! I think that waiting until it's complete is fine.