sysv-generator: Replace Provides: symlinks with real units

Since commit b7e7184 the SysV generator creates symlinks for all "Provides:" in
the LSB header. However, this is too greedy; there are cases where the
creation of a unit .service file fails because of an already existing
symlink with the same name:

 - Backup files such as /etc/init.d/foo.bak still have "Provides: foo", and
   thus get a foo.service -> foo.bak.service link. foo.bak would not be enabled
   in rcN.d/, but we (deliberately) create units for all executables in init.d/
   so that a manual "systemctl start" works. If foo.bak is processed before,
   the symlink already exists.

 - init.d/bar has "Provides: foo", while there also is a real init.d/foo. The
   former would create a link foo.service -> bar.service, while the latter
   would fail to create the real foo.service.

If we encounter an existing symlink, just remove it before writing a real unit.

Note that two init.d scripts "foo" and "bar" which both provide the same name
"common" already work. The first processed init script wins and creates the
"common.service" symlink, and the second just fails to create the symlink
again. Thus create an additional test case for this to ensure that it keeps
working sensibly.

https://bugs.debian.org/775404

diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c
index a47b0722e0aa50a5afe894dfff2c7e669696dadc..3279c7f44482ef6f0e2ead2a17b25e4de985df8f 100644 (file)
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -147,6 +147,7 @@ static int generate_unit_file(SysvStub *s) {
         _cleanup_free_ char *wants = NULL;
         _cleanup_free_ char *conflicts = NULL;
         int r;
         _cleanup_free_ char *wants = NULL;
         _cleanup_free_ char *conflicts = NULL;
         int r;

+        struct stat st;

 
         before = strv_join(s->before, " ");
         if (!before)
 
         before = strv_join(s->before, " ");
         if (!before)


@@ -168,6 +169,14 @@ static int generate_unit_file(SysvStub *s) {
         if (!unit)
                 return log_oom();
 
         if (!unit)
                 return log_oom();
 

+        /* We might already have a symlink with the same name from a Provides:,
+         * or from backup files like /etc/init.d/foo.bak. Real scripts always win,
+         * so remove an existing link */
+        if (lstat(unit, &st) == 0 && S_ISLNK(st.st_mode)) {
+                log_warning("Overwriting existing symlink %s with real service", unit);
+                unlink(unit);
+        }
+

         f = fopen(unit, "wxe");
         if (!f)
                 return log_error_errno(errno, "Failed to create unit file %s: %m", unit);
         f = fopen(unit, "wxe");
         if (!f)
                 return log_error_errno(errno, "Failed to create unit file %s: %m", unit);


@@ -343,6 +352,8 @@ static int load_sysv(SysvStub *s) {
         if (!f)
                 return errno == ENOENT ? 0 : -errno;
 
         if (!f)
                 return errno == ENOENT ? 0 : -errno;
 

+        log_debug("Loading SysV script %s", s->path);
+

         while (!feof(f)) {
                 char l[LINE_MAX], *t;
 
         while (!feof(f)) {
                 char l[LINE_MAX], *t;
 


@@ -492,6 +503,7 @@ static int load_sysv(SysvStub *s) {
                                                 continue;
 
                                         if (unit_name_to_type(m) == UNIT_SERVICE) {
                                                 continue;
 
                                         if (unit_name_to_type(m) == UNIT_SERVICE) {

+                                                log_debug("Adding Provides: alias '%s' for '%s'", m, s->name);

                                                 r = add_alias(s->name, m);
                                         } else {
                                                 /* NB: SysV targets
                                                 r = add_alias(s->name, m);
                                         } else {
                                                 /* NB: SysV targets

diff --git a/test/sysv-generator-test.py b/test/sysv-generator-test.py
index df13c98e299012ac69d5c829fd2c7fa59ef2e648..5098519073fe461e75a6e17fff669c570cdd7b72 100644 (file)
--- a/test/sysv-generator-test.py
+++ b/test/sysv-generator-test.py
@@ -271,6 +271,25 @@ class SysvGeneratorTest(unittest.TestCase):
             self.assertEqual(os.readlink(os.path.join(self.out_dir, f)),
                              'foo.service')
 
             self.assertEqual(os.readlink(os.path.join(self.out_dir, f)),
                              'foo.service')
 

+    def test_same_provides_in_multiple_scripts(self):
+        '''multiple init.d scripts provide the same name'''
+
+        self.add_sysv('foo', {'Provides': 'foo common'}, enable=True, prio=1)
+        self.add_sysv('bar', {'Provides': 'bar common'}, enable=True, prio=2)
+        err, results = self.run_generator()
+        self.assertEqual(sorted(results), ['bar.service', 'foo.service'])
+        # should create symlink for the alternative name for either unit
+        self.assertIn(os.readlink(os.path.join(self.out_dir, 'common.service')),
+                      ['foo.service', 'bar.service'])
+
+    def test_provide_other_script(self):
+        '''init.d scripts provides the name of another init.d script'''
+
+        self.add_sysv('foo', {'Provides': 'foo bar'}, enable=True)
+        self.add_sysv('bar', {'Provides': 'bar'}, enable=True)
+        err, results = self.run_generator()
+        self.assertEqual(sorted(results), ['bar.service', 'foo.service'])
+

     def test_nonexecutable_script(self):
         '''ignores non-executable init.d script'''
 
     def test_nonexecutable_script(self):
         '''ignores non-executable init.d script'''
 


@@ -328,6 +347,26 @@ class SysvGeneratorTest(unittest.TestCase):
 
         self.assert_enabled('foo.service', [2, 3, 4, 5])
 
 
         self.assert_enabled('foo.service', [2, 3, 4, 5])
 

+    def test_backup_file(self):
+        '''init.d script with backup file'''
+
+        script = self.add_sysv('foo', {}, enable=True)
+        # backup files (not enabled in rcN.d/)
+        shutil.copy(script, script + '.bak')
+        shutil.copy(script, script + '.old')
+
+        err, results = self.run_generator()
+        print(err)
+        self.assertEqual(sorted(results),
+                         ['foo.bak.service', 'foo.old.service', 'foo.service'])
+
+        # ensure we don't try to create a symlink to itself
+        self.assertNotIn(err, 'itself')
+
+        self.assert_enabled('foo.service', [2, 3, 4, 5])
+        self.assert_enabled('foo.bak.service', [])
+        self.assert_enabled('foo.old.service', [])
+

 
 if __name__ == '__main__':
     unittest.main(testRunner=unittest.TextTestRunner(stream=sys.stdout, verbosity=2))
 
 if __name__ == '__main__':
     unittest.main(testRunner=unittest.TextTestRunner(stream=sys.stdout, verbosity=2))