diff options
author | Tristan Van Berkom <tristan.vanberkom@codethink.co.uk> | 2017-03-18 15:32:38 +0900 |
---|---|---|
committer | Tristan Van Berkom <tristan.vanberkom@codethink.co.uk> | 2017-03-18 15:32:38 +0900 |
commit | da394300561d4fd65eb238152f1ced15d2a41dbe (patch) | |
tree | de221838d072db74ba0cef954c4b03376bc3f3b9 | |
parent | 0d55bd32641eda6c3a99a5774f98b829fb5a0e6c (diff) | |
download | buildstream-da394300561d4fd65eb238152f1ced15d2a41dbe.tar.gz |
Refactor: Untangling element/source names
o The metaelements and metasources now carry the name, the loader
resolves source names now.
o Element/Source factories dont require a name anymore as they
are already in the meta objects
o Pipeline no longer composes names
o Element.name is now the original project relative filename,
this allows plugins to identify that name in their dependencies,
allowing one to express configuration which identifies elements
by the same name that the user used in the dependencies.
o Removed plugin._get_display_name() in favor of the plugin.name
o Added Element.normal_name, for the cases where we need to have
a normalized name for creating directories and log files
o Updated frontend and test cases and all callers to use the
new naming
-rw-r--r-- | buildstream/_artifactcache.py | 2 | ||||
-rw-r--r-- | buildstream/_elementfactory.py | 5 | ||||
-rw-r--r-- | buildstream/_frontend/main.py | 4 | ||||
-rw-r--r-- | buildstream/_frontend/status.py | 6 | ||||
-rw-r--r-- | buildstream/_frontend/widget.py | 4 | ||||
-rw-r--r-- | buildstream/_loader.py | 29 | ||||
-rw-r--r-- | buildstream/_metasource.py | 4 | ||||
-rw-r--r-- | buildstream/_pipeline.py | 4 | ||||
-rw-r--r-- | buildstream/_scheduler.py | 4 | ||||
-rw-r--r-- | buildstream/_sourcefactory.py | 5 | ||||
-rw-r--r-- | buildstream/element.py | 25 | ||||
-rw-r--r-- | buildstream/plugin.py | 40 | ||||
-rw-r--r-- | buildstream/source.py | 4 | ||||
-rw-r--r-- | tests/loader/variants.py | 22 | ||||
-rw-r--r-- | tests/pipeline/load.py | 32 | ||||
-rw-r--r-- | tests/sources/fixture.py | 1 |
16 files changed, 102 insertions, 89 deletions
diff --git a/buildstream/_artifactcache.py b/buildstream/_artifactcache.py index dc932d58e..191d821e8 100644 --- a/buildstream/_artifactcache.py +++ b/buildstream/_artifactcache.py @@ -36,7 +36,7 @@ def buildref(element): key = element._get_cache_key() # Normalize ostree ref unsupported chars - element_name = element.name.replace('+', 'X') + element_name = element.normal_name.replace('+', 'X') # assume project and element names are not allowed to contain slashes return '{0}/{1}/{2}'.format(project.name, element_name, key) diff --git a/buildstream/_elementfactory.py b/buildstream/_elementfactory.py index 5c37b6f17..cd3c2755d 100644 --- a/buildstream/_elementfactory.py +++ b/buildstream/_elementfactory.py @@ -47,7 +47,6 @@ class ElementFactory(PluginContext): # # Args: # kind (str): The kind of Element to create - # display_name (str): A name to display the Element # context (object): The Context object for processing # project (object): The project object # meta (object): The loaded MetaElement @@ -58,6 +57,6 @@ class ElementFactory(PluginContext): # PluginError (if the kind lookup failed) # LoadError (if the element itself took issue with the config) # - def create(self, kind, display_name, context, project, artifacts, meta): + def create(self, kind, context, project, artifacts, meta): element_type = self.lookup(kind) - return element_type(display_name, context, project, artifacts, meta) + return element_type(context, project, artifacts, meta) diff --git a/buildstream/_frontend/main.py b/buildstream/_frontend/main.py index 30a7352d9..f2bb2124f 100644 --- a/buildstream/_frontend/main.py +++ b/buildstream/_frontend/main.py @@ -242,7 +242,7 @@ def show(app, target, arch, variant, deps, order, format): profile_start(Topics.SHOW, target.replace(os.sep, '-') + '-' + arch) for element in dependencies: - line = p.fmt_subst(format, 'name', element._get_display_name(), fg='blue', bold=True) + line = p.fmt_subst(format, 'name', element.name, fg='blue', bold=True) cache_key = element._get_display_key() consistency = element._consistency() @@ -342,7 +342,7 @@ def shell(app, target, arch, variant, builddir, scope): click.echo("") click.echo("Missing elements for staging an environment for a shell:") for dep in missing_deps: - click.echo(" {}".format(dep._get_display_name())) + click.echo(" {}".format(dep.name)) click.echo("") click.echo("Try building them first") sys.exit(-1) diff --git a/buildstream/_frontend/status.py b/buildstream/_frontend/status.py index 795a50984..ced456d8f 100644 --- a/buildstream/_frontend/status.py +++ b/buildstream/_frontend/status.py @@ -238,7 +238,7 @@ class StatusJob(): # Calculate the size needed to display self.size = 10 # Size of time code self.size += len(action_name) - self.size += len(element._get_display_name()) + self.size += len(element.name) self.size += 3 # '[' + ':' + ']' # render() @@ -253,11 +253,11 @@ class StatusJob(): text = self.time_code.render_time(elapsed) # Add padding after the display name, before terminating ']' - display_name = self.element._get_display_name() + (' ' * padding) + name = self.element.name + (' ' * padding) text += self.format_profile.fmt('[') + \ self.content_profile.fmt(self.action_name) + \ self.format_profile.fmt(':') + \ - self.content_profile.fmt(display_name) + \ + self.content_profile.fmt(name) + \ self.format_profile.fmt(']') return text diff --git a/buildstream/_frontend/widget.py b/buildstream/_frontend/widget.py index af7634677..50ef0e0eb 100644 --- a/buildstream/_frontend/widget.py +++ b/buildstream/_frontend/widget.py @@ -166,7 +166,7 @@ class ElementName(Widget): def size_request(self, pipeline): longest_name = 0 for plugin in pipeline.dependencies(Scope.ALL, include_sources=True): - longest_name = max(len(plugin._get_display_name()), longest_name) + longest_name = max(len(plugin.name), longest_name) # Put a cap at a specific width, usually some elements cause the line # to be too long, just live with the unaligned columns in that case @@ -176,7 +176,7 @@ class ElementName(Widget): def render(self, message): if message.unique_id is not None: plugin = _plugin_lookup(message.unique_id) - name = plugin._get_display_name() + name = plugin.name else: name = '' diff --git a/buildstream/_loader.py b/buildstream/_loader.py index e34f7deaa..944cf8900 100644 --- a/buildstream/_loader.py +++ b/buildstream/_loader.py @@ -152,7 +152,7 @@ class LoadElement(): self.filename = filename self.data = data self.arch = arch - self.name = element_name_from_filename(filename) + self.name = filename self.elements = elements # These are shared with the owning Loader object @@ -310,7 +310,7 @@ def extract_depends_from_node(owner, data): for dep in depends: if isinstance(dep, str): - dependency = Dependency(owner, element_name_from_filename(dep), filename=dep) + dependency = Dependency(owner, dep, filename=dep) elif isinstance(dep, Mapping): # Make variant optional, for this we set it to None after @@ -329,8 +329,7 @@ def extract_depends_from_node(owner, data): (str(provenance), dep_type)) filename = _yaml.node_get(dep, str, Symbol.FILENAME) - name = element_name_from_filename(filename) - dependency = Dependency(owner, name, variant_name=variant, filename=filename, dep_type=dep_type) + dependency = Dependency(owner, filename, variant_name=variant, filename=filename, dep_type=dep_type) else: index = depends.index(dep) @@ -348,12 +347,6 @@ def extract_depends_from_node(owner, data): return output_deps -def element_name_from_filename(filename): - element_name = filename.replace(os.sep, '-') - element_name = os.path.splitext(element_name)[0] - return element_name - - ################################################# # The Loader # ################################################# @@ -383,7 +376,7 @@ class Loader(): # Target bst filename self.target_filename = filename - self.target = element_name_from_filename(filename) + self.target = filename # Optional variant self.target_variant = variant @@ -458,16 +451,15 @@ class Loader(): self.loaded_files[filename] = True # Raise error if two files claim the same name - element_name = element_name_from_filename(filename) - if element_name in self.elements: - element = self.elements[element_name] + if filename in self.elements: + element = self.elements[filename] raise LoadError(LoadErrorReason.INVALID_DATA, "Tried to load file '%s' but existing file '%s' has the same name" % (filename, element.filename)) # Call the ticker if ticker: - ticker(element_name) + ticker(filename) fullpath = os.path.join(self.basedir, filename) @@ -475,7 +467,7 @@ class Loader(): data = _yaml.load(fullpath, shortname=filename, copy_tree=rewritable) element = LoadElement(data, filename, self.basedir, self.arch, self.elements) - self.elements[element_name] = element + self.elements[filename] = element # Load all possible dependency files for the new LoadElement for dep in element.base_deps: @@ -754,7 +746,10 @@ class Loader(): else: directory = None - meta_source = MetaSource(kind, source, directory, + index = sources.index(source) + source_name = "{}-{}".format(element_name, index) + + meta_source = MetaSource(source_name, kind, source, directory, provenance.node, provenance.toplevel, provenance.filename) diff --git a/buildstream/_metasource.py b/buildstream/_metasource.py index 75bf0142a..8413ddbd8 100644 --- a/buildstream/_metasource.py +++ b/buildstream/_metasource.py @@ -26,13 +26,15 @@ class MetaSource(): # An abstract object holding data suitable for constructing a Source # # Args: + # name: The name of the source, for display purposes # kind: The kind of the source # config: The configuration data for the source # origin_node: The original YAML dictionary node defining this source # origin_toplevel: The toplevel YAML loaded from the original file # origin_filename: The filename in which the node was loaded from # - def __init__(self, kind, config, directory, origin_node, origin_toplevel, origin_filename): + def __init__(self, name, kind, config, directory, origin_node, origin_toplevel, origin_filename): + self.name = name self.kind = kind self.config = config self.directory = directory diff --git a/buildstream/_pipeline.py b/buildstream/_pipeline.py index e716c2eb9..9adc2deee 100644 --- a/buildstream/_pipeline.py +++ b/buildstream/_pipeline.py @@ -352,7 +352,6 @@ class Pipeline(): ticker(meta_element.name) element = self.element_factory.create(meta_element.kind, - meta_element.provenance.filename, self.context, self.project, self.artifacts, @@ -368,11 +367,8 @@ class Pipeline(): # resolve sources for meta_source in meta_element.sources: - index = meta_element.sources.index(meta_source) - display_name = "{}-{}".format(meta_element.provenance.filename, index) element._add_source( self.source_factory.create(meta_source.kind, - display_name, self.context, self.project, meta_source) diff --git a/buildstream/_scheduler.py b/buildstream/_scheduler.py index 7a5d56ca6..d188e9efe 100644 --- a/buildstream/_scheduler.py +++ b/buildstream/_scheduler.py @@ -592,7 +592,7 @@ class Job(): elt_env = utils._node_sanitize(element._Element__environment) env_dump = yaml.round_trip_dump(elt_env, default_flow_style=False, allow_unicode=True) self.message(element, MessageType.LOG, - "Build environment for element {}".format(element._get_display_name()), + "Build environment for element {}".format(element.name), detail=env_dump, logfile=filename) try: @@ -627,7 +627,7 @@ class Job(): INDENT = " " EMPTYTIME = "--:--:--" - name = '[' + plugin._get_display_name() + ']' + name = '[' + plugin.name + ']' fmt = "[{timecode: <8}] {type: <7} {name: <15}: {message}" detail = '' diff --git a/buildstream/_sourcefactory.py b/buildstream/_sourcefactory.py index c83d98aa6..78fb7ec5f 100644 --- a/buildstream/_sourcefactory.py +++ b/buildstream/_sourcefactory.py @@ -47,7 +47,6 @@ class SourceFactory(PluginContext): # # Args: # kind (str): The kind of Source to create - # display_name (str): A name to display the Source # context (object): The Context object for processing # project (object): The project object # meta (object): The loaded MetaSource @@ -59,6 +58,6 @@ class SourceFactory(PluginContext): # PluginError (if the kind lookup failed) # LoadError (if the source itself took issue with the config) # - def create(self, kind, display_name, context, project, meta): + def create(self, kind, context, project, meta): source_type = self.lookup(kind) - return source_type(display_name, context, project, meta) + return source_type(context, project, meta) diff --git a/buildstream/element.py b/buildstream/element.py index 9464b4513..27fb950c5 100644 --- a/buildstream/element.py +++ b/buildstream/element.py @@ -70,12 +70,17 @@ class Element(Plugin): __defaults = {} # The defaults from the yaml file and project __defaults_set = False # Flag, in case there are no defaults at all - def __init__(self, display_name, context, project, artifacts, meta): + def __init__(self, context, project, artifacts, meta): - super().__init__(display_name, context, project, meta.provenance, "element") + super().__init__(meta.name, context, project, meta.provenance, "element") - self.name = meta.name - """The element name""" + self.normal_name = os.path.splitext(self.name.replace(os.sep, '-'))[0] + """A normalized element name + + This is the original element without path separators or + the extension, it's used mainly for composing log file names + and creating directory names and such. + """ self.__runtime_dependencies = [] # Direct runtime dependency Elements self.__build_dependencies = [] # Direct build dependency Elements @@ -316,9 +321,9 @@ class Element(Plugin): for dep in self.dependencies(scope): o, i = dep.stage(sandbox, path=path, splits=splits, orphans=orphans) if o: - overwrites[dep._get_display_name()] = o + overwrites[dep.name] = o if i: - ignored[dep._get_display_name()] = i + ignored[dep.name] = i if overwrites: detail = "Staged files overwrite existing files in staging area:\n" @@ -648,7 +653,7 @@ class Element(Plugin): # Explicitly clean it up, keep the build dir around if exceptions are raised os.makedirs(context.builddir, exist_ok=True) - rootdir = tempfile.mkdtemp(prefix="{}-".format(self.name), dir=context.builddir) + rootdir = tempfile.mkdtemp(prefix="{}-".format(self.normal_name), dir=context.builddir) # Cleanup the build directory on explicit SIGTERM def cleanup_rootdir(): @@ -709,7 +714,7 @@ class Element(Plugin): logfile = "{key}-{action}.{pid}.log".format( key=key, action=action, pid=pid) - directory = os.path.join(context.logdir, project.name, self.name) + directory = os.path.join(context.logdir, project.name, self.normal_name) os.makedirs(directory, exist_ok=True) return os.path.join(directory, logfile) @@ -726,7 +731,7 @@ class Element(Plugin): # Write one last line to the log and flush it to disk def flush_log(): logfile.write('\n\nAction {} for element {} forcefully terminated\n' - .format(action_name, self._get_display_name())) + .format(action_name, self.name)) logfile.flush() self._set_log_handle(logfile) @@ -780,7 +785,7 @@ class Element(Plugin): else: os.makedirs(context.builddir, exist_ok=True) - rootdir = tempfile.mkdtemp(prefix="{}-".format(self.name), dir=context.builddir) + rootdir = tempfile.mkdtemp(prefix="{}-".format(self.normal_name), dir=context.builddir) # Recursive contextmanager... with self.__sandbox(scope, rootdir, stdout=stdout, stderr=stderr) as sandbox: diff --git a/buildstream/plugin.py b/buildstream/plugin.py index ed32133b8..a78fd1e24 100644 --- a/buildstream/plugin.py +++ b/buildstream/plugin.py @@ -41,13 +41,23 @@ class Plugin(): Some common features to both Sources and Elements are found in this class. """ - def __init__(self, display_name, context, project, provenance, type_tag): + def __init__(self, name, context, project, provenance, type_tag): + + self.name = name + """The plugin name + + For elements, this is the project relative bst filename, + for sources this is the owning element's name with a suffix + indicating it's index on the owning element. + + For sources this is for display purposes only. + """ + self.__context = context # The Context object self.__project = project # The Project object self.__provenance = provenance # The Provenance information self.__type_tag = type_tag # The type of plugin (element or source) self.__unique_id = _plugin_register(self) # Unique ID - self.__display_name = display_name # Plugin display name self.__log = None # The log handle when running a task self.debug("Created: {}".format(self)) @@ -93,7 +103,7 @@ class Plugin(): """Iterate over a dictionary loaded from YAML Args: - dict: The YAML loaded dictionary object + node (dict): The YAML loaded dictionary object Returns: list: List of key/value tuples to iterate over @@ -109,6 +119,22 @@ class Plugin(): continue yield (key, value) + def node_provenance(self, node, member_name=None): + """Gets the provenance for `node` and `member_name` + + This reports a string with file, line and column information suitable + for reporting an error or warning. + + Args: + node (dict): The YAML loaded dictionary object + member_name (str): The name of the member to check, or None for the node itself + + Returns: + (str): A string describing the provenance of the node and member + """ + provenance = _yaml.node_get_provenance(node, key=member_name) + return str(provenance) + def node_get_member(self, node, expected_type, member_name, default_value=None): """Fetch the value of a node member, raising an error if the value is missing or incorrectly typed. @@ -479,14 +505,6 @@ class Plugin(): # Private Methods used in BuildStream # ############################################################# - # _get_display_name(): - # - # Fetch the plugin's display name, to be used in message titles - # and such. - # - def _get_display_name(self): - return self.__display_name - # _get_unique_id(): # # Fetch the plugin's unique identifier diff --git a/buildstream/source.py b/buildstream/source.py index 5c7c27253..fb4c9d95a 100644 --- a/buildstream/source.py +++ b/buildstream/source.py @@ -56,9 +56,9 @@ class Source(Plugin): All Sources derive from this class, this interface defines how the core will be interacting with Sources. """ - def __init__(self, display_name, context, project, meta): + def __init__(self, context, project, meta): provenance = _yaml.node_get_provenance(meta.config) - super().__init__(display_name, context, project, provenance, "source") + super().__init__(meta.name, context, project, provenance, "source") self.__directory = meta.directory # Staging relative directory self.__origin_node = meta.origin_node # YAML node this Source was loaded from diff --git a/tests/loader/variants.py b/tests/loader/variants.py index 39869b844..0e20faf93 100644 --- a/tests/loader/variants.py +++ b/tests/loader/variants.py @@ -162,7 +162,7 @@ def test_variant_simple_dependency_default(datafiles): assert(element.kind == 'pony') # Test that the default is a pink pony - assert_dependency(element, 0, 'elements-simply-pink', 'color', 'pink') + assert_dependency(element, 0, 'elements/simply-pink.bst', 'color', 'pink') @pytest.mark.datafiles(DATA_DIR) @@ -177,7 +177,7 @@ def test_variant_simple_dependency_pink_pony(datafiles): assert(element.kind == 'pony') # Test that the explicit pink dependency is correct - assert_dependency(element, 0, 'elements-simply-pink', 'color', 'pink') + assert_dependency(element, 0, 'elements/simply-pink.bst', 'color', 'pink') @pytest.mark.datafiles(DATA_DIR) @@ -192,7 +192,7 @@ def test_variant_simple_dependency_blue_pony(datafiles): assert(element.kind == 'pony') # Test that the explicit blue dependency is correct - assert_dependency(element, 0, 'elements-simply-blue', 'color', 'blue') + assert_dependency(element, 0, 'elements/simply-blue.bst', 'color', 'blue') @pytest.mark.datafiles(DATA_DIR) @@ -208,10 +208,10 @@ def test_variant_indirect_dependency_default(datafiles): # Test that the default is a blue pony-color by default simple = assert_dependency( - element, 0, 'elements-simple-dependency-variants', 'pony-color', 'blue') + element, 0, 'elements/simple-dependency-variants.bst', 'pony-color', 'blue') # Test that the element we depend on now depends on the blue color - assert_dependency(simple, 0, 'elements-simply-blue', 'color', 'blue') + assert_dependency(simple, 0, 'elements/simply-blue.bst', 'color', 'blue') @pytest.mark.datafiles(DATA_DIR) @@ -227,10 +227,10 @@ def test_variant_indirect_dependency_blue_pony(datafiles): # Test for a blue pony-color simple = assert_dependency( - element, 0, 'elements-simple-dependency-variants', 'pony-color', 'blue') + element, 0, 'elements/simple-dependency-variants.bst', 'pony-color', 'blue') # Test that the element we depend on now depends on the blue color - assert_dependency(simple, 0, 'elements-simply-blue', 'color', 'blue') + assert_dependency(simple, 0, 'elements/simply-blue.bst', 'color', 'blue') @pytest.mark.datafiles(DATA_DIR) @@ -246,10 +246,10 @@ def test_variant_indirect_dependency_pink_pony(datafiles): # Test for a blue pony-color simple = assert_dependency( - element, 0, 'elements-simple-dependency-variants', 'pony-color', 'pink') + element, 0, 'elements/simple-dependency-variants.bst', 'pony-color', 'pink') # Test that the element we depend on now depends on the blue color - assert_dependency(simple, 0, 'elements-simply-pink', 'color', 'pink') + assert_dependency(simple, 0, 'elements/simply-pink.bst', 'color', 'pink') @pytest.mark.datafiles(DATA_DIR) @@ -263,8 +263,8 @@ def test_engine_resolve_agreement(datafiles): assert(element.kind == 'tricky') # Test the first dependency - first = assert_dependency(element, 0, 'elements-tricky-first', 'choice', 'second') - second = assert_dependency(element, 1, 'elements-tricky-second', 'choice', 'second') + first = assert_dependency(element, 0, 'elements/tricky-first.bst', 'choice', 'second') + second = assert_dependency(element, 1, 'elements/tricky-second.bst', 'choice', 'second') @pytest.mark.datafiles(DATA_DIR) diff --git a/tests/pipeline/load.py b/tests/pipeline/load.py index 6f4fffe43..c58abc4b1 100644 --- a/tests/pipeline/load.py +++ b/tests/pipeline/load.py @@ -54,13 +54,13 @@ def test_iterate_scope_all(datafiles, tmpdir): element_list = list(element_list) assert(len(element_list) == 7) - assert(element_list[0].name == "build-build") - assert(element_list[1].name == "run-build") - assert(element_list[2].name == "build") - assert(element_list[3].name == "dep-one") - assert(element_list[4].name == "run") - assert(element_list[5].name == "dep-two") - assert(element_list[6].name == "target") + assert(element_list[0].name == "build-build.bst") + assert(element_list[1].name == "run-build.bst") + assert(element_list[2].name == "build.bst") + assert(element_list[3].name == "dep-one.bst") + assert(element_list[4].name == "run.bst") + assert(element_list[5].name == "dep-two.bst") + assert(element_list[6].name == "target.bst") @pytest.mark.datafiles(os.path.join(DATA_DIR, 'iterate')) @@ -80,10 +80,10 @@ def test_iterate_scope_run(datafiles, tmpdir): element_list = list(element_list) assert(len(element_list) == 4) - assert(element_list[0].name == "dep-one") - assert(element_list[1].name == "run") - assert(element_list[2].name == "dep-two") - assert(element_list[3].name == "target") + assert(element_list[0].name == "dep-one.bst") + assert(element_list[1].name == "run.bst") + assert(element_list[2].name == "dep-two.bst") + assert(element_list[3].name == "target.bst") @pytest.mark.datafiles(os.path.join(DATA_DIR, 'iterate')) @@ -104,9 +104,9 @@ def test_iterate_scope_build(datafiles, tmpdir): assert(len(element_list) == 3) - assert(element_list[0].name == "dep-one") - assert(element_list[1].name == "run") - assert(element_list[2].name == "dep-two") + assert(element_list[0].name == "dep-one.bst") + assert(element_list[1].name == "run.bst") + assert(element_list[2].name == "dep-two.bst") @pytest.mark.datafiles(os.path.join(DATA_DIR, 'iterate')) @@ -133,5 +133,5 @@ def test_iterate_scope_build_of_child(datafiles, tmpdir): assert(len(element_list) == 2) - assert(element_list[0].name == "run-build") - assert(element_list[1].name == "build") + assert(element_list[0].name == "run-build.bst") + assert(element_list[1].name == "build.bst") diff --git a/tests/sources/fixture.py b/tests/sources/fixture.py index 20025f46c..c6a9c0452 100644 --- a/tests/sources/fixture.py +++ b/tests/sources/fixture.py @@ -43,7 +43,6 @@ class Setup(): base = PluginBase(package='buildstream.plugins') self.factory = SourceFactory(base) self.source = self.factory.create(self.meta_source.kind, - "test", self.context, self.project, self.meta_source) |