All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Locke <kevin@kevinlocke.name>
To: "John W. Linville" <linville@tuxdriver.com>
Cc: netdev@vger.kernel.org, Jesse Brandeburg <jesse.brandeburg@intel.com>
Subject: Re: [PATCH v2] ethtool: Add bash-completion script
Date: Tue, 16 Apr 2019 20:53:33 -0600	[thread overview]
Message-ID: <20190417025333.GA28674@kevinolos> (raw)
In-Reply-To: <20190416183714.GA14097@tuxdriver.com>

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.

  reply	other threads:[~2019-04-17  2:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-08  2:19 [PATCH] ethtool: Add bash-completion script Kevin Locke
2019-04-11 16:14 ` Jesse Brandeburg
2019-04-11 16:47   ` Kevin Locke
2019-04-11 17:39 ` [PATCH v2] " Kevin Locke
2019-04-16 18:37   ` John W. Linville
2019-04-17  2:53     ` Kevin Locke [this message]
2019-04-18 16:05       ` John W. Linville
2019-04-20  0:16   ` [PATCH v3] " Kevin Locke
2019-04-24 19:54     ` John W. Linville

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190417025333.GA28674@kevinolos \
    --to=kevin@kevinlocke.name \
    --cc=jesse.brandeburg@intel.com \
    --cc=linville@tuxdriver.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.