This is an automated email from the ASF dual-hosted git repository. tvb pushed a commit to branch traveltissues/mr4 in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit b97e32612da29addcadf2e1aa2d11b2ab5908b25 Author: Darius Makovsky <[email protected]> AuthorDate: Mon Jan 13 09:06:11 2020 +0000 Rename workspace.last_successful to workspace.last_build Add last_dep, built_incrementally and expand the criteria for can_build_incrementally. Also add several fixmes for incremental support. --- src/buildstream/_loader/loader.py | 2 +- src/buildstream/_stream.py | 2 +- src/buildstream/_workspaces.py | 37 ++++++++++--- src/buildstream/element.py | 79 +++++++++++++++++++--------- src/buildstream/plugins/sources/workspace.py | 8 +-- 5 files changed, 90 insertions(+), 38 deletions(-) diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py index 0c6c725..bbc16d2 100644 --- a/src/buildstream/_loader/loader.py +++ b/src/buildstream/_loader/loader.py @@ -456,7 +456,7 @@ class Loader: if workspace: workspace_node = {"kind": "workspace"} workspace_node["path"] = workspace.get_absolute_path() - workspace_node["last_successful"] = str(workspace.to_dict().get("last_successful", "")) + workspace_node["last_build"] = str(workspace.to_dict().get("last_build", "")) node[Symbol.SOURCES] = [workspace_node] skip_workspace = False diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py index db9794c..b2a888c 100644 --- a/src/buildstream/_stream.py +++ b/src/buildstream/_stream.py @@ -931,7 +931,7 @@ class Stream: if soft: workspace.prepared = False - workspace.last_successful = None + workspace.last_build = None self._message( MessageType.INFO, "Reset workspace state for {} at: {}".format(element.name, workspace_path) ) diff --git a/src/buildstream/_workspaces.py b/src/buildstream/_workspaces.py index 49b76a7..9c383c7 100644 --- a/src/buildstream/_workspaces.py +++ b/src/buildstream/_workspaces.py @@ -232,22 +232,34 @@ class WorkspaceProjectCache: # An object to contain various helper functions and data required for # workspaces. # -# last_successful, path and running_files are intended to be public +# last_build, path and running_files are intended to be public # properties, but may be best accessed using this classes' helper # methods. # # Args: # toplevel_project (Project): Top project. Will be used for resolving relative workspace paths. # path (str): The path that should host this workspace -# last_successful (str): The key of the last successful build of this workspace +# last_build (str): The key of the last attempted build of this workspace # running_files (dict): A dict mapping dependency elements to files # changed between failed builds. Should be # made obsolete with failed build artifacts. # class Workspace: - def __init__(self, toplevel_project, *, last_successful=None, path=None, prepared=False, running_files=None): + def __init__( + self, + toplevel_project, + *, + last_build=None, + last_dep=None, + path=None, + built_incrementally=False, + prepared=False, + running_files=None + ): self.prepared = prepared - self.last_successful = last_successful + self.built_incrementally = built_incrementally + self.last_build = last_build + self.last_dep = last_dep self._path = path self.running_files = running_files if running_files is not None else {} @@ -262,9 +274,16 @@ class Workspace: # (dict) A dict representation of the workspace # def to_dict(self): - ret = {"prepared": self.prepared, "path": self._path, "running_files": self.running_files} - if self.last_successful is not None: - ret["last_successful"] = self.last_successful + ret = { + "built_incrementally": self.built_incrementally, + "prepared": self.prepared, + "path": self._path, + "running_files": self.running_files, + } + if self.last_build is not None: + ret["last_build"] = self.last_build + if self.last_dep is not None: + ret["last_dep"] = self.last_dep return ret # from_dict(): @@ -585,8 +604,10 @@ class Workspaces: dictionary = { "prepared": node.get_bool("prepared", default=False), + "built_incrementally": node.get_bool("built_incrementally", default=False), "path": node.get_str("path"), - "last_successful": node.get_str("last_successful", default=None), + "last_build": node.get_str("last_build", default=None), + "last_dep": node.get_str("last_dep", default=None), "running_files": running_files, } return Workspace.from_dict(self._toplevel_project, dictionary) diff --git a/src/buildstream/element.py b/src/buildstream/element.py index d333a40..6ba3595 100644 --- a/src/buildstream/element.py +++ b/src/buildstream/element.py @@ -750,27 +750,27 @@ class Element(Plugin): workspace = self._get_workspace() context = self._get_context() - if self.__can_build_incrementally() and workspace.last_successful: + if self.__can_build_incrementally(): # Try to perform an incremental build if the last successful # build is still in the artifact cache # - if self.__artifacts.contains(self, workspace.last_successful): - last_successful = Artifact(self, context, strong_key=workspace.last_successful) - # Get a dict of dependency strong keys - old_dep_keys = last_successful.get_metadata_dependencies() - else: - # Last successful build is no longer in the artifact cache, - # so let's reset it and perform a full build now. - workspace.prepared = False - workspace.last_successful = None - - self.info("Resetting workspace state, last successful build is no longer in the cache") - - # In case we are staging in the main process - if utils._is_main_process(): - context.get_workspaces().save_config() - + last_artifact = Artifact(self, context, strong_key=workspace.last_build) + # Get a dict of dependency strong keys + old_dep_keys = last_artifact.get_metadata_dependencies() + elif workspace: + # Last successful build is no longer in the artifact cache, + # so let's reset it and perform a full build now. + workspace.prepared = False + workspace.last_build = None + + self.info("Resetting workspace state, last successful build is no longer in the cache") + + # In case we are staging in the main process + if utils._is_main_process(): + context.get_workspaces().save_config() + + # FIXME: for incremental builds, if the build deps have changed then we must fallback to a full build for dep in self.dependencies(scope): # If we are workspaced, and we therefore perform an # incremental build, we must ensure that we update the mtimes @@ -1402,6 +1402,7 @@ class Element(Plugin): # perform incremental builds. if self.__can_build_incrementally(): sandbox.mark_directory(directory) + # FIXME: incremental builds should merge the source into the last artifact before staging # Stage all sources that need to be copied sandbox_vroot = sandbox.get_virtual_directory() @@ -1581,7 +1582,14 @@ class Element(Plugin): # key = self._get_cache_key() workspace = self._get_workspace() - workspace.last_successful = key + workspace.last_build = key + # FIXME: get last dep hash and save to the workspace config + workspace.last_dep = None + # FIXME: merge the cached source into the cached buildtree + # if the digest of this is equivalent to the digest of the cached + # buildtree then the workspace was built incrementally (that is, + # the sourcetree is a subset of the buildtree) + workspace.built_incrementally = False workspace.clear_running_files() self._get_context().get_workspaces().save_config() @@ -2363,7 +2371,32 @@ class Element(Plugin): # (bool): Whether this element can be built incrementally # def __can_build_incrementally(self): - return bool(self._get_workspace()) + # FIXME: + # in order to build incrementally the element must be: + # 1. workspaced + # 2. workspace config provides the previous key, previous artifact and previous dependency hash + # 3. the previous artifact is cached + # 4. the workspace advertises that artifact and key can merge (calculated after previous build and saved in the config + # 5. dependency hash is unchanged + + # 1 + workspace = self._get_workspace() + if workspace: + assert len(self.__sources) == 1 + # 2 + if not workspace.last_dep and workspace.last_build: + return False + # 4 + if not workspace.built_incrementally: + return False + # 3 + if not self.__artifacts.contains(self, workspace.last_build): + return False + # 5 + # TODO + return True + + return False # __configure_sandbox(): # @@ -2386,11 +2419,9 @@ class Element(Plugin): workspace = self._get_workspace() prepared = False if workspace and workspace.prepared: - # FIXME: ideally we don't have to check this, eventually we would - # like to get the saved old_ref and apply the new workspace on top - # to support incremental builds. - if [s._key for s in self.__sources] == [workspace.last_successful]: - prepared = True + # FIXME: ideally we have already merged the workspace.last_build and + # can consider this prepared, for now we mark it as unprepared + prepared = False if not prepared: self.prepare(sandbox) diff --git a/src/buildstream/plugins/sources/workspace.py b/src/buildstream/plugins/sources/workspace.py index 5225b1a..375299b 100644 --- a/src/buildstream/plugins/sources/workspace.py +++ b/src/buildstream/plugins/sources/workspace.py @@ -55,16 +55,16 @@ class WorkspaceSource(Source): self.__unique_key = None # the digest of the Directory following the import of the workspace self.__digest = None - # the cache key of the last successful workspace - self.__last_successful = None + # the cache key of the last workspace build + self.__last_build = None def track(self) -> SourceRef: # pylint: disable=arguments-differ return None def configure(self, node: MappingNode) -> None: - node.validate_keys(["path", "last_successful", "kind"]) + node.validate_keys(["path", "last_build", "kind"]) self.path = node.get_str("path") - self.__last_successful = node.get_str("last_successful") + self.__last_build = node.get_str("last_build") def preflight(self) -> None: pass # pragma: nocover
