On Thu, Oct 08, 2009 at 08:02:06AM -0700, Shawn O. Pearce wrote: > Kirill Smelkov <kirr@xxxxxxxxxx> wrote: > > diff --git a/contrib/completion/Makefile b/contrib/completion/Makefile > > new file mode 100644 > > index 0000000..a0fbb66 > > --- /dev/null > > +++ b/contrib/completion/Makefile > > @@ -0,0 +1,11 @@ > > +all : git-completion.bash > > + > > + > > +git-completion.bash: git-completion.bash.in git-completion.bash.generate > > + # Generate completions for binaries we have just built > > + PATH="$(shell pwd)/..:$$PATH" ./git-completion.bash.generate > > Is only one .. enough? Isn't that putting us into the contrib > directory, and therefore not finding the 'git' we just compiled? Fixed, thanks. > I'm also concerned that git-completion.bash.generate requires > bash to compile the completion for bash. IMHO, if we are building > this code at compile time we shouldn't assume bash is available. > What if this is a sandboxed build environment using another shell > and /bin/bash isn't installed? > > I think the git-completion.bash.generate code needs to be a bit > more sh agnostic than the completion routines themselves are. I've reworked it not to depend on bash in 2nd patch. > > +# pregenerated stuff (to save load time) > > +__git_merge_strategylist=__GIT_MERGE_STRATEGYLIST > > +__git_all_commandlist=__GIT_ALL_COMMANDLIST > > +__git_porcelain_commandlist=__GIT_PORCELAIN_COMMANDLIST > > This also makes testing the completion a bit more difficult, now > we have to build it before we can load it, making the testing cycle > actually be: > > make && . git-completion.bash > > We probably should place a quick comment here to remind folks that > they need to build the script in order to test it properly. I've added some sort of protection, so that git-completion.bash.in can't be sourced at all. Is it ok? (interdiff for patch 1) diff --git a/contrib/completion/Makefile b/contrib/completion/Makefile index a0fbb66..90aa225 100644 --- a/contrib/completion/Makefile +++ b/contrib/completion/Makefile @@ -3,7 +3,7 @@ all : git-completion.bash git-completion.bash: git-completion.bash.in git-completion.bash.generate # Generate completions for binaries we have just built - PATH="$(shell pwd)/..:$$PATH" ./git-completion.bash.generate + PATH="$(shell pwd)/../..:$$PATH" ./git-completion.bash.generate clean: diff --git a/contrib/completion/git-completion.bash.in b/contrib/completion/git-completion.bash.in index cf1b5fd..e1ab612 100644 --- a/contrib/completion/git-completion.bash.in +++ b/contrib/completion/git-completion.bash.in @@ -54,6 +54,21 @@ # git@xxxxxxxxxxxxxxx # + +# pregenerated stuff (to save load time) +__git_merge_strategylist=__GIT_MERGE_STRATEGYLIST +__git_all_commandlist=__GIT_ALL_COMMANDLIST +__git_porcelain_commandlist=__GIT_PORCELAIN_COMMANDLIST + +# remind folks that git-completion.bash.in can't be sourced +case "$__git_merge_strategylist" in +__GIT*) + echo "E: git-completion.bash.in can't be sourced" + return 1 ;; +esac + + + case "$COMP_WORDBREAKS" in *:*) : great ;; *) COMP_WORDBREAKS="$COMP_WORDBREAKS:" @@ -79,12 +94,6 @@ __gitdir () } -# pregenerated stuff (to save load time) -__git_merge_strategylist=__GIT_MERGE_STRATEGYLIST -__git_all_commandlist=__GIT_ALL_COMMANDLIST -__git_porcelain_commandlist=__GIT_PORCELAIN_COMMANDLIST - - # __git_ps1 accepts 0 or 1 arguments (i.e., format string) # returns text to add to bash PS1 prompt (includes branch name) Here are new patches themselves: ---- 8< ---- From: Kirill Smelkov <kirr@xxxxxxxxxx> Date: Mon, 5 Oct 2009 13:36:15 +0400 Subject: [PATCH v3 1/2] Speedup bash completion loading On my slow laptop (P3 700MHz), system-wide bash completions take too much time to load (> 1s), and significant fraction of this time is spent loading git-completion.bash: $ time bash -c '. git-completion.bash' # before this patch real 0m0.317s user 0m0.250s sys 0m0.060s I've tracked down that the most time is spent warming up merge_strategy, all_command & porcelain_command caches. Initially I thought that since git is not used in each and every interactive xterm, it would be perfectly ok to load completion support with cold caches, and then load needed thing lazily. But for me this strategy turned out to be difficult to implement in simple and maintainable way -- bash does not provide a way to return values from inside functions, so one will have to use e.g. ${__git_all_commandlist:=$(__git_all_commands)} everywhere in place where $(__git_all_commands) we used before, so as also Ted Pavlic suggested let's pregenerate everything at build time so that we have nothing to compute at runtime when git-completion.bash script is loaded. The result is that loading completion is significantly faster now: $ time bash -c '. git-completion.bash' # after this patch real 0m0.068s user 0m0.060s sys 0m0.010s Cc: Ted Pavlic <ted@xxxxxxxxxxxxx> Signed-off-by: Kirill Smelkov <kirr@xxxxxxxxxx> --- contrib/completion/.gitignore | 1 + contrib/completion/Makefile | 11 ++ contrib/completion/git-completion.bash.generate | 128 ++++++++++++++++ ...{git-completion.bash => git-completion.bash.in} | 161 +++----------------- 4 files changed, 162 insertions(+), 139 deletions(-) create mode 100644 contrib/completion/.gitignore create mode 100644 contrib/completion/Makefile create mode 100755 contrib/completion/git-completion.bash.generate rename contrib/completion/{git-completion.bash => git-completion.bash.in} (90%) mode change 100755 => 100644 diff --git a/contrib/completion/.gitignore b/contrib/completion/.gitignore new file mode 100644 index 0000000..578e6a8 --- /dev/null +++ b/contrib/completion/.gitignore @@ -0,0 +1 @@ +git-completion.bash diff --git a/contrib/completion/Makefile b/contrib/completion/Makefile new file mode 100644 index 0000000..90aa225 --- /dev/null +++ b/contrib/completion/Makefile @@ -0,0 +1,11 @@ +all : git-completion.bash + + +git-completion.bash: git-completion.bash.in git-completion.bash.generate + # Generate completions for binaries we have just built + PATH="$(shell pwd)/../..:$$PATH" ./git-completion.bash.generate + + +clean: + rm -f git-completion.bash + diff --git a/contrib/completion/git-completion.bash.generate b/contrib/completion/git-completion.bash.generate new file mode 100755 index 0000000..fa998dc --- /dev/null +++ b/contrib/completion/git-completion.bash.generate @@ -0,0 +1,128 @@ +#!/bin/bash +# +# Generate bash completion for git. +# +# Precompute everything that can be known in advance at build time, so that +# actual bash completion script is loaded faster. + +__git_merge_strategies () +{ + git merge -s help 2>&1 | + sed -n -e '/[Aa]vailable strategies are: /,/^$/{ + s/\.$// + s/.*:// + s/^[ ]*// + s/[ ]*$// + p + }' +} + +__git_all_commands () +{ + local i IFS=" "$'\n' + for i in $(git help -a|egrep '^ ') + do + case $i in + *--*) : helper pattern;; + *) echo $i;; + esac + done +} + + +__git_porcelain_commands () +{ + local i IFS=" "$'\n' + for i in "help" $(__git_all_commands) + do + case $i in + *--*) : helper pattern;; + applymbox) : ask gittus;; + applypatch) : ask gittus;; + archimport) : import;; + cat-file) : plumbing;; + check-attr) : plumbing;; + check-ref-format) : plumbing;; + checkout-index) : plumbing;; + commit-tree) : plumbing;; + count-objects) : infrequent;; + cvsexportcommit) : export;; + cvsimport) : import;; + cvsserver) : daemon;; + daemon) : daemon;; + diff-files) : plumbing;; + diff-index) : plumbing;; + diff-tree) : plumbing;; + fast-import) : import;; + fast-export) : export;; + fsck-objects) : plumbing;; + fetch-pack) : plumbing;; + fmt-merge-msg) : plumbing;; + for-each-ref) : plumbing;; + hash-object) : plumbing;; + http-*) : transport;; + index-pack) : plumbing;; + init-db) : deprecated;; + local-fetch) : plumbing;; + lost-found) : infrequent;; + ls-files) : plumbing;; + ls-remote) : plumbing;; + ls-tree) : plumbing;; + mailinfo) : plumbing;; + mailsplit) : plumbing;; + merge-*) : plumbing;; + mktree) : plumbing;; + mktag) : plumbing;; + pack-objects) : plumbing;; + pack-redundant) : plumbing;; + pack-refs) : plumbing;; + parse-remote) : plumbing;; + patch-id) : plumbing;; + peek-remote) : plumbing;; + prune) : plumbing;; + prune-packed) : plumbing;; + quiltimport) : import;; + read-tree) : plumbing;; + receive-pack) : plumbing;; + reflog) : plumbing;; + repo-config) : deprecated;; + rerere) : plumbing;; + rev-list) : plumbing;; + rev-parse) : plumbing;; + runstatus) : plumbing;; + sh-setup) : internal;; + shell) : daemon;; + show-ref) : plumbing;; + send-pack) : plumbing;; + show-index) : plumbing;; + ssh-*) : transport;; + stripspace) : plumbing;; + symbolic-ref) : plumbing;; + tar-tree) : deprecated;; + unpack-file) : plumbing;; + unpack-objects) : plumbing;; + update-index) : plumbing;; + update-ref) : plumbing;; + update-server-info) : daemon;; + upload-archive) : plumbing;; + upload-pack) : plumbing;; + write-tree) : plumbing;; + var) : infrequent;; + verify-pack) : infrequent;; + verify-tag) : plumbing;; + *) echo $i;; + esac + done +} + + +__git_merge_strategylist=$(__git_merge_strategies | tr '\n' ' ') +__git_all_commandlist="$(__git_all_commands | tr '\n' ' ')" +__git_porcelain_commandlist="$(__git_porcelain_commands | tr '\n' ' ')" + + +sed -e "s/__GIT_MERGE_STRATEGYLIST/\"$__git_merge_strategylist\"/" \ + -e "s/__GIT_ALL_COMMANDLIST/\"$__git_all_commandlist\"/" \ + -e "s/__GIT_PORCELAIN_COMMANDLIST/\"$__git_porcelain_commandlist\"/" \ + git-completion.bash.in > git-completion.bash + diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash.in old mode 100755 new mode 100644 similarity index 90% rename from contrib/completion/git-completion.bash rename to contrib/completion/git-completion.bash.in index 88b1b3c..67d03c3 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash.in @@ -21,13 +21,7 @@ # 2) Added the following line to your .bashrc: # source ~/.git-completion.sh # -# 3) You may want to make sure the git executable is available -# in your PATH before this script is sourced, as some caching -# is performed while the script loads. If git isn't found -# at source time then all lookups will be done on demand, -# which may be slightly slower. -# -# 4) Consider changing your PS1 to also show the current branch: +# 3) Consider changing your PS1 to also show the current branch: # PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ ' # # The argument to __git_ps1 will be displayed only if you @@ -60,6 +54,21 @@ # git@xxxxxxxxxxxxxxx # + +# pregenerated stuff (to save load time) +__git_merge_strategylist=__GIT_MERGE_STRATEGYLIST +__git_all_commandlist=__GIT_ALL_COMMANDLIST +__git_porcelain_commandlist=__GIT_PORCELAIN_COMMANDLIST + +# remind folks that git-completion.bash.in can't be sourced +case "$__git_merge_strategylist" in +__GIT*) + echo "E: git-completion.bash.in can't be sourced" + return 1 ;; +esac + + + case "$COMP_WORDBREAKS" in *:*) : great ;; *) COMP_WORDBREAKS="$COMP_WORDBREAKS:" @@ -324,23 +333,6 @@ __git_remotes () done } -__git_merge_strategies () -{ - if [ -n "${__git_merge_strategylist-}" ]; then - echo "$__git_merge_strategylist" - return - fi - git merge -s help 2>&1 | - sed -n -e '/[Aa]vailable strategies are: /,/^$/{ - s/\.$// - s/.*:// - s/^[ ]*// - s/[ ]*$// - p - }' -} -__git_merge_strategylist= -__git_merge_strategylist=$(__git_merge_strategies 2>/dev/null) __git_complete_file () { @@ -476,128 +468,19 @@ __git_complete_strategy () { case "${COMP_WORDS[COMP_CWORD-1]}" in -s|--strategy) - __gitcomp "$(__git_merge_strategies)" + __gitcomp "$__git_merge_strategylist" return 0 esac local cur="${COMP_WORDS[COMP_CWORD]}" case "$cur" in --strategy=*) - __gitcomp "$(__git_merge_strategies)" "" "${cur##--strategy=}" + __gitcomp "$__git_merge_strategylist" "" "${cur##--strategy=}" return 0 ;; esac return 1 } -__git_all_commands () -{ - if [ -n "${__git_all_commandlist-}" ]; then - echo "$__git_all_commandlist" - return - fi - local i IFS=" "$'\n' - for i in $(git help -a|egrep '^ ') - do - case $i in - *--*) : helper pattern;; - *) echo $i;; - esac - done -} -__git_all_commandlist= -__git_all_commandlist="$(__git_all_commands 2>/dev/null)" - -__git_porcelain_commands () -{ - if [ -n "${__git_porcelain_commandlist-}" ]; then - echo "$__git_porcelain_commandlist" - return - fi - local i IFS=" "$'\n' - for i in "help" $(__git_all_commands) - do - case $i in - *--*) : helper pattern;; - applymbox) : ask gittus;; - applypatch) : ask gittus;; - archimport) : import;; - cat-file) : plumbing;; - check-attr) : plumbing;; - check-ref-format) : plumbing;; - checkout-index) : plumbing;; - commit-tree) : plumbing;; - count-objects) : infrequent;; - cvsexportcommit) : export;; - cvsimport) : import;; - cvsserver) : daemon;; - daemon) : daemon;; - diff-files) : plumbing;; - diff-index) : plumbing;; - diff-tree) : plumbing;; - fast-import) : import;; - fast-export) : export;; - fsck-objects) : plumbing;; - fetch-pack) : plumbing;; - fmt-merge-msg) : plumbing;; - for-each-ref) : plumbing;; - hash-object) : plumbing;; - http-*) : transport;; - index-pack) : plumbing;; - init-db) : deprecated;; - local-fetch) : plumbing;; - lost-found) : infrequent;; - ls-files) : plumbing;; - ls-remote) : plumbing;; - ls-tree) : plumbing;; - mailinfo) : plumbing;; - mailsplit) : plumbing;; - merge-*) : plumbing;; - mktree) : plumbing;; - mktag) : plumbing;; - pack-objects) : plumbing;; - pack-redundant) : plumbing;; - pack-refs) : plumbing;; - parse-remote) : plumbing;; - patch-id) : plumbing;; - peek-remote) : plumbing;; - prune) : plumbing;; - prune-packed) : plumbing;; - quiltimport) : import;; - read-tree) : plumbing;; - receive-pack) : plumbing;; - reflog) : plumbing;; - repo-config) : deprecated;; - rerere) : plumbing;; - rev-list) : plumbing;; - rev-parse) : plumbing;; - runstatus) : plumbing;; - sh-setup) : internal;; - shell) : daemon;; - show-ref) : plumbing;; - send-pack) : plumbing;; - show-index) : plumbing;; - ssh-*) : transport;; - stripspace) : plumbing;; - symbolic-ref) : plumbing;; - tar-tree) : deprecated;; - unpack-file) : plumbing;; - unpack-objects) : plumbing;; - update-index) : plumbing;; - update-ref) : plumbing;; - update-server-info) : daemon;; - upload-archive) : plumbing;; - upload-pack) : plumbing;; - write-tree) : plumbing;; - var) : infrequent;; - verify-pack) : infrequent;; - verify-tag) : plumbing;; - *) echo $i;; - esac - done -} -__git_porcelain_commandlist= -__git_porcelain_commandlist="$(__git_porcelain_commands 2>/dev/null)" - __git_aliases () { local i IFS=$'\n' @@ -1077,7 +960,7 @@ _git_help () return ;; esac - __gitcomp "$(__git_all_commands) + __gitcomp "$__git_all_commandlist attributes cli core-tutorial cvs-migration diffcore gitk glossary hooks ignore modules repository-layout tutorial tutorial-2 @@ -1423,7 +1306,7 @@ _git_config () return ;; pull.twohead|pull.octopus) - __gitcomp "$(__git_merge_strategies)" + __gitcomp "$__git_merge_strategylist" return ;; color.branch|color.diff|color.interactive|\ @@ -1524,7 +1407,7 @@ _git_config () pager.*) local pfx="${cur%.*}." cur="${cur#*.}" - __gitcomp "$(__git_all_commands)" "$pfx" "$cur" + __gitcomp "$__git_all_commandlist" "$pfx" "$cur" return ;; remote.*.*) @@ -2116,7 +1999,7 @@ _git () --help " ;; - *) __gitcomp "$(__git_porcelain_commands) $(__git_aliases)" ;; + *) __gitcomp "$__git_porcelain_commandlist $(__git_aliases)" ;; esac return fi -- 1.6.5.rc2.18.g84f98.dirty From: Kirill Smelkov <kirr@xxxxxxxxxx> Date: Fri, 9 Oct 2009 12:45:30 +0400 Subject: [PATCH 2/2] bash: make git-completion.bash.generate bash agnostic We've just moved some code from git-completion.bash into git-completion.bash.generate, but as Shawn O. Pearce notes, this code is now used at compile time, so we shouldn't assume bash is avalable. In more details: we used IFS=" "$'\n' which works in bash, but not e.g. in dash, but it turns out that we can avoid setting IFS at all, look: $ ./git help -a | egrep '^ ' [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS] add get-tar-commit-id rebase add--interactive grep rebase--interactive am gui receive-pack ... First, there are some unneedded lines, with e.g. [--bare], and this does produce noise, since e.g. in bash $ for i in "[--bare]"; do echo $i ; done a b so we kill it though grepping more explicitely: $ ./git help -a | egrep '^ [^ ]' add get-tar-commit-id rebase add--interactive grep rebase--interactive am gui receive-pack ... And then, plain "for in in $(git help -a|egrep '^ [^ ]')" works in both bash and dash, which was the goal. Signed-off-by: Kirill Smelkov <kirr@xxxxxxxxxx> --- contrib/completion/git-completion.bash.generate | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contrib/completion/git-completion.bash.generate b/contrib/completion/git-completion.bash.generate index fa998dc..04460eb 100755 --- a/contrib/completion/git-completion.bash.generate +++ b/contrib/completion/git-completion.bash.generate @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh # # Generate bash completion for git. # @@ -19,8 +19,8 @@ __git_merge_strategies () __git_all_commands () { - local i IFS=" "$'\n' - for i in $(git help -a|egrep '^ ') + local i + for i in $(git help -a|egrep '^ [^ ]') do case $i in *--*) : helper pattern;; @@ -32,7 +32,7 @@ __git_all_commands () __git_porcelain_commands () { - local i IFS=" "$'\n' + local i for i in "help" $(__git_all_commands) do case $i in -- 1.6.5.rc2.18.g84f98.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html