Merge pull request #454 from darko-poljak/fix-conflicting-reqs

Revert dependency change (see bd9008794c)

cdist/emulator.py

@@ -77,9 +77,6 @@ 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()
 
@@ -155,7 +152,6 @@ class Emulator(object):
         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:
                 errmsg = ("Object %s already exists with conflicting "
                     "parameters:\n%s: %s\n%s: %s" % (self.cdist_object.name,
@@ -201,6 +197,34 @@ class Emulator(object):
             except EnvironmentError as e:
                 raise cdist.Error('Failed to read from stdin: %s' % e)
 
+
+    def record_requirement(self, requirement):
+        """record requirement and return recorded requirement"""
+
+        # Raises an error, if object cannot be created
+        try:
+            cdist_object = self.cdist_object.object_from_name(requirement)
+        except core.cdist_type.NoSuchTypeError as e:
+            self.log.error(("%s requires object %s, but type %s does not"
+                    " exist. Defined at %s"  % (self.cdist_object.name,
+                        requirement, e.name, self.object_source)))
+            raise
+        except core.cdist_object.MissingObjectIdError as e:
+            self.log.error(("%s requires object %s without object id."
+                " Defined at %s"  % (self.cdist_object.name, requirement,
+                    self.object_source)))
+            raise
+
+        self.log.debug("Recording requirement: %s", requirement)
+
+        # Save the sanitised version, not the user supplied one
+        # (__file//bar => __file/bar)
+        # This ensures pattern matching is done against sanitised list
+        self.cdist_object.requirements.append(cdist_object.name)
+
+        return cdist_object.name
+
+
     def record_requirements(self):
         """record requirements"""
 
@@ -221,47 +245,13 @@ 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)
             for requirement in requirements.split(" "):
                 # Ignore empty fields - probably the only field anyway
                 if len(requirement) == 0: continue
-
+                self.record_requirement(requirement)
-                # Raises an error, if object cannot be created
-                try:
-                    cdist_object = self.cdist_object.object_from_name(requirement)
-                except core.cdist_type.NoSuchTypeError as e:
-                    self.log.error("%s requires object %s, but type %s does not exist. Defined at %s"  % (self.cdist_object.name, requirement, e.name, self.object_source))
-                    raise
-                except core.cdist_object.MissingObjectIdError as e:
-                    self.log.error("%s requires object %s without object id. Defined at %s"  % (self.cdist_object.name, requirement, self.object_source))
-                    raise
-
-                self.log.debug("Recording requirement: %s", requirement)
-
-                # Save the sanitised version, not the user supplied one
-                # (__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):

cdist/test/emulator/__init__.py

@@ -155,7 +155,7 @@ class EmulatorConflictingRequirementsTestCase(test.CdistTestCase):
     def tearDown(self):
         shutil.rmtree(self.temp_dir)
 
-    def test_object_conflicting_requirements_req_none(self):
+    def test_object_different_requirements_req_none(self):
         argv = ['__directory', 'spam']
         emu = emulator.Emulator(argv, env=self.env)
         emu.run()
@@ -167,9 +167,14 @@ class EmulatorConflictingRequirementsTestCase(test.CdistTestCase):
         if 'require' in self.env:
             del self.env['require']
         emu = emulator.Emulator(argv, env=self.env)
-        self.assertRaises(cdist.Error, emu.run)
+        emu.run()
 
-    def test_object_conflicting_requirements_none_req(self):
+        cdist_type = core.CdistType(self.local.type_path, '__file')
+        cdist_object = core.CdistObject(cdist_type, self.local.object_path, self.local.object_marker_name, 'eggs')
+        reqs = set(('__directory/spam',))
+        self.assertEqual(reqs, set(cdist_object.requirements))
+
+    def test_object_different_requirements_none_req(self):
         argv = ['__directory', 'spam']
         emu = emulator.Emulator(argv, env=self.env)
         emu.run()
@@ -181,7 +186,37 @@ class EmulatorConflictingRequirementsTestCase(test.CdistTestCase):
         argv = ['__file', 'eggs']
         self.env['require'] = '__directory/spam'
         emu = emulator.Emulator(argv, env=self.env)
-        self.assertRaises(cdist.Error, emu.run)
+        emu.run()
+
+        cdist_type = core.CdistType(self.local.type_path, '__file')
+        cdist_object = core.CdistObject(cdist_type, self.local.object_path, self.local.object_marker_name, 'eggs')
+        reqs = set(('__directory/spam',))
+        self.assertEqual(reqs, set(cdist_object.requirements))
+
+    def test_object_different_requirements(self):
+        argv = ['__directory', 'spam']
+        emu = emulator.Emulator(argv, env=self.env)
+        emu.run()
+        argv = ['__directory', 'spameggs']
+        emu = emulator.Emulator(argv, env=self.env)
+        emu.run()
+
+        argv = ['__file', 'eggs']
+        if 'require' in self.env:
+            del self.env['require']
+        self.env['require'] = '__directory/spam'
+        emu = emulator.Emulator(argv, env=self.env)
+        emu.run()
+
+        argv = ['__file', 'eggs']
+        self.env['require'] = '__directory/spameggs'
+        emu = emulator.Emulator(argv, env=self.env)
+        emu.run()
+
+        cdist_type = core.CdistType(self.local.type_path, '__file')
+        cdist_object = core.CdistObject(cdist_type, self.local.object_path, self.local.object_marker_name, 'eggs')
+        reqs = set(('__directory/spam', '__directory/spameggs',))
+        self.assertEqual(reqs, set(cdist_object.requirements))
 
 
 class AutoRequireEmulatorTestCase(test.CdistTestCase):

docs/changelog

@@ -4,7 +4,6 @@ Changelog
 next:
 	* Documentation: Restructure and fix and improve docs and manpages (Darko Poljak)
 	* Core: Add files directory for static files (Darko Poljak)
-	* 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)