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

Add gitstatus backend for git segment #104

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SuperSandro2000
Copy link
Contributor

This PR adds gitstatus as an backend for the git segment. On my local copy of nixpkgs this improves the speed of the segment drastically. With normal git it takes ~58ms for the segment and with gitstatus only ~5 ms like their readme advertises.

I am happy to integrate feedback.

sbp.bash Outdated Show resolved Hide resolved
sbp.bash Outdated

PS1=$(bash "${SBP_PATH}/src/main.bash" "$command_status" "$command_duration")
# gitstatus.plugin.sh requires an interactive shell
PS1=$(bash --noediting --noprofile --norc -i "${SBP_PATH}/src/main.bash" "$command_status" "$command_duration" "$GITSTATUS_DIR" "$GITSTATUS_DAEMON_PID" "$_GITSTATUS_REQ_FD" "$_GITSTATUS_RESP_FD")
Copy link
Owner

Choose a reason for hiding this comment

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

Did you verify that this is the case? I can't see the rationale for this looking at the repository at least as the gitstatus binary is doing the actual work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you look at the script you can see the same return that the default .bashrc has.

If we would vendor the script we could remove the check and remove this giant workaround. Not sure what to do.

Copy link
Owner

Choose a reason for hiding this comment

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

ah right. Would we be able to limit the interactive shell to just the invocation of the gitplugin that we are doing in the segment script though? I'm thinking that this could probably be a separate segment script, as it's using a completely different approach than the regular git segment. Does that make sense to you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure. We can't easily forward all variables we need without explicitly listing all of them.

source "$segment_script"
the segments are loaded here as far as I can tell and converting source to bash is not that easy.

I wouldn't make this a different segment because it is still sharing some of the code.

@brujoand
Copy link
Owner

brujoand commented Oct 9, 2020

This is a difficult one, I really like the work romkatv has done on getting the speed increase for git status. Which is by far the slowest segment in SBP. In the current suggestion though I think we are taking on too much responsibility for the gitstatus plugin. Would it be possible to instead link to instructions to setup the deamon, and have the segment use it directly? Or could we then query the daemon directly based on environment variables in the shell?

@SuperSandro2000
Copy link
Contributor Author

SuperSandro2000 commented Oct 9, 2020

Would it be possible to instead link to instructions to setup the deamon, and have the segment use it directly?
Or could we then query the daemon directly based on environment variables in the shell?

🤔 I should have thought about that earlier. Instruct people to put it into their bashrc and we would make a lot of stuff easier and they can use it with other scripts. I try to do that and test it over the weekend.

This would also mean we could remove the setting because if the PID variable of gitstatus is present we assume it should be used. This also solves the config loading issue.

This is probably the way to go. Thanks for the idea!!

@brujoand
Copy link
Owner

brujoand commented Oct 9, 2020

Cool! 👍

@SuperSandro2000
Copy link
Contributor Author

@brujoand I tested this over the last weeks and I finally got the history fully working like it should. Also sometimes gitstatus does not show as much information as git-status. I think this is only the case if the remote branch does not exist but I am not sure if this a limitation of gitstatus or my code. Anyway I don't think this is a blocker right now.

@@ -48,6 +48,21 @@ truncation/compacting for that specific segment.
shows the git branch and current status, set ´SEGMENTS_GIT_BRANCH_ONLY=true´
to speed up execution on large repos. The default value is ´false´.

Enother option to speed up execution is to use gitstatus as a backend.
Install gitstatus and then add the following to your .bashrc_:
Copy link
Owner

Choose a reason for hiding this comment

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

Do we still require sbp to start an interactive shell with this approach?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brujoand
Copy link
Owner

Apologies again for taking forever to get back to you on this one. It really bugged me that gitstatus would have to become so interconnected into sbp to achieve this. And looking through the bash code it uses, it's quite extensive and not trivial to understand. At the same time, I've been working on a monorepo the last couple of years and the git segement has been really really slow. But today I discovered this https://github.blog/2022-06-29-improve-git-monorepo-performance-with-a-file-system-monitor/

By enabling untrackedcache and fsmonitor the git segment execution moved from 160ms to 45ms. Which is pretty much the same results I had with gitstatus.

$ git config core.fsmonitor true
$ git config core.untrackedcache true

This would be much easier for people to use, it's straight from git and doesn't require us to use an interactive shell. So I think this is the correct approach to solve this.

@SuperSandro2000
Copy link
Contributor Author

I have untrackedcache enabled through feature.manyFiles since a long time but the fsmonitor is not available on linux and the alternative hook daemons I have tried before where all either not really faster or leaked hundreds of megabytes of RAM over a short amount of time per open shell.

 ➜ git fsmonitor--daemon
fatal: fsmonitor--daemon not supported on this platform

gitstatus is faster especially on large repositories with many changes.
This is an alternative to untracked cache and fsmonitor.
@brujoand
Copy link
Owner

Yeah, I’ve been using the fs-monitor for a while now on osx, and I’m not super happy with the performance.

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