[Buildroot] [PATCH] Config.in files: add missing dependencies to toolchain option comments
Arnout Vandecappelle
arnout at mind.be
Mon Nov 4 21:26:59 UTC 2013
On 04/11/13 08:56, Thomas De Schampheleire wrote:
> When a package A depends on B and toolchain option C, then the comment that
> is given when C is not fulfilled should also depend on B. For example:
>
> config BR2_PACKAGE_A
> depends on BR2_PACKAGE_B
> depends on BR2_LARGEFILE
> depends on BR2_WCHAR
>
> comment "A needs a toolchain w/ largefile, wchar"
> depends on !BR2_LARGEFILE || !BR2_WCHAR
>
> This comment should actually be:
>
> comment "A needs a toolchain w/ largefile, wchar"
> depends on BR2_PACKAGE_B
> depends on !BR2_LARGEFILE || !BR2_WCHAR
Actually, _most_ of the fixes you make here are for architecture
options, not package dependencies. Not that that matters much... except
that I don't really agree with making this change for package dependencies.
For package dependencies, I much prefer to if...endif construct because
this draws the attention to the fact that the whole Config.in is disabled
when the dependent package isn't selected. But I guess it's mostly a
matter of taste anyway. Hm, maybe even moving all the arch and package
dependencies to a global if...endif would be a good idea.
>
> Otherwise, the comment would be visible even though the other dependencies
> are not met.
>
> This patch adds such missing dependencies, and changes existing such
> dependencies from
> depends on BR2_BASE_DEP && !BR2_TOOLCHAIN_USES_GLIBC
> to
> depends on BR2_BASE_DEP
> depends on !BR2_TOOLCHAIN_USES_GLIBC
> so that (positive) base dependencies are separate from the (negative)
> toolchain dependencies. This strategy makes it easier to write such comments
> (because one can simply copy the base dependency from the actual package
> config option), but also avoids complex and long boolean expressions.
>
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>
Even though I have a bunch of comments, the patch is good as it is (and
also a bit fragile because of the large number of changes), so:
Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout at mind.be>
(untested)
>
> ---
> Note: this patch is written on top of my earlier patch:
> "Config.in files: add/update comments on (e)glibc dependencies"
> which is in ThomasP's for-peter branch.
>
>
> While browsing through the Config.in files, I noticed following problems
> that I am planning to fix in later patches:
> - missing comments for some dependencies on toolchain options
> - unification of dependency on Linux kernel
> - unification of dependency on udev /dev management
> - unification of dependency on python, libgtk2, xorg7 (with comment or not)
> - unification of dependency on locale support (and update manual)
> - spaces instead of tabs on some lines, and trailing whitespace
Nice!
[snip]
> diff --git a/package/alsamixergui/Config.in b/package/alsamixergui/Config.in
> --- a/package/alsamixergui/Config.in
> +++ b/package/alsamixergui/Config.in
> @@ -14,4 +14,5 @@ config BR2_PACKAGE_ALSAMIXERGUI
> http://www.iua.upf.es/~mdeboer/projects/alsamixergui/
>
> comment "alsamixergui needs a toolchain w/ C++, threads"
> - depends on (!BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS) && BR2_PACKAGE_XORG7
> + depends on BR2_PACKAGE_XORG7 && BR2_USE_MMU
> + depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_HAS_THREADS
I think there should be a 'depends on BR2_PACKAGE_XORG7' around all of
the X libraries and applications. On the other hand, there are a few that
don't actually depend on X (liberation fonts, xkeyboard-config), and
there may be more in the future that can run on either X or wayland...
[snip]
> diff --git a/package/flex/Config.in b/package/flex/Config.in
> --- a/package/flex/Config.in
> +++ b/package/flex/Config.in
> @@ -19,4 +19,5 @@ config BR2_PACKAGE_FLEX_BINARY
> Install the flex binary tool in the target filesystem.
>
> comment "flex binary needs a toolchain w/ wchar"
> + depends on BR2_USE_MMU && BR2_PACKAGE_FLEX
This one should _definitely_ use an if...endif construct.
> depends on !BR2_USE_WCHAR
[snip]
> diff --git a/package/gstreamer/gst-ffmpeg/Config.in b/package/gstreamer/gst-ffmpeg/Config.in
> --- a/package/gstreamer/gst-ffmpeg/Config.in
> +++ b/package/gstreamer/gst-ffmpeg/Config.in
> @@ -14,4 +14,5 @@ config BR2_PACKAGE_GST_FFMPEG
> http://gstreamer.freedesktop.org/
>
> comment "gst-ffmpeg needs a toolchain w/ largefile, IPv6"
> + depends on BR2_PACKAGE_GSTREAMER
> depends on !(BR2_LARGEFILE && BR2_INET_IPV6)
There are a few more in the gstreamer/ directory that could be
rewritten. But here as well, I would prefer an if...endif in
gstreamer/Config.in.
[snip]
> diff --git a/package/zmqpp/Config.in b/package/zmqpp/Config.in
> --- a/package/zmqpp/Config.in
> +++ b/package/zmqpp/Config.in
> @@ -16,6 +16,7 @@ config BR2_PACKAGE_ZMQPP
> http://github.com/benjamg/zmqpp
>
> comment "zmqpp needs a toolchain w/ C++, IPv6, largefile, wchar, threads"
> + depends on !BR2_avr32
> depends on !(BR2_INSTALL_LIBSTDCPP && BR2_INET_IPV6 && BR2_LARGEFILE \
> && BR2_USE_WCHAR && BR2_TOOLCHAIN_HAS_THREADS)
>
> @@ -30,4 +31,5 @@ config BR2_PACKAGE_ZMQPP_CLIENT
> used to listen or send to zeromq sockets.
>
> comment "zmqpp client needs a toolchain w/ threads"
> - depends on BR2_PACKAGE_ZMQPP && !BR2_TOOLCHAIN_HAS_THREADS
> + depends on BR2_PACKAGE_ZMQPP
> + depends on !BR2_TOOLCHAIN_HAS_THREADS
This one should also use an if...endif.
Regards,
Arnout
[snip]
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
More information about the buildroot
mailing list