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

Timebox git status checks inside prompt_info #3725

Closed
wants to merge 1 commit into from

Conversation

apjanke
Copy link
Contributor

@apjanke apjanke commented Mar 26, 2015

UPDATE: Can't use this because it'll break any use of coproc by the user. Nevermind.

Summary

This adds a user-configurable timeout to the git status call done inside git_prompt_info() so the prompt doesn't hang when you are in a slow-to-check repo, such as a very large repo or one accessed over a network file system.

Fixes #3009.
Fixes #3706.
Fixes #3723.
Maybe #3351.
Fixes #1160.
Maybe #38.

When it times out, it'll include an explicit "?" indicator in the prompt to show that it doesn't know what the repo's status is, instead of spuriously reporting either clean or dirty.

Gives "robbyrussell" theme support for the timed-out status check, with a "?" indicator.

The current way to deal with slow repos is to set git config --add oh-my-zsh.hide-status 1 to turn off status display. This has a couple drawbacks:

  • It requires you to identify and manually configure each slow repo.
  • It affects all views of configured repos, regardless of how they're accessed. Some repos may only be slow when they're accessed via a remote filesystem, so you'd still want to see their status when viewed on a fast local filesystem.
  • It is indistinguishable from a clean repo in the prompt's output, so you have to remember that it's been set for that repo.

Implementation

Uses zsh's coproc and read -p -t to put a timeout on the git status call inside git_prompt_info().

Introduces new theme configuration variables:

  • $ZSH_THEME_GIT_PROMPT_TIMEDOUT - how the timeout appears in the theme's prompt
  • $ZSH_THEME_SCM_CHECK_TIMEOUT - how long the timeout is

I chose the generic "SCM" instead of "GIT" in $ZSH_THEME_SCM_CHECK_TIMEOUT so the same control could be reused for hg and other source control systems if they want to implement timeboxing, too.

Effects

When you're in a slow git repo using a theme that includes git prompt info, it'll give up and show a "?" or other theme-defined status indicator instead of a clean/dirty indicator. Fast repos (where git status is <1 second, by default) work just as they did before.

If you really want it to grind out a status for every prompt, you can just set the $ZSH_THEME_SCM_CHECK_TIMEOUT to a very large number in your ~/.zshrc.

Here's an example of what it looks like in the robbyrussell theme. When in a repo over a slow network connection (under /Volumes/janke), it times out and shows a "?". In a fast local repo (under ~), it shows the normal clean/dirty indicator.

git timeboxing in robbyrussell theme

Future enhancements

If this works out, it could be improved further by having git.zsh track which repos have been slow in this session, and skip future git status checks for them for the rest of the session, so the prompt would be instantaneous, instead of eating the 1-second timeout each time it's presented.

…g the zsh prompt.

Give "robbyrussell" them support for the timed-out check.

Uses zsh's `coproc` to put a timeout on the `git status` call inside git_prompt_info()
Introduces $ZSH_THEME_GIT_PROMPT_TIMEDOUT and $ZSH_THEME_SCM_CHECK_TIMEOUT theme configuration variables to control how long the timeout is and how the timed-out indicator appears inside the prompt.
@apjanke
Copy link
Contributor Author

apjanke commented Mar 26, 2015

Wait, crap: we can't use this because zsh can only have one coproc, so if the user is running their own coproc, this will break it. Nevermind; don't merge. I'll see if there's a different mechanism for doing an asynchronous git status call.

@apjanke apjanke closed this Mar 26, 2015
@mcornella
Copy link
Member

Dude you rock!

About the user coproc thing, I haven't read about it but isn't it impossible by default to have something run while the PROMPT is being updated?

I'm surely missing out something so don't reply if my question is stupid 😉. I'll catch up with coproc literature later this evening.

@apjanke
Copy link
Contributor Author

apjanke commented Mar 27, 2015

That's the thing about the coproc – it's a background process so it can stay running through multiple prompt presentations. Oops.

I think this approach could be modified to use a regular background process or job instead of the special coproc (which I think is really just a special named slot for a background job), in which case we'd be good to go.

@apjanke
Copy link
Contributor Author

apjanke commented Mar 27, 2015

And thanks!

@mcornella
Copy link
Member

Maybe a normal "job" works?
http://zsh.sourceforge.net/Doc/Release/Jobs-_0026-Signals.html

@apjanke
Copy link
Contributor Author

apjanke commented Mar 31, 2015

Okay: I was able to change the implementation to use a regular background process and FIFO instead of the coproc. Outline of the new FIFO-based implementation:

    local OMZ_TMPDIR=$TMPDIR/oh-my-zsh
    local GIT_FIFO=$OMZ_TMPDIR/omz-parse_git_dirty-git-status.$$
    mkfifo $GIT_FIFO
    command git status ${FLAGS} >$GIT_FIFO 2>/dev/null &
    # Use dummy "__unset__" to distinguish timeouts from empty output
    local STATUS=__unset__
    read -t $ZSH_THEME_SCM_CHECK_TIMEOUT STATUS <$GIT_FIFO
    rm $GIT_FIFO

This version should be safe to use inside prompt info and won't conflict with other process control stuff the way the coproc implementation did. I added the commits to my git-status-timebox branch, but they're not showing up here, maybe because this PR is closed. Going to test this on a couple more systems and then rebase and put in a new PR.

Want to give it a test?

@mcornella
Copy link
Member

Hi @apjanke, sorry for the delay. I just got to trying it out, and it's late so I can't say much about it. The only thing I can report is that the SYS_TMPDIR logic surprisingly didn't work on my Debian system, since neither $TMPDIR nor $TEMP are set.
Here's a quick fix for that:

commit 08320a07a49ffff6a2b67080250e674f5c9b6b29
Author: Marc Cornellà <marc.cornella@live.com>
Date:   Thu Apr 23 00:13:23 2015 +0200

    Use /tmp as fallback when neither $TMPDIR nor $TEMP are set

diff --git a/lib/git.zsh b/lib/git.zsh
index bf270a9..39a2448 100644
--- a/lib/git.zsh
+++ b/lib/git.zsh
@@ -25,7 +25,7 @@ function parse_git_dirty() {
 #  RETURN: 0 if git command completed, 1 if timed out or other error occurred
 function _git_status_timeboxed_oneline() {
   local OUT_VAR=$1
-  local SYS_TMPDIR=${TMPDIR:-$TEMP}
+  local SYS_TMPDIR=${${TMPDIR:-$TEMP}:-/tmp}
   local OMZ_TMPDIR=$SYS_TMPDIR/oh-my-zsh
   if [[ ! -d $OMZ_TMPDIR ]]; then
     if ! mkdir -p $OMZ_TMPDIR; then

I'll be using it these days to see how it behaves, and hopefully I can get some sort of benchmark going (I remember there's some zsh module to do that) and report back the improvements.

Thank you!

@apjanke
Copy link
Contributor Author

apjanke commented Apr 23, 2015

Thanks Marc.

Yep: I see the same behavior on my Debian box. I guess TMPDIR and TEMP aren't as universal as I thought.

I incorporated your patch in to my branch (along with a paranoid check to see if /tmp actually exists). Looks like it's working for me; I'll run it as well for a while.

(There's another change on the branch that speeds up the parsing for the full git_prompt_status(), but most themes do not use that one.)

This logic is starting to feel a little long. Maybe it would be better to use mktemp or one of the related temp file commands. I don't know how universal those are either; I'll look in to them. I was also hoping to implement this without having to issue many commands or create many files, since it does get called on every prompt. (Especially for Cygwin users, where command execution overhead is higher.) (That's why I'm using a FIFO: it should require just a directory entry, and no data actually going to disk.) I suppose that in most cases this would all happen inside disk cache since the files are very short-lived. But I don't want to assume too much about people's systems.

Open to suggestions on what the Right Way to pick temp files is here, especially if it would generalize to other other temp file uses in OMZ.

If you're benchmarking, note that this change isn't intended to speed up the normal git_prompt_dirty() case where git status completes and you get the clean/dirty info for your prompt. It just avoids you getting a hung or unusably slow shell in the case where you're in a slow repo. So the worst case should be 1 second now; and ordinary cases should be unchanged. Just watch out for if it accidentally made things slower.

@apjanke
Copy link
Contributor Author

apjanke commented Apr 23, 2015

I'm also open to suggestions on how to time-box the full git status call for git_prompt_status, where it uses the entire output instead of just the first line. It's tricky because you need to preserve the full status output, and need to timeout with respect to it being complete, not just some output appearing. There's no builtin for "wait for a process to complete, with a timeout" as far as I can tell.

The best I've come up with so far is:

  1. Run (STATUS_$$_$RANDOM=$(git status); echo DONE > $TEMPFILE) as a list in the background.
  2. Wait on $TEMPFILE with a timeout using read -t 1 <$TEMPFILE
  3. If it times out, parse the output of ps looking for that $RANDOM value to locate the git process and get its PID, and kill it. Doing it this way is necessary because $! only gives the PID of the last process in a backgrounded list. I can't figure out a way to directly get the PID of git when run with a completion-indication command following it.

@apjanke
Copy link
Contributor Author

apjanke commented Apr 23, 2015

(And once I get these working, I'm going to split out the timeboxing from the parsing optimizations if possible, rebase them, and put them in as new separate Pull Requests. Too many commits on this branch now.)

@apjanke
Copy link
Contributor Author

apjanke commented May 20, 2015

I redid this, squashed and rebased, as #3914.

@mcornella
Copy link
Member

Hi @apjanke, I recently came across this thread that points to a suggestion by Bart Schaefer to implement a "background process" prompt: olivierverdier/zsh-git-prompt#11 (comment). Maybe it applies here and we can push this further.

@apjanke
Copy link
Contributor Author

apjanke commented Aug 1, 2015

Interesting. That <<(...) syntax is new to me, and might be a better way of dealing with coproc-like interaction. I'll look in to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants