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.

[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]

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.

[scop]

That sounds mostly sane, but I think (offhand) that reading system locations before in-tree ones makes it harder to work on bash-completion as well as to replace the system installed one with a user one.

[scop]

#1579 is one example suffering from our current load dir precedence issues.

[akinomyoga]

That sounds mostly sane, but I think (offhand) that reading system locations before in-tree ones makes it harder to work on bash-completion

Could you explain it in a bit more detail?

I think that is the original intent of the present separation of the directory. The original change for the separation of in-tree completions into completions-core intended to allow a system-installed completion to override the in-tree completion; it previously required renaming the in-tree completion from <cmd> to _<cmd> to avoid the conflict on the path completions/<cmd>, but that doesn't work until bash-completion in the market is entirely replaced with the new version.

For the test suite of bash-completion, I think we already set BASH_COMPLETION_USER_DIR and XDG_DATA_DIR to disable users' and system completions. If it still loads the system-installed one intead of the in-tree completion, it is probably found by 3) (i.e., from the prefix where the command file exists).

as well as to replace the system installed one with a user one.

In my understanding, replacing the system-installed one with a user one is easy because the user-provided one is searched frist in the suggestion #1569 (comment).

#1579 is one example suffering from our current load dir precedence issues.

As I mentioned above, we already set BASH_COMPLETION_USER_DIR and XDG_DATA_DIR, so I guess they are found through 3) in _comp_load. We might add a configuration variable to turn off the search 3).

[scop]

In my understanding, replacing the system-installed one with a user one is easy because the user-provided one is searched frist in the suggestion

That suggestion notes this happens through $BASH_COMPLETION_USER_DIR. This variable was not required before for this to happen. I have run my personal setup for ages so that I do have the distro/system bash-completion installed, but I have my bash-completion working tree checked out somewhere in my home dir, and I source it with

. ~/path/to/bash-completion/bash_completion

...in my bashrc, and it has Just Worked: things in my home dir one get precedence without having to set any other variables.

[scop]

I've reworked this PR pretty much altogether.

It documents my thoughts regarding this fairly completely I hope, and implements them in a way that is easier to read and reason. Could be it's also faster in some cases because it returns as soon as it finds what needs to be found instead of computing dirs that might never be even looked at.

This is very lightly tested as it stands.

[akinomyoga]

Thank you for updating the PR. I have a question. In the new version, <cmd>.bash in system directories are still searched after the in-tree completions-core, or in other words, steps 2 and 3 in #1569 (comment) are swapped. Is this intentional?

The current version still has the problem described in #1569 (comment) with a user installation of bash-completion; even if the upstream starts to offer the completion setting at <cmd>.bash (as suggested in #1385 (comment)) to replace an in-tree completion in completions-core, it continues to be masked until we move the in-tree completion to completions-fallback and the user updates bash-completion to the latest version. Then, when the user installs bash-completion in a home directory, there is technically no way for the upstream to replace the completion setting without waiting for bash-completion to be updated.