Making element overriding explicit
This is a re-factor of element_dependencies to achieve two things -- centralising override policy and storing path names. Firstly we want to make the override policy for elements completely explicit. Currently, elements that wish to copy parts of other elements walk ELEMENTS_PATH themselves and look for elements in IMAGE_ELEMENT. How they handle duplicate elements can differ, leading to inconsistent behaviour. We introduce logic in element-info to find elements in each of the directories in ELEMENT_PATHS in *reverse* order -- that is to say, earlier entries in the paths will overwrite later ones. For example ELEMENT_PATHS=foo:bar:baz will mean that "foo/element" will override "baz/element", since "foo" is first. This should be sane to anyone familiar with $PATH. Documentation is clarified around this point and a test-case is added. The second thing is that we want to keep the complete path of the elements we have chosen. We want the aforementioned elements that walk the element list to use these canonical paths to pickup files; this way they don't need to make local decisions about element overrides, but can simply iterate a list and copy/merge files if they exist. A follow-on change (I7092e1845942f249175933d67ab121188f3511fd) will expose this data in a separate variable that can be parsed by elements (a further follow-on I0a64b45e9f2cfa28e84b2859d76b065a6c4590f0 modifies the elements to use this information). Thus this does not change the status-quo -- elements that are walking ELEMENTS_PATH themselves and can/will continue doing that. Change-Id: I2a29861c67de2d25c595cb35d850e92807d26ac6
This commit is contained in:
Ian Wienand
parent
ca53af1184
commit
274be6de55
@ -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
|
||||
|
||||
@ -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):
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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.
|
||||
Loading…
Reference in New Issue
Block a user