Supressing output from completion script

[kevinoid]

While tracking down a syntax error in a bash completion script, I noticed that stdout and stderr are redirected to /dev/null for all completion scripts. It looks like this has been present since 20c05b4, and probably for good reason, but it seems like a big hammer which may be papering over many errors (such as the one I eventually tracked down to a syntax error, due to a missing ;). Since I hadn't seen it discussed before, I'm opening this issue to discuss what problems it may be solving and whether it's the best approach.

Thanks for considering,
Kevin

[scop]

That's a fair question. I think we should not mute everything, #508 has a candidate fix. Thoughts?

[kevinoid]

Looks great! Thanks for considering it and for working up a fix. I appreciate it, and I think it'll be a positive change.

It wouldn't surprise me if some third-party bash-completion scripts inadvertently produce output. When I get a chance, I'll see if I can work up a reasonable test to see how many bash-completion scripts in the Debian archive produce output. That might give us an idea of how much pain the change may cause and whether any triage is required first.

From a quick look through the archive, I found 3 packages which put their completion script in a subdirectory of /usr/share/bash-completion/completions/:

• crystal-lang - /usr/share/bash-completion/completions/crystal-lang/completion.bash
• libgadap-dev - /usr/share/bash-completion/completions/bash_completion.d/gadap-config
• trace-cmd - /usr/share/bash-completion/completions/trace-cmd/trace-cmd.bash

I assume all of these are buggy and will investigate and file bugs shortly.

[kevinoid]

Yep, they were all packaging bugs:

• #984955 for crystal-lang.
• #984957 for libgadap-dev.
• #984959 for trace-cmd.

I included a trivial patch for each. Hopefully they will be fixed quickly.

[kevinoid]

I ran some quick tests on bash-completion scripts in the Debian archive. I'm still analyzing the results, but the most prominent is scripts which print:

bash: have: command not found

to stderr when sourced. Scripts with this issue:

Package	Script
alltray	/usr/share/bash-completion/completions/alltray
apt-xapian-index	/usr/share/bash-completion/completions/axi-cache
bumblebee	/usr/share/bash-completion/completions/bumblebee
coccinelle	/usr/share/bash-completion/completions/spatch
debfoster	/usr/share/bash-completion/completions/debfoster
dosage	/usr/share/bash-completion/completions/dosage-completion
dpatch	/usr/share/bash-completion/completions/dpatch_edit_patch
dupload	/usr/share/bash-completion/completions/dupload
fcoe-utils	/usr/share/bash-completion/completions/fcoeadm
fcoe-utils	/usr/share/bash-completion/completions/fcoemon
freecontact	/usr/share/bash-completion/completions/freecontact.bash-completion
insserv	/usr/share/bash-completion/completions/insserv
jackd1	/usr/share/bash-completion/completions/jackd1
jackd2	/usr/share/bash-completion/completions/jackd
konwert	/usr/share/bash-completion/completions/konwert
linkchecker	/usr/share/bash-completion/completions/linkchecker-completion
lldpad	/usr/share/bash-completion/completions/lldpad
lldpad	/usr/share/bash-completion/completions/lldptool
lyx-common	/usr/share/bash-completion/completions/lyx
ngraph-gtk	/usr/share/bash-completion/completions/ngraph
nmh	/usr/share/bash-completion/completions/nmh
ocaml-findlib	/usr/share/bash-completion/completions/ocamlfind
ola	/usr/share/bash-completion/completions/ola
patool	/usr/share/bash-completion/completions/patool.bash-completion
pmount	/usr/share/bash-completion/completions/pmount
porg	/usr/share/bash-completion/completions/porg_bash_completion
primus	/usr/share/bash-completion/completions/primus
python3-pastescript	/usr/share/bash-completion/completions/paster3
rasmol	/usr/share/bash-completion/completions/rasmol
tgt	/usr/share/bash-completion/completions/tgt
tuxpaint	/usr/share/bash-completion/completions/tuxpaint-completion.bash
ubuntu-dev-tools	/usr/share/bash-completion/completions/pbuilder-dist
unar	/usr/share/bash-completion/completions/unar
vdr	/usr/share/bash-completion/completions/svdrpsend
vobsub2srt	/usr/share/bash-completion/completions/vobsub2srt
wbar	/usr/share/bash-completion/completions/wbar

I'm guessing these scripts were written/tested with eager sourcing (e.g. in /etc/bash_completion.d where the issue does not occur) when have is still defined and became broken when sourced-on-use after have has been unset (e.g. in /usr)?

So, be aware that this change will likely reveal broken completion scripts (which I think is a good thing).

For this case, do you think it would be worth defining have before sourcing completion scripts from /usr (and unsetting it after to avoid leaking have) as a workaround for these scripts? Or is it better to let them break and have developers/maintainers fix them?

[scop]

Ooh, awesome work, thanks!

I'm leaning towards letting those scripts break "loudly", I think this is a good thing too. If those scripts were defining their completions conditionally based on have return value, they haven't been defining them at all in quite some time.

But even if we don't provide a workaround here in the upstream project, it would be appropriate to provide maintainers a recipe/instructions for doing that "cleanly" (for some values of it). A crude example would be an /etc/profile.d script that gets sourced after our bash-completion.sh there, and just does have() { return 0; }.

Some of those completion file names will also not satisfy the rules for what gets dynamically loaded. Need to change them to use the command's basename or basename.bash, see __load_completion for details (underscore filename prefix is reserved for our internal deprecation use). Suspects:

• dosage-completion
• freecontact.bash-completion
• linkchecker-completion
• patool.bash-completion
• porg_bash_completion

I wonder how would I remember to add a prominent note to our release notes for maintainers about this. Maybe we should just not sit on this for too long but just push a release with it out Soon(tm).

[scop]

(Keeping open for a bit even though #508 was merged as a reminder to work on filing reports about the packaging issues.)

[scop]

Another option for the documented maintainer workaround would be to do the on-the-fly have definition/undefinition, but even though cleaner in a way, that'd require them to patch it in instead of just dropping a separate file out there.