fix: load from more new style completion dirs

[scop]

This might not be spot on off the bat, but it restores some of the way we used to load completions from.

In particular, before this, completions-core and completions-fallback from the bash_completion location were not attempted, which is a regression at least for my workflow: I rely on being able to modify files in my bash-completion working dir and have them take precendence over any system installed ones.

[scop]

The result of this is that we end up loading completions without the .bash suffix from more dirs we did before this change. We may want to tighten that up some.

Could check if the dir ends with -core or -fallback and load only .bash for them. That isn't 100% correct in the case if someone has for whatever reason configured their BASH_COMPLETION_USER_DIR to have one of those suffixes (we'd want to load both with and without .bash from there for backwards compatibility).

[akinomyoga]

I agree to check the suffix -{core,fallback} (probably inside _comp_load__visit_file).

[akinomyoga]

Thank you for creating the PR! This is exactly the issue that I described in #1557 (comment).

When using the new version, I also noticed a search-order problem of _comp_load when the system bash-completion < 2.18 and the user's bash-completion >= 2.18 coexist. I'll later describe it. I would like to discuss it and fix issues of _comp_load including it.

I was naively thinking of swapping the directory precedence of 2) and 3), and inserting the search for completions-core before 4), but I haven't yet considered it deeply enough.

[akinomyoga]

BASH_COMPLETION_USER_DIR is supposed to contain a path like ~/.local/share/bash-completion, so we need to suffix /completion to them too. I think we can use _comp_compgen. For example, something like this: _comp_compgen -Rv dirs -F : -- -W '$BASH_COMPLETION_USER_DIR' -S /completions, though I haven't tested this carefully.

[scop]

I haven't thought of this too deeply, but I think we do not need to do that. This dir is a single directory to load completions from, solely under the user's control, I don't think we need to add any more structure or segments to it. Adding would also be a breaking change.

[scop]

Hm README.md seems to disagree with me, I misremembered. $BASH_COMPLETION_USER_DIR is by the way not documented in doc/configuration.md, only its $XDG_CONFIG_HOME/bash_completion fallback is. We should fix that.

[akinomyoga]

Should we search the system completions-{core,fallback} after none were found in the in-tree completions-{core,fallback} (i.e., $_comp__base_directory/completions-{core,fallback} in the tree of the current bash_completion)? I think all default/fallback completions should already be found in the in-tree completions-{core,fallback}. Are there cases where the completion file is not found in the in-tree completions-{core,fallback} but found in the system completions-{core,fallback}?

I also think loading external completions-{core,fallback} would cause the compatibility problem with the internal API. We've been assuming that we can use the latest internal APIs defined in bash_completion inside the in-tree completions of the same version. When the in-tree completions-{core,fallback} doesn't contain the completion for a command while the system completions-{core,fallback} contains one, I think that implies the system bash-completion is newer than the current bash-completion. In this case, we should avoid loading the system completions-{core,fallback} because it is likely to contain uses of new internal APIs.

In short, I think the completions in completions-{core,fallback} are tightly bound, in the sense of internal API, to the exact version of bash_completion in the same tree. It's not safe to load completions-{core,fallback} outside the tree.

[scop]

Yep, agreed.

[akinomyoga]

For the same reason, I think we don't want to load completions-{core,fallback} in the bin directories.

Also, as I mentioned above, I would like to suggest exchanging the ordering of 2) and 3).

[akinomyoga]

Another issue is that when, the current bash_completion's tree ($_comp__base_directory) is different from the system's one (/usr/share/bash-completion), with the suggested search ordering, external completions installed in the system directory /usr/share/bash-completion/completions/<cmd>.bash will be masked by our completions in completions-{core,fallback}. This implies that the way to override bash-completion's in-tree completion is missing, even if the upstream decides to start hosting the completion file.

• I think completions-fallback should at least be checked at the very last step as before this PR.

• We should also be aware of the version time lag of the move of the file completions-{core => fallback}; even if we move a completion file to a fallback directory on request from the upstream, our in-tree completion continues to take precedence until the bash-completion version in the market is entirely replaced (which takes several to ten years).

  For this reason, I think the search ordering that we currently discuss in this PR should be a temporary one; when new bash-completion versions (having in-tree completions in completions-{core,fallback}) are available in distributions, I think we should move back the search order of the in-tree completions-core after 1)..4) directories.

[akinomyoga]

• For this reason, I think the search ordering that we currently discuss in this PR should be a temporary one; when new bash-completion versions (having in-tree completions in completions-{core,fallback}) are available in distributions, I think we should move back the search order of the in-tree completions-core after 1)..4) directories.

Or, maybe we can keep the current master, and set something like XDG_DATA_DIRS=/dev/null for testing purposes.

Or, another possibility is to search i) 1-4 with .bash, ii) in-tree completions-core, iii) 1-4 without suffix, iv) completions-fallback. The biggest problem currently is that completions in /usr/share/bash-completion/completions/<cmd> provided by the system bash-completion package have higher precedence than the in-tree completions. In the current search ordering, we need to wait until distributed versions of bash-completion are updated to be the ones with the new directory structure. Loading of old completions provided by the system bash-completion package can be avoided by checking the .bash suffix. That's the idea.

[akinomyoga]

Or, another possibility is to search i) 1-4 with .bash, ii) in-tree completions-core, iii) 1-4 without suffix, iv) completions-fallback.

Currently, I like this idea, though I need to think about it more later. It seems to solve the main issues cleanly with the minimum change.

diff --git a/bash_completion b/bash_completion
index e67cc3d51..d9c1d1cce 100644
--- a/bash_completion
+++ b/bash_completion
@@ -3508,15 +3508,17 @@ _comp_load()
     shift
     local -a source_args=("$@")

-    local i
-    for i in "${!dirs[@]}"; do
-        dir=${dirs[i]}/completions
-        [[ -d $dir ]] || continue
-        for compfile in "$cmdname.bash" "$cmdname"; do
-            _comp_load__visit_file "$dir/$compfile" && return 0
-        done
+    local dir
+    for dir in "${dirs[@]}"; do
+        _comp_load__visit_file "$dir/completions/$cmdname.bash" && return 0
     done
+
     _comp_load__visit_file "$_comp__base_directory/completions-core/$cmdname.bash" && return 0
+
+    for dir in "${dirs[@]}"; do
+        _comp_load__visit_file "$dir/completions/$cmdname" && return 0
+    done
+
     _comp_load__visit_file "$_comp__base_directory/completions-fallback/$cmdname.bash" && return 0

     # Look up simple "xspec" completions

[scop]

Currently, I like this idea, though I need to think about it more later. It seems to solve the main issues cleanly with the minimum change.

I like how it looks too, but it contains a behavioral change: *.bash from the base completions-core now has higher precedence than plain non-*.bash from other dirs such as the user specified one. Strictly speaking that sounds like a breaking change.

[akinomyoga]

Currently, I like this idea, though I need to think about it more later. It seems to solve the main issues cleanly with the minimum change.

I like how it looks too, but it contains a behavioral change: *.bash from the base completions-core now has higher precedence than plain non-*.bash from other dirs such as the user specified one. Strictly speaking that sounds like a breaking change.

That is a good point.

Then, how about using different rules for the user-specified completion files and the package-provided completion files?

1. We first look for <cmd>.bash and <cmd> in the user directories (specified through $BASH_COMPLETION_USER_DIR).
2. Then, we look for completions/<cmd>.bash in system directories,
3. the in-tree completions-core/<cmd>.bash,
4. completions/<cmd> in system directories, and
5. the in-tree completions-fallback/<cmd>.bash, in order.

Example Implementation: bc5dfff...4366ead

Rationale for checking completion/<cmd> in system directories after the in-tree completions-core/<cmd>.bash

I think we don't have to care about the completion file <cmd> provided by the upstream <cmd> project. While users may put their custom version at $BASH_COMPLETION_USER_DIR/completions/<cmd> to override the one provided by bash-completion or the upstream <cmd> project, I think the upstream <cmd> project has been supposed to notify us that they start to ship the completion file so that we rename the file { => _}<cmd>.

• When we had completions/<cmd> before the directory structure change, while the upstream provided its completion at completions/<cmd> in a different install prefix, the situation is unchanged. Our in-tree completions/<cmd> had higher precedence than the upstream one, so it isn't a new problem that our completions-core/<cmd>.bash masks the upstream completions/<cmd> in the new directory structure.
• When we renamed the in-tree completion to completions/_<cmd> for the upstream completions/<cmd>, the problem doesn't arise. That's because we moved completions/_<cmd> to completions-fallback/<cmd>.bash, and the upstream completions/<cmd> still has higher precedence than completions-fallback.
• When the upstream <cmd> project starts to provide their completion file in the future, they can install it at completions/<cmd>.bash with a suffix .bash. Then, it has higher precedence than the in-tree completions-core/<cmd>.bash in the new directory structure.
• There might still be a project newly installing a completion file at completions/<cmd>, but it causes a conflict even with the old directory structure, or it is masked by our in-tree completion with the old directory structure.