All of lore.kernel.org
 help / color / mirror / Atom feed
From: "John W. Linville" <linville@tuxdriver.com>
To: Kevin Locke <kevin@kevinlocke.name>
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 14:37:15 -0400	[thread overview]
Message-ID: <20190416183714.GA14097@tuxdriver.com> (raw)
In-Reply-To: <4dedb8b19e812805abdd4ffbdfddac5df755b8a5.1555004349.git.kevin@kevinlocke.name>

On Thu, Apr 11, 2019 at 11:39:32AM -0600, Kevin Locke wrote:
> To aid users constructing a valid ethtool invocation, create a
> [bash-completion] script to provide [programmable completion] of ethtool
> arguments.  It supports all current command options.
> 
> The script is named shell-completion/bash/ethtool, similar to [kmod],
> and installed to `pkg-config --variable=completionsdir bash-completion`
> (with fallback to $datadir/bash-completion/completions) by default.
> This can be disabled by passing --without-bash-completion-dir or changed
> by passing --with-bash-completion-dir=$anypath to ./configure.  It
> requires pkg-config 0.18 or later to be installed on the build system
> which runs aclocal (for the PKG_CHECK_MODULES m4 macro).
> 
> To install the script manually for the current user, copy or link
> shell-completion/bash to $BASH_COMPLETION_USER_DIR/completions
> (default $XDG_DATA_HOME/bash-completion/completions
> (default ~/.local/share/bash-completion/completions)).
> To install system-wide, copy shell-completion/bash to completionsdir
> from pkg-config (default /usr/share/bash-completion/completions)
> discussed above.  Restarting bash may be necessary to pick up changes to
> the script (if a previous version had already been loaded).
> 
> Note: In [scop/bash-completion#289] the bash-completion maintainer
> suggested shipping this completion with ethtool rather than
> bash-completion, due to assumptions about the ethtool command-line
> format made by the script.  That pull request also contains an extensive
> test suite in Python which is not included in this commit, but may be
> ported to a format suitable for inclusion if there is sufficient
> interest and agreement about how to achieve that.
> 
> [bash-completion]: https://github.com/scop/bash-completion
> [programmable completion]: https://www.gnu.org/software/bash/manual/html_node/Programmable-Completion.html
> [kmod]: https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/tree/
> [scop/bash-completion#289]: https://github.com/scop/bash-completion/pull/289
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
> 
> Changes in v2:
> * Describe manual install and ./configure arguments in commit message.

Overall, it looks good to me. But when I build with "make distcheck",
I get this output:

...
make[3]: Leaving directory '/home/linville/git/ethtool/ethtool-5.0/_build/sub'
make[2]: Leaving directory '/home/linville/git/ethtool/ethtool-5.0/_build/sub'
make[1]: Leaving directory '/home/linville/git/ethtool/ethtool-5.0/_build/sub'
make[1]: Entering directory '/home/linville/git/ethtool/ethtool-5.0/_build/sub'
make[2]: Entering directory '/home/linville/git/ethtool/ethtool-5.0/_build/sub'
 /usr/bin/mkdir -p '/home/linville/git/ethtool/ethtool-5.0/_inst/sbin'
  /usr/bin/install -c ethtool '/home/linville/git/ethtool/ethtool-5.0/_inst/sbin'
 /usr/bin/mkdir -p '/usr/share/bash-completion/completions'
 /usr/bin/install -c -m 644 ../../shell-completion/bash/ethtool '/usr/share/bash-completion/completions'
/usr/bin/install: cannot create regular file '/usr/share/bash-completion/completions/ethtool': Permission denied
make[2]: *** [Makefile:2005: install-dist_bashcompletionDATA] Error 1
make[2]: Leaving directory '/home/linville/git/ethtool/ethtool-5.0/_build/sub'
make[1]: *** [Makefile:2438: install-am] Error 2
make[1]: Leaving directory '/home/linville/git/ethtool/ethtool-5.0/_build/sub'
make: *** [Makefile:2347: distcheck] Error 1

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,

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

  reply	other threads:[~2019-04-16 18:45 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 [this message]
2019-04-17  2:53     ` Kevin Locke
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=20190416183714.GA14097@tuxdriver.com \
    --to=linville@tuxdriver.com \
    --cc=jesse.brandeburg@intel.com \
    --cc=kevin@kevinlocke.name \
    --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.