On Monday, July 27 2020, Gabriel F. T. Gomes wrote:
> On 17 Jul 2020, Anthony Fok wrote:
>
>>I ran into the following error while packaging the latest version of
>>hugo package:
>>
>> dh_bash-completion: error: Cannot resolve variable
>> "${BASH_COMP_DEBUG_FILE}" in debian/hugo.bash-completion (line 5)
>>
>>where debian/hugo.bash-completion is a proper completion snippet.
>
> How to reproduce (note to myself):
>
> On a system with debhelper >= 13:
> $ git clone https://salsa.debian.org/go-team/packages/hugo.git
> $ cd hugo
> # apt install hugo
> $ hugo gen autocomplete --completionfile=debian/hugo.bash-completion
> $ dh_bash-completion
> dh_bash-completion: error: Cannot resolve variable
> "${BASH_COMP_DEBUG_FILE}" in debian/hugo.bash-completion (line 5)
Thanks for this :-).
>>While one could try to work around the issue by changing
>>"${foo}" to "$foo" in the proper completion snippet,
>>quoting variables with curly braces like ${foo} is nonetheless
>>perfectly valid bash syntax, and it is often unavoidable
>>in cases such as "${foo}bar" or "${foo}_bar".
>
> I agree, it would be terrible to forbid scripts from using it.
+1.
IMHO, the problem here was that debhelper chose to use the ${}-format
without considering dh_bash-completion's case. Of course, with the
ever-growing number of debhelper scripts it's impossible to take every
corner case into account, even though ${} is a pretty overloaded format
for addressing variables ;-).
When I read this bug the first time I thought that it might have been
good if they'd provided a filedoublearray_noexpand or some such, but
then it occurred to me that we do want to support debhelper variables
inside file-lists, so...
>>Perhaps there are better ways to distinguish whether
>>debian/package.bash-completion is a file list or proper snippet
>>than sending it to filedoublearray() and see if it fails or not?
>
> I really suck at perl programming, but, as far as I can tell, that's
> not the only reason to call filedoublearray(). dh_bash-completion uses
> the *output* of filedoublearray(), not its return code, to detect
> whether the file is a proper snippet. However, since debhelper 13,
> filedoublearray() fails during variable expansion (if we track the
> failure down, it happens in _variable_substitution()), so
> dh_bash-completion doesn't actually get a chance to check if the file
> is a proper snippet or a list of files.
>
> I'll try and find a solution for this, but I might need help from perl
> experts. :)
I'm far from being a Perl expert, but I *think* I came up with a
solution.
So, here's the thing. We can't blindly rely on debhelper's
filedoublearray anymore, because of the problem you guys pointed out
above. Which means that bash-completion will probably have to have its
own stripped-down, poor-man's version of filedoublearray. Actually,
given the way dh_bash-completion works, it should be enough to have a
function that tries to determine whether the file being examined is (a)
a bash-completion script, or (b) a file-list. If (a), then the file
itself should be installed. If (b), then we install each file listed in
it.
In order to hack this new function I used a little bit of what
filedoublearray does in the beginning, and then I crafted a few regexes
that will perform a "heuristic" to see if we catch some well-known bash
constructions in the file. If we succeed, then just assume that the
file is a bash-completion script and be done with it. Otherwise, it's
(probably) a file-list.
Now, in order to test this approach, here's what I did (inside a schroot
session):
- Go to sources.d.o, look for all the packages that depend on
bash-completion.
- Download the list, iterate over it and do an "apt source" on all of
them.
- Check for a few interesting cases in this list. Examples I could find
are: consul, cargo, caffe, 2ping, xkcdpass, virtualenvwrapper.
- Enter their directories and perform a "dh_bash-completion -v".
As far as I have tested, everything works OK. Of course, this is a
heuristic approach and it is possible to craft a problematic file that
will cause an error, but it's better than what we have now, IMHO.
BTW: while I was hacking I had another idea, which is to use a
try...catch block around filedoublearray and see if it "throws" anything
like ".*cannot resolve variable.*", but that doesn't really work: if the
error is triggered, it might mean that we're dealing with a
bash-completion script, *but* it might also mean that the file-list file
is broken (e.g., referencing a wrong or non-existent variable), which
means that we would need to perform some kind of parsing to determine
what's really happening anyway (assuming that we want to do a good job
at detecting the error).
Hopefully this will help. I'm not tagging this as "+patch" because I'd
like to hear your opinions first.
Cheers,
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
https://sergiodj.net/
diff --git a/debian/extra/debhelper/dh_bash-completion
b/debian/extra/debhelper/dh_bash-completion
index f96704b..ffc1854 100755
--- a/debian/extra/debhelper/dh_bash-completion
+++ b/debian/extra/debhelper/dh_bash-completion
@@ -33,6 +33,60 @@ completion snippet after. The file format is as follows:
=cut
+# This helper function tries to determine (using some poor man's
+# heuristics) whether $file (its first and only argument) is a
+# filelist containing a list of files to be installed by us, or a
+# bash-completion script, which should itself be installed.
+#
+# If we're dealing with a filelist, return 1. Otherwise, return 0.
+sub is_filelist {
+ # The file to be checked.
+ my ($file) = @_;
+
+ open (DH_FILE, '<', $file) || error("cannot read $file: $!");
+
+ while (<DH_FILE>) {
+ # Get rid of lines containing just spaces or comments.
+ chomp;
+ s/^\s++//;
+ next if /^#/;
+ s/\s++$//;
+
+ # We always ignore/permit empty lines
+ next if $_ eq '';
+
+ # This is the heart of the script. Here, we check for some
+ # well-known idioms on bash scripts, and try to determine if
+ # they're present in the file we're examining. We assume that
+ # if they are, then this means the file is a bash-completion
+ # script.
+ #
+ # The regexes check:
+ #
+ # - If we're calling the bash function "complete", which is a
+ # pretty common thing to do in bash-completion scripts, or
+ #
+ # - If we're using the $(program) way of executing a program.
+ # We don't take into account multi-line statements. Or
+ #
+ # - If we're calling the bash function "compgen", which is
+ # also a pretty common thing that bash-completion scripts
+ # do. Or
+ #
+ # - If we see an "if...then" construction in the file. We
+ # take into account multi-line statements.
+ if (/\s*complete.*-[A-Za-z].*/
+ || /\$\(.*\)/
+ || /\s*compgen.*-[A-Za-z].*/
+ || /\s*if.*;.*then/s) {
+ return 0;
+ }
+ }
+
+ # If we reached the end, this is not a bash-completion script.
+ return 1;
+}
+
init();
my $srcdir = '.';
@@ -53,6 +107,15 @@ PKG: foreach my $package (@{$dh{DOPACKAGES}}) {
if ($completions) {
install_dir($bc_dir);
+ # Invoke our heuristic function to try and determine
+ # if we're dealing with a filelist or with a
+ # bash-completion script.
+ if (!is_filelist($completions)) {
+ verbose_print "detected $completions as a
bash-completion script";
+ install_file($completions, "$bc_dir/$package");
+ next PKG
+ }
+
# try parsing a list of files
@install = filedoublearray($completions);
foreach my $set (@install) {
signature.asc
Description: PGP signature
