| Message ID | 20231130202404.89791-3-stanhu@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | completion: refactor and support reftables backend | expand |
On Thu, Nov 30, 2023 at 12:24:04PM -0800, Stan Hu wrote: > In contrib/completion/git-completion.bash, there are a bunch of > instances where we read special refs like HEAD, MERGE_HEAD, > REVERT_HEAD, and others via the filesystem. However, the upcoming > reftable refs backend won't use '.git/HEAD' at all but instead will > write an invalid refname as placeholder for backwards compatibility, > which will break the git-completion script. > > Update the '__git_ref_exists' function to: > > 1. Recognize the placeholder '.git/HEAD' written by the reftable > backend (its content is specified in the reftable specs). > 2. If reftable is in use, use 'git rev-parse' to determine whether the > given ref exists. > 3. Otherwise, continue to use 'test -f' to check for the ref's filename. Nit, not worth a reroll: you already document this in the code, but I think it could help to also briefly explain why we're going through all of these hoops here instead of just using git-rev-parse(1) everywhere in the commit message. Patrick
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 9fbdc13f9a..f5b630ba99 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -122,12 +122,35 @@ __git () ${__git_dir:+--git-dir="$__git_dir"} "$@" 2>/dev/null } +# Helper function to read the first line of a file into a variable. +# __git_eread requires 2 arguments, the file path and the name of the +# variable, in that order. +# +# This is taken from git-prompt.sh. +__git_eread () +{ + test -r "$1" && IFS=$'\r\n' read -r "$2" <"$1" +} + # Runs git in $__git_repo_path to determine whether a ref exists. # 1: The ref to search __git_ref_exists () { local ref=$1 + # If the reftable is in use, we have to shell out to 'git rev-parse' + # to determine whether the ref exists instead of looking directly in + # the filesystem to determine whether the ref exists. Otherwise, use + # Bash builtins since executing Git commands are expensive on some + # platforms. + if __git_eread "$__git_repo_path/HEAD" head; then + b="${head#ref: }" + if [ "$b" == "refs/heads/.invalid" ]; then + __git -C "$__git_repo_path" rev-parse --verify --quiet "$ref" 2>/dev/null + return $? + fi + fi + [ -f "$__git_repo_path/$ref" ] }
In contrib/completion/git-completion.bash, there are a bunch of instances where we read special refs like HEAD, MERGE_HEAD, REVERT_HEAD, and others via the filesystem. However, the upcoming reftable refs backend won't use '.git/HEAD' at all but instead will write an invalid refname as placeholder for backwards compatibility, which will break the git-completion script. Update the '__git_ref_exists' function to: 1. Recognize the placeholder '.git/HEAD' written by the reftable backend (its content is specified in the reftable specs). 2. If reftable is in use, use 'git rev-parse' to determine whether the given ref exists. 3. Otherwise, continue to use 'test -f' to check for the ref's filename. Reviewed-by: Patrick Steinhardt <ps@pks.im> Reviewed-by: Christian Couder <christian.couder@gmail.com> Signed-off-by: Stan Hu <stanhu@gmail.com> --- contrib/completion/git-completion.bash | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)