diff --git a/diskimage_builder/element_dependencies.py b/diskimage_builder/element_dependencies.py index 93ba504d..6c2f6086 100644 --- a/diskimage_builder/element_dependencies.py +++ b/diskimage_builder/element_dependencies.py @@ -15,6 +15,7 @@ from __future__ import print_function import argparse import collections +import errno import logging import os import sys @@ -24,69 +25,64 @@ import diskimage_builder.logging_config logger = logging.getLogger(__name__) +class Element(object): + """An element""" + def __init__(self, name, path): + """A new element + + :param name: The element name + :param path: Full path to element. element-deps and + element-provides files will be parsed + """ + self.name = name + self.path = path + self.provides = set() + self.depends = set() + + # read the provides & depends files for this element into a + # set; if the element has them. + provides = os.path.join(path, 'element-provides') + depends = os.path.join(path, 'element-deps') + try: + with open(provides) as p: + self.provides = set([line.strip() for line in p]) + except IOError as e: + if e.errno == errno.ENOENT: + pass + else: + raise + try: + with open(depends) as d: + self.depends = set([line.strip() for line in d]) + except IOError as e: + if e.errno == errno.ENOENT: + pass + else: + raise + + logger.debug("New element : %s", str(self)) + + def __str__(self): + return '%s p:<%s> d:<%s>' % (self.name, + ','.join(self.provides), + ','.join(self.depends)) + + def get_elements_dir(): if not os.environ.get('ELEMENTS_PATH'): raise Exception("$ELEMENTS_PATH must be set.") return os.environ['ELEMENTS_PATH'] -def _get_set(element, fname, elements_dir=None): - if elements_dir is None: - elements_dir = get_elements_dir() - - for path in elements_dir.split(':'): - element_deps_path = (os.path.join(path, element, fname)) - try: - with open(element_deps_path) as element_deps: - return set([line.strip() for line in element_deps]) - except IOError as e: - if os.path.exists(os.path.join(path, element)) and e.errno == 2: - return set() - if e.errno == 2: - continue - else: - raise - - logger.error("Element '%s' not found in '%s'" % (element, elements_dir)) - sys.exit(-1) - - -def provides(element, elements_dir=None): - """Return the set of elements provided by the specified element. - - :param element: name of a single element - :param elements_dir: the elements dir to read from. If not supplied, - inferred by calling get_elements_dir(). - - :return: a set just containing all elements that the specified element - provides. - """ - return _get_set(element, 'element-provides', elements_dir) - - -def dependencies(element, elements_dir=None): - """Return the non-transitive set of dependencies for a single element. - - :param element: name of a single element - :param elements_dir: the elements dir to read from. If not supplied, - inferred by calling get_elements_dir(). - - :return: a set just containing all elements that the specified element - depends on. - """ - return _get_set(element, 'element-deps', elements_dir) - - -def expand_dependencies(user_elements, elements_dir=None): +def expand_dependencies(user_elements, all_elements): """Expand user requested elements using element-deps files. Arguments: :param user_elements: iterable enumerating the elements a user requested - :param elements_dir: the elements dir to read from. Passed directly to - dependencies() + :param all_elements: Element object dictionary from find_all_elements - :return: a set containing user_elements and all dependent elements - including any transitive dependencies. + :return: a set containing the names of user_elements and all + dependent elements including any transitive dependencies. """ final_elements = set(user_elements) check_queue = collections.deque(user_elements) @@ -99,8 +95,14 @@ def expand_dependencies(user_elements, elements_dir=None): element = check_queue.popleft() if element in provided: continue - element_deps = dependencies(element, elements_dir) - element_provides = provides(element, elements_dir) + elif element not in all_elements: + logger.error("Element '%s' not found", element) + sys.exit(1) + + element_obj = all_elements[element] + + element_deps = element_obj.depends + element_provides = element_obj.provides # save which elements provide another element for potential # error message for provide in element_provides: @@ -122,9 +124,60 @@ def expand_dependencies(user_elements, elements_dir=None): logger.error("%s : already provided by %s" % (element, provided_by[element])) sys.exit(-1) + return final_elements - provided +def find_all_elements(paths=None): + """Build a dictionary Element() objects + + Walk ELEMENTS_PATH and find all elements. Make an Element object + for each element we wish to consider. Note we process overrides + such that elements specified earlier in the ELEMENTS_PATH override + those seen later. + + :param paths: A list of paths to find elements in. If None will + use ELEMENTS_PATH + + :return: a dictionary of all elements + + """ + + all_elements = {} + + # note we process the later entries *first*, so that earlier + # entries will override later ones. i.e. with + # ELEMENTS_PATH=path1:path2:path3 + # we want the elements in "path1" to override "path3" + if not paths: + paths = reversed(get_elements_dir().split(':')) + for path in paths: + if not os.path.isdir(path): + logger.error("ELEMENT_PATH entry '%s' is not a directory", path) + sys.exit(1) + + # In words : make a list of directories in "path". Since an + # element is a directory, this is our list of elements. + elements = [os.path.realpath(os.path.join(path, f)) + for f in os.listdir(path) + if os.path.isdir(os.path.join(path, f))] + + for element in elements: + # the element name is the last part of the full path in + # element (these are all directories, we know that from + # above) + name = os.path.basename(element) + + new_element = Element(name, element) + if name in all_elements: + logger.warning("Element <%s> overrides <%s>", + new_element.path, all_elements[name].path) + + all_elements[name] = new_element + + return all_elements + + def main(argv): diskimage_builder.logging_config.setup() @@ -138,9 +191,11 @@ def main(argv): args = parser.parse_args(argv[1:]) + all_elements = find_all_elements() + if args.expand_dependencies: logger.warning("expand-dependencies flag is deprecated, " "and is now on by default.", file=sys.stderr) - print(' '.join(expand_dependencies(args.elements))) + print(' '.join(expand_dependencies(args.elements, all_elements))) return 0 diff --git a/diskimage_builder/tests/test_elementdeps.py b/diskimage_builder/tests/test_elementdeps.py index c2f20bcd..ef747397 100644 --- a/diskimage_builder/tests/test_elementdeps.py +++ b/diskimage_builder/tests/test_elementdeps.py @@ -39,7 +39,14 @@ class TestElementDeps(testtools.TestCase): def setUp(self): super(TestElementDeps, self).setUp() - self.element_dir = self.useFixture(fixtures.TempDir()).path + self.element_root_dir = self.useFixture(fixtures.TempDir()).path + + self.element_dir = os.path.join(self.element_root_dir, 'elements') + self.element_override_dir = os.path.join(self.element_root_dir, + 'element-override') + os.mkdir(self.element_dir) + os.mkdir(self.element_override_dir) + self.log_fixture = self.useFixture( fixtures.FakeLogger(level=logging.DEBUG)) _populate_element(self.element_dir, 'requires-foo', ['foo']) @@ -74,45 +81,51 @@ class TestElementDeps(testtools.TestCase): 'requires_new_virtual', ['new_virtual']) + # second element should override the first one here + _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]) + def test_non_transitive_deps(self): result = element_dependencies.expand_dependencies( - ['requires-foo'], - elements_dir=self.element_dir) + ['requires-foo'], self.all_elements) self.assertEqual(set(['requires-foo', 'foo']), result) def test_missing_deps(self): self.assertRaises(SystemExit, element_dependencies.expand_dependencies, ['fake'], - self.element_dir) + self.all_elements) self.assertIn("Element 'fake' not found", self.log_fixture.output) def test_transitive_deps(self): result = element_dependencies.expand_dependencies( - ['requires-requires-foo'], elements_dir=self.element_dir) + ['requires-requires-foo'], self.all_elements) self.assertEqual(set(['requires-requires-foo', 'requires-foo', 'foo']), result) def test_no_deps(self): result = element_dependencies.expand_dependencies( - ['foo'], elements_dir=self.element_dir) + ['foo'], self.all_elements) self.assertEqual(set(['foo']), result) def test_self(self): result = element_dependencies.expand_dependencies( - ['self', 'foo'], elements_dir=self.element_dir) + ['self', 'foo'], self.all_elements) self.assertEqual(set(['self', 'foo']), result) def test_circular(self): result = element_dependencies.expand_dependencies( - ['circular1'], elements_dir=self.element_dir) + ['circular1'], self.all_elements) self.assertEqual(set(['circular1', 'circular2']), result) def test_provide(self): result = element_dependencies.expand_dependencies( ['provides_virtual', 'requires_virtual'], - elements_dir=self.element_dir) + self.all_elements) self.assertEqual(set(['requires_virtual', 'provides_virtual']), result) def test_provide_conflict(self): @@ -124,7 +137,7 @@ class TestElementDeps(testtools.TestCase): def test_provide_virtual_ordering(self): result = element_dependencies.expand_dependencies( ['requires_new_virtual', 'provides_new_virtual'], - elements_dir=self.element_dir) + self.all_elements) self.assertEqual(set(['requires_new_virtual', 'provides_new_virtual']), result) @@ -132,7 +145,7 @@ class TestElementDeps(testtools.TestCase): self.assertRaises(SystemExit, element_dependencies.expand_dependencies, ['provides_virtual'], - elements_dir=self.element_dir) + self.all_elements) self.assertIn("Please include an operating system element", self.log_fixture.output) @@ -140,12 +153,20 @@ class TestElementDeps(testtools.TestCase): self.assertRaises(SystemExit, element_dependencies.expand_dependencies, ['circular1', 'operating-system'], - elements_dir=self.element_dir) + self.all_elements) # ensure we get the error message about what's providing the # conflicting package self.assertIn("operating-system : already provided by ['circular1']", self.log_fixture.output) + 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) + class TestElements(testtools.TestCase): def test_depends_on_env(self): diff --git a/doc/source/developer/invocation.rst b/doc/source/developer/invocation.rst index e6cab7a9..7635c139 100644 --- a/doc/source/developer/invocation.rst +++ b/doc/source/developer/invocation.rst @@ -10,11 +10,13 @@ sudo, so if you want them to run non-interactively, you should either run them as root, with sudo -E, or allow your build user to run any sudo command without password. -Using the variable ``ELEMENTS_PATH`` will allow to specify multiple -elements locations. It is a colon (:) separated path list, and it -will work in a first path/element found, first served approach. The -included elements tree is used when no path is supplied, and is added -to the end of the path if a path is supplied. +The variable ``ELEMENTS_PATH`` is a colon (:) separated path list to +search for elements. The included ``elements`` tree is used when no +path is supplied and is always added to the end of the path if a path +is supplied. Earlier elements will override later elements, i.e. with +``ELEMENTS_PATH=foo:bar`` the element ``my-element`` will be chosen +from ``foo/my-element`` over ``bar/my-element``, or any in-built +element of the same name. By default, the image building scripts will not overwrite existing disk images, allowing you to compare the newly built image with the diff --git a/releasenotes/notes/element-override-ccda78c24ab4a4ff.yaml b/releasenotes/notes/element-override-ccda78c24ab4a4ff.yaml new file mode 100644 index 00000000..ae931628 --- /dev/null +++ b/releasenotes/notes/element-override-ccda78c24ab4a4ff.yaml @@ -0,0 +1,7 @@ +--- +deprecations: + + - Element override behavior is now defined, with elements found in + earlier entries of ``ELEMENTS_PATH`` overriding later ones + (e.g. the same semantics as ``$PATH``). Previously the behavior + was undefined. \ No newline at end of file