When git and yadm are both installed via brew, the yadm bash-completion script clobbers the git completion

[davidpfarrell]

Describe the bug

When git and yadm are both installed via brew, the yadm bash-completion script clobbers the git completion.

To reproduce

Can this be reproduced with the yadm/testbed docker image: [Yes/No]

Steps to reproduce the behavior:

Invoke bash completion when git is installed and yadm is not installed:

$ . /usr/local/etc/profile.d/bash_completion

$ complete -p git

complete -o bashdefault -o default -o nospace -F __git_wrap__git_main git

Invoke bash completion when both git and yadm are installed

$ . /usr/local/etc/profile.d/bash_completion

$ complete -p git

complete -F _minimal git

Expected behavior

Expect git completion to be unchanged

Environment

• Operating system: MacOS BigSur 11.1
• Version yadm: 3.0.2
• Version Git: 2.30.0

Additional context

The following logic in the yadm bash completion script seems faulty:

# test if git completion is missing, but loader exists, attempt to load
if ! declare -F _git > /dev/null && declare -F _completion_loader > /dev/null; then
  _completion_loader git
fi

# only operate if git completion is present
if declare -F _git > /dev/null; then

When the _git function is not found, the invoked _completion_loader git ends up clobbering the existing completion and replacing it with a minimal completion.

The official git bash completion script doesn't appear to create any functions named _git. It does, however, create __git:

# Runs git with all the options given as argument, respecting any
# '--git-dir=<path>' and '-C <path>' options present on the command line
__git ()
{
	git ${__git_C_args:+"${__git_C_args[@]}"} \
		${__git_dir:+--git-dir="$__git_dir"} "$@" 2>/dev/null
}

I am not sure if this is the method it was intended to check for/call?

A couple of side notes:

• There is a more-proper technique for checking if a completion is configured:

$ complete -p git &>/dev/null || echo "NOT CONFIGURED"

• There may be better techniques for utilizing the git completions within your completions. I'm not skilled enough in the workings of the complete command to say for sure, but commands like sudo and others have the ability to delegate to sub-command completions, so there may be some techniques there that could be used.

[rra]

This same issue caused https://bugs.debian.org/980754 (bash completion stops working with the latest version of Git) for the same reason (_git no longer exists). The upstream Git commit that removed that code is git/git@441ecda

The following hack of a patch seems to make things work again, but it's kind of messy. I'm not sure if there's a better way.

--- yadm.bash.orig      2021-01-31 12:49:38.002713695 -0800
+++ yadm.bash   2021-01-31 12:46:10.194337545 -0800
@@ -1,10 +1,10 @@
 # test if git completion is missing, but loader exists, attempt to load
-if ! declare -F _git > /dev/null && declare -F _completion_loader > /dev/null; then
+if ! declare -F _git > /dev/null && ! declare -F __git > /dev/null && declare -F _completion_loader > /dev/null; then
   _completion_loader git
 fi
 
 # only operate if git completion is present
-if declare -F _git > /dev/null; then
+if declare -F _git > /dev/null || declare -F __git > /dev/null; then
 
   _yadm() {
 
@@ -66,7 +66,11 @@
     if [[ " ${yadm_switches[*]} " != *" $penultimate "* ]]; then
       # TODO: somehow solve the problem with [--yadm-xxx option] being
       #       incompatible with what git expects, namely [--arg=option]
-      _git
+      if declare -F _git > /dev/null; then
+        _git
+      else
+        __git_wrap__git_main
+      fi
     fi
     if [[ "$current" =~ ^- ]]; then
       local matching

[TheLocehiliosan]

@davidpfarrell Thanks for reporting these issues. @rra Thanks for the suggested solutions. I will likely do something close to that patch, utilizing _git when it exists, otherwise adopting the function _git was using.

[TheLocehiliosan]

Incidentally, I only have the trouble of yadm completions being broken with the combination of brew installed yadm and git (3.0.2/2.30.0). Not and difference in how Git completions were defined. But I'm also not using BigSur, maybe that makes a difference.

[TheLocehiliosan]

A fix for this problem has been pushed to branch fix/318 c4327d0. I have tested this with the completion from Git 2.29.2 & 2.30.0. I've tested it on MacOS & Linux (Fedora). If others could confirm this works for them, it would be great.

[rra]

Confirmed this works for me on Debian unstable. Thanks!

[davidpfarrell]

Copied the file into my /usr/local/etc/bash_completion.d to test and appears to work as expected.

The main thing that finding the correct function name does for me is that it avoids the call to _completion_loader git which is what breaks the already-present completion.

Are you sure that, in general (i.e outside of the brew git / brew yadm install), _completion_loader git should work?

I'm just not sure if the bug is one of :

• git's completion doesn't work with the dynamic completion loader (in the brew context)
  • i.e Either brew's bash_completion or brew's git-completion (or both) are mis-configured
• git's completion doesn't work with the dynamic completion loader (in any context)
  • i.e YADM shouldn't expect this to work
• _completion_loader doesn't expect to be invoked if the completion is already defined
  • A cursory look suggests the function does not check to see if a completion is already defined
  • (At least in brew context), the function does not discover the git completion

Anyway, as-is, the fix prevents my git completions from being clobbered - so thats great, thanks !

[TheLocehiliosan]

This change has been merged into the develop branch and will be part of the next release.