| Message ID | 20171213130131.15744-1-thomas.petazzoni@free-electrons.com |
|---|---|
| Headers | show |
| Series | Handle conflicting files with Busybox | expand |
Thomas, All, On 2017-12-13 14:01 +0100, Thomas Petazzoni spake thusly: > As we discussed during the Prague Buildroot Developers meeting, in > order to implement per-package SDK, we need to ensure that no package > overwrites files installed by another package. > > This RFC series is an attempt at solving this problem for Busybox. I > have not fixed all packages yet: since it is a very boring task to do, > I wanted to first get some feedback on whether the approach looks > reasonable or not. > > If the feedback is positive, I'll go ahead and submit proper patches > that fix all packages that conflict with Busybox. As I previously said on IRC: I do not much like the big list we will now have to maintain; that's sad... However, I like the fact that we can get rid of the many dependencies in so many packages here and there. :-) What I would have suggested, though, is to do what Baruch hinted at: use the noclobber install of Busybox, and then have Busybox depend on all the packages it provides applets for: BUSYBOX_DEPENDENCIES = \ $(if $(BR2_PACKAGE_COREUTILS),coreutils) \ $(if $(BR2_PACKAGE_util_LINUX),util-linux) \ etc... But unfortunately, the noclobber install option is not usable: - first, there is no way to cause a noclobber install; - second, the noclobber is not accounted for in the case shell wrappers are used. So, I'm afraid we don't have much choice but to do as your series does... Regards, Yann E. MORIN. > Thanks for your feedback! > > Thomas > > Thomas Petazzoni (2): > busybox: avoid conflict with other packages > packages: drop no longer needed busybox dependencies > > package/bc/bc.mk | 5 ---- > package/binutils/binutils.mk | 5 ---- > package/busybox/busybox.mk | 58 ++++++++++++++++++++++++++++++++++++++ > package/coreutils/coreutils.mk | 6 ---- > package/cpio/cpio.mk | 1 - > package/dcron/dcron.mk | 5 ---- > package/debianutils/debianutils.mk | 2 -- > package/diffutils/diffutils.mk | 4 --- > package/fbset/fbset.mk | 5 ---- > package/kmod/kmod.mk | 3 -- > package/util-linux/util-linux.mk | 6 ---- > 11 files changed, 58 insertions(+), 42 deletions(-) > > -- > 2.14.3 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Hello, Thanks for the feedback. On Thu, 28 Dec 2017 18:00:17 +0100, Yann E. MORIN wrote: > > This RFC series is an attempt at solving this problem for Busybox. I > > have not fixed all packages yet: since it is a very boring task to do, > > I wanted to first get some feedback on whether the approach looks > > reasonable or not. > > > > If the feedback is positive, I'll go ahead and submit proper patches > > that fix all packages that conflict with Busybox. > > As I previously said on IRC: I do not much like the big list we will now > have to maintain; that's sad... Even though I agree it's not nice to maintain, I think it's unavoidable if we want to solve the overwriting issue. > However, I like the fact that we can get rid of the many dependencies in > so many packages here and there. :-) > > What I would have suggested, though, is to do what Baruch hinted at: use > the noclobber install of Busybox, and then have Busybox depend on all > the packages it provides applets for: > > BUSYBOX_DEPENDENCIES = \ > $(if $(BR2_PACKAGE_COREUTILS),coreutils) \ > $(if $(BR2_PACKAGE_util_LINUX),util-linux) \ > etc... > > But unfortunately, the noclobber install option is not usable: > > - first, there is no way to cause a noclobber install; > > - second, the noclobber is not accounted for in the case shell > wrappers are used. > > So, I'm afraid we don't have much choice but to do as your series > does... There is however one remaining debate: my patch series tweaks the Busybox configuration to not build the support for applets for which the functionality is provided by a full-featured program. But Baruch didn't like this tweaking of the Busybox configuration, and would prefer to not install the symlinks, and leave the Busybox configuration unchanged (which means we have lots of Busybox applets built into Busybox that are not really used on the target). Do you have an opinion on this specific topic ? Another question is whether we want to have this logic centralized in busybox.mk, or spread into the packages that provide the full-featured variants of the applets ? The latter may have some variable definition ordering issues though. Thanks, Thomas
Thomas, All, On 2017-12-28 18:04 +0100, Thomas Petazzoni spake thusly: > Hello, > > Thanks for the feedback. > > On Thu, 28 Dec 2017 18:00:17 +0100, Yann E. MORIN wrote: > > > > This RFC series is an attempt at solving this problem for Busybox. I > > > have not fixed all packages yet: since it is a very boring task to do, > > > I wanted to first get some feedback on whether the approach looks > > > reasonable or not. > > > > > > If the feedback is positive, I'll go ahead and submit proper patches > > > that fix all packages that conflict with Busybox. > > > > As I previously said on IRC: I do not much like the big list we will now > > have to maintain; that's sad... > > Even though I agree it's not nice to maintain, I think it's unavoidable > if we want to solve the overwriting issue. So that we are on the same line: I do agree with the underlying reason for the change, yes. I'm just trying to see if there are better ways to go with that. > > However, I like the fact that we can get rid of the many dependencies in > > so many packages here and there. :-) > > > > What I would have suggested, though, is to do what Baruch hinted at: use > > the noclobber install of Busybox, and then have Busybox depend on all > > the packages it provides applets for: > > > > BUSYBOX_DEPENDENCIES = \ > > $(if $(BR2_PACKAGE_COREUTILS),coreutils) \ > > $(if $(BR2_PACKAGE_util_LINUX),util-linux) \ > > etc... > > > > But unfortunately, the noclobber install option is not usable: > > > > - first, there is no way to cause a noclobber install; > > > > - second, the noclobber is not accounted for in the case shell > > wrappers are used. > > > > So, I'm afraid we don't have much choice but to do as your series > > does... > > There is however one remaining debate: my patch series tweaks the > Busybox configuration to not build the support for applets for which > the functionality is provided by a full-featured program. But Baruch > didn't like this tweaking of the Busybox configuration, and would > prefer to not install the symlinks, and leave the Busybox configuration > unchanged (which means we have lots of Busybox applets built into > Busybox that are not really used on the target). > > Do you have an opinion on this specific topic ? I prefer they be explicitly disabled as you did. First, because this is a security issue that there is dead code: these applets are still usable by calling 'busybox foo' for example, and that is a security issue. Second, yes it gains a bit of space. But that is not so compelling, becasue if you already have the big buddies enabled, a few kB lost inBusybox is not that much of a burden anyway... > Another question is whether we want to have this logic centralized in > busybox.mk, or spread into the packages that provide the full-featured > variants of the applets ? The latter may have some variable definition > ordering issues though. Please leave it centralised in busybox.mk. Regards, Yann E. MORIN.
Thomas, All On Dec 28, 2017 11:20 AM, "Yann E. MORIN" <yann.morin.1998@free.fr> wrote: Thomas, All, [Snip] I prefer they be explicitly disabled as you did. First, because this is a security issue that there is dead code: these applets are still usable by calling 'busybox foo' for example, and that is a security issue. Second, yes it gains a bit of space. But that is not so compelling, becasue if you already have the big buddies enabled, a few kB lost inBusybox is not that much of a burden anyway... When it comes to maintaining a custom BusyBox config (which could now be dynamically updated), maybe we should output a warning when we detect that we change a config value so the user can know to update their customizations. Unsure if this is best as a altered.configs file or just printing it out..... (Pardon the html response, no computer this week) Matt <div dir="auto"><div>Thomas, All<br><div class="gmail_extra"><br><div class="gmail_quote">On Dec 28, 2017 11:20 AM, "Yann E. MORIN" <<a href="mailto:yann.morin.1998@free.fr">yann.morin.1998@free.fr</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Thomas, All,<br><br></blockquote></div></div></div><div dir="auto">[Snip]</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="quoted-text"> <br> </div>I prefer they be explicitly disabled as you did.<br> <br> First, because this is a security issue that there is dead code: these<br> applets are still usable by calling 'busybox foo' for example, and that<br> is a security issue.<br> <br> Second, yes it gains a bit of space. But that is not so compelling,<br> becasue if you already have the big buddies enabled, a few kB lost<br> inBusybox is not that much of a burden anyway...<br></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">When it comes to maintaining a custom BusyBox config (which could now be dynamically updated), maybe we should output a warning when we detect that we change a config value so the user can know to update their customizations. Unsure if this is best as a altered.configs file or just printing it out.....</div><div dir="auto"><br></div><div dir="auto">(Pardon the html response, no computer this week)</div><div dir="auto"><br></div><div dir="auto">Matt</div></div>
Hi Yann, On Thu, Dec 28, 2017 at 06:20:30PM +0100, Yann E. MORIN wrote: > On 2017-12-28 18:04 +0100, Thomas Petazzoni spake thusly: > > Thanks for the feedback. > > > > On Thu, 28 Dec 2017 18:00:17 +0100, Yann E. MORIN wrote: > > > > > > This RFC series is an attempt at solving this problem for Busybox. I > > > > have not fixed all packages yet: since it is a very boring task to do, > > > > I wanted to first get some feedback on whether the approach looks > > > > reasonable or not. > > > > > > > > If the feedback is positive, I'll go ahead and submit proper patches > > > > that fix all packages that conflict with Busybox. > > > > > > As I previously said on IRC: I do not much like the big list we will now > > > have to maintain; that's sad... > > > > Even though I agree it's not nice to maintain, I think it's unavoidable > > if we want to solve the overwriting issue. > > So that we are on the same line: I do agree with the underlying reason > for the change, yes. I'm just trying to see if there are better ways to > go with that. > > > > However, I like the fact that we can get rid of the many dependencies in > > > so many packages here and there. :-) > > > > > > What I would have suggested, though, is to do what Baruch hinted at: use > > > the noclobber install of Busybox, and then have Busybox depend on all > > > the packages it provides applets for: > > > > > > BUSYBOX_DEPENDENCIES = \ > > > $(if $(BR2_PACKAGE_COREUTILS),coreutils) \ > > > $(if $(BR2_PACKAGE_util_LINUX),util-linux) \ > > > etc... > > > > > > But unfortunately, the noclobber install option is not usable: > > > > > > - first, there is no way to cause a noclobber install; > > > > > > - second, the noclobber is not accounted for in the case shell > > > wrappers are used. > > > > > > So, I'm afraid we don't have much choice but to do as your series > > > does... > > > > There is however one remaining debate: my patch series tweaks the > > Busybox configuration to not build the support for applets for which > > the functionality is provided by a full-featured program. But Baruch > > didn't like this tweaking of the Busybox configuration, and would > > prefer to not install the symlinks, and leave the Busybox configuration > > unchanged (which means we have lots of Busybox applets built into > > Busybox that are not really used on the target). > > > > Do you have an opinion on this specific topic ? > > I prefer they be explicitly disabled as you did. As I said earlier, I don't like this heavy handed modification of the Busybox configuration. I'm not sure how robust this solution would be, as there might be unintended consequences. Kconfig 'select' and 'depends' contribute to that. I prefer to keep config modifications to minimum. > First, because this is a security issue that there is dead code: these > applets are still usable by calling 'busybox foo' for example, and that > is a security issue. That is no different than the current behaviour. > Second, yes it gains a bit of space. But that is not so compelling, > becasue if you already have the big buddies enabled, a few kB lost > inBusybox is not that much of a burden anyway... I agree. > > Another question is whether we want to have this logic centralized in > > busybox.mk, or spread into the packages that provide the full-featured > > variants of the applets ? The latter may have some variable definition > > ordering issues though. > > Please leave it centralised in busybox.mk. I agree here as well. baruch
Baruch, Thomas, All, On 2017-12-28 20:01 +0200, Baruch Siach spake thusly: > On Thu, Dec 28, 2017 at 06:20:30PM +0100, Yann E. MORIN wrote: > > On 2017-12-28 18:04 +0100, Thomas Petazzoni spake thusly: [--SNIP--] > > > There is however one remaining debate: my patch series tweaks the > > > Busybox configuration to not build the support for applets for which > > > the functionality is provided by a full-featured program. But Baruch > > > didn't like this tweaking of the Busybox configuration, and would > > > prefer to not install the symlinks, and leave the Busybox configuration > > > unchanged (which means we have lots of Busybox applets built into > > > Busybox that are not really used on the target). > > > > > > Do you have an opinion on this specific topic ? > > > > I prefer they be explicitly disabled as you did. > > As I said earlier, I don't like this heavy handed modification of the Busybox > configuration. I'm not sure how robust this solution would be, as there might > be unintended consequences. Kconfig 'select' and 'depends' contribute to that. > I prefer to keep config modifications to minimum. Agreed that the 'select' things in busybox' Kconfig could be an issue. But do we have applets that get select-ed by another? In current busybox, there is no applet that is select-ed: $ git grep -E 'config:[[:space:]]+\<select ' |cut -d : -f 2- |sort -u //config:select FEATURE_BZIP2_DECOMPRESS //config:se lect FEATURE_GZIP_DECOMPRESS //config:select FEATURE_IP_ADDRESS //conf ig:select FEATURE_IP_LINK //config:select FEATURE_IP_NEIGH //config: select FEATURE_IP_ROUTE //config:select FEATURE_IP_RULE //config:sele ct FEATURE_IP_TUNNEL //config:select FEATURE_SEAMLESS_GZ //config:sel ect FEATURE_SYSLOG //config:select LONG_OPTS //config:select PLATFORM _LINUX //config:select PLATFORM_LINUX # statfs() //config:select PLAT FORM_LINUX #sysinfo() //config:select TLS //config:select VOLUMEID None of those are actual applets. But OK, there is no guarantee that this will continue to be the case. As an aside, we currently disable a bunch of Busybox options, of which only three applets: swapon, swapoff, ash. > > First, because this is a security issue that there is dead code: these > > applets are still usable by calling 'busybox foo' for example, and that > > is a security issue. > > That is no different than the current behaviour. Indeed not really. But it's not because we're doing something bad today, that we should pursue in this direction! ;-) In the end, my preference is still to do as Thomas proposed... Regards, Yann E. MORIN.