[PATCH] base-passwd: Fix the broken preinst/postinstall

[PATCH] base-passwd: Fix the broken preinst/postinstall

[Richard Purdie]

The preinst accesses file which may not yet have been unpacked.
The postinst is too late for the creation of these files
for at least the opkg backend.

This patch therefore encodes the file contents into the preinst,
resolving the various issues once and for all.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
diff --git a/meta/recipes-core/base-passwd/base-passwd_3.5.22.bb b/meta/recipes-core/base-passwd/base-passwd_3.5.22.bb
index aa90a6d..05be23f 100644
--- a/meta/recipes-core/base-passwd/base-passwd_3.5.22.bb
+++ b/meta/recipes-core/base-passwd/base-passwd_3.5.22.bb
@@ -1,7 +1,7 @@
 SUMMARY = "Base system master password/group files."
 DESCRIPTION = "The master copies of the user database files (/etc/passwd and /etc/group).  The update-passwd tool is also provided to keep the system databases synchronized with these master files."
 SECTION = "base"
-PR = "r4"
+PR = "r5"
 LICENSE = "GPLv2+"
 LIC_FILES_CHKSUM = "file://COPYING;md5=eb723b61539feef013de476e68b5c50a"
 
@@ -37,36 +37,6 @@ do_install () {
 	install -p -m 644 debian/copyright ${D}${docdir}/${BPN}/
 }
 
-pkg_preinst_${PN} () {
-	set -e
-
-	# Used for rootfs generation. On in-target install this will be run
-        # before the unpack so the files won't be available
-
-	if [ ! -e $D${sysconfdir}/passwd ] && [ -e $D${datadir}/base-passwd/passwd.master ]; then
-		cp $D${datadir}/base-passwd/passwd.master $D${sysconfdir}/passwd
-	fi
-
-	if [ ! -e $D${sysconfdir}/group ] && [ -e $D${datadir}/base-passwd/group.master ]; then
-		cp $D${datadir}/base-passwd/group.master $D${sysconfdir}/group
-	fi
-
-	exit 0
-}
-
-pkg_postinst_${PN} () {
-	set -e
-
-	if [ ! -e $D${sysconfdir}/passwd ] ; then
-		cp $D${datadir}/base-passwd/passwd.master $D${sysconfdir}/passwd
-	fi
-
-	if [ ! -e $D${sysconfdir}/group ] ; then
-		cp $D${datadir}/base-passwd/group.master $D${sysconfdir}/group
-	fi
-	exit 0
-}
-
 base_passwd_sstate_postinst() {
 	if [ "${BB_CURRENTTASK}" = "populate_sysroot" -o "${BB_CURRENTTASK}" = "populate_sysroot_setscene" ]
 	then
@@ -80,3 +50,31 @@ base_passwd_sstate_postinst() {
 		install -p -m 644 ${STAGING_DIR_TARGET}${datadir}/base-passwd/group.master ${STAGING_DIR_TARGET}${sysconfdir}/group
 	fi
 }
+
+python populate_packages_prepend() {
+	# Add in the preinst function for ${PN}
+	# We have to do this here as prior to this, passwd/group.master
+	# would be unavailable. We need to create these files at preinst
+	# time before the files from the package may be available, hence
+	# storing the data from the files in the preinst directly.
+
+	f = open(bb.data.expand("${STAGING_DATADIR}/base-passwd/passwd.master", d), 'r')
+	passwd = "".join(f.readlines())
+	f.close()
+	f = open(bb.data.expand("${STAGING_DATADIR}/base-passwd/group.master", d), 'r')
+	group = "".join(f.readlines())
+	f.close()
+
+	preinst = """#!/bin/sh
+if [ ! -e $D${sysconfdir}/passwd ]; then
+	cat << EOF > $D${sysconfdir}/passwd
+""" + passwd + """EOF
+fi
+if [ ! -e $D${sysconfdir}/group ]; then
+	cat << EOF > $D${sysconfdir}/group
+""" + group + """EOF
+fi
+"""
+	d.setVar('pkg_preinst_${PN}', preinst)
+}
+

Re: [PATCH] base-passwd: Fix the broken preinst/postinstall

[Phil Blundell]

On Mon, 2011-11-14 at 12:49 +0000, Richard Purdie wrote:
> The preinst accesses file which may not yet have been unpacked.
> The postinst is too late for the creation of these files
> for at least the opkg backend.
> 
> This patch therefore encodes the file contents into the preinst,
> resolving the various issues once and for all.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

If you're going to do that (which does look reasonable) then you might
as well stop the package from shipping passwd.master and group.master at
all.  I don't think anything else uses those files so they're just
wasting space at the moment.

p.

Re: [PATCH] base-passwd: Fix the broken preinst/postinstall

[Otavio Salvador]

+1 :D

On Mon, Nov 14, 2011 at 10:52, Phil Blundell <philb@gnu.org> wrote:
> On Mon, 2011-11-14 at 12:49 +0000, Richard Purdie wrote:
>> The preinst accesses file which may not yet have been unpacked.
>> The postinst is too late for the creation of these files
>> for at least the opkg backend.
>>
>> This patch therefore encodes the file contents into the preinst,
>> resolving the various issues once and for all.
>>
>> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>
> If you're going to do that (which does look reasonable) then you might
> as well stop the package from shipping passwd.master and group.master at
> all.  I don't think anything else uses those files so they're just
> wasting space at the moment.
>
> p.
>
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>



-- 
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

Re: [PATCH] base-passwd: Fix the broken preinst/postinstall

[Koen Kooi]

[-- Attachment #1: Type: text/plain, Size: 880 bytes --]


Op 14 nov. 2011, om 13:52 heeft Phil Blundell het volgende geschreven:

> On Mon, 2011-11-14 at 12:49 +0000, Richard Purdie wrote:
>> The preinst accesses file which may not yet have been unpacked.
>> The postinst is too late for the creation of these files
>> for at least the opkg backend.
>> 
>> This patch therefore encodes the file contents into the preinst,
>> resolving the various issues once and for all.
>> 
>> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> 
> If you're going to do that (which does look reasonable) then you might
> as well stop the package from shipping passwd.master and group.master at
> all.  I don't think anything else uses those files so they're just
> wasting space at the moment.

IIRC 'update-passwd' will use those. That's not a criticism on Phils suggestion, I like it very much.

regards,

Koen

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 169 bytes --]

[PATCH] base-passwd: Move update-passwd into a separate package

[Richard Purdie]

update-passwd is the only user of the passwd/group.master files
and was never used by OE since it wasn't run.

This patch packages this separately and adds an appropriate postinst
to make the package useful so people can include it as they wish.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
diff --git a/meta/recipes-core/base-passwd/base-passwd_3.5.22.bb
b/meta/recipes-core/base-passwd/base-passwd_3.5.22.bb
index 05be23f..f6e17f9 100644
--- a/meta/recipes-core/base-passwd/base-passwd_3.5.22.bb
+++ b/meta/recipes-core/base-passwd/base-passwd_3.5.22.bb
@@ -1,7 +1,7 @@
 SUMMARY = "Base system master password/group files."
 DESCRIPTION = "The master copies of the user database files
(/etc/passwd and /etc/group).  The update-passwd tool is also provided
to keep the system databases synchronized with these master files."
 SECTION = "base"
-PR = "r5"
+PR = "r9"
 LICENSE = "GPLv2+"
 LIC_FILES_CHKSUM =
"file://COPYING;md5=eb723b61539feef013de476e68b5c50a"
 
@@ -16,6 +16,11 @@ S = "${WORKDIR}/base-passwd"
 
 inherit autotools
 
+PACKAGES =+ "${PN}-update"
+FILES_${PN}-update = "${sbindir}/* ${datadir}/${PN}"
+
+ALLOW_EMPTY_${PN} = "1"
+
 SSTATEPOSTINSTFUNCS += "base_passwd_sstate_postinst"
 
 do_install () {
@@ -78,3 +83,10 @@ fi
 	d.setVar('pkg_preinst_${PN}', preinst)
 }
 
+pkg_postinst_${PN}-update () {
+#!/bin/sh
+if [ "x$D" != "x" ]; then
+	exit 0
+fi
+${sbindir}/update-passwd
+}

Re: [PATCH] base-passwd: Fix the broken preinst/postinstall

[Richard Purdie]

On Mon, 2011-11-14 at 14:37 +0100, Koen Kooi wrote:
> Op 14 nov. 2011, om 13:52 heeft Phil Blundell het volgende geschreven:
> 
> > On Mon, 2011-11-14 at 12:49 +0000, Richard Purdie wrote:
> >> The preinst accesses file which may not yet have been unpacked.
> >> The postinst is too late for the creation of these files
> >> for at least the opkg backend.
> >> 
> >> This patch therefore encodes the file contents into the preinst,
> >> resolving the various issues once and for all.
> >> 
> >> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > 
> > If you're going to do that (which does look reasonable) then you might
> > as well stop the package from shipping passwd.master and group.master at
> > all.  I don't think anything else uses those files so they're just
> > wasting space at the moment.
> 
> IIRC 'update-passwd' will use those. That's not a criticism on Phils
> suggestion, I like it very much.

I've posted a second patch which moves these pieces into a separate
package and actually uses update-passwd.

Cheers,

Richard

Re: [PATCH] base-passwd: Move update-passwd into a separate package

[Phil Blundell]

On Mon, 2011-11-14 at 13:57 +0000, Richard Purdie wrote:
> +pkg_postinst_${PN}-update () {
> +#!/bin/sh
> +if [ "x$D" != "x" ]; then
> +	exit 0
> +fi
> +${sbindir}/update-passwd
> +}

Not that it really matters, but (in the interests of not perpetuating
unnecessary cruft) this "x" paradigm is unnecessary here.  If you quote
the values then empty strings are permissible, so you could have
written:

if [ "$D" != "" ]; then
   ...
fi

But, in fact, all reasonable implementations of /bin/sh support "test
-n" (it's required by POSIX) so you can write it even more concisely:

if [ -n $D ]; then
  ...
fi

Of course, it sucks fairly badly that we need to have all this scar
tissue in the installed postinsts at all.  For the opkg backend at
least, it would be fairly straightforward to add support for an "offline
postinst" as a separate script, which could be run by rootfs_ipk but
never actually installed into the rootfs.  But I don't know what the
impact on the other backends would be for that, which I guess might make
it hard to deploy in practice.

p.

Re: [PATCH] base-passwd: Move update-passwd into a separate package

[Richard Purdie]

On Mon, 2011-11-14 at 15:15 +0000, Phil Blundell wrote:
> On Mon, 2011-11-14 at 13:57 +0000, Richard Purdie wrote:
> > +pkg_postinst_${PN}-update () {
> > +#!/bin/sh
> > +if [ "x$D" != "x" ]; then
> > +	exit 0
> > +fi
> > +${sbindir}/update-passwd
> > +}
> 
> Not that it really matters, but (in the interests of not perpetuating
> unnecessary cruft) this "x" paradigm is unnecessary here.  If you quote
> the values then empty strings are permissible, so you could have
> written:
> 
> if [ "$D" != "" ]; then
>    ...
> fi
> 
> But, in fact, all reasonable implementations of /bin/sh support "test
> -n" (it's required by POSIX) so you can write it even more concisely:
> 
> if [ -n $D ]; then
>   ...
> fi

Right, I picked a bad example to copy :/.

> Of course, it sucks fairly badly that we need to have all this scar
> tissue in the installed postinsts at all.  For the opkg backend at
> least, it would be fairly straightforward to add support for an "offline
> postinst" as a separate script, which could be run by rootfs_ipk but
> never actually installed into the rootfs.  But I don't know what the
> impact on the other backends would be for that, which I guess might make
> it hard to deploy in practice.

Why can't opkg just wipe the postinst's its run off the disk? Its not
like it needs them any longer...

Cheers,

Richard

Re: [PATCH] base-passwd: Move update-passwd into a separate package

[Phil Blundell]

On Mon, 2011-11-14 at 17:19 +0000, Richard Purdie wrote:
> Why can't opkg just wipe the postinst's its run off the disk? Its not
> like it needs them any longer...

Yes, that's true.  In fact for my purposes I end up wiping the whole of
the opkg metadata anyway so it is a moot issue. :-)  But you're right,
"opkg configure" is a one-shot operation and it will never use the
postinst again so it might as well delete them for itself.

p.