| Message ID | 20250123201229.182266-1-marex@denx.de |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] bash: Drop /bin/sh from RPROVIDES | expand |
On Thu, Jan 23, 2025 at 12:12 PM Marek Vasut via lists.openembedded.org <marex=denx.de@lists.openembedded.org> wrote: > > Remove /bin/sh from bash RPROVIDES as this has a side-effect which > confuses rpm package manager when also busybox provides /bin/sh and > base-files depend on /bin/sh . The problem is broken down below. > > First, bash depends on base-files and bash pkg_postinst must run > after base-files was installed, because it requires /etc/shells > provided by base-files to be in place. > > Second, base-files depends on /bin/sh, which is provided by either > bash or busybox in this case. This is the actual problem here, if > bash is selected as /bin/sh provider, then there is cyclic dependency > between bash and base-files, and that confuses dnf which may install > the packages in the wrong order, bash first and base-files second . > > To make this worse, if busybox is also /bin/sh provider, it can and > does happen that some systems pick busybox as the /bin/sh provider, > while others pick bash as the /bin/sh provider, and that cyclic > dependency does not always appear. > > Attempt to break this dependency, drop RPROVIDES /bin/sh from bash > recipe to always force base-files to pick /bin/sh from busybox . > (I am really unsure about this approach) > > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Cc: Alexander Kanavin <alex@linutronix.de> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > Cc: Richard Purdie <richard.purdie@linuxfoundation.org> > --- > meta/recipes-extended/bash/bash.inc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/meta/recipes-extended/bash/bash.inc b/meta/recipes-extended/bash/bash.inc > index 634209c9115..6a061b8a612 100644 > --- a/meta/recipes-extended/bash/bash.inc > +++ b/meta/recipes-extended/bash/bash.inc > @@ -134,4 +134,4 @@ FILES:${PN}-loadable += "${libdir}/bash/*" > > # Limit the RPROVIDES here to class target so that if usrmerge is enabled for nativesdk, it does not > # include host system paths in /bin/ > -RPROVIDES:${PN}:append:class-target = " ${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', '/bin/sh /bin/bash', '', d)}" > +RPROVIDES:${PN}:append:class-target = " ${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', '/bin/bash', '', d)}" Does this now mean that system will always need busybox ? some distros may not want that and simply use other variants to fill in the shoes. I wonder if shell choice could be a distro option or image feature which can then help sort this out ? > -- > 2.45.2 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#210212): https://lists.openembedded.org/g/openembedded-core/message/210212 > Mute This Topic: https://lists.openembedded.org/mt/110778651/1997914 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- >
On Thu, 23 Jan 2025 at 21:12, Marek Vasut via lists.openembedded.org <marex=denx.de@lists.openembedded.org> wrote: > # Limit the RPROVIDES here to class target so that if usrmerge is enabled for nativesdk, it does not > # include host system paths in /bin/ > -RPROVIDES:${PN}:append:class-target = " ${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', '/bin/sh /bin/bash', '', d)}" > +RPROVIDES:${PN}:append:class-target = " ${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', '/bin/bash', '', d)}" This was added for a reason (that you can find in git history), if we partially remove it, then the reason will come back and bite us. What I'd like to understand is how bash's postinst is actually failing when /etc/shells is not there yet: grep -q "^${base_bindir}/bash$" $D${sysconfdir}/shells || echo ${base_bindir}/bash >> $D${sysconfdir}/shells Basically you either need to show me how to reproduce the issue, or show an error message you're getting, or an incorrect outcome on the rootfs. Alex
On 1/24/25 1:43 PM, Alexander Kanavin wrote: > On Thu, 23 Jan 2025 at 21:12, Marek Vasut via lists.openembedded.org > <marex=denx.de@lists.openembedded.org> wrote: > >> # Limit the RPROVIDES here to class target so that if usrmerge is enabled for nativesdk, it does not >> # include host system paths in /bin/ >> -RPROVIDES:${PN}:append:class-target = " ${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', '/bin/sh /bin/bash', '', d)}" >> +RPROVIDES:${PN}:append:class-target = " ${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', '/bin/bash', '', d)}" > > This was added for a reason (that you can find in git history), if we > partially remove it, then the reason will come back and bite us. That's why there is [RFC] tag in subject. > What I'd like to understand is how bash's postinst is actually failing > when /etc/shells is not there yet: > > grep -q "^${base_bindir}/bash$" $D${sysconfdir}/shells || echo > ${base_bindir}/bash >> $D${sysconfdir}/shells Grep will complain about the missing file /etc/shells . I can totally imagine we can take the easy way out here for all the postinst scripts and do something like : " # Make sure $D${sysconfdir}/shells exists touch $D${sysconfdir}/shells # Remove previous bash entry (if any) sed -i '/^${base_bindir}/bash$/ d' $D${sysconfdir}/shells # Add new bash entry , now surely a singular one echo ${base_bindir}/bash >> $D${sysconfdir}/shells " But I have the bad feeling that this would be hiding some deeper issue. > Basically you either need to show me how to reproduce the issue I cannot, because so far this only happens in internal CI. I am not able to reproduce it on multiple local build machines. > , or > show an error message you're getting The grep -q fails on missing /etc/shells , because base-files which was supposed to add it was not installed yet. > , or an incorrect outcome on the > rootfs. That incorrect rootfs will only contain /etc/shells from base-files , the base-files which were installed AFTER bash was installed already , which will be missing the bash entry because that was never added because the bash postinst ran before base-files was installed . (and I still plan to reply to Khem and also got some hint from Martin Jansa, which I have yet to explore)
On 1/23/25 10:18 PM, Khem Raj wrote: > On Thu, Jan 23, 2025 at 12:12 PM Marek Vasut via > lists.openembedded.org <marex=denx.de@lists.openembedded.org> wrote: >> >> Remove /bin/sh from bash RPROVIDES as this has a side-effect which >> confuses rpm package manager when also busybox provides /bin/sh and >> base-files depend on /bin/sh . The problem is broken down below. >> >> First, bash depends on base-files and bash pkg_postinst must run >> after base-files was installed, because it requires /etc/shells >> provided by base-files to be in place. >> >> Second, base-files depends on /bin/sh, which is provided by either >> bash or busybox in this case. This is the actual problem here, if >> bash is selected as /bin/sh provider, then there is cyclic dependency >> between bash and base-files, and that confuses dnf which may install >> the packages in the wrong order, bash first and base-files second . >> >> To make this worse, if busybox is also /bin/sh provider, it can and >> does happen that some systems pick busybox as the /bin/sh provider, >> while others pick bash as the /bin/sh provider, and that cyclic >> dependency does not always appear. >> >> Attempt to break this dependency, drop RPROVIDES /bin/sh from bash >> recipe to always force base-files to pick /bin/sh from busybox . >> (I am really unsure about this approach) >> >> Signed-off-by: Marek Vasut <marex@denx.de> >> --- >> Cc: Alexander Kanavin <alex@linutronix.de> >> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> >> Cc: Richard Purdie <richard.purdie@linuxfoundation.org> >> --- >> meta/recipes-extended/bash/bash.inc | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/meta/recipes-extended/bash/bash.inc b/meta/recipes-extended/bash/bash.inc >> index 634209c9115..6a061b8a612 100644 >> --- a/meta/recipes-extended/bash/bash.inc >> +++ b/meta/recipes-extended/bash/bash.inc >> @@ -134,4 +134,4 @@ FILES:${PN}-loadable += "${libdir}/bash/*" >> >> # Limit the RPROVIDES here to class target so that if usrmerge is enabled for nativesdk, it does not >> # include host system paths in /bin/ >> -RPROVIDES:${PN}:append:class-target = " ${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', '/bin/sh /bin/bash', '', d)}" >> +RPROVIDES:${PN}:append:class-target = " ${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', '/bin/bash', '', d)}" > > Does this now mean that system will always need busybox ? The system will require some other /bin/sh provider than bash . I suspect there will always be one , but maybe I am wrong and there are systems without busybox and with bash only (probably yes) ? > some distros > may not want that and simply use > other variants to fill in the shoes. I wonder if shell choice could be > a distro option or image feature which can then > help sort this out ? I don't think this specific fix is the correct solution, I think what I need to figure out is how to make sure base-files is installed before bash, even if there is this cyclic dependency which confuses rpm/dnf into installing the two packages in arbitrary order: base-files -depends_on-> /bin/sh -depends_on-> bash (the /bin/sh provider) bash -depends_on-> base-files I still have to check the input from Martin Jansa , there was some hint which might be the better fix for this. Any ideas ?
I wonder if we can just drop /etc/shells altogether from base-files? Alex On Fri 24. Jan 2025 at 16.26, Marek Vasut via lists.openembedded.org <marex= denx.de@lists.openembedded.org> wrote: > On 1/23/25 10:18 PM, Khem Raj wrote: > > On Thu, Jan 23, 2025 at 12:12 PM Marek Vasut via > > lists.openembedded.org <marex=denx.de@lists.openembedded.org> wrote: > >> > >> Remove /bin/sh from bash RPROVIDES as this has a side-effect which > >> confuses rpm package manager when also busybox provides /bin/sh and > >> base-files depend on /bin/sh . The problem is broken down below. > >> > >> First, bash depends on base-files and bash pkg_postinst must run > >> after base-files was installed, because it requires /etc/shells > >> provided by base-files to be in place. > >> > >> Second, base-files depends on /bin/sh, which is provided by either > >> bash or busybox in this case. This is the actual problem here, if > >> bash is selected as /bin/sh provider, then there is cyclic dependency > >> between bash and base-files, and that confuses dnf which may install > >> the packages in the wrong order, bash first and base-files second . > >> > >> To make this worse, if busybox is also /bin/sh provider, it can and > >> does happen that some systems pick busybox as the /bin/sh provider, > >> while others pick bash as the /bin/sh provider, and that cyclic > >> dependency does not always appear. > >> > >> Attempt to break this dependency, drop RPROVIDES /bin/sh from bash > >> recipe to always force base-files to pick /bin/sh from busybox . > >> (I am really unsure about this approach) > >> > >> Signed-off-by: Marek Vasut <marex@denx.de> > >> --- > >> Cc: Alexander Kanavin <alex@linutronix.de> > >> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> > >> Cc: Richard Purdie <richard.purdie@linuxfoundation.org> > >> --- > >> meta/recipes-extended/bash/bash.inc | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/meta/recipes-extended/bash/bash.inc > b/meta/recipes-extended/bash/bash.inc > >> index 634209c9115..6a061b8a612 100644 > >> --- a/meta/recipes-extended/bash/bash.inc > >> +++ b/meta/recipes-extended/bash/bash.inc > >> @@ -134,4 +134,4 @@ FILES:${PN}-loadable += "${libdir}/bash/*" > >> > >> # Limit the RPROVIDES here to class target so that if usrmerge is > enabled for nativesdk, it does not > >> # include host system paths in /bin/ > >> -RPROVIDES:${PN}:append:class-target = " > ${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', '/bin/sh /bin/bash', > '', d)}" > >> +RPROVIDES:${PN}:append:class-target = " > ${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', '/bin/bash', '', d)}" > > > > Does this now mean that system will always need busybox ? > > The system will require some other /bin/sh provider than bash . > > I suspect there will always be one , but maybe I am wrong and there are > systems without busybox and with bash only (probably yes) ? > > > some distros > > may not want that and simply use > > other variants to fill in the shoes. I wonder if shell choice could be > > a distro option or image feature which can then > > help sort this out ? > I don't think this specific fix is the correct solution, I think what I > need to figure out is how to make sure base-files is installed before > bash, even if there is this cyclic dependency which confuses rpm/dnf > into installing the two packages in arbitrary order: > > base-files -depends_on-> /bin/sh -depends_on-> bash (the /bin/sh provider) > > bash -depends_on-> base-files > > I still have to check the input from Martin Jansa , there was some hint > which might be the better fix for this. > > Any ideas ? > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#210223): > https://lists.openembedded.org/g/openembedded-core/message/210223 > Mute This Topic: https://lists.openembedded.org/mt/110778651/1686489 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [ > alex.kanavin@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- > >
diff --git a/meta/recipes-extended/bash/bash.inc b/meta/recipes-extended/bash/bash.inc index 634209c9115..6a061b8a612 100644 --- a/meta/recipes-extended/bash/bash.inc +++ b/meta/recipes-extended/bash/bash.inc @@ -134,4 +134,4 @@ FILES:${PN}-loadable += "${libdir}/bash/*" # Limit the RPROVIDES here to class target so that if usrmerge is enabled for nativesdk, it does not # include host system paths in /bin/ -RPROVIDES:${PN}:append:class-target = " ${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', '/bin/sh /bin/bash', '', d)}" +RPROVIDES:${PN}:append:class-target = " ${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', '/bin/bash', '', d)}"
Remove /bin/sh from bash RPROVIDES as this has a side-effect which confuses rpm package manager when also busybox provides /bin/sh and base-files depend on /bin/sh . The problem is broken down below. First, bash depends on base-files and bash pkg_postinst must run after base-files was installed, because it requires /etc/shells provided by base-files to be in place. Second, base-files depends on /bin/sh, which is provided by either bash or busybox in this case. This is the actual problem here, if bash is selected as /bin/sh provider, then there is cyclic dependency between bash and base-files, and that confuses dnf which may install the packages in the wrong order, bash first and base-files second . To make this worse, if busybox is also /bin/sh provider, it can and does happen that some systems pick busybox as the /bin/sh provider, while others pick bash as the /bin/sh provider, and that cyclic dependency does not always appear. Attempt to break this dependency, drop RPROVIDES /bin/sh from bash recipe to always force base-files to pick /bin/sh from busybox . (I am really unsure about this approach) Signed-off-by: Marek Vasut <marex@denx.de> --- Cc: Alexander Kanavin <alex@linutronix.de> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> Cc: Richard Purdie <richard.purdie@linuxfoundation.org> --- meta/recipes-extended/bash/bash.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)