| Message ID | 20190826173751.13832-1-alejandro.gonzalez.correo@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] package/iputils: add configs to select which binaries are built | expand |
Hi Alejandro, On 26/08/2019 19:37, Alejandro González wrote: > By default, the iputils build script might build binaries which are > useless for certain applications, like tftpd. Those binaries will bloat > the target filesystem unless a post-build script removes them manually, > which is cumbersome and doesn't shorten build times. Generally, we only add such options if the space savings are significant. Could you indicate how much can be saved? It's sufficient to just compare the size of 1 tool with the size of all together (and for good measure, the total filesystem size as well). > > These changes add Kconfig options which let the user select what > binaries are built with ease. > > Signed-off-by: Alejandro González <alejandro.gonzalez.correo@gmail.com> > --- > package/iputils/Config.in | 64 ++++++++++++++++++++ > package/iputils/iputils.mk | 119 ++++++++++++++++++++++++++++++------- > 2 files changed, 163 insertions(+), 20 deletions(-) > > diff --git a/package/iputils/Config.in b/package/iputils/Config.in > index b5d9141a7d..cb3d03072a 100644 > --- a/package/iputils/Config.in > +++ b/package/iputils/Config.in > @@ -7,3 +7,67 @@ config BR2_PACKAGE_IPUTILS > etc. > > https://github.com/iputils/iputils [snip] > +config BR2_PACKAGE_IPUTILS_NINFOD > + bool "ninfod" > + depends on BR2_TOOLCHAIN_HAS_THREADS # ninfod requires <pthread.h> It also requires a crypto package, according to the .mk file. So you should add something like: select BR2_PACKAGE_NETTLE if !BR2_PACKAGE_LIBGCRYPT && !BR2_PACKAGE_OPENSSL > + default y > + help > + Installs ninfod. > + > +endif > diff --git a/package/iputils/iputils.mk b/package/iputils/iputils.mk > index 4a06581790..291114abb9 100644 > --- a/package/iputils/iputils.mk > +++ b/package/iputils/iputils.mk > @@ -17,6 +17,68 @@ IPUTILS_LICENSE = GPL-2.0+, BSD-3-Clause > IPUTILS_LICENSE_FILES = LICENSE Documentation/LICENSE.BSD3 Documentation/LICENSE.GPL2 > IPUTILS_DEPENDENCIES = $(TARGET_NLS_DEPENDENCIES) > > +# Selectively build binaries > + > +ifeq ($(BR2_PACKAGE_IPUTILS_ARPING),y) > +IPUTILS_CONF_OPTS += -DBUILD_ARPING=true > +else > +IPUTILS_CONF_OPTS += -DBUILD_ARPING=false > +endif Since no dependencies need to be added here, we generally prefer the following pattern: IPUTILS_CONF_OPTS = \ -DBUID_ARPING=$(if $(BR2_PACKAGE_IPUTILS_ARPING),true,false) \ ... (but it's not so important) On the other hand, we also like it if a condition is checked only once, and all modifications that need to be done for that condition are collected below it. So you'd have something like: ifeq ($(BR2_PACKAGE_IPUTILS_ARPING),y) IPUTILS_CONF_OPTS += -DBUILD_ARPING=true ... define hook ... add arping to permissions else IPUTILS_CONF_OPTS += -DBUILD_ARPING=false endif However, the "add arping to permissions" bit is not so easy to do :-( [snip] > ifeq ($(BR2_PACKAGE_LIBCAP),y) > IPUTILS_CONF_OPTS += -DUSE_CAP=true > IPUTILS_DEPENDENCIES += libcap > @@ -49,57 +111,74 @@ IPUTILS_CONF_OPTS += -DUSE_CRYPTO=none > IPUTILS_CONF_OPTS += -DBUILD_NINFOD=false This bit can be removed since it's already handled by the Config.in. > endif > > -# ninfod requires <pthread.h> > -ifneq ($(BR2_TOOLCHAIN_HAS_THREADS),y) > -IPUTILS_CONF_OPTS += -DBUILD_NINFOD=false > -endif > - > ifeq ($(BR2_SYSTEM_ENABLE_NLS),y) > IPUTILS_CONF_OPTS += -DUSE_GETTEXT=true > else > IPUTILS_CONF_OPTS += -DUSE_GETTEXT=false > endif > > -IPUTILS_CONF_OPTS += -DBUILD_TRACEROUTE6=true > - > # XSL Stylesheets for DocBook 5 not packaged for buildroot > IPUTILS_CONF_OPTS += -DBUILD_MANS=false -DBUILD_HTML_MANS=false > > # move iputils binaries to the same location as where Busybox installs > # the corresponding applets, so that we have a single version of the > # tools (from iputils) > -define IPUTILS_MOVE_BINARIES > +define IPUTILS_MOVE_ARPING_BINARY > mv $(TARGET_DIR)/usr/bin/arping $(TARGET_DIR)/usr/sbin/arping > +endef > +ifeq ($(BR2_PACKAGE_IPUTILS_ARPING),y) For hooks, we generally also put the hook definition inside the condition. So just move the ifeq just before the define. > +IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_MOVE_ARPING_BINARY > +endif > + > +define IPUTILS_MOVE_PING_BINARY > $(if $(BR2_ROOTFS_MERGED_USR),,\ This could be simplified together with the ping condition, into: ifeq ($(BR2_PACKAGE_IPUTILS_PING):$(BR2_ROOTFS_MERGED_USR),y:) define IPUTILS_MOVE_PING_BINARY mv $(TARGET_DIR)/usr/bin/ping $(TARGET_DIR)/bin/ping) endef IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_MOVE_PING_BINARY endif > mv $(TARGET_DIR)/usr/bin/ping $(TARGET_DIR)/bin/ping) > +endef > +ifeq ($(BR2_PACKAGE_IPUTILS_PING),y) > +IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_MOVE_PING_BINARY > +endif > + > +define IPUTILS_MOVE_TFTPD_BINARY > mv $(TARGET_DIR)/usr/bin/tftpd $(TARGET_DIR)/usr/sbin/tftpd > endef > -IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_MOVE_BINARIES > +ifeq ($(BR2_PACKAGE_IPUTILS_TFTPD),y) > +IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_MOVE_TFTPD_BINARY > +endif > > # upstream requires distros to create symlink > define IPUTILS_CREATE_PING6_SYMLINK > ln -sf $(TARGET_DIR)/bin/ping $(TARGET_DIR)/bin/ping6 > endef > +ifeq ($(BR2_PACKAGE_IPUTILS_PING),y) > IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_CREATE_PING6_SYMLINK > +endif > > # handle permissions ourselves > IPUTILS_CONF_OPTS += -DNO_SETCAP_OR_SUID=true > ifeq ($(BR2_ROOTFS_DEVICE_TABLE_SUPPORTS_EXTENDED_ATTRIBUTES),y) > define IPUTILS_PERMISSIONS > - /usr/sbin/arping f 755 0 0 - - - - - > - /usr/bin/clockdiff f 755 0 0 - - - - - > - |xattr cap_net_raw+p > - /bin/ping f 755 0 0 - - - - - > - |xattr cap_net_raw+p > - /usr/bin/traceroute6 f 755 0 0 - - - - - > - |xattr cap_net_raw+p > + $(if $(BR2_PACKAGE_IPUTILS_ARPING),y,\ I guess you haven't tested this, because it's wrong :-) $(if ...) is not like ifeq, it can only check for non-empty. So if should be $(if $(BR2_PACKAGE_IPUTILS_ARPING),\ This is getting pretty ugly though... But I don't see an easy way to make it better. Regards, Arnout > + /usr/sbin/arping f 755 0 0 - - - - -) > + $(if $(BR2_PACKAGE_IPUTILS_CLOCKDIFF),y,\ > + /usr/bin/clockdiff f 755 0 0 - - - - - > + |xattr cap_net_raw+p) > + $(if $(BR2_PACKAGE_IPUTILS_PING),y,\ > + /bin/ping f 755 0 0 - - - - - > + |xattr cap_net_raw+p) > + $(if $(BR2_PACKAGE_IPUTILS_TRACEROUTE6),y,\ > + /usr/bin/traceroute6 f 755 0 0 - - - - - > + |xattr cap_net_raw+p) > endef > else > define IPUTILS_PERMISSIONS > - /usr/sbin/arping f 755 0 0 - - - - - > - /usr/bin/clockdiff f 4755 0 0 - - - - - > - /bin/ping f 4755 0 0 - - - - - > - /usr/bin/traceroute6 f 4755 0 0 - - - - - > + $(if $(BR2_PACKAGE_IPUTILS_ARPING),y,\ > + /usr/sbin/arping f 755 0 0 - - - - -) > + $(if $(BR2_PACKAGE_IPUTILS_CLOCKDIFF),y,\ > + /usr/bin/clockdiff f 4755 0 0 - - - - -) > + $(if $(BR2_PACKAGE_IPUTILS_PING),y,\ > + /bin/ping f 4755 0 0 - - - - -) > + $(if $(BR2_PACKAGE_IPUTILS_TRACEROUTE6),y,\ > + /usr/bin/traceroute6 f 4755 0 0 - - - - -) > endef > endif > >
Hello Arnout. Thank you very much for your insightful comment. El 26/8/19 a las 23:33, Arnout Vandecappelle escribió: > Hi Alejandro, > > On 26/08/2019 19:37, Alejandro González wrote: >> By default, the iputils build script might build binaries which are >> useless for certain applications, like tftpd. Those binaries will bloat >> the target filesystem unless a post-build script removes them manually, >> which is cumbersome and doesn't shorten build times. > > Generally, we only add such options if the space savings are significant. Could > you indicate how much can be saved? It's sufficient to just compare the size of > 1 tool with the size of all together (and for good measure, the total filesystem > size as well). Of course! I have just taken some numbers on a Buildroot toolchain I have already set up for a project. It uses GCC 8.3 configured to optimize for speed for the aarch64 arquitecture, with v8 floating point operations, musl C library, and the latest binutils version offered by buildroot. The binaries are stripped from debugging symbols, and the host is an usual amd64 laptop. All of the measures have been taken with the same configuration, same host PC and the same Buildroot version, with this patch applied. These were the commands I've executed to measure the sizes, in the order that is shown: $ make menuconfig (Here I changed the relevant iputils configs. I left the rest as they were) $ make iputils-dirclean (To force iputils to rebuild with the latest config) $ rm -rf target/ && find -name ".stamp_target_installed" -exec rm -f {} \; (To force the target filesystem files to be regenerated from stratch) $ make $ du --block-size=1024 --summarize --apparent-size target/ (To measure the total size of the target files) I've repeated them three times, to get three different measures. Without iputils selected, the target folder size was 34669 KiB. With iputils selected, but only the ping binary selected, the size increased to 34732 KiB (34732 - 34669 = 63 KiB more). WIth every iputils option selected, the size jumped to 34888 KiB (34888 - 34669 = 219 additional KiB). It might seem that ~200 KiB are not worth optimizing in general, but if we take into account that by only selecting the ping binary we can save a 71.2% of the full iputils package size, one might agree that in relative terms the savings are notorious. On the other hand, I've been thinking that reducing the target filesystem size would not be the only use of this patch (let's be honest, ~200 KiB are not much these days :-) ). I've noticed that upstream recently disabled building tftpd by default: https://github.com/iputils/iputils/commit/737d8a91e394518d2ccdaf398bb16283eb8e4a81 . The day that a version with this commit in it comes to Buildroot, there might be users who complain about iputils no longer shipping the tftpd implementation they use for PXE booting, and that could make the updating process more difficult if Buildroot doesn't enable the corresponding build flag manually. And other binaries in the package may suffer the same fate; we don't know. This patch implicitly provides a "line of defense" against such potentially breaking upgrades with no end user and Buildroot developers effort, and my opinion is that this might be its most compelling reason to apply. Moreover, the generic security advantage of reducing the attack surface by only having the networking daemons you need installed apply, too. And, if someone is still using the relic of RARP these days, he or she will find useful being able to select the rarpd daemon for building. Should I edit the commit description to reflect these other potential reaasons for this patch? > [snip] >> +config BR2_PACKAGE_IPUTILS_NINFOD >> + bool "ninfod" >> + depends on BR2_TOOLCHAIN_HAS_THREADS # ninfod requires <pthread.h> > > It also requires a crypto package, according to the .mk file. So you should add > something like: > > select BR2_PACKAGE_NETTLE if !BR2_PACKAGE_LIBGCRYPT && !BR2_PACKAGE_OPENSSL > (snip) > Since no dependencies need to be added here, we generally prefer the following > pattern: > > IPUTILS_CONF_OPTS = \ > -DBUID_ARPING=$(if $(BR2_PACKAGE_IPUTILS_ARPING),true,false) \ > ... > > (but it's not so important) > > On the other hand, we also like it if a condition is checked only once, and all > modifications that need to be done for that condition are collected below it. So > you'd have something like: > > ifeq ($(BR2_PACKAGE_IPUTILS_ARPING),y) > IPUTILS_CONF_OPTS += -DBUILD_ARPING=true > ... define hook > ... add arping to permissions > else > IPUTILS_CONF_OPTS += -DBUILD_ARPING=false > endif > > However, the "add arping to permissions" bit is not so easy to do :-( > > [snip] >> ifeq ($(BR2_PACKAGE_LIBCAP),y) >> IPUTILS_CONF_OPTS += -DUSE_CAP=true >> IPUTILS_DEPENDENCIES += libcap >> @@ -49,57 +111,74 @@ IPUTILS_CONF_OPTS += -DUSE_CRYPTO=none >> IPUTILS_CONF_OPTS += -DBUILD_NINFOD=false > > This bit can be removed since it's already handled by the Config.in. > (snip) >> # move iputils binaries to the same location as where Busybox installs >> # the corresponding applets, so that we have a single version of the >> # tools (from iputils) >> -define IPUTILS_MOVE_BINARIES >> +define IPUTILS_MOVE_ARPING_BINARY >> mv $(TARGET_DIR)/usr/bin/arping $(TARGET_DIR)/usr/sbin/arping >> +endef >> +ifeq ($(BR2_PACKAGE_IPUTILS_ARPING),y) > > For hooks, we generally also put the hook definition inside the condition. So > just move the ifeq just before the define. > >> +IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_MOVE_ARPING_BINARY >> +endif >> + >> +define IPUTILS_MOVE_PING_BINARY >> $(if $(BR2_ROOTFS_MERGED_USR),,\ > > This could be simplified together with the ping condition, into: > > ifeq ($(BR2_PACKAGE_IPUTILS_PING):$(BR2_ROOTFS_MERGED_USR),y:) > define IPUTILS_MOVE_PING_BINARY > mv $(TARGET_DIR)/usr/bin/ping $(TARGET_DIR)/bin/ping) > endef > IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_MOVE_PING_BINARY > endif > I agree on that with those refactorings the code could look more elegant. I appreciate you took some time to explain them, and I will apply them in the next version of this patch. (snip) >> # handle permissions ourselves >> IPUTILS_CONF_OPTS += -DNO_SETCAP_OR_SUID=true >> ifeq ($(BR2_ROOTFS_DEVICE_TABLE_SUPPORTS_EXTENDED_ATTRIBUTES),y) >> define IPUTILS_PERMISSIONS >> - /usr/sbin/arping f 755 0 0 - - - - - >> - /usr/bin/clockdiff f 755 0 0 - - - - - >> - |xattr cap_net_raw+p >> - /bin/ping f 755 0 0 - - - - - >> - |xattr cap_net_raw+p >> - /usr/bin/traceroute6 f 755 0 0 - - - - - >> - |xattr cap_net_raw+p >> + $(if $(BR2_PACKAGE_IPUTILS_ARPING),y,\ > > I guess you haven't tested this, because it's wrong :-) $(if ...) is not like > ifeq, it can only check for non-empty. So if should be > > $(if $(BR2_PACKAGE_IPUTILS_ARPING),\ > Indeed, my tests didn't show up my mistake there. I've noticed it on other parts of the code and fixed them before submitting, but I forgot about that one. Thank you for pointing that out! :-) > > This is getting pretty ugly though... But I don't see an easy way to make it > better. > > Regards, > Arnout Agreed. I'm open to more suggestions about how can I make this better. Regards.
On 27/08/2019 12:56, Alejandro González wrote: > Hello Arnout. Thank you very much for your insightful comment. > > El 26/8/19 a las 23:33, Arnout Vandecappelle escribió: >> Hi Alejandro, >> >> On 26/08/2019 19:37, Alejandro González wrote: >>> By default, the iputils build script might build binaries which are >>> useless for certain applications, like tftpd. Those binaries will bloat >>> the target filesystem unless a post-build script removes them manually, >>> which is cumbersome and doesn't shorten build times. >> >> Generally, we only add such options if the space savings are significant. Could >> you indicate how much can be saved? It's sufficient to just compare the size of >> 1 tool with the size of all together (and for good measure, the total filesystem >> size as well). > > Of course! I have just taken some numbers on a Buildroot toolchain I have already > set up for a project. It uses GCC 8.3 configured to optimize for speed for the > aarch64 arquitecture, with v8 floating point operations, musl C library, and the > latest binutils version offered by buildroot. The binaries are stripped from debugging > symbols, and the host is an usual amd64 laptop. All of the measures have been taken > with the same configuration, same host PC and the same Buildroot version, with this > patch applied. > > These were the commands I've executed to measure the sizes, in the order that is shown: > $ make menuconfig > (Here I changed the relevant iputils configs. I left the rest as they were) > $ make iputils-dirclean > (To force iputils to rebuild with the latest config) > $ rm -rf target/ && find -name ".stamp_target_installed" -exec rm -f {} \; > (To force the target filesystem files to be regenerated from stratch) > $ make > $ du --block-size=1024 --summarize --apparent-size target/ > (To measure the total size of the target files) > > I've repeated them three times, to get three different measures. Without iputils > selected, the target folder size was 34669 KiB. With iputils selected, but only > the ping binary selected, the size increased to 34732 KiB (34732 - 34669 = 63 KiB > more). WIth every iputils option selected, the size jumped to 34888 KiB (34888 - > 34669 = 219 additional KiB). It might seem that ~200 KiB are not worth optimizing > in general, but if we take into account that by only selecting the ping binary we > can save a 71.2% of the full iputils package size, one might agree that in relative > terms the savings are notorious. Yes, it's only the relative savings that are relevant. 34MB is anyway huge - a normal rootfs with just musl+busybox should be around 500K by itself. Could you mention this briefly in the commit log? E.g.: The size of complete iputils on aarch64 is 220KiB, while ping by itself is only 63KiB. 63KiB is still surprisingly large BTW. > On the other hand, I've been thinking that reducing the target filesystem size > would not be the only use of this patch (let's be honest, ~200 KiB are not much > these days :-) ). I've noticed that upstream recently disabled building tftpd by > default: https://github.com/iputils/iputils/commit/737d8a91e394518d2ccdaf398bb16283eb8e4a81 . > The day that a version with this commit in it comes to Buildroot, there might be > users who complain about iputils no longer shipping the tftpd implementation they > use for PXE booting, and that could make the updating process more difficult if > Buildroot doesn't enable the corresponding build flag manually. And other binaries > in the package may suffer the same fate; we don't know. This patch implicitly provides a > "line of defense" against such potentially breaking upgrades with no end user and > Buildroot developers effort, and my opinion is that this might be its most compelling > reason to apply. Moreover, the generic security advantage of reducing the attack > surface by only having the networking daemons you need installed apply, too. And, > if someone is still using the relic of RARP these days, he or she will find useful > being able to select the rarpd daemon for building. > > Should I edit the commit description to reflect these other potential reaasons for > this patch? Yes, it's worth mentioning this in the commit message. Briefly :-) [snip] >>> + $(if $(BR2_PACKAGE_IPUTILS_ARPING),y,\ >> >> I guess you haven't tested this, because it's wrong :-) $(if ...) is not like >> ifeq, it can only check for non-empty. So if should be >> >> $(if $(BR2_PACKAGE_IPUTILS_ARPING),\ >> > > Indeed, my tests didn't show up my mistake there. I've noticed it on other parts of the > code and fixed them before submitting, but I forgot about that one. Thank you for > pointing that out! :-) Actually, you had fixed it in v2, but I hadn't noticed your v2 and replied to the v1. Regards, Arnout > >> >> This is getting pretty ugly though... But I don't see an easy way to make it >> better. >> >> Regards, >> Arnout > > Agreed. I'm open to more suggestions about how can I make this better. > > Regards. >
El 27/8/19 a las 14:13, Arnout Vandecappelle escribió: > (snip) > > Yes, it's only the relative savings that are relevant. 34MB is anyway huge - a > normal rootfs with just musl+busybox should be around 500K by itself. > > Could you mention this briefly in the commit log? E.g.: > > The size of complete iputils on aarch64 is 220KiB, while ping by itself is only > 63KiB. > > 63KiB is still surprisingly large BTW. I should mention that I'm using the full coreutils and util-linux packages instead of Busybox, with eudev (but no HWDB) and System V init, and I'm compiling with strong stack protection enabled. Of course, all of that adds to the final target filesystem size, but perhaps the difference from 500 KiB to 34 MiB has other reasons behind it too. In any case, I hope that my numbers are consistently huge, in the sense that relative savings still apply, although raw size numbers are in a lower scale. >> On the other hand, I've been thinking that reducing the target filesystem size >> would not be the only use of this patch (let's be honest, ~200 KiB are not much >> these days :-) ). I've noticed that upstream recently disabled building tftpd by >> default: https://github.com/iputils/iputils/commit/737d8a91e394518d2ccdaf398bb16283eb8e4a81 . >> The day that a version with this commit in it comes to Buildroot, there might be >> users who complain about iputils no longer shipping the tftpd implementation they >> use for PXE booting, and that could make the updating process more difficult if >> Buildroot doesn't enable the corresponding build flag manually. And other binaries >> in the package may suffer the same fate; we don't know. This patch implicitly provides a >> "line of defense" against such potentially breaking upgrades with no end user and >> Buildroot developers effort, and my opinion is that this might be its most compelling >> reason to apply. Moreover, the generic security advantage of reducing the attack >> surface by only having the networking daemons you need installed apply, too. And, >> if someone is still using the relic of RARP these days, he or she will find useful >> being able to select the rarpd daemon for building. >> >> Should I edit the commit description to reflect these other potential reaasons for >> this patch? > > Yes, it's worth mentioning this in the commit message. Briefly :-) Okay, I will edit it then for the next version of this patch :-) > [snip] >>>> + $(if $(BR2_PACKAGE_IPUTILS_ARPING),y,\ >>> >>> I guess you haven't tested this, because it's wrong :-) $(if ...) is not like >>> ifeq, it can only check for non-empty. So if should be >>> >>> $(if $(BR2_PACKAGE_IPUTILS_ARPING),\ >>> >> >> Indeed, my tests didn't show up my mistake there. I've noticed it on other parts of the >> code and fixed them before submitting, but I forgot about that one. Thank you for >> pointing that out! :-) > > Actually, you had fixed it in v2, but I hadn't noticed your v2 and replied to > the v1. No problem, I admit that confused me a bit too. I'm not yet so used to the workflow of sending Git patches via e-mail, so I sent two messages in quick sucession to fix my previous mistakes in a rather confusing manner. Regards.
diff --git a/package/iputils/Config.in b/package/iputils/Config.in index b5d9141a7d..cb3d03072a 100644 --- a/package/iputils/Config.in +++ b/package/iputils/Config.in @@ -7,3 +7,67 @@ config BR2_PACKAGE_IPUTILS etc. https://github.com/iputils/iputils + +if BR2_PACKAGE_IPUTILS + +config BR2_PACKAGE_IPUTILS_ARPING + bool "arping" + default y + help + Installs arping. + +config BR2_PACKAGE_IPUTILS_CLOCKDIFF + bool "clockdiff" + default y + help + Installs clockdiff. + +config BR2_PACKAGE_IPUTILS_PING + bool "ping" + default y + help + Installs ping. + +config BR2_PACKAGE_IPUTILS_RARPD + bool "rarpd" + help + Installs rarpd. + +config BR2_PACKAGE_IPUTILS_RDISC + bool "rdisc" + default y + help + Installs rdisc. + +config BR2_PACKAGE_IPUTILS_RDISC_SERVER + bool "rdisc (server code)" + depends on BR2_PACKAGE_IPUTILS_RDISC + default y + help + Builds rdisc with server code. + +config BR2_PACKAGE_IPUTILS_TFTPD + bool "tftpd" + help + Installs tftpd. + +config BR2_PACKAGE_IPUTILS_TRACEPATH + bool "tracepath" + default y + help + Installs tracepath. + +config BR2_PACKAGE_IPUTILS_TRACEROUTE6 + bool "traceroute6" + default y + help + Installs traceroute6. + +config BR2_PACKAGE_IPUTILS_NINFOD + bool "ninfod" + depends on BR2_TOOLCHAIN_HAS_THREADS # ninfod requires <pthread.h> + default y + help + Installs ninfod. + +endif diff --git a/package/iputils/iputils.mk b/package/iputils/iputils.mk index 4a06581790..291114abb9 100644 --- a/package/iputils/iputils.mk +++ b/package/iputils/iputils.mk @@ -17,6 +17,68 @@ IPUTILS_LICENSE = GPL-2.0+, BSD-3-Clause IPUTILS_LICENSE_FILES = LICENSE Documentation/LICENSE.BSD3 Documentation/LICENSE.GPL2 IPUTILS_DEPENDENCIES = $(TARGET_NLS_DEPENDENCIES) +# Selectively build binaries + +ifeq ($(BR2_PACKAGE_IPUTILS_ARPING),y) +IPUTILS_CONF_OPTS += -DBUILD_ARPING=true +else +IPUTILS_CONF_OPTS += -DBUILD_ARPING=false +endif + +ifeq ($(BR2_PACKAGE_IPUTILS_CLOCKDIFF),y) +IPUTILS_CONF_OPTS += -DBUILD_CLOCKDIFF=true +else +IPUTILS_CONF_OPTS += -DBUILD_CLOCKDIFF=false +endif + +ifeq ($(BR2_PACKAGE_IPUTILS_PING),y) +IPUTILS_CONF_OPTS += -DBUILD_PING=true +else +IPUTILS_CONF_OPTS += -DBUILD_PING=false +endif + +ifeq ($(BR2_PACKAGE_IPTUILS_RARPD),y) +IPUTILS_CONF_OPTS += -DBUILD_RARPD=true +else +IPUTILS_CONF_OPTS += -DBUILD_RARPD=false +endif + +ifeq ($(BR2_PACKAGE_IPUTILS_RDISC),y) +IPUTILS_CONF_OPTS += -DBUILD_RDISC=true +else +IPUTILS_CONF_OPTS += -DBUILD_RDISC=false +endif + +ifeq ($(BR2_PACKAGE_IPUTILS_RDISC_SERVER),y) +IPUTILS_CONF_OPTS += -DENABLE_RDISC_SERVER=true +else +IPUTILS_CONF_OPTS += -DENABLE_RDISC_SERVER=false +endif + +ifeq ($(BR2_PACKAGE_IPUTILS_TFTPD),y) +IPUTILS_CONF_OPTS += -DBUILD_TFTPD=true +else +IPUTILS_CONF_OPTS += -DBUILD_TFTPD=false +endif + +ifeq ($(BR2_PACKAGE_IPUTILS_TRACEPATH),y) +IPUTILS_CONF_OPTS += -DBUILD_TRACEPATH=true +else +IPUTILS_CONF_OPTS += -DBUILD_TRACEPATH=false +endif + +ifeq ($(BR2_PACKAGE_IPUTILS_TRACEROUTE6),y) +IPUTILS_CONF_OPTS += -DBUILD_TRACEROUTE6=true +else +IPUTILS_CONF_OPTS += -DBUILD_TRACEROUTE6=false +endif + +ifeq ($(BR2_PACKAGE_IPUTILS_NINFOD),y) +IPUTILS_CONF_OPTS += -DBUILD_NINFOD=true +else +IPUTILS_CONF_OPTS += -DBUILD_NINFOD=false +endif + ifeq ($(BR2_PACKAGE_LIBCAP),y) IPUTILS_CONF_OPTS += -DUSE_CAP=true IPUTILS_DEPENDENCIES += libcap @@ -49,57 +111,74 @@ IPUTILS_CONF_OPTS += -DUSE_CRYPTO=none IPUTILS_CONF_OPTS += -DBUILD_NINFOD=false endif -# ninfod requires <pthread.h> -ifneq ($(BR2_TOOLCHAIN_HAS_THREADS),y) -IPUTILS_CONF_OPTS += -DBUILD_NINFOD=false -endif - ifeq ($(BR2_SYSTEM_ENABLE_NLS),y) IPUTILS_CONF_OPTS += -DUSE_GETTEXT=true else IPUTILS_CONF_OPTS += -DUSE_GETTEXT=false endif -IPUTILS_CONF_OPTS += -DBUILD_TRACEROUTE6=true - # XSL Stylesheets for DocBook 5 not packaged for buildroot IPUTILS_CONF_OPTS += -DBUILD_MANS=false -DBUILD_HTML_MANS=false # move iputils binaries to the same location as where Busybox installs # the corresponding applets, so that we have a single version of the # tools (from iputils) -define IPUTILS_MOVE_BINARIES +define IPUTILS_MOVE_ARPING_BINARY mv $(TARGET_DIR)/usr/bin/arping $(TARGET_DIR)/usr/sbin/arping +endef +ifeq ($(BR2_PACKAGE_IPUTILS_ARPING),y) +IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_MOVE_ARPING_BINARY +endif + +define IPUTILS_MOVE_PING_BINARY $(if $(BR2_ROOTFS_MERGED_USR),,\ mv $(TARGET_DIR)/usr/bin/ping $(TARGET_DIR)/bin/ping) +endef +ifeq ($(BR2_PACKAGE_IPUTILS_PING),y) +IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_MOVE_PING_BINARY +endif + +define IPUTILS_MOVE_TFTPD_BINARY mv $(TARGET_DIR)/usr/bin/tftpd $(TARGET_DIR)/usr/sbin/tftpd endef -IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_MOVE_BINARIES +ifeq ($(BR2_PACKAGE_IPUTILS_TFTPD),y) +IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_MOVE_TFTPD_BINARY +endif # upstream requires distros to create symlink define IPUTILS_CREATE_PING6_SYMLINK ln -sf $(TARGET_DIR)/bin/ping $(TARGET_DIR)/bin/ping6 endef +ifeq ($(BR2_PACKAGE_IPUTILS_PING),y) IPUTILS_POST_INSTALL_TARGET_HOOKS += IPUTILS_CREATE_PING6_SYMLINK +endif # handle permissions ourselves IPUTILS_CONF_OPTS += -DNO_SETCAP_OR_SUID=true ifeq ($(BR2_ROOTFS_DEVICE_TABLE_SUPPORTS_EXTENDED_ATTRIBUTES),y) define IPUTILS_PERMISSIONS - /usr/sbin/arping f 755 0 0 - - - - - - /usr/bin/clockdiff f 755 0 0 - - - - - - |xattr cap_net_raw+p - /bin/ping f 755 0 0 - - - - - - |xattr cap_net_raw+p - /usr/bin/traceroute6 f 755 0 0 - - - - - - |xattr cap_net_raw+p + $(if $(BR2_PACKAGE_IPUTILS_ARPING),y,\ + /usr/sbin/arping f 755 0 0 - - - - -) + $(if $(BR2_PACKAGE_IPUTILS_CLOCKDIFF),y,\ + /usr/bin/clockdiff f 755 0 0 - - - - - + |xattr cap_net_raw+p) + $(if $(BR2_PACKAGE_IPUTILS_PING),y,\ + /bin/ping f 755 0 0 - - - - - + |xattr cap_net_raw+p) + $(if $(BR2_PACKAGE_IPUTILS_TRACEROUTE6),y,\ + /usr/bin/traceroute6 f 755 0 0 - - - - - + |xattr cap_net_raw+p) endef else define IPUTILS_PERMISSIONS - /usr/sbin/arping f 755 0 0 - - - - - - /usr/bin/clockdiff f 4755 0 0 - - - - - - /bin/ping f 4755 0 0 - - - - - - /usr/bin/traceroute6 f 4755 0 0 - - - - - + $(if $(BR2_PACKAGE_IPUTILS_ARPING),y,\ + /usr/sbin/arping f 755 0 0 - - - - -) + $(if $(BR2_PACKAGE_IPUTILS_CLOCKDIFF),y,\ + /usr/bin/clockdiff f 4755 0 0 - - - - -) + $(if $(BR2_PACKAGE_IPUTILS_PING),y,\ + /bin/ping f 4755 0 0 - - - - -) + $(if $(BR2_PACKAGE_IPUTILS_TRACEROUTE6),y,\ + /usr/bin/traceroute6 f 4755 0 0 - - - - -) endef endif
By default, the iputils build script might build binaries which are useless for certain applications, like tftpd. Those binaries will bloat the target filesystem unless a post-build script removes them manually, which is cumbersome and doesn't shorten build times. These changes add Kconfig options which let the user select what binaries are built with ease. Signed-off-by: Alejandro González <alejandro.gonzalez.correo@gmail.com> --- package/iputils/Config.in | 64 ++++++++++++++++++++ package/iputils/iputils.mk | 119 ++++++++++++++++++++++++++++++------- 2 files changed, 163 insertions(+), 20 deletions(-)