From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] package: add powertop 2.7
Date: Fri, 20 Mar 2015 16:33:16 +0100 [thread overview]
Message-ID: <20150320163316.695aa4ed@free-electrons.com> (raw)
In-Reply-To: <1426743621-1652-1-git-send-email-steven@uplinklabs.net>
Dear Steven Noonan,
On Wed, 18 Mar 2015 22:40:21 -0700, Steven Noonan wrote:
> Signed-off-by: Steven Noonan <steven@uplinklabs.net>
I've applied, but to be honest, I found that they were too many trivial
issues that should have been discovered before the patch was submitted.
Here is the list of things that I had to change:
[Thomas:
- fix commit title
- powertop wants libintl unconditionally, so make sure
BR2_PACKAGE_GETTEXT is selected when BR2_NEEDS_GETTEXT is set,
and add gettext to the dependencies.
- add missing comment about thread dependency.
- add missing dependency on host-pkgconf, without which powertop
cannot find libnl.
- patch src/Makefile.am to not pass -fstack-protector, which fails
to build if the toolchain does not have SSP support.
- rename patch powertop-autotune.patch to confirm to the patch
naming convention.]
Commit title: we normally expect "<foo>: new package" for package
additions. I agree, this is pure nitpicking :-)
The libintl issue was trivially found by using a uClibc toolchain,
which is the default in Buildroot. You can for example use
http://autobuild.buildroot.org/toolchains/configs/br-arm-full.config as
a base configuration file to quickly test things against uClibc.
The missing thread comment was trivial, it's documented in the section
"17.2.2. Dependencies on target and toolchain options" in the Buildroot
manual.
The missing dependency on host-pkgconf is seen when you do a build with
just powertop. Whenever you submit a new package, make sure you do a
build from scratch with just this package enabled, so that you have
some minimal verification that the dependencies are correct.
The -fstack-protector issue was also seen because I'm using a standard
uClibc toolchain, which do not have SSP enabled by default.
The patch naming convention is also documented in the Buildroot manual.
In the light of those comments, could you have a second look at your
other "new package" submissions and retest them with a uClibc
toolchain, and with a Buildroot build having just each new package
enabled, in order to discover missing dependencies?
Thanks a lot!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
next prev parent reply other threads:[~2015-03-20 15:33 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-19 5:40 [Buildroot] [PATCH] package: add powertop 2.7 Steven Noonan
2015-03-20 15:33 ` Thomas Petazzoni [this message]
2015-03-20 20:28 ` Steven Noonan
2015-03-20 20:54 ` Thomas Petazzoni
-- strict thread matches above, loose matches on Subject: below --
2015-03-19 3:48 [Buildroot] [PATCH] package: add hwloc 1.10.1 Steven Noonan
2015-03-19 3:48 ` [Buildroot] [PATCH] package: add powertop 2.7 Steven Noonan
2015-03-19 5:31 ` Baruch Siach
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=20150320163316.695aa4ed@free-electrons.com \
--to=thomas.petazzoni@free-electrons.com \
--cc=buildroot@busybox.net \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox