Bash completion file not installed by the build system?

[hartwork]

Hi!

My impression is that the current Bash completion is not installed by the build system. I can manually copy the file at packaging stage of course but then every distro has to do that manually and before that there is no bash completion. Are there plans to make the build system install it for everyone?

Thanks and best, Sebastian

[davidpolverari]

Hi, Sebastian!

As not every user uses bash, and as every distribution has a different place in which to drop bash completion scripts, I think a good approach would be to add an optional, disabled by default, configure switch that would allow users to specify whether they want to build with bash completion. Then we could have automake creating (or not) the proper variables to install it, something akin to what William Swanson proposes on his blog.

Do you think it would help packaging? If so, I will try to implement it as soon as I have time.

Regards,

David.

[hartwork]

Hi @davidpolverari thanks for your reply! I didn't expect or intend to make this big and complicated. I think plain…

bashcompletiondir = $(datadir)/bash-completion/completions
dist_bashcompletion_DATA = doc/dcfldd-bash_completion

…would already go a long way, and then you could go from there as needed. Maybe it doesn't have to be as complex as the scenario in that blog post, e.g. installing a Bash completion file for people that do not use Bash as their main shell would be tolerable in my world. What do you think? Should I make the above a pull request?

[davidpolverari]

Sorry for the delayed response. I have already prepared a solution along the lines of the linked blog post on the bash-completion-install branch. The reason is twofold: it would avoid installing the bash-completion file if the user doesn't use/need bash-completion or even bash itself and it wouldn't do anything different than it does now by default.

I think that approach would allow you to easily install bash-completions on a directory of your choosing during packaging and it wouldn't disrupt any existing packaging efforts (eg. Debian's). Thus, I would kindly ask you and @eribertomota to have a look at it and tell me whether it looks good to you, before we merge it into master.

[hartwork]

@davidpolverari thanks for your work on the topic! :+1:

I reviewed and tried out these changes now. 90% look and operate perfectly. For the other 10%: ~The current use offers AC_ARG_ENABLE-like functionality (e.g. a mere bool rather than accepting a non-bool value like a custom a directory) dressed up as AC_ARG_WITH; it currently needs literal --with-bash-completion-dir=yes to enable. For a true bool --enable-bash-completion would be more organic. The advertised --with-bash-completion-dir[=PATH] is explicitly not supported, because ${withval} is ignored. I recommend to fully buy into AC_ARG_WITH, support ${withval} and check for != xno rather than == xyes.~

On a side note, my vote for enabling by default. From what I remember, Debian has an inclusion list of what files to pull into the package after "make install" so they should be robust towards changes like these and will need a new list in their inclusion list to ship the file. Happy to be corrected on that.

I should also mention that due to the use of pkg-config (nothing against that :+1:) the Bash completion file will currently be installed in /usr/share rather than /usr/local/share by default. Adding --define-variable=datadir="${prefix}/share" to the invocation of pkg-config could be a fix or part of a fix. I'm not sure if all pkg-config implementations support it, I think there are two at least. The idea behind that is this finding:

# grep -E '^(completionsdir|datadir)=' /usr/share/pkgconfig/bash-completion.pc
datadir=/usr/share
completionsdir=${datadir}/bash-completion/completions

Please note how datadir=/usr/share does not use ${prefix} here so plain --define-variable=prefix="${prefix}" would not work for this very file.

What do you think?

[davidpolverari]

@davidpolverari thanks for your work on the topic! +1

You're welcome!

I reviewed and tried out these changes now. 90% look and operate perfectly. For the other 10%: ~The current use offers AC_ARG_ENABLE-like functionality (e.g. a mere bool rather than accepting a non-bool value like a custom a directory) dressed up as AC_ARG_WITH; it currently needs literal --with-bash-completion-dir=yes to enable. For a true bool --enable-bash-completion would be more organic. The advertised --with-bash-completion-dir[=PATH] is explicitly not supported, because ${withval} is ignored. I recommend to fully buy into AC_ARG_WITH, support ${withval} and check for != xno rather than == xyes.~

The current switch is intended to work both AC_ARG_ENABLE-like when used without specifying a directory, and use the provided path otherwise. I tested it and it worked for me, and judging by the strike-through text, I believe you have also tested it and considered it ok, is that correct?

On a side note, my vote for enabling by default. From what I remember, Debian has an inclusion list of what files to pull into the package after "make install" so they should be robust towards changes like these and will need a new list in their inclusion list to ship the file. Happy to be corrected on that.

Yes, Debian packaging indeed offers such a mechanism ^1. Anyways, I find it odd installing a bash-completion script if bash-completion is not installed. Do you think the current implementation hinders packaging for other distros? Please let me know if you foresee any problems with that.

I should also mention that due to the use of pkg-config (nothing against that +1) the Bash completion file will currently be installed in /usr/share rather than /usr/local/share by default. Adding --define-variable=datadir="${prefix}/share" to the invocation of pkg-config could be a fix or part of a fix. I'm not sure if all pkg-config implementations support it, I think there are two at least. The idea behind that is this finding:

# grep -E '^(completionsdir|datadir)=' /usr/share/pkgconfig/bash-completion.pc
datadir=/usr/share
completionsdir=${datadir}/bash-completion/completions

Please note how datadir=/usr/share does not use ${prefix} here so plain --define-variable=prefix="${prefix}" would not work for this very file.

What do you think?

Oh, I see it now. I'm a little bit busy these days, but I will try to implement your suggestion and test it, as it seems to be the right approach. I think testing it under the original Freedesktop.org original pkg-config should suffice, as other implementations are meant to be drop-in replacements (it is never that simple, of course :). In the meantime, any PRs are welcome too.

[hartwork]

and judging by the strike-through text, I believe you have also tested it and considered it ok, is that correct?

@davidpolverari that's the correct reading, yes, I was too quick with conclusions the first time.

Anyways, I find it odd installing a bash-completion script if bash-completion is not installed. Do you think the current implementation hinders packaging for other distros? Please let me know if you foresee any problems with that.

I think it's a question of both purity (your argument) and politics (my argument) in some sense, I don't expect any hinderage (if that word exists), no. Let's go with the shy and pure version then and focus on the rest of it :+1:

I should also mention that due to the use of pkg-config (nothing against that +1) the Bash completion file will currently be installed in /usr/share rather than /usr/local/share by default. Adding --define-variable=datadir="${prefix}/share" to the invocation of pkg-config could be a fix or part of a fix. I'm not sure if all pkg-config implementations support it, I think there are two at least. The idea behind that is this finding:

# grep -E '^(completionsdir|datadir)=' /usr/share/pkgconfig/bash-completion.pc
datadir=/usr/share
completionsdir=${datadir}/bash-completion/completions

Please note how datadir=/usr/share does not use ${prefix} here so plain --define-variable=prefix="${prefix}" would not work for this very file. What do you think?

Oh, I see it now. I'm a little bit busy these days, but I will try to implement your suggestion and test it, as it seems to be the right approach.

Cool! :+1:

[eribertomota]

Hi guys, sorry for my delay. I saw my name in this discussion today. I am travelling and I will be able to test any solution on Debian in 15 days.

Debian works fine with autotools and I think that all will be right.

[hartwork]

~@davidpolverari is branch bash-completion-install ready to be merged (or be turned into a pull request)?~

@davidpolverari any news on --define-variable=datadir="${prefix}"/share?

[davidpolverari]

~@davidpolverari is branch bash-completion-install ready to be merged (or be turned into a pull request)?~

@davidpolverari any news on --define-variable=datadir="${prefix}"/share?

I'm really sorry for the delay. I was busy with other work stuff before, and I didn't have time to work on this issue.

I should also mention that due to the use of pkg-config (nothing against that +1) the Bash completion file will currently be installed in /usr/share rather than /usr/local/share by default.

Now that I was trying to implement it, I had some doubts whether this will be the right approach. So, my question is: does this happen even when bash-completion is installed on /usr/local? The reason I ask is that to me it seemed the expected behavior seeing bash-completion files installed using the same prefix as the one used for bash-completion, and not the one used for dcfldd. But if it pkg-config points to /usr/share even when bash-completion was installed on /usr/local, then it would be really wrong.

Besides, if we want to install our bash-completion file to a different directory (at $HOME, to destination directory before even the bash-completion package gets installed, or any directory inside /usr/local, for instance), we can optionally specify the directory using --with-bash-completion-dir=<dir>. What do you think about it? Would that be a solution? Or did I get it all wrong? Sorry if I missed something.

[hartwork]

Hi @davidpolverari, thanks for your reply!

Would you mind creating a pull request from the branch so that it is less hidden and we get the full GitHub comfort for code review?

Yes, whatever prefix bash-completion was installed to wins, so if the prefix of bash-completion and dcfldd do not match, we get weird mix-ups, which could be trouble in particular when mixing /usr/local (where the primary package manger should have no say) and /usr (where there primary package manager should be the only thing to have a say). I was able to produce these two mix-up variations easily with Docker (details below):

a) bash-completion /usr/local + dcfldd /usr

#19 [16/17] RUN find ROOT/ | sort | grep '/bash-completion\|/bin' | xargs ls -ld
#19 sha256:d66c009504271a293b1df3ee6e741eaac459f11f458a2079068af465ec04aacc
#19 0.268 drwxr-xr-x 2 root root     20 Apr 28 15:09 ROOT/usr/bin
#19 0.268 -rwxr-xr-x 1 root root 322472 Apr 28 15:09 ROOT/usr/bin/dcfldd
#19 0.268 drwxr-xr-x 3 root root     25 Apr 28 15:09 ROOT/usr/local/share/bash-completion
#19 0.268 drwxr-xr-x 2 root root     36 Apr 28 15:09 ROOT/usr/local/share/bash-completion/completions
#19 0.268 -rw-r--r-- 1 root root   2105 Apr 28 15:09 ROOT/usr/local/share/bash-completion/completions/dcfldd-bash_completion
#19 DONE 0.3s

b) bash-completion /usr + dcfldd /usr/local

#19 [16/17] RUN find ROOT/ | sort | grep '/bash-completion\|/bin' | xargs ls -ld
#19 sha256:842c122e6c0dd58ef4bd66f1643c5a55981304c48da0c91c8690b647fbb03a3c
#19 0.303 drwxr-xr-x 2 root root     20 Apr 28 15:19 ROOT/usr/local/bin
#19 0.303 -rwxr-xr-x 1 root root 322472 Apr 28 15:19 ROOT/usr/local/bin/dcfldd
#19 0.303 drwxr-xr-x 3 root root     25 Apr 28 15:19 ROOT/usr/share/bash-completion
#19 0.303 drwxr-xr-x 2 root root     36 Apr 28 15:19 ROOT/usr/share/bash-completion/completions
#19 0.303 -rw-r--r-- 1 root root   2105 Apr 28 15:19 ROOT/usr/share/bash-completion/completions/dcfldd-bash_completion
#19 DONE 0.4s

My Dockerfile for (a) was this:

FROM debian:bullseye
RUN apt-get update && apt-get install --no-install-recommends --yes -V \
        autoconf \
        automake \
        build-essential \
        ca-certificates \
        git \
        pkg-config

RUN git clone --depth 1 https://github.com/scop/bash-completion
WORKDIR bash-completion
RUN autoreconf -i
RUN ./configure --prefix=/usr/local
RUN make -j2
RUN make -j2 install
WORKDIR ..

RUN for i in $(pkg-config --print-variables bash-completion | sort) ; do echo "$i=$(pkg-config --variable="$i" bash-completion)" ; done

RUN git clone --depth 1 --branch bash-completion-install https://github.com/resurrecting-open-source-projects/dcfldd
WORKDIR dcfldd
RUN ./autogen.sh
RUN ./configure --prefix=/usr --with-bash-completion-dir
RUN make -j2
RUN make install DESTDIR="$PWD"/ROOT
RUN find ROOT/ | sort | grep '/bash-completion\|/bin' | xargs ls -ld

So the reason we get these mix-ups is that pkg-config is happy to mix .pc files from /usr and /usr/local. While that may not be a problem for someone building packages in a near vacuum, it may be a problem to e.g. source based Linux and BSD distros when /usr/local contains bash-completion, so these users would need to (1) be aware and (2) pass an explicit an directory or end up with potentially unwanted results.

I'm unsure if passing --define-variable=datadir="${prefix}"/share would be more of a bug or more of a feature, I guess it's arguable. What do you think?

PS: Can we rename --with-bash-completion-dir to --with-bash-completion? The version without arguments just looks too wrong with -dir at the end.

[davidpolverari]

Hi @davidpolverari, thanks for your reply!

You're welcome!

Would you mind creating a pull request from the branch so that it is less hidden and we get the full GitHub comfort for code review?

Sure, I have just created it.

Yes, whatever prefix bash-completion was installed to wins, so if the prefix of bash-completion and dcfldd do not match, we get weird mix-ups, which could be trouble in particular when mixing /usr/local (where the primary package manger should have no say) and /usr (where there primary package manager should be the only thing to have a say). I was able to produce these two mix-up variations easily with Docker (details below):

Thanks for taking your time to make those tests.

So the reason we get these mix-ups is that pkg-config is happy to mix .pc files from /usr and /usr/local. While that may not be a problem for someone building packages in a near vacuum, it may be a problem to e.g. source based Linux and BSD distros when /usr/local contains bash-completion, so these users would need to (1) be aware and (2) pass an explicit an directory or end up with potentially unwanted results.

That is one of the reasons that I resisted to enable that option by default, besides the fact that not every dcfldd user uses bash as their shell.

I'm unsure if passing --define-variable=datadir="${prefix}"/share would be more of a bug or more of a feature, I guess it's arguable. What do you think?

I'm also unsure about it. I tend to think of it as more of a bug than a feature, mainly because it doesn't seem right to fix that through pkg-config. I was afraid that solution could move the problem elsewhere, instead of solving it. I was trying to come up with another solution, but still didn't manage to think of any. But if the solution you proposed solves your immediate problems with packaging, please send your proposed changes, and we could test and merge it for the moment, if it doesn't break anything. Then we can think of another solution later. Please let me know if you are willing to do that, or if you think of another possible solution.

PS: Can we rename --with-bash-completion-dir to --with-bash-completion? The version without arguments just looks too wrong with -dir at the end.

Sure! I already changed it in another commit on the same PR. I found it strange too on a second thought :).

[hartwork]

@davidpolverari thanks for #18!

I believe what we have now is user friendly, arguably behaves reasonable, and can be overriden to enforce desired off-default behavior during packaging when needed. I think that's pretty good, my vote for a merge.

[davidpolverari]

Ok, so I will rebase the commits and merge them into master. Thanks for your help!