Patchwork [meta-networking] vsftpd: install volatiles file based on init system

login
register
mail settings
Submitter Joe MacDonald
Date Dec. 12, 2013, 7 p.m.
Message ID <1386874857-8596-1-git-send-email-joe@deserted.net>
Download mbox | patch
Permalink /patch/63295/
State Not Applicable, archived
Headers show

Comments

Joe MacDonald - Dec. 12, 2013, 7 p.m.
The sysvinit populate-volatile.sh scans for volatiles in
/etc/default/volatiles.  systemd expects the same format files to live in
/etc/tmpfiles.d.  Depedning on the DISTRO_FEATURE list, install vsftpd's
volatiles file to the expected location.  While we're here, drop the
creation of the empty ${localstatedir}/run/ hierarchy since they should be
created by the volatiles processing.

Signed-off-by: Joe MacDonald <joe@deserted.net>
---
 meta-networking/recipes-daemons/vsftpd/vsftpd_3.0.0.bb |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
Paul Eggleton - Dec. 13, 2013, 10:33 a.m.
Hi Joe,

On Thursday 12 December 2013 14:00:57 Joe MacDonald wrote:
> The sysvinit populate-volatile.sh scans for volatiles in
> /etc/default/volatiles.  systemd expects the same format files to live in
> /etc/tmpfiles.d.  Depedning on the DISTRO_FEATURE list, install vsftpd's
> volatiles file to the expected location.  While we're here, drop the
> creation of the empty ${localstatedir}/run/ hierarchy since they should be
> created by the volatiles processing.
> 
> Signed-off-by: Joe MacDonald <joe@deserted.net>
> ---
>  meta-networking/recipes-daemons/vsftpd/vsftpd_3.0.0.bb |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/meta-networking/recipes-daemons/vsftpd/vsftpd_3.0.0.bb
> b/meta-networking/recipes-daemons/vsftpd/vsftpd_3.0.0.bb index
> 0698a63..9d82fd7 100644
> --- a/meta-networking/recipes-daemons/vsftpd/vsftpd_3.0.0.bb
> +++ b/meta-networking/recipes-daemons/vsftpd/vsftpd_3.0.0.bb
> @@ -59,8 +59,13 @@ do_install() {
>      install -m 600 ${WORKDIR}/vsftpd.conf ${D}${sysconfdir}/vsftpd.conf
>      install -d ${D}${sysconfdir}/init.d/
>      install -m 755 ${WORKDIR}/init ${D}${sysconfdir}/init.d/vsftpd
> -    install -d ${D}/${sysconfdir}/default/volatiles
> -    install -m 644 ${WORKDIR}/volatiles.99_vsftpd
> ${D}/${sysconfdir}/default/volatiles/99_vsftpd +    if
> ${@base_contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then +
>        install -d ${D}/${sysconfdir}/tmpfiles.d
> +        install -m 644 ${WORKDIR}/volatiles.99_vsftpd
> ${D}/${sysconfdir}/tmpfiles.d/99_vsftpd +    else
> +        install -d ${D}/${sysconfdir}/default/volatiles
> +        install -m 644 ${WORKDIR}/volatiles.99_vsftpd
> ${D}/${sysconfdir}/default/volatiles/99_vsftpd +    fi
> 
>      install -m 600 ${WORKDIR}/vsftpd.ftpusers ${D}${sysconfdir}/
>      install -m 600 ${WORKDIR}/vsftpd.user_list ${D}${sysconfdir}/
> @@ -70,7 +75,6 @@ do_install() {
>          sed -i "s:/lib/security:${base_libdir}/security:"
> ${D}${sysconfdir}/pam.d/vsftpd sed -i "s:ftpusers:vsftpd.ftpusers:"
> ${D}${sysconfdir}/pam.d/vsftpd fi
> -    install -d ${D}${localstatedir}/run/vsftpd/empty
>  }
> 
>  INITSCRIPT_PACKAGES = "${PN}"

I think there might be a problem with this: the DISTRO_FEATURES options aren't 
mutually exclusive - both can be enabled at the same time (usually where you 
want sysvinit for some rescue/initramfs image and systemd for the main image).

Cheers,
Paul
Joe MacDonald - Dec. 13, 2013, 2:34 p.m.
[Re: [oe] [meta-networking][PATCH] vsftpd: install volatiles file based on init system] On 13.12.13 (Fri 10:33) Paul Eggleton wrote:

> Hi Joe,
> 
> On Thursday 12 December 2013 14:00:57 Joe MacDonald wrote:
> > The sysvinit populate-volatile.sh scans for volatiles in
> > /etc/default/volatiles.  systemd expects the same format files to live in
> > /etc/tmpfiles.d.  Depedning on the DISTRO_FEATURE list, install vsftpd's
> > volatiles file to the expected location.  While we're here, drop the
> > creation of the empty ${localstatedir}/run/ hierarchy since they should be
> > created by the volatiles processing.
> > 
> > Signed-off-by: Joe MacDonald <joe@deserted.net>
> > ---
> >  meta-networking/recipes-daemons/vsftpd/vsftpd_3.0.0.bb |   10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/meta-networking/recipes-daemons/vsftpd/vsftpd_3.0.0.bb
> > b/meta-networking/recipes-daemons/vsftpd/vsftpd_3.0.0.bb index
> > 0698a63..9d82fd7 100644
> > --- a/meta-networking/recipes-daemons/vsftpd/vsftpd_3.0.0.bb
> > +++ b/meta-networking/recipes-daemons/vsftpd/vsftpd_3.0.0.bb
> > @@ -59,8 +59,13 @@ do_install() {
> >      install -m 600 ${WORKDIR}/vsftpd.conf ${D}${sysconfdir}/vsftpd.conf
> >      install -d ${D}${sysconfdir}/init.d/
> >      install -m 755 ${WORKDIR}/init ${D}${sysconfdir}/init.d/vsftpd
> > -    install -d ${D}/${sysconfdir}/default/volatiles
> > -    install -m 644 ${WORKDIR}/volatiles.99_vsftpd
> > ${D}/${sysconfdir}/default/volatiles/99_vsftpd +    if
> > ${@base_contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then +
> >        install -d ${D}/${sysconfdir}/tmpfiles.d
> > +        install -m 644 ${WORKDIR}/volatiles.99_vsftpd
> > ${D}/${sysconfdir}/tmpfiles.d/99_vsftpd +    else
> > +        install -d ${D}/${sysconfdir}/default/volatiles
> > +        install -m 644 ${WORKDIR}/volatiles.99_vsftpd
> > ${D}/${sysconfdir}/default/volatiles/99_vsftpd +    fi
> > 
> >      install -m 600 ${WORKDIR}/vsftpd.ftpusers ${D}${sysconfdir}/
> >      install -m 600 ${WORKDIR}/vsftpd.user_list ${D}${sysconfdir}/
> > @@ -70,7 +75,6 @@ do_install() {
> >          sed -i "s:/lib/security:${base_libdir}/security:"
> > ${D}${sysconfdir}/pam.d/vsftpd sed -i "s:ftpusers:vsftpd.ftpusers:"
> > ${D}${sysconfdir}/pam.d/vsftpd fi
> > -    install -d ${D}${localstatedir}/run/vsftpd/empty
> >  }
> > 
> >  INITSCRIPT_PACKAGES = "${PN}"
> 
> I think there might be a problem with this: the DISTRO_FEATURES options aren't 
> mutually exclusive - both can be enabled at the same time (usually where you 
> want sysvinit for some rescue/initramfs image and systemd for the main image).

Hey Paul,

That's a good point and I probably should've at least made mention of it
in my email.  I'd thought about it (thought I try to only have either
sysvinit or systemd enabled in a configuration at a time, multiple init
systems running concurrently make my teeth itch) but I had thought that
in this case vsftpd would only be relying on one of them (preferring
systemd if available) to create the volatiles.  Since I try very hard
not to be in the scenario where I include both, though, I could be
overlooking a case here.  Do you think I should be doing something more
like this:

 62    if ${@base_contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
 63        install -d ${D}/${sysconfdir}/tmpfiles.d
 64        install -m 644 ${WORKDIR}/volatiles.99_vsftpd ${D}/${sysconfdir}/tmpfiles.d/99_vsftpd
 65    fi
 66    if ${@base_contains('DISTRO_FEATURES', 'sysvinit', 'true', 'false', d)}; then
 67        install -d ${D}/${sysconfdir}/default/volatiles
 68        install -m 644 ${WORKDIR}/volatiles.99_vsftpd ${D}/${sysconfdir}/default/volatiles/99_vsftpd
 69    fi
Paul Eggleton - Dec. 13, 2013, 2:45 p.m.
On Friday 13 December 2013 09:34:36 Joe MacDonald wrote:
> [Re: [oe] [meta-networking][PATCH] vsftpd: install volatiles file based on init system] On 13.12.13 (Fri 10:33) Paul Eggleton wrote:
> > I think there might be a problem with this: the DISTRO_FEATURES options
> > aren't mutually exclusive - both can be enabled at the same time (usually
> > where you want sysvinit for some rescue/initramfs image and systemd for
> > the main image).
> 
> That's a good point and I probably should've at least made mention of it
> in my email.  I'd thought about it (thought I try to only have either
> sysvinit or systemd enabled in a configuration at a time, multiple init
> systems running concurrently make my teeth itch) but I had thought that
> in this case vsftpd would only be relying on one of them (preferring
> systemd if available) to create the volatiles.  Since I try very hard
> not to be in the scenario where I include both, though, I could be
> overlooking a case here.  Do you think I should be doing something more
> like this:
> 
> 62    if ${@base_contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
> 63        install -d ${D}/${sysconfdir}/tmpfiles.d
> 64        install -m 644 ${WORKDIR}/volatiles.99_vsftpd ${D}/${sysconfdir}/tmpfiles.d/99_vsftpd
> 65    fi
> 66    if ${@base_contains('DISTRO_FEATURES', 'sysvinit', 'true', 'false', d)}; then
> 67        install -d ${D}/${sysconfdir}/default/volatiles
> 68        install -m 644 ${WORKDIR}/volatiles.99_vsftpd ${D}/${sysconfdir}/default/volatiles/99_vsftpd
> 69    fi

Assuming it's OK to have both of these when both options are on (I haven't
tried it myself), that would seem to be the right thing to do.

Cheers,
Paul
Javier Viguera - March 11, 2014, 4:17 p.m.
Hi all,

Was this patch finally merged?

At the moment building *vsftpd* in a sysvinit-based system is *broken*. 
The failure is:

ERROR: QA Issue: vsftpd: Files/directories were installed but not shipped
   /run
   /run/vsftpd
   /run/vsftpd/empty
ERROR: QA run found fatal errors. Please consider fixing them.

In such sysvinit systems, '/run' usually is a TMPFS and 
'/run/vsftpd/empty' is created at *runtime* via volatiles bootscript.

Creating the directory in the recipe at build time leads to the above 
mentioned error.

-
Javier Viguera
Software Engineer
Digi International® Spain S.A.U.
Joe MacDonald - March 11, 2014, 5:05 p.m.
[Re: [oe] [meta-networking][PATCH] vsftpd: install volatiles file based on init system] On 14.03.11 (Tue 17:17) Javier Viguera wrote:

> Hi all,
> 
> Was this patch finally merged?

Sorry, no, not yet.  At the end of the thread I think I had a workable
solution but I've not merged the result.  If you have an updated patch
that addresses all of the systemd/sysvinit/systemd+sysvinit
combinations, I'll be happy to merge it instead of my work.

-J.

> 
> At the moment building *vsftpd* in a sysvinit-based system is *broken*. The
> failure is:
> 
> ERROR: QA Issue: vsftpd: Files/directories were installed but not shipped
>   /run
>   /run/vsftpd
>   /run/vsftpd/empty
> ERROR: QA run found fatal errors. Please consider fixing them.
> 
> In such sysvinit systems, '/run' usually is a TMPFS and '/run/vsftpd/empty'
> is created at *runtime* via volatiles bootscript.
> 
> Creating the directory in the recipe at build time leads to the above
> mentioned error.
> 
> -
> Javier Viguera
> Software Engineer
> Digi International® Spain S.A.U.
Javier Viguera - March 11, 2014, 5:40 p.m.
Hi Joe,

On 11/03/14 18:05, Joe MacDonald wrote:
> Sorry, no, not yet.  At the end of the thread I think I had a workable
> solution but I've not merged the result.  If you have an updated patch
> that addresses all of the systemd/sysvinit/systemd+sysvinit
> combinations, I'll be happy to merge it instead of my work.

Unfortunately I have never played with *systemd*, so i don't have a 
patch handy.

At the moment in my custom BSP layer i have workarounded it creating a 
bbappend with just:

INSANE_SKIP_${PN} = "installed-vs-shipped"

I know this is not the proper fix but at least it allows me to complete 
the build. Otherwise it fails.

-
Javier Viguera
Software Engineer
Digi International® Spain S.A.U.
Joe MacDonald - March 14, 2014, 12:47 p.m.
[Re: [oe] [meta-networking][PATCH] vsftpd: install volatiles file based on init system] On 14.03.11 (Tue 18:40) Javier Viguera wrote:

> Hi Joe,
> 
> On 11/03/14 18:05, Joe MacDonald wrote:
> >Sorry, no, not yet.  At the end of the thread I think I had a workable
> >solution but I've not merged the result.  If you have an updated patch
> >that addresses all of the systemd/sysvinit/systemd+sysvinit
> >combinations, I'll be happy to merge it instead of my work.
> 
> Unfortunately I have never played with *systemd*, so i don't have a patch
> handy.
> 
> At the moment in my custom BSP layer i have workarounded it creating a
> bbappend with just:
> 
> INSANE_SKIP_${PN} = "installed-vs-shipped"
> 
> I know this is not the proper fix but at least it allows me to complete the
> build. Otherwise it fails.

Understood.  I'll finish up my version that is intended to address the
other combinations as soon as I can and I'll follow up so you know the
status.

Patch

diff --git a/meta-networking/recipes-daemons/vsftpd/vsftpd_3.0.0.bb b/meta-networking/recipes-daemons/vsftpd/vsftpd_3.0.0.bb
index 0698a63..9d82fd7 100644
--- a/meta-networking/recipes-daemons/vsftpd/vsftpd_3.0.0.bb
+++ b/meta-networking/recipes-daemons/vsftpd/vsftpd_3.0.0.bb
@@ -59,8 +59,13 @@  do_install() {
     install -m 600 ${WORKDIR}/vsftpd.conf ${D}${sysconfdir}/vsftpd.conf
     install -d ${D}${sysconfdir}/init.d/
     install -m 755 ${WORKDIR}/init ${D}${sysconfdir}/init.d/vsftpd
-    install -d ${D}/${sysconfdir}/default/volatiles
-    install -m 644 ${WORKDIR}/volatiles.99_vsftpd ${D}/${sysconfdir}/default/volatiles/99_vsftpd
+    if ${@base_contains('DISTRO_FEATURES', 'systemd', 'true', 'false', d)}; then
+        install -d ${D}/${sysconfdir}/tmpfiles.d
+        install -m 644 ${WORKDIR}/volatiles.99_vsftpd ${D}/${sysconfdir}/tmpfiles.d/99_vsftpd
+    else
+        install -d ${D}/${sysconfdir}/default/volatiles
+        install -m 644 ${WORKDIR}/volatiles.99_vsftpd ${D}/${sysconfdir}/default/volatiles/99_vsftpd
+    fi
 
     install -m 600 ${WORKDIR}/vsftpd.ftpusers ${D}${sysconfdir}/
     install -m 600 ${WORKDIR}/vsftpd.user_list ${D}${sysconfdir}/
@@ -70,7 +75,6 @@  do_install() {
         sed -i "s:/lib/security:${base_libdir}/security:" ${D}${sysconfdir}/pam.d/vsftpd
         sed -i "s:ftpusers:vsftpd.ftpusers:" ${D}${sysconfdir}/pam.d/vsftpd
     fi
-    install -d ${D}${localstatedir}/run/vsftpd/empty
 }
 
 INITSCRIPT_PACKAGES = "${PN}"