Hey all,

the recent fix for sysv-generator's Provides: handling [1] caused, or
rather uncovered, another bug which now creates symlinks to itself
"foo.service -> foo.service" for any /etc/init.d/foo.sh.

The generator would output an error message like

  Failed to create unit file <path...>/foo.service: File exists

instead of creating the actual foo.service file. I. e. this completely
breaks translating init scripts with .sh.

Fix with corresponding test case attached. This is a test case for the
test suite I sent in my previous mail; that might still need some
masssaging, so if you are ok with this fix, I'll commit that without
the test case, and add the test case to the suite separately.

Martin

[1] http://cgit.freedesktop.org/systemd/systemd/commit/?id=b7e7184634d5
-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
From cbd7f20422f044ba192d11afe2d3a29bcf7a5f69 Mon Sep 17 00:00:00 2001
From: Martin Pitt <martin.p...@ubuntu.com>
Date: Tue, 20 Jan 2015 16:41:31 +0100
Subject: [PATCH 2/2] sysv-generator: Handle .sh suffixes when translating
 Provides:

When deciding whether the provided name equals the file name in
sysv_translate_facility(), also consider them equal if the file name has a
".sh" suffix.

This was uncovered by commit b7e7184 which then created a symlink
"<name>.service" to itself for ".sh" suffixed init.d scripts.

For additional robustness, refuse to create symlinks to itself in add_alias().
---
 src/sysv-generator/sysv-generator.c | 16 +++++++++++++++-
 test/sysv-generator-test.py         | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
index 4774981..6334fd3 100644
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -119,6 +119,11 @@ static int add_alias(const char *service, const char *alias) {
         assert(service);
         assert(alias);
 
+        if (streq(service, alias)) {
+                log_error("Ignoring creation of an alias %s for itself", service);
+                return 0;
+        }
+
         link = strjoin(arg_dest, "/", alias, NULL);
         if (!link)
                 return log_oom();
@@ -263,6 +268,7 @@ static int sysv_translate_facility(const char *name, const char *filename, char
         unsigned i;
         char *r;
         const char *n;
+        _cleanup_free_ char *filename_no_sh = NULL;
 
         assert(name);
         assert(_r);
@@ -284,6 +290,14 @@ static int sysv_translate_facility(const char *name, const char *filename, char
                 goto finish;
         }
 
+        /* strip ".sh" suffix from file name for comparison */
+        filename_no_sh = strdup(filename);
+        if (!filename_no_sh)
+                return -ENOMEM;
+        if (endswith(filename, ".sh"))
+                filename_no_sh[strlen(filename)-3] = '\0';
+        log_debug("sysv_translate_facility: translating %s fname %s no_sh %s", name, filename, filename_no_sh);
+
         /* If we don't know this name, fallback heuristics to figure
          * out whether something is a target or a service alias. */
 
@@ -293,7 +307,7 @@ static int sysv_translate_facility(const char *name, const char *filename, char
 
                 /* Facilities starting with $ are most likely targets */
                 r = unit_name_build(n, NULL, ".target");
-        } else if (filename && streq(name, filename))
+        } else if (filename && streq(name, filename_no_sh))
                 /* Names equaling the file name of the services are redundant */
                 return 0;
         else
diff --git a/test/sysv-generator-test.py b/test/sysv-generator-test.py
index e281a7f..53d729a 100644
--- a/test/sysv-generator-test.py
+++ b/test/sysv-generator-test.py
@@ -172,6 +172,43 @@ class SysvGeneratorTest(unittest.TestCase):
             self.assertEqual(os.readlink(os.path.join(self.out_dir, f)),
                              'foo.service')
 
+    def test_sh_suffix(self):
+        '''init.d script with .sh suffix'''
+
+        self.add_sysv('foo.sh', {}, enable=True)
+        err, results = self.run_generator()
+        s = results['foo.service']
+
+        self.assertEqual(s.sections(), ['Unit', 'Service'])
+        # should not have a .sh
+        self.assertEqual(s.get('Unit', 'Description'), 'LSB: test foo service')
+
+        # calls correct script with .sh
+        init_script = os.path.join(self.init_d_dir, 'foo.sh')
+        self.assertEqual(s.get('Service', 'ExecStart'),
+                         '%s start' % init_script)
+        self.assertEqual(s.get('Service', 'ExecStop'),
+                         '%s stop' % init_script)
+
+        # should be enabled
+        target = os.readlink(os.path.join(
+            self.out_dir, 'runlevel2.target.wants', 'foo.service'))
+        self.assertTrue(os.path.exists(target))
+        self.assertEqual(os.path.basename(target), 'foo.service')
+
+    def test_sh_suffix_with_provides(self):
+        '''init.d script with .sh suffix and Provides:'''
+
+        self.add_sysv('foo.sh', {'Provides': 'bar'})
+        err, results = self.run_generator()
+        self.assertEqual(sorted(results.keys()), ['bar.service', 'foo.service'])
+        s = results['foo.service']
+
+        # should create symlink for the alternative name
+        self.assertEqual(os.readlink(os.path.join(self.out_dir, 'bar.service')),
+                         'foo.service')
+
+
 
 if __name__ == '__main__':
     unittest.main(testRunner=unittest.TextTestRunner(stream=sys.stdout, verbosity=2))
-- 
2.1.4

Attachment: signature.asc
Description: Digital signature

_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Reply via email to