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]

Message-ID: <20190418160533.GA22913@tuxdriver.com>
Date:   Thu, 18 Apr 2019 12:05:34 -0400
From:   "John W. Linville" <linville@...driver.com>
To:     Kevin Locke <kevin@...inlocke.name>, netdev@...r.kernel.org,
        Jesse Brandeburg <jesse.brandeburg@...el.com>
Subject: Re: [PATCH v2] ethtool: Add bash-completion script

On Tue, Apr 16, 2019 at 08:53:33PM -0600, Kevin Locke wrote:
> 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.

Option #2 seems reasonable, and your "P.S." seems fine too.

Thanks,

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@...driver.com			might be all we have.  Be ready.