Merge pull request #445 from darko-poljak/fix-dependency-conflict

Conflicting requirements bugfix.

cdist/emulator.py

@@ -77,6 +77,9 @@ class Emulator(object):
 
         self.type_name      = os.path.basename(argv[0])
         self.cdist_type     = core.CdistType(self.type_base_path, self.type_name)
+        # if set then object already exists and this var holds existing
+        # requirements
+        self._existing_reqs = None
 
         self.__init_log()
 
@@ -150,10 +153,18 @@ class Emulator(object):
                 self.parameters[key] = value
 
         if self.cdist_object.exists and not 'CDIST_OVERRIDE' in self.env:
+            # make existing requirements a set, so we can compare it
+            # later with new requirements
+            self._existing_reqs = set(self.cdist_object.requirements)
             if self.cdist_object.parameters != self.parameters:
-                raise cdist.Error("Object %s already exists with conflicting parameters:\n%s: %s\n%s: %s"
-                    % (self.cdist_object.name, " ".join(self.cdist_object.source), self.cdist_object.parameters, self.object_source, self.parameters)
-            )
+                errmsg = ("Object %s already exists with conflicting "
+                    "parameters:\n%s: %s\n%s: %s" % (self.cdist_object.name,
+                        " ".join(self.cdist_object.source),
+                        self.cdist_object.parameters,
+                        self.object_source,
+                        self.parameters))
+                self.log.error(errmsg)
+                raise cdist.Error(errmsg)
         else:
             if self.cdist_object.exists:
                 self.log.debug('Object %s override forced with CDIST_OVERRIDE',self.cdist_object.name)
@@ -210,7 +221,7 @@ class Emulator(object):
                     # if no second last line, we are on the first type, so do not set a requirement
                     pass
 
-
+        reqs = set()
         if "require" in self.env:
             requirements = self.env['require']
             self.log.debug("reqs = " + requirements)
@@ -234,6 +245,24 @@ class Emulator(object):
                 # (__file//bar => __file/bar)
                 # This ensures pattern matching is done against sanitised list
                 self.cdist_object.requirements.append(cdist_object.name)
+                reqs.add(cdist_object.name)
+        if self._existing_reqs is not None:
+            # if object exists then compare existing and new requirements
+            self.log.debug("OBJ: {} {}".format(self.cdist_type, self.object_id))
+            self.log.debug("EXISTING REQS: {}".format(self._existing_reqs))
+            self.log.debug("REQS: {}".format(reqs))
+
+            if self._existing_reqs != reqs:
+                errmsg = ("Object {} already exists with conflicting "
+                    "requirements:\n{}: {}\n{}: {}".format(
+                        self.cdist_object.name,
+                        " ".join(self.cdist_object.source),
+                        self._existing_reqs,
+                        self.object_source,
+                        reqs))
+                self.log.error(errmsg)
+                raise cdist.Error(errmsg)
+
 
     def record_auto_requirements(self):
         """An object shall automatically depend on all objects that it defined in it's type manifest.

cdist/test/emulator/__init__.py

@@ -133,6 +133,56 @@ class EmulatorTestCase(test.CdistTestCase):
         self.assertEqual(list(file_object.requirements), ['__planet/mars'])
         # if we get here all is fine
 
+class EmulatorConflictingRequirementsTestCase(test.CdistTestCase):
+
+    def setUp(self):
+        self.temp_dir = self.mkdtemp()
+        handle, self.script = self.mkstemp(dir=self.temp_dir)
+        os.close(handle)
+        base_path = self.temp_dir
+
+        self.local = local.Local(
+            target_host=self.target_host,
+            base_path=base_path,
+            exec_path=test.cdist_exec_path,
+            add_conf_dirs=[conf_dir])
+        self.local.create_files_dirs()
+
+        self.manifest = core.Manifest(self.target_host, self.local)
+        self.env = self.manifest.env_initial_manifest(self.script)
+        self.env['__cdist_object_marker'] = self.local.object_marker_name
+
+    def tearDown(self):
+        shutil.rmtree(self.temp_dir)
+
+    def test_object_conflicting_requirements_req_none(self):
+        argv = ['__directory', 'spam']
+        emu = emulator.Emulator(argv, env=self.env)
+        emu.run()
+        argv = ['__file', 'eggs']
+        self.env['require'] = '__directory/spam'
+        emu = emulator.Emulator(argv, env=self.env)
+        emu.run()
+        argv = ['__file', 'eggs']
+        if 'require' in self.env:
+            del self.env['require']
+        emu = emulator.Emulator(argv, env=self.env)
+        self.assertRaises(cdist.Error, emu.run)
+
+    def test_object_conflicting_requirements_none_req(self):
+        argv = ['__directory', 'spam']
+        emu = emulator.Emulator(argv, env=self.env)
+        emu.run()
+        argv = ['__file', 'eggs']
+        if 'require' in self.env:
+            del self.env['require']
+        emu = emulator.Emulator(argv, env=self.env)
+        emu.run()
+        argv = ['__file', 'eggs']
+        self.env['require'] = '__directory/spam'
+        emu = emulator.Emulator(argv, env=self.env)
+        self.assertRaises(cdist.Error, emu.run)
+
 
 class AutoRequireEmulatorTestCase(test.CdistTestCase):
 
@@ -366,3 +416,8 @@ class StdinTestCase(test.CdistTestCase):
             stdin_saved_by_emulator = fd.read()
 
         self.assertEqual(random_string, stdin_saved_by_emulator)
+
+
+if __name__ == '__main__':
+    import unittest
+    unittest.main()

docs/changelog

@@ -2,6 +2,7 @@ Changelog
 ---------
 
 next:
+	* Core: Fix conflicting requirements (Darko Poljak)
 	* Custom: Add bash and zsh completions (Darko Poljak)
 	* Core: Improve error reporting for local and remote run command (Darko Poljak)
 	* New type: __jail_freebsd9: Handle jail management on FreeBSD <= 9.X (Jake Guffey)