diff --git a/diskimage_builder/element_dependencies.py b/diskimage_builder/element_dependencies.py index dfae210f..bc6aedbe 100644 --- a/diskimage_builder/element_dependencies.py +++ b/diskimage_builder/element_dependencies.py @@ -26,6 +26,22 @@ import diskimage_builder.logging_config logger = logging.getLogger(__name__) +class MissingElementException(Exception): + pass + + +class AlreadyProvidedException(Exception): + pass + + +class MissingOSException(Exception): + pass + + +class InvalidElementDir(Exception): + pass + + class Element(object): """An element""" def __init__(self, name, path): @@ -63,19 +79,25 @@ class Element(object): logger.debug("New element : %s", str(self)) + def __eq__(self, other): + return self.name == other.name + + def __repr__(self): + return self.name + def __str__(self): return '%s p:<%s> d:<%s>' % (self.name, ','.join(self.provides), ','.join(self.depends)) -def get_elements_dir(): +def _get_elements_dir(): if not os.environ.get('ELEMENTS_PATH'): raise Exception("$ELEMENTS_PATH must be set.") return os.environ['ELEMENTS_PATH'] -def expand_dependencies(user_elements, all_elements): +def _expand_element_dependencies(user_elements, all_elements): """Expand user requested elements using element-deps files. Arguments: @@ -97,8 +119,7 @@ def expand_dependencies(user_elements, all_elements): if element in provided: continue elif element not in all_elements: - logger.error("Element '%s' not found", element) - sys.exit(1) + raise MissingElementException("Element '%s' not found" % element) element_obj = all_elements[element] @@ -112,11 +133,6 @@ def expand_dependencies(user_elements, all_elements): check_queue.extend(element_deps - (final_elements | provided)) final_elements.update(element_deps) - if "operating-system" not in provided: - logger.error( - "Please include an operating system element.") - sys.exit(-1) - conflicts = set(user_elements) & provided if conflicts: logger.error( @@ -124,12 +140,16 @@ def expand_dependencies(user_elements, all_elements): for element in conflicts: logger.error("%s : already provided by %s" % (element, provided_by[element])) - sys.exit(-1) + raise AlreadyProvidedException() - return final_elements - provided + if "operating-system" not in provided: + raise MissingOSException("Please include an operating system element") + + out = final_elements - provided + return([all_elements[element] for element in out]) -def find_all_elements(paths=None): +def _find_all_elements(paths=None): """Build a dictionary Element() objects Walk ELEMENTS_PATH and find all elements. Make an Element object @@ -138,10 +158,9 @@ def find_all_elements(paths=None): those seen later. :param paths: A list of paths to find elements in. If None will - use ELEMENTS_PATH + use ELEMENTS_PATH from environment :return: a dictionary of all elements - """ all_elements = {} @@ -151,11 +170,16 @@ def find_all_elements(paths=None): # ELEMENTS_PATH=path1:path2:path3 # we want the elements in "path1" to override "path3" if not paths: - paths = reversed(get_elements_dir().split(':')) + paths = reversed(_get_elements_dir().split(':')) + else: + paths = reversed(paths.split(':')) + + logger.debug("ELEMENTS_PATH is: %s" % paths) + for path in paths: if not os.path.isdir(path): - logger.error("ELEMENT_PATH entry '%s' is not a directory", path) - sys.exit(1) + raise InvalidElementDir("ELEMENTS_PATH entry '%s' " + "is not a directory " % path) # In words : make a list of directories in "path". Since an # element is a directory, this is our list of elements. @@ -179,6 +203,93 @@ def find_all_elements(paths=None): return all_elements +def _get_elements(elements, paths=None): + """Return the canonical list of Element objects + + This function returns Element objects. For exernal calls, use + get_elements which returns a simple tuple & list. + + :param elements: user specified list of elements + :param paths: element paths, default to environment + + """ + all_elements = _find_all_elements(paths) + return _expand_element_dependencies(elements, all_elements) + + +def get_elements(elements, paths=None): + """Return the canonical list of elements with their dependencies + + .. note:: + + You probably do not want to use this! Elements that require + access to the list of all other elements should generally use + the environment variables exported by disk-image-create below. + + :param elements: user specified elements + :param paths: Alternative ELEMENTS_PATH; default is to use from env + + :return: A de-duplicated list of tuples [(element, path), + (element, path) ...] with all elements and their + dependents, including any transitive dependencies. + """ + + elements = _get_elements(elements, paths) + return [(element.name, element.path) for element in elements] + + +def expand_dependencies(user_elements, element_dirs): + """Deprecated method for expanding element dependencies. + + .. warning:: + + DO NOT USE THIS FUNCTION. For compatability reasons, this + function does not provide paths to the returned elements. This + means the caller must process override rules if two elements + with the same name appear in element_dirs + + :param user_elements: iterable enumerating the elements a user requested + :param elements_dir: The ELEMENTS_PATH to process + + :return: a set contatining user_elements and all dependent + elements including any transitive dependencies. + """ + logger.warning("expand_dependencies() deprecated, use get_elements") + elements = _get_elements(user_elements, element_dirs) + return set([element.name for element in elements]) + + +def _output_env_vars(elements): + """Output eval-able bash strings for IMAGE_ELEMENT vars + + :param elements: list of Element objects to represent + """ + # first the "legacy" environment variable that just lists the + # elements + print("export IMAGE_ELEMENT='%s'" % + ' '.join([element.name for element in elements])) + + # Then YAML + output = {} + for element in elements: + output[element.name] = element.path + print("export IMAGE_ELEMENT_YAML='%s'" % yaml.safe_dump(output)) + + # Then bash array. Unfortunately, bash can't export array + # variables. So we take a compromise and produce an exported + # function that outputs the string to re-create the array. + # You can then simply do + # eval declare -A element_array=$(get_image_element_array) + # and you have it. + output = "" + for element in elements: + output += '[%s]=%s ' % (element.name, element.path) + print("function get_image_element_array {\n" + " echo \"%s\"\n" + "};\n" + "export -f get_image_element_array;" % output) + + def main(): diskimage_builder.logging_config.setup() @@ -192,36 +303,13 @@ def main(): args = parser.parse_args(sys.argv[1:]) - all_elements = find_all_elements() - - elements = expand_dependencies(args.elements, all_elements) + elements = _get_elements(args.elements) if args.env: - # first the "legacy" environment variable that just lists the - # elements - print("export IMAGE_ELEMENT='%s'" % ' '.join(elements)) - - # Then YAML - output = {} - for element in elements: - output[element] = all_elements[element].path - print("export IMAGE_ELEMENT_YAML='%s'" % yaml.safe_dump(output)) - - # Then bash array. Unfortunately, bash can't export array - # variables. So we take a compromise and produce an exported - # function that outputs the string to re-create the array. - # You can then simply do - # eval declare -A element_array=$(get_image_element_array) - # and you have it. - output = "" - for element in elements: - output += '[%s]=%s ' % (element, all_elements[element].path) - print("function get_image_element_array {\n" - " echo \"%s\"\n" - "};\n" - "export -f get_image_element_array;" % output) + _output_env_vars(elements) else: - print(' '.join(elements)) + # deprecated compatability output; doesn't include paths. + print(' '.join([element.name for element in elements])) return 0 diff --git a/diskimage_builder/tests/test_elementdeps.py b/diskimage_builder/tests/test_elementdeps.py index ef747397..0c263acf 100644 --- a/diskimage_builder/tests/test_elementdeps.py +++ b/diskimage_builder/tests/test_elementdeps.py @@ -20,11 +20,14 @@ import testtools from diskimage_builder import element_dependencies +logger = logging.getLogger(__name__) + data_dir = os.path.abspath( os.path.join(os.path.dirname(__file__), 'test-elements')) def _populate_element(element_dir, element_name, element_deps=[], provides=[]): + logger.debug("Populate %s <%s>" % (element_name, element_dir)) element_home = os.path.join(element_dir, element_name) os.mkdir(element_home) deps_path = os.path.join(element_home, 'element-deps') @@ -85,75 +88,96 @@ class TestElementDeps(testtools.TestCase): _populate_element(self.element_dir, 'override_element', []) _populate_element(self.element_override_dir, 'override_element', []) - self.all_elements = element_dependencies.find_all_elements( - [self.element_dir, self.element_override_dir]) + # This simulates $ELEMENTS_PATH + self.element_dirs = "%s:%s" % (self.element_override_dir, + self.element_dir) + + # helper to return an (element, path) tuple from the standard dir + def _e(self, element): + return (element, os.path.join(self.element_dir, element)) + + # helper to return an (element, path) tuple from the override dir + def _eo(self, element): + return (element, os.path.join(self.element_override_dir, element)) def test_non_transitive_deps(self): - result = element_dependencies.expand_dependencies( - ['requires-foo'], self.all_elements) - self.assertEqual(set(['requires-foo', 'foo']), result) + result = element_dependencies.get_elements(['requires-foo'], + self.element_dirs) + self.assertItemsEqual([self._e('foo'), self._e('requires-foo')], + result) def test_missing_deps(self): - self.assertRaises(SystemExit, - element_dependencies.expand_dependencies, ['fake'], - self.all_elements) - self.assertIn("Element 'fake' not found", - self.log_fixture.output) + e = self.assertRaises(element_dependencies.MissingElementException, + element_dependencies.get_elements, + ['fake'], + self.element_dirs) + self.assertIn("Element 'fake' not found", str(e)) + + def test_invalid_element_dir(self): + e = self.assertRaises(element_dependencies.InvalidElementDir, + element_dependencies.get_elements, + ['fake'], + self.element_dirs + ":/not/a/dir") + self.assertIn("ELEMENTS_PATH entry '/not/a/dir' is not a directory", + str(e)) def test_transitive_deps(self): - result = element_dependencies.expand_dependencies( - ['requires-requires-foo'], self.all_elements) - self.assertEqual(set(['requires-requires-foo', - 'requires-foo', - 'foo']), result) + result = element_dependencies.get_elements( + ['requires-requires-foo'], self.element_dirs) + + self.assertItemsEqual([self._e('requires-requires-foo'), + self._e('requires-foo'), + self._e('foo')], result) def test_no_deps(self): - result = element_dependencies.expand_dependencies( - ['foo'], self.all_elements) - self.assertEqual(set(['foo']), result) + result = element_dependencies.get_elements(['foo'], self.element_dirs) + self.assertEqual([self._e('foo')], result) def test_self(self): - result = element_dependencies.expand_dependencies( - ['self', 'foo'], self.all_elements) - self.assertEqual(set(['self', 'foo']), result) + result = element_dependencies.get_elements(['self', 'foo'], + self.element_dirs) + self.assertItemsEqual([self._e('self'), + self._e('foo')], result) def test_circular(self): - result = element_dependencies.expand_dependencies( - ['circular1'], self.all_elements) - self.assertEqual(set(['circular1', 'circular2']), result) + result = element_dependencies.get_elements(['circular1'], + self.element_dirs) + self.assertItemsEqual([self._e('circular1'), + self._e('circular2')], result) def test_provide(self): - result = element_dependencies.expand_dependencies( - ['provides_virtual', 'requires_virtual'], - self.all_elements) - self.assertEqual(set(['requires_virtual', 'provides_virtual']), result) + result = element_dependencies.get_elements( + ['provides_virtual', 'requires_virtual'], + self.element_dirs) + self.assertItemsEqual([self._e('requires_virtual'), + self._e('provides_virtual')], result) def test_provide_conflict(self): - self.assertRaises(SystemExit, - element_dependencies.expand_dependencies, - ['virtual', 'provides_virtual'], - self.element_dir) + self.assertRaises(element_dependencies.AlreadyProvidedException, + element_dependencies.get_elements, + ['virtual', 'provides_virtual'], + self.element_dirs) def test_provide_virtual_ordering(self): - result = element_dependencies.expand_dependencies( - ['requires_new_virtual', 'provides_new_virtual'], - self.all_elements) - self.assertEqual(set(['requires_new_virtual', 'provides_new_virtual']), - result) + result = element_dependencies.get_elements( + ['requires_new_virtual', 'provides_new_virtual'], + self.element_dirs) + self.assertItemsEqual( + [self._e('requires_new_virtual'), + self._e('provides_new_virtual')], result) def test_no_os_element(self): - self.assertRaises(SystemExit, - element_dependencies.expand_dependencies, + self.assertRaises(element_dependencies.MissingOSException, + element_dependencies.get_elements, ['provides_virtual'], - self.all_elements) - self.assertIn("Please include an operating system element", - self.log_fixture.output) + self.element_dirs) def test_duplicated_os_passed_as_element(self): - self.assertRaises(SystemExit, - element_dependencies.expand_dependencies, - ['circular1', 'operating-system'], - self.all_elements) + self.assertRaises( + element_dependencies.AlreadyProvidedException, + element_dependencies.get_elements, + ['circular1', 'operating-system'], + self.element_dirs) # ensure we get the error message about what's providing the # conflicting package self.assertIn("operating-system : already provided by ['circular1']", @@ -162,18 +186,33 @@ class TestElementDeps(testtools.TestCase): def test_element_override(self): # make sure we picked up "override_element" from the override dir, # not the base dir - self.assertTrue('override_element' in self.all_elements) - self.assertEqual(os.path.join(self.element_override_dir, - 'override_element'), - self.all_elements['override_element'].path) + result = element_dependencies.get_elements(['override_element', 'foo'], + self.element_dirs) + self.assertItemsEqual([self._e('foo'), + self._eo('override_element')], + result) + + def test_expand_dependencies_deprecated(self): + # test the deprecated expand_dependencies call + result = element_dependencies.expand_dependencies( + ['foo', 'requires-foo'], self.element_dirs) + self.assertItemsEqual(['foo', 'requires-foo'], result) + + def test_output_sanity(self): + # very basic output sanity test + elements = element_dependencies._get_elements(['foo', 'requires-foo'], + self.element_dirs) + element_dependencies._output_env_vars(elements) class TestElements(testtools.TestCase): def test_depends_on_env(self): self.useFixture( fixtures.EnvironmentVariable('ELEMENTS_PATH', '/foo/bar')) - self.assertEqual('/foo/bar', element_dependencies.get_elements_dir()) + self.assertEqual('/foo/bar', + element_dependencies._get_elements_dir()) def test_env_not_set(self): self.useFixture(fixtures.EnvironmentVariable('ELEMENTS_PATH', '')) - self.assertRaises(Exception, element_dependencies.get_elements_dir, ()) + self.assertRaises(Exception, + element_dependencies._get_elements_dir, ()) diff --git a/tox.ini b/tox.ini index d4746467..67b309e4 100644 --- a/tox.ini +++ b/tox.ini @@ -36,5 +36,5 @@ commands = python setup.py build_sphinx commands = sphinx-build -a -W -E -d releasenotes/build/doctrees -b html releasenotes/source releasenotes/build/html [flake8] -ignore = E125,H202,H302,H803 +ignore = E126,E127,E125,H202,H302,H803 exclude = .venv,.git,.tox,dist,doc,*lib/python*,*egg,build,conf.py