search

jaraco / keyring

MIT License
1.24k stars 152 forks source link

homebrew installs invalid keyring completion file, causing "usage: install" messages at shell startup #671

Closed actualben closed 4 months ago

actualben commented 5 months ago

Describe the bug Homebrew seems to be installing an invalid completion file in /opt/homebrew/etc/bash_completion.d/keyring. These files are sourced at shell startup but the one homebrew is installing is not valid shell code. I'm not sure if the problem is here or in the homebrew package - the recent keyring package release on homebrew doesn't seem like the kind of change that could cause this, but I'm not sure.

$ ls -al /opt/homebrew/etc/bash_completion.d/keyring 
lrwxr-xr-x  1 user  admin    57B Mar 25 08:44 /opt/homebrew/etc/bash_completion.d/keyring@ -> ../../Cellar/keyring/25.0.0/etc/bash_completion.d/keyring

$ cat /opt/homebrew/etc/bash_completion.d/keyring 
Install keyring[completion] for completion support.

As a result of this when I open a new interactive shell session, I see:

$ bash -l
usage: install [-bCcpSsv] [-B suffix] [-f flags] [-g group] [-m mode]
               [-o owner] file1 file2
       install [-bCcpSsv] [-B suffix] [-f flags] [-g group] [-m mode]
               [-o owner] file1 ... fileN directory
       install -d [-v] [-g group] [-m mode] [-o owner] directory ...

Because the bash_completion.d/keyring launches /usr/bin/install due to the case-insensitive filesystem nonsense on macOS.

To Reproduce Steps to reproduce the behavior:

  1. brew install bash bash-completion keyring
  2. cat $(brew --prefix)/etc/bash_completion.d/keyring
  3. Look for not-shell-funciton output

Expected behavior A valid shell completion function definition should be in bash_completion.d/keyring or no file should be installed here

Environment

Additional context

==> bash: stable 5.2.26 (bottled), HEAD
/opt/homebrew/Cellar/bash/5.2.26 (162 files, 12.4MB) *
  Poured from bottle using the formulae.brew.sh API on 2024-01-15 at 02:14:06

==> bash-completion: stable 1.3 (bottled)
/opt/homebrew/Cellar/bash-completion/1.3_3 (189 files, 608.4KB) *
  Poured from bottle using the formulae.brew.sh API on 2023-03-13 at 07:50:32

==> keyring: stable 25.0.0 (bottled)
/opt/homebrew/Cellar/keyring/25.0.0 (90 files, 436.9KB) *
  Poured from bottle using the formulae.brew.sh API on 2024-03-25 at 08:44:14

Here's a trace of my shell startup showing the issue:

$ echo 'exit' | bash -lxe 2>&1 | grep -A 10 -B 10 'install'
--
+++++ unset -v have
+++++ PATH=/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/usr/libexec:/sbin:/usr/sbin:/usr/local/sbin
+++++ type k3b
++++ for i in $(LC_ALL=C command ls "$BASH_COMPLETION_COMPAT_DIR")
++++ i=/opt/homebrew/etc/bash_completion.d/keyring
++++ [[ keyring != @(*~|*.bak|*.swp|\#*\#|*.dpkg*|*.rpm@(orig|new|save)|Makefile*) ]]
++++ [[ -f /opt/homebrew/etc/bash_completion.d/keyring ]]
++++ [[ -r /opt/homebrew/etc/bash_completion.d/keyring ]]
++++ . /opt/homebrew/etc/bash_completion.d/keyring
+++++ Install 'keyring[completion]' for completion support.
usage: install [-bCcpSsv] [-B suffix] [-f flags] [-g group] [-m mode]
               [-o owner] file1 file2
       install [-bCcpSsv] [-B suffix] [-f flags] [-g group] [-m mode]
               [-o owner] file1 ... fileN directory
       install -d [-v] [-g group] [-m mode] [-o owner] directory ...
++++ for i in $(LC_ALL=C command ls "$BASH_COMPLETION_COMPAT_DIR")
++++ i=/opt/homebrew/etc/bash_completion.d/kldload
++++ [[ kldload != @(*~|*.bak|*.swp|\#*\#|*.dpkg*|*.rpm@(orig|new|save)|Makefile*) ]]
++++ [[ -f /opt/homebrew/etc/bash_completion.d/kldload ]]
++++ [[ -r /opt/homebrew/etc/bash_completion.d/kldload ]]
++++ . /opt/homebrew/etc/bash_completion.d/kldload
+++++ '[' Darwin = FreeBSD ']'
++++ for i in $(LC_ALL=C command ls "$BASH_COMPLETION_COMPAT_DIR")
++++ i=/opt/homebrew/etc/bash_completion.d/larch
++++ [[ larch != @(*~|*.bak|*.swp|\#*\#|*.dpkg*|*.rpm@(orig|new|save)|Makefile*) ]]
--
charlievieth commented 5 months ago

Looks like the issue is that the output of keyring --print-completion bash (which brew uses for completion) is invalid:

Install keyring[completion] for completion support.
actualben commented 5 months ago

Homebrew has now switched to shtab for keyring completions so a brew upgrade should address the symptoms for homebrew users.

jaraco commented 4 months ago

Looks like the issue is that the output of keyring --print-completion bash (which brew uses for completion) is invalid:

Install keyring[completion] for completion support.

And that error message is indicating the problem. When running keyring without the completion extra, it will emit that message.

 ~ @ pip-run keyring -- -m keyring --print-completion bash
Install keyring[completion] for completion support.

When running with the extra, it emits the completion logic:

 ~ @ pip-run 'keyring[completion]' -- -m keyring --print-completion bash | head -n 5
# AUTOMATICALLY GENERATED by `shtab`

_shtab___main___py_option_strings=('-h' '--help' '-p' '--keyring-path' '-b' '--keyring-backend' '--list-backends' '--disable' '--print-completion')

It sounds like the homebrew recipe just needed to include the completion extra in its install. Since the only package installed by the completion extra is shtab, the presence or absence of that package will determine whether the completion generation completes or not.

I do wonder if perhaps keyring should exit with a non-zero exit code here when the completion dependencies aren't present.

Would that have helped catch this issue earlier (prior to release)?

actualben commented 4 months ago

Yeah I think non-zero exit makes sense and also having the error message go to sys.stderr instead of stdout may prevent this kind of thing.

jaraco commented 4 months ago

Fixed in v25.0.1.