From 72345f551620019f13f1b5710e82dd145f73263c Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Thu, 30 Jun 2016 20:31:01 +0200 Subject: [PATCH 1/3] Make union of existing and new requirements instead of conflict error. --- cdist/emulator.py | 60 +++++++++++++++++++++------------ cdist/test/emulator/__init__.py | 43 ++++++++++++++++++++--- 2 files changed, 77 insertions(+), 26 deletions(-) diff --git a/cdist/emulator.py b/cdist/emulator.py index f5a9f645..d526bf77 100644 --- a/cdist/emulator.py +++ b/cdist/emulator.py @@ -201,6 +201,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""" @@ -228,24 +256,8 @@ class Emulator(object): for requirement in requirements.split(" "): # Ignore empty fields - probably the only field anyway if len(requirement) == 0: continue - - # 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) + req = self.record_requirement(requirement) + reqs.add(req) 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)) @@ -253,15 +265,19 @@ class Emulator(object): self.log.debug("REQS: {}".format(reqs)) if self._existing_reqs != reqs: - errmsg = ("Object {} already exists with conflicting " - "requirements:\n{}: {}\n{}: {}".format( + dbgmsg = ("Object {} already exists with different " + "requirements:\n{}: {}\n{}: {}. Merging sets.".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) + self.log.debug(dbgmsg) + all_reqs = reqs | self._existing_reqs + self.log.debug("All requirements: {}".format(all_reqs)) + for x in all_reqs: + if not x in self.cdist_object.requirements: + self.record_requirement(x) def record_auto_requirements(self): diff --git a/cdist/test/emulator/__init__.py b/cdist/test/emulator/__init__.py index b91c1e8f..ee6c3cea 100644 --- a/cdist/test/emulator/__init__.py +++ b/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): From 92d96c14b936502a3fdb2c53ece8a5250e076c21 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Mon, 4 Jul 2016 10:11:11 +0200 Subject: [PATCH 2/3] Undo reqs conflict detection, continue appending new reqs. --- cdist/emulator.py | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/cdist/emulator.py b/cdist/emulator.py index d526bf77..0e4a41c4 100644 --- a/cdist/emulator.py +++ b/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, @@ -249,35 +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 - req = self.record_requirement(requirement) - reqs.add(req) - 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: - dbgmsg = ("Object {} already exists with different " - "requirements:\n{}: {}\n{}: {}. Merging sets.".format( - self.cdist_object.name, - " ".join(self.cdist_object.source), - self._existing_reqs, - self.object_source, - reqs)) - self.log.debug(dbgmsg) - all_reqs = reqs | self._existing_reqs - self.log.debug("All requirements: {}".format(all_reqs)) - for x in all_reqs: - if not x in self.cdist_object.requirements: - self.record_requirement(x) + self.record_requirement(requirement) def record_auto_requirements(self): From 43ef5ec1ae664d5b51ec3ed61fb83564ace65b28 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Mon, 4 Jul 2016 10:14:19 +0200 Subject: [PATCH 3/3] Remove conflicting reqs changelog. --- docs/changelog | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/changelog b/docs/changelog index 8e701bdd..f1aead98 100644 --- a/docs/changelog +++ b/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)