On Wed, Jan 21, 2015 at 10:46:03AM +0100, Martin Pitt wrote: > Keeping track of which alias symlinks we actually want is error prone, and > restricting the creation of services for enabled init.d scripts would reduce > the utility of the generator (for manual starting disabled init.d scripts) as > well as not cover the second case. So if we encounter an existing symlink, > just > remove it before writing a real unit. Looks fine. Although the code is clearer than the description :)
> 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 > --- > src/sysv-generator/sysv-generator.c | 12 ++++++++++++ > test/sysv-generator-test.py | 38 > +++++++++++++++++++++++++++++++++++++ > 2 files changed, 50 insertions(+) > > diff --git a/src/sysv-generator/sysv-generator.c > b/src/sysv-generator/sysv-generator.c > index a47b072..fd3ee20 100644 > --- 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; > + struct stat st; > > before = strv_join(s->before, " "); > if (!before) > @@ -168,6 +169,14 @@ static int generate_unit_file(SysvStub *s) { > 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); > @@ -343,6 +352,8 @@ static int load_sysv(SysvStub *s) { > if (!f) > return errno == ENOENT ? 0 : -errno; > > + log_debug("loading SysV script %s", s->path); Capital "L"? > + > 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) { > + log_debug("Adding Provides: > alias '%s' for '%s'", m, s->name); > 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 a3daa9f..63a10ec 100644 > --- 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') > > + 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''' > > @@ -313,6 +332,25 @@ class SysvGeneratorTest(unittest.TestCase): > self.assertEqual(os.readlink(os.path.join(self.out_dir, > 'bar.service')), > 'foo.service') > > + 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', []) Looks fine from my POV. Zbyszek _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel