base-passwd: Fix the broken preinst/postinstall

PATCH In review Richard Purdie 4 participants 9 messages Nov 14, 2011

Share & tools

Richard Purdie

author

2011-11-14 12:56 UTC #0 lore raw

PATCH 1/1 base-passwd: Fix the broken preinst/postinstall

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 <[email protected]>

---

meta/recipes-core/base-passwd/base-passwd_3.5.22.bb +29 ~~-31~~

index aa90a6d..05be23f 100644
@@ -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)
+}
+

Phil Blundell

2011-11-14 12:59 UTC #1 lore raw

Quoted 8 lines · 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 <[email protected]>

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.

Otavio Salvador

2011-11-14 13:15 UTC #2 lore raw

+1 :D

Quoted 24 lines · Phil Blundell

Quoted 8 lines · 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 <[email protected]>
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
[email protected]
http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core

Signature · 3 lines

Otavio Salvador                             O.S. Systems
E-mail: [email protected]  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

Koen Kooi

2011-11-14 13:43 UTC #3 lore raw

In reply to Phil Blundell

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

Quoted 8 lines · Richard Purdie

Quoted 7 lines
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.

Quoted 6 lines

Quoted 1 line
Signed-off-by Richard Purdie <[email protected]>
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

Richard Purdie

author

2011-11-14 14:03 UTC #4 lore raw

PATCH 1/1 base-passwd: Move update-passwd into a separate package

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 <[email protected]>

---

(quoted diff) +0 -0

b/meta/recipes-core/base-passwd/base-passwd_3.5.22.bb index 05be23f..f6e17f9 100644

meta/recipes-core/base-passwd/base-passwd_3.5.22.bb +0 -0

@@ -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
+}

Phil Blundell

2011-11-14 15:22 UTC #5 lore raw

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

Review

Quoted 7 lines · Richard Purdie

(quoted diff) +7 -0

+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.

Richard Purdie

author

2011-11-14 17:26 UTC #6 lore raw

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

Quoted 18 lines · Phil Blundell

Review
Quoted 7 lines · Richard Purdie
(quoted diff) +7 -0
+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

Quoted 6 lines

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 :/.

Quoted 7 lines

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

Phil Blundell

2011-11-14 17:44 UTC #7 lore raw

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

Quoted 2 lines · Richard Purdie

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.

Richard Purdie

author

2011-11-14 14:05 UTC #8 lore raw

In reply to Koen Kooi

Quoted 12 lines · Koen Kooi

Op 14 nov. 2011, om 13:52 heeft Phil Blundell het volgende geschreven:
Quoted 10 lines · Richard Purdie
Quoted 8 lines
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 <[email protected]>

Quoted 7 lines

Quoted 4 lines
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