netdev - Re: [PATCH v2] ethtool: Add bash-completion script

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 16 Apr 2019 20:53:33 -0600
From:   Kevin Locke <kevin@...inlocke.name>
To:     "John W. Linville" <linville@...driver.com>
Cc:     netdev@...r.kernel.org,
        Jesse Brandeburg <jesse.brandeburg@...el.com>
Subject: Re: [PATCH v2] ethtool: Add bash-completion script

On Tue, 2019-04-16 at 14:37 -0400, John W. Linville wrote:
> Overall, it looks good to me. But when I build with "make distcheck",
> I get this output:
> 
> [...]
> 
> It looks like somewhere you are using "$(bashcompletiondir)" instead of
> "$(DESTDIR)$(bashcompletiondir)", but I can't seem to find it. Perhaps
> you can find the change required to avoid this build error?

Thanks for taking a look at it!  Good catch.  DESTDIR would have been
my guess as well.  Interestingly, the issue is that make distcheck
expects `./configure --prefix=foo && make install` to only install
files below foo, which fails because --with-bash-completion-dir
defaults to `pkg-config --variable=completionsdir bash-completion`
(usually /usr/share/bash-completion/completions).

I can think of a few different ways to fix this:

1. Add =--without-bash-completion-dir or
   --with-bashcompletiondir=$something to DISTCHECK_CONFIGURE_FLAGS
   in Makefile.am to avoid installing the script during distcheck.
2. Replace the prefix for bash-completion (using
   `pkg-config --variable=prefix bash-completion`) in
   $bashcompletiondir with --prefix passed to configure.
3. Stop using pkg-config and install to
   $datadir/bash-completion/completions by default.

Option 1 has the disadvantage that users may not expect files to be
installed outside of --prefix, that the script does not install to
/usr/local (with everything else) by default, and that
--with-bash-completion-dir= must be passed for non-root installs.
kmod passes
--with-bashcompletiondir=$$dc_install_base/$(bashcompletiondir)
to DISTCHECK_CONFIGURE_FLAGS, which has the additional disadvantage of
using the undocumented (AFAICT) Automake $$dc_install_base variable.

Options 2 and 3 have the disadvantage that passing --prefix= which is
not the prefix of $XDG_DATA_HOME or $XDG_DATA_DIRS will install the
script to a directory that bash-completion doesn't use.

Option 3 has the additional disadvantage of ignoring the upstream
recommendations, which could install the script to a directory not
used by bash-completion for customized or future versions.  It has the
advantage of being extremely simple and understandable.

My personal preference is #2, but I would defer to your judgement.
Let me know which you would prefer and I'll update the patch.

Best,
Kevin

P.S.  I noticed that the PKG_CHECK_MODULES macro does unnecessary work
handling BASH_COMPLETION_CFLAGS and BASH_COMPLETION_LIBS and adds them
to the configure --help text, so I will remove it in favor of calling
$PKG_CONFIG directly, unless you object.