[OE-core] [PATCH] systemd: quote SYSTEMD_SERVICE in prerm and postinst

[Damien Riegel]

v2 looks like the snippet below, but I'm not very fond of these path
manipulations. Any suggestion for a better solution?

Regards,
-- 
Damien

diff --git a/meta/classes/systemd.bbclass b/meta/classes/systemd.bbclass
index 589e73c855..9bacdecb9b 100644
--- a/meta/classes/systemd.bbclass
+++ b/meta/classes/systemd.bbclass
@@ -81,10 +81,27 @@ python systemd_populate_packages() {
         if not pkg_systemd in packages.split():
             bb.error('%s does not appear in package list, please add it' % pkg_systemd)
 
+    def systemd_path_replace(pkg, old, new):
+         services = d.getVar('SYSTEMD_SERVICE_' + pkg, True)
+         if services:
+             services = services.replace(old, new)
+             d.setVar('SYSTEMD_SERVICE_' + pkg, services
+
+    def systemd_path_unescape(pkg):
+        systemd_path_replace(pkg, r'\\x2d', r'\x2d')
+
+    def systemd_path_escape(pkg):
+        systemd_path_replace(pkg, r'\x2d', r'\\x2d')
 
     def systemd_generate_package_scripts(pkg):
         bb.debug(1, 'adding systemd calls to postinst/postrm for %s' % pkg)
 
+        # Some systemd config files may have paths that contain escape
+        # characters, but systemctl will fail to install these units if
+        # paths are not escaped, so tweak these paths during generation
+        # of prerm and postinst scripts
+        systemd_path_escape(pkg)
+
         # Add pkg to the overrides so that it finds the SYSTEMD_SERVICE_pkg
         # variable.
         localdata = d.createCopy()
@@ -102,6 +119,8 @@ python systemd_populate_packages() {
         prerm += localdata.getVar('systemd_prerm')
         d.setVar('pkg_prerm_%s' % pkg, prerm)
 
+        systemd_path_unescape(pkg)
+
 
     # Add files to FILES_*-systemd if existent and not already done
     def systemd_append_file(pkg_systemd, file_append):
@@ -130,7 +149,7 @@ python systemd_populate_packages() {
                 systemd_add_files_and_parse(pkg_systemd, path, service_base + '@.service', keys)
             for key in keys.split():
                 # recurse all dependencies found in keys ('Also';'Conflicts';..) and add to files
-                cmd = "grep %s %s | sed 's,%s=,,g' | tr ',' '\\n'" % (key, fullpath, key)
+                cmd = "grep %s '%s' | sed 's,%s=,,g' | tr ',' '\\n'" % (key, fullpath, key)
                 pipe = os.popen(cmd, 'r')
                 line = pipe.readline()
                 while line:


On Thu, Jun 21, 2018 at 10:35:56PM -0400, Damien Riegel wrote:
> Hi,
> 
> On Fri, Jun 22, 2018 at 09:29:37AM +0800, ChenQi wrote:
> > Hi Damien,
> > 
> > If some recipe has more than one unit file, this patch will cause problem
> > for it.
> 
> That's a good point, let me see if I can come up with a solution that
> addresses that.
> 
> Regards,
> -- 
> Damien
> 
> > 
> > Best Regards,
> > Chen Qi
> > 
> > 
> > On 06/22/2018 04:35 AM, Damien Riegel wrote:
> > > Systemd mount configuration file must have a name that match the mount
> > > point directory they control. So for instance, if a mount file contains
> > > 
> > >      [Mount]
> > >      ...
> > >      Where=/mnt/my-data
> > > 
> > > The file must be named `mnt-my\x2ddata.mount`, or systemd will refuse to
> > > honour it.
> > > 
> > > If this config file contains an [Install] section, it will silently fail
> > > because the unit file is not quoted when systemctl is called. To fix
> > > that, quote `${SYSTEMD_SERVICE}` in prerm and postinst hooks.
> > > 
> > > Signed-off-by: Damien Riegel <damien.riegel at savoirfairelinux.com>
> > > ---
> > >   meta/classes/systemd.bbclass | 8 ++++----
> > >   1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/meta/classes/systemd.bbclass b/meta/classes/systemd.bbclass
> > > index 1b134322fb..589e73c855 100644
> > > --- a/meta/classes/systemd.bbclass
> > > +++ b/meta/classes/systemd.bbclass
> > > @@ -34,10 +34,10 @@ if type systemctl >/dev/null 2>/dev/null; then
> > >   		systemctl daemon-reload
> > >   	fi
> > > -	systemctl $OPTS ${SYSTEMD_AUTO_ENABLE} ${SYSTEMD_SERVICE}
> > > +	systemctl $OPTS ${SYSTEMD_AUTO_ENABLE} "${SYSTEMD_SERVICE}"
> > >   	if [ -z "$D" -a "${SYSTEMD_AUTO_ENABLE}" = "enable" ]; then
> > > -		systemctl --no-block restart ${SYSTEMD_SERVICE}
> > > +		systemctl --no-block restart "${SYSTEMD_SERVICE}"
> > >   	fi
> > >   fi
> > >   }
> > > @@ -51,10 +51,10 @@ fi
> > >   if type systemctl >/dev/null 2>/dev/null; then
> > >   	if [ -z "$D" ]; then
> > > -		systemctl stop ${SYSTEMD_SERVICE}
> > > +		systemctl stop "${SYSTEMD_SERVICE}"
> > >   	fi
> > > -	systemctl $OPTS disable ${SYSTEMD_SERVICE}
> > > +	systemctl $OPTS disable "${SYSTEMD_SERVICE}"
> > >   fi
> > >   }
> > 
> > 
> > -- 
> > _______________________________________________
> > Openembedded-core mailing list
> > Openembedded-core at lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-core
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core at lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core