busybox: fix to support FEATURE_INDIVIDUAL

PATCH In review [email protected] 2 participants 6 messages May 30, 2013 – Jun 3, 2013

lore

Share & tools

lore

[email protected]

author

2013-05-30 08:51 UTC #0 lore raw

cover busybox: fix to support FEATURE_INDIVIDUAL

From: Chen Qi <[email protected]>

The following changes since commit 5cb59cc6910d8f3165528c4a71e29e4de897d242:

yocto-bsp: re-enable AutoAddDevices in xorg.conf for generated BSPs (2013-05-29 22:25:41 +0100)

are available in the git repository at:

git://git.pokylinux.org/poky-contrib ChenQi/busybox-individual
http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=ChenQi/busybox-individual

Chen Qi (1):
busybox: fix to support FEATURE_INDIVIDUAL

 meta/recipes-core/busybox/busybox.inc |   16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

Signature · 1 line

1.7.9.5

[email protected]

author

2013-05-30 08:51 UTC #1 lore raw

PATCH 1/1 busybox: fix to support FEATURE_INDIVIDUAL

From: Chen Qi <[email protected]>

Previously, if CONFIG_FEATURE_INDIVIDUAL was enabled for busybox, yocto-based systems could start correctly.

This is because if busybox is built as individual apps, '/bin/busybox' may not be present, so setting the default ALTERNATIVE_TARGET to '/bin/busybox' is not appropriate and could lead to errors.

This patch fixes this problem by checking the existence of '/bin/busybox' before setting the ALTERNATIVE_TARGET to '/bin/busybox'.

After this change, if busybox is built as individual apps, we'll have links like '/bin/ls -> /bin/ls.busybox', otherwise, we'll have links like '/bin/ls -> /bin/busybox'.

Besides, this patch removes the pkg_postinst part in the busybox recipe, because it's redundant.

[YOCTO #4570]

Signed-off-by Chen Qi <[email protected]>

---
 meta/recipes-core/busybox/busybox.inc |   16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

meta/recipes-core/busybox/busybox.inc +5 ~~-11~~

index c8908b0..6abe167 100644
@@ -171,7 +171,7 @@ do_install () {
 			install -m 0755 "0_lib/$NAME" "${D}$FILE.${BPN}"
 		done
 		# add suid bit where needed
-		for i in `grep -E "APPLET.*_BB_SUID_((MAYBE|REQUIRE))" include/applets.h | grep -v _BB_SUID_DROP | cut -f 3 -d '(' | cut -f 1 -d ','`; do
+		for i in `grep -E "APPLET.*BB_SUID_((MAYBE|REQUIRE))" include/applets.h | grep -v _BB_SUID_DROP | cut -f 3 -d '(' | cut -f 1 -d ','`; do
 			find ${D} -name $i.${BPN} -exec chmod a+s {} \;
 		done
 		install -m 0755 0_lib/libbusybox.so.${PV} ${D}${libdir}/libbusybox.so.${PV}

@@ -242,14 +242,16 @@ ALTERNATIVE_TARGET[syslog-init] = "${sysconfdir}/init.d/syslog.${BPN}"
 ALTERNATIVE_LINK_NAME[syslog-startup-conf] = "${sysconfdir}/syslog-startup.conf"
 ALTERNATIVE_TARGET[syslog-startup-conf] = "${sysconfdir}/syslog-startup.conf.${BPN}"
 
-ALTERNATIVE_TARGET = "/bin/busybox"
-
 python do_package_prepend () {
     # We need to load the full set of busybox provides from the /etc/busybox.links
     # Use this to see the update-alternatives with the right information
 
     dvar = d.getVar('D', True)
     pn = d.getVar('PN', True)
+
+    if os.path.exists('%s/bin/busybox' % (dvar)):
+        d.setVar('ALTERNATIVE_TARGET', "/bin/busybox")
+
     f = open('%s/etc/busybox.links' % (dvar), 'r')
 
     for alt_link_name in f:

@@ -265,14 +267,6 @@ python do_package_prepend () {
     f.close()
 }
 
-pkg_postinst_${PN} () {
-	# If we are not making an image we create links for the utilities that doesn't exist
-	# so the update-alternatives script will get the utilities it needs
-	# (update-alternatives have no problem replacing links later anyway)
-	test -n 2> /dev/null || alias test='busybox test'
-	if test "x$D" = "x"; then while read link; do if test ! -h "$link"; then case "$link" in /*/*/*) to="../../bin/busybox";; /bin/*) to="busybox";; /*/*) to="../bin/busybox";; esac; busybox ln -s $to $link; fi; done </etc/busybox.links; fi
-}
-
 pkg_prerm_${PN} () {
 	# This is so you can make busybox commit suicide - removing busybox with no other packages
 	# providing its files, this will make update-alternatives work, but the update-rc.d part

Signature · 1 line

1.7.9.5

Saul Wold

2013-05-30 12:26 UTC #2 lore raw

Quoted 9 lines · On 05/30/2013 05:51 PM, [email protected] wrote:

From: Chen Qi <[email protected]>
Previously, if CONFIG_FEATURE_INDIVIDUAL was enabled for busybox, yocto-based systems could start correctly.
This is because if busybox is built as individual apps, '/bin/busybox' may not be present, so setting the default ALTERNATIVE_TARGET to '/bin/busybox' is not appropriate and could lead to errors.

Quoted 7 lines

This patch fixes this problem by checking the existence of '/bin/busybox' before setting the ALTERNATIVE_TARGET to '/bin/busybox'.
After this change, if busybox is built as individual apps, we'll have links like '/bin/ls -> /bin/ls.busybox', otherwise, we'll have links like '/bin/ls -> /bin/busybox'.

Does this add size to the image? In this example does ls.busybox link to something else or is it a binary on it's own?

Sau!

Quoted 58 lines

Besides, this patch removes the pkg_postinst part in the busybox recipe, because it's redundant.

[YOCTO #4570]

Signed-off-by Chen Qi <[email protected]>

---
  meta/recipes-core/busybox/busybox.inc |   16 +++++-----------
  1 file changed, 5 insertions(+), 11 deletions(-)

meta/recipes-core/busybox/busybox.inc +5 ~~-11~~

index c8908b0..6abe167 100644
@@ -171,7 +171,7 @@ do_install () {
  			install -m 0755 "0_lib/$NAME" "${D}$FILE.${BPN}"
  		done
  		# add suid bit where needed
-		for i in `grep -E "APPLET.*_BB_SUID_((MAYBE|REQUIRE))" include/applets.h | grep -v _BB_SUID_DROP | cut -f 3 -d '(' | cut -f 1 -d ','`; do
+		for i in `grep -E "APPLET.*BB_SUID_((MAYBE|REQUIRE))" include/applets.h | grep -v _BB_SUID_DROP | cut -f 3 -d '(' | cut -f 1 -d ','`; do
  			find ${D} -name $i.${BPN} -exec chmod a+s {} \;
  		done
  		install -m 0755 0_lib/libbusybox.so.${PV} ${D}${libdir}/libbusybox.so.${PV}

@@ -242,14 +242,16 @@ ALTERNATIVE_TARGET[syslog-init] = "${sysconfdir}/init.d/syslog.${BPN}"
  ALTERNATIVE_LINK_NAME[syslog-startup-conf] = "${sysconfdir}/syslog-startup.conf"
  ALTERNATIVE_TARGET[syslog-startup-conf] = "${sysconfdir}/syslog-startup.conf.${BPN}"
 
-ALTERNATIVE_TARGET = "/bin/busybox"
-
  python do_package_prepend () {
      # We need to load the full set of busybox provides from the /etc/busybox.links
      # Use this to see the update-alternatives with the right information
 
      dvar = d.getVar('D', True)
      pn = d.getVar('PN', True)
+
+    if os.path.exists('%s/bin/busybox' % (dvar)):
+        d.setVar('ALTERNATIVE_TARGET', "/bin/busybox")
+
      f = open('%s/etc/busybox.links' % (dvar), 'r')
 
      for alt_link_name in f:

@@ -265,14 +267,6 @@ python do_package_prepend () {
      f.close()
  }
 
-pkg_postinst_${PN} () {
-	# If we are not making an image we create links for the utilities that doesn't exist
-	# so the update-alternatives script will get the utilities it needs
-	# (update-alternatives have no problem replacing links later anyway)
-	test -n 2> /dev/null || alias test='busybox test'
-	if test "x$D" = "x"; then while read link; do if test ! -h "$link"; then case "$link" in /*/*/*) to="../../bin/busybox";; /bin/*) to="busybox";; /*/*) to="../bin/busybox";; esac; busybox ln -s $to $link; fi; done </etc/busybox.links; fi
-}
-
  pkg_prerm_${PN} () {
  	# This is so you can make busybox commit suicide - removing busybox with no other packages
  	# providing its files, this will make update-alternatives work, but the update-rc.d part

ChenQi

author

2013-06-03 02:19 UTC #3 lore raw

Quoted 13 lines · Saul Wold

Quoted 12 lines · On 05/30/2013 05:51 PM, [email protected] wrote:
From: Chen Qi <[email protected]>
Previously, if CONFIG_FEATURE_INDIVIDUAL was enabled for busybox, yocto-based systems could start correctly.
This is because if busybox is built as individual apps, '/bin/busybox' may not be present, so setting the default ALTERNATIVE_TARGET to '/bin/busybox' is not appropriate and could lead to errors.
This patch fixes this problem by checking the existence of '/bin/busybox' before setting the ALTERNATIVE_TARGET to '/bin/busybox'.

Quoted 9 lines

Quoted 4 lines
After this change, if busybox is built as individual apps, we'll have links like '/bin/ls -> /bin/ls.busybox', otherwise, we'll have links like '/bin/ls -> /bin/busybox'.
Does this add size to the image? In this example does ls.busybox link to something else or is it a binary on it's own?
Sau!

It increases about 1M of the whole image size.
ls.busybox, for example, is a binary.

Best Regards,
Chen Qi

Quoted 74 lines

Quoted 72 lines
Besides, this patch removes the pkg_postinst part in the busybox recipe, because it's redundant.
[YOCTO #4570]
Signed-off-by Chen Qi <[email protected]>
---
  meta/recipes-core/busybox/busybox.inc |   16 +++++-----------
  1 file changed, 5 insertions(+), 11 deletions(-)
meta/recipes-core/busybox/busybox.inc +0 -1
index c8908b0..6abe167 100644
@@ -171,7 +171,7 @@ do_install () {
              install -m 0755 "0_lib/$NAME" "${D}$FILE.${BPN}"
          done
          # add suid bit where needed
-        for i in `grep -E "APPLET.*_BB_SUID_((MAYBE|REQUIRE))" 
include/applets.h | grep -v _BB_SUID_DROP | cut -f 3 -d '(' | cut -f 1 -d ','`; do
+ for i in `grep -E "APPLET.*BB_SUID_((MAYBE|REQUIRE))"
include/applets.h | grep -v _BB_SUID_DROP | cut -f 3 -d '(' | cut -f 1 -d ','`; do
              find ${D} -name $i.${BPN} -exec chmod a+s {} \;
          done
          install -m 0755 0_lib/libbusybox.so.${PV} 
${D}${libdir}/libbusybox.so.${PV}
meta/recipes-core/busybox/busybox.inc +0 -0
@@ -242,14 +242,16 @@ ALTERNATIVE_TARGET[syslog-init] = 
"${sysconfdir}/init.d/syslog.${BPN}"
ALTERNATIVE_LINK_NAME[syslog-startup-conf] =
"${sysconfdir}/syslog-startup.conf"
ALTERNATIVE_TARGET[syslog-startup-conf] =
"${sysconfdir}/syslog-startup.conf.${BPN}"
meta/recipes-core/busybox/busybox.inc +0 -2
-ALTERNATIVE_TARGET = "/bin/busybox"
-
  python do_package_prepend () {
      # We need to load the full set of busybox provides from the 
/etc/busybox.links
# Use this to see the update-alternatives with the right
information
      dvar = d.getVar('D', True)
      pn = d.getVar('PN', True)
meta/recipes-core/busybox/busybox.inc +4 -2
+
+    if os.path.exists('%s/bin/busybox' % (dvar)):
+        d.setVar('ALTERNATIVE_TARGET', "/bin/busybox")
+
      f = open('%s/etc/busybox.links' % (dvar), 'r')
 
      for alt_link_name in f:
@@ -265,14 +267,6 @@ python do_package_prepend () {
      f.close()
  }
 
-pkg_postinst_${PN} () {
-    # If we are not making an image we create links for the 
utilities that doesn't exist
meta/recipes-core/busybox/busybox.inc +0 -2
-    # so the update-alternatives script will get the utilities it needs
-    # (update-alternatives have no problem replacing links later 
anyway)
meta/recipes-core/busybox/busybox.inc +0 -2
-    test -n 2> /dev/null || alias test='busybox test'
-    if test "x$D" = "x"; then while read link; do if test ! -h 
"$link"; then case "$link" in /*/*/*) to="../../bin/busybox";; /bin/*) to="busybox";; /*/*) to="../bin/busybox";; esac; busybox ln
meta/recipes-core/busybox/busybox.inc +0 -3
-s $to $link; fi; done </etc/busybox.links; fi
-}
-
  pkg_prerm_${PN} () {
      # This is so you can make busybox commit suicide - removing 
busybox with no other packages
      # providing its files, this will make update-alternatives work, 
but the update-rc.d part

Saul Wold

2013-06-03 04:10 UTC #4 lore raw

Quoted 19 lines · ChenQi

Quoted 18 lines · Saul Wold
Quoted 17 lines · On 05/30/2013 05:51 PM, [email protected] wrote:
From: Chen Qi <[email protected]>
Previously, if CONFIG_FEATURE_INDIVIDUAL was enabled for busybox, yocto-based systems could start correctly.
This is because if busybox is built as individual apps, '/bin/busybox' may not be present, so setting the default ALTERNATIVE_TARGET to '/bin/busybox' is not appropriate and could lead to errors.
This patch fixes this problem by checking the existence of '/bin/busybox'
before setting the ALTERNATIVE_TARGET to '/bin/busybox'.
After this change, if busybox is built as individual apps, we'll have links like '/bin/ls -> /bin/ls.busybox', otherwise, we'll have links like '/bin/ls -> /bin/busybox'.

Quoted 9 lines

Quoted 6 lines
Does this add size to the image? In this example does ls.busybox link to something else or is it a binary on it's own?
Sau!
It increases about 1M of the whole image size.
ls.busybox, for example, is a binary.

That's NOT ok, we are looking to reduce the image size especially for core-image-minimal, I believe the static test I did with building a busybox with only a tinylogin config was actually smaller that the current tinylogin binary!

Please investigate this route, we also need to understand what other parts of busybox are using SetUID.

Thanks
Sau!

Quoted 84 lines

Best Regards,
Chen Qi
Quoted 78 lines
Quoted 76 lines
Besides, this patch removes the pkg_postinst part in the busybox recipe, because it's redundant.
[YOCTO #4570]
Signed-off-by Chen Qi <[email protected]>
---
  meta/recipes-core/busybox/busybox.inc |   16 +++++-----------
  1 file changed, 5 insertions(+), 11 deletions(-)
meta/recipes-core/busybox/busybox.inc +0 -1
index c8908b0..6abe167 100644
@@ -171,7 +171,7 @@ do_install () {
              install -m 0755 "0_lib/$NAME" "${D}$FILE.${BPN}"
          done
          # add suid bit where needed
-        for i in `grep -E "APPLET.*_BB_SUID_((MAYBE|REQUIRE))"
include/applets.h | grep -v _BB_SUID_DROP | cut -f 3 -d '(' | cut -f 1 -d ','`; do
+ for i in `grep -E "APPLET.*BB_SUID_((MAYBE|REQUIRE))"
include/applets.h | grep -v _BB_SUID_DROP | cut -f 3 -d '(' | cut -f 1 -d ','`; do
              find ${D} -name $i.${BPN} -exec chmod a+s {} \;
          done
          install -m 0755 0_lib/libbusybox.so.${PV}
${D}${libdir}/libbusybox.so.${PV}
meta/recipes-core/busybox/busybox.inc +0 -0
@@ -242,14 +242,16 @@ ALTERNATIVE_TARGET[syslog-init] =
"${sysconfdir}/init.d/syslog.${BPN}"
ALTERNATIVE_LINK_NAME[syslog-startup-conf] =
"${sysconfdir}/syslog-startup.conf"
ALTERNATIVE_TARGET[syslog-startup-conf] =
"${sysconfdir}/syslog-startup.conf.${BPN}"
meta/recipes-core/busybox/busybox.inc +0 -2
-ALTERNATIVE_TARGET = "/bin/busybox"
-
  python do_package_prepend () {
      # We need to load the full set of busybox provides from the
/etc/busybox.links
# Use this to see the update-alternatives with the right
information
      dvar = d.getVar('D', True)
      pn = d.getVar('PN', True)
meta/recipes-core/busybox/busybox.inc +4 -2
+
+    if os.path.exists('%s/bin/busybox' % (dvar)):
+        d.setVar('ALTERNATIVE_TARGET', "/bin/busybox")
+
      f = open('%s/etc/busybox.links' % (dvar), 'r')
 
      for alt_link_name in f:
@@ -265,14 +267,6 @@ python do_package_prepend () {
      f.close()
  }
 
-pkg_postinst_${PN} () {
-    # If we are not making an image we create links for the
utilities that doesn't exist
meta/recipes-core/busybox/busybox.inc +0 -2
-    # so the update-alternatives script will get the utilities it needs
-    # (update-alternatives have no problem replacing links later
anyway)
meta/recipes-core/busybox/busybox.inc +0 -2
-    test -n 2> /dev/null || alias test='busybox test'
-    if test "x$D" = "x"; then while read link; do if test ! -h
"$link"; then case "$link" in /*/*/*) to="../../bin/busybox";; /bin/*) to="busybox";; /*/*) to="../bin/busybox";; esac; busybox ln
meta/recipes-core/busybox/busybox.inc +0 -3
-s $to $link; fi; done </etc/busybox.links; fi
-}
-
  pkg_prerm_${PN} () {
      # This is so you can make busybox commit suicide - removing
busybox with no other packages
      # providing its files, this will make update-alternatives work,
but the update-rc.d part

ChenQi

author

2013-06-03 04:33 UTC #5 lore raw

Quoted 26 lines · Saul Wold

Quoted 25 lines · ChenQi
Quoted 24 lines · Saul Wold
Quoted 9 lines · On 05/30/2013 05:51 PM, [email protected] wrote:
From: Chen Qi <[email protected]>
Previously, if CONFIG_FEATURE_INDIVIDUAL was enabled for busybox, yocto-based systems could start correctly.
This is because if busybox is built as individual apps, '/bin/busybox' may not be present, so setting the default ALTERNATIVE_TARGET to '/bin/busybox' is not appropriate and could lead to errors.
Quoted 8 lines
This patch fixes this problem by checking the existence of '/bin/busybox'
before setting the ALTERNATIVE_TARGET to '/bin/busybox'.
After this change, if busybox is built as individual apps, we'll have links like '/bin/ls -> /bin/ls.busybox', otherwise, we'll have links like '/bin/ls -> /bin/busybox'.
Does this add size to the image? In this example does ls.busybox link to something else or is it a binary on it's own?
Sau!

Quoted 7 lines

Quoted 3 lines
It increases about 1M of the whole image size.
ls.busybox, for example, is a binary.
That's NOT ok, we are looking to reduce the image size especially for core-image-minimal, I believe the static test I did with building a busybox with only a tinylogin config was actually smaller that the current tinylogin binary!
Please investigate this route, we also need to understand what other parts of busybox are using SetUID.

This is a different bug.
(busybox in Yocto/OE doesn't support FEATURE_INDIVIDUAL https://bugzilla.yoctoproject.org/show_bug.cgi?id=4570)
I encountered this bug when I was exploring busybox.

I won't use this 'FEATURE_INDIVIDUAL' approach to fix bug#4207, as you said above, it increases image size, and that's not what we want. (Switch to using busybox instead of tinylogin https://bugzilla.yoctoproject.org/show_bug.cgi?id=4207).

Best Regards,
Chen Qi

Quoted 91 lines

Thanks
Sau!
Quoted 85 lines
Best Regards,
Chen Qi
Quoted 79 lines
Quoted 77 lines
Besides, this patch removes the pkg_postinst part in the busybox recipe, because it's redundant.
[YOCTO #4570]
Signed-off-by Chen Qi <[email protected]>
---
  meta/recipes-core/busybox/busybox.inc |   16 +++++-----------
  1 file changed, 5 insertions(+), 11 deletions(-)
meta/recipes-core/busybox/busybox.inc +0 -1
index c8908b0..6abe167 100644
@@ -171,7 +171,7 @@ do_install () {
              install -m 0755 "0_lib/$NAME" "${D}$FILE.${BPN}"
          done
          # add suid bit where needed
-        for i in `grep -E "APPLET.*_BB_SUID_((MAYBE|REQUIRE))"
include/applets.h | grep -v _BB_SUID_DROP | cut -f 3 -d '(' | cut -f 1 -d ','`; do
+ for i in `grep -E "APPLET.*BB_SUID_((MAYBE|REQUIRE))"
include/applets.h | grep -v _BB_SUID_DROP | cut -f 3 -d '(' | cut -f 1 -d ','`; do
              find ${D} -name $i.${BPN} -exec chmod a+s {} \;
          done
          install -m 0755 0_lib/libbusybox.so.${PV}
${D}${libdir}/libbusybox.so.${PV}
meta/recipes-core/busybox/busybox.inc +0 -0
@@ -242,14 +242,16 @@ ALTERNATIVE_TARGET[syslog-init] =
"${sysconfdir}/init.d/syslog.${BPN}"
ALTERNATIVE_LINK_NAME[syslog-startup-conf] =
"${sysconfdir}/syslog-startup.conf"
ALTERNATIVE_TARGET[syslog-startup-conf] =
"${sysconfdir}/syslog-startup.conf.${BPN}"
meta/recipes-core/busybox/busybox.inc +0 -2
-ALTERNATIVE_TARGET = "/bin/busybox"
-
  python do_package_prepend () {
      # We need to load the full set of busybox provides from the
/etc/busybox.links
# Use this to see the update-alternatives with the right
information
      dvar = d.getVar('D', True)
      pn = d.getVar('PN', True)
meta/recipes-core/busybox/busybox.inc +4 -2
+
+    if os.path.exists('%s/bin/busybox' % (dvar)):
+        d.setVar('ALTERNATIVE_TARGET', "/bin/busybox")
+
      f = open('%s/etc/busybox.links' % (dvar), 'r')
 
      for alt_link_name in f:
@@ -265,14 +267,6 @@ python do_package_prepend () {
      f.close()
  }
 
-pkg_postinst_${PN} () {
-    # If we are not making an image we create links for the
utilities that doesn't exist
- # so the update-alternatives script will get the utilities it
needs
- # (update-alternatives have no problem replacing links later
anyway)
meta/recipes-core/busybox/busybox.inc +0 -2
-    test -n 2> /dev/null || alias test='busybox test'
-    if test "x$D" = "x"; then while read link; do if test ! -h
"$link"; then case "$link" in /*/*/*) to="../../bin/busybox";; /bin/*) to="busybox";; /*/*) to="../bin/busybox";; esac; busybox ln
meta/recipes-core/busybox/busybox.inc +0 -3
-s $to $link; fi; done </etc/busybox.links; fi
-}
-
  pkg_prerm_${PN} () {
      # This is so you can make busybox commit suicide - removing
busybox with no other packages
      # providing its files, this will make update-alternatives work,
but the update-rc.d part