This is an automated email from the ASF dual-hosted git repository. tvb pushed a commit to branch tristan/optional-project in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit 60c1ccb447f219b0b062cdf93abaae37fea0cdc1 Author: Tristan van Berkom <[email protected]> AuthorDate: Thu Nov 19 16:35:33 2020 +0900 Allow certain operations to work without loading a project This patch removes the requirement to load a project.conf at startup time, accomodating certain operations which do not need to observe workspace state or load elements. Summary of changes: * _frontend/app.py: Don't error out if a project cannot be loaded. The project is optional, and Stream() will raise an error if a project is required for the operation it is asked to perform. * _stream.py: Handle cases where the main project is None, and raise an error in cases where a project is required. * _workspaces.py: Handle cases where the project directory is None, in case we are looking at an ArtifactProject which has no directory. * tests/format/project.py: Update the missing project test to expect a different error, and observe multiple cases where a project might be required. We still have an error for missing projects, but it is only issued for cases where a project is expected, we need to continue testing this. --- src/buildstream/_frontend/app.py | 23 +++++++------ src/buildstream/_stream.py | 72 +++++++++++++++++++++++++++++----------- src/buildstream/_workspaces.py | 10 ++++-- tests/format/project.py | 7 ++-- 4 files changed, 77 insertions(+), 35 deletions(-) diff --git a/src/buildstream/_frontend/app.py b/src/buildstream/_frontend/app.py index 9937470..ce1610f 100644 --- a/src/buildstream/_frontend/app.py +++ b/src/buildstream/_frontend/app.py @@ -284,23 +284,24 @@ class App: cli_options=self._main_options["option"], default_mirror=self._main_options.get("default_mirror"), ) - - self.stream.set_project(self.project) except LoadError as e: - # Help users that are new to BuildStream by suggesting 'init'. - # We don't want to slow down users that just made a mistake, so - # don't stop them with an offer to create a project for them. - if e.reason == LoadErrorReason.MISSING_PROJECT_CONF: - click.echo("No project found. You can create a new project like so:", err=True) - click.echo("", err=True) - click.echo(" bst init", err=True) - - self._error_exit(e, "Error loading project") + # If there was no project.conf at all then there was just no project found. + # + # Don't error out in this case, as Stream() supports some operations which + # do not require a project. If Stream() requires a project and it is missing, + # then it will raise an error. + # + if e.reason != LoadErrorReason.MISSING_PROJECT_CONF: + self._error_exit(e, "Error loading project") except BstError as e: self._error_exit(e, "Error loading project") + # Set the project on the Stream, this can be None if there is no project. + # + self.stream.set_project(self.project) + # Run the body of the session here, once everything is loaded try: yield diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py index 22feab9..6eb25e8 100644 --- a/src/buildstream/_stream.py +++ b/src/buildstream/_stream.py @@ -124,7 +124,8 @@ class Stream: def set_project(self, project): assert self._project is None self._project = project - self._project.load_context.set_fetch_subprojects(self._fetch_subprojects) + if self._project: + self._project.load_context.set_fetch_subprojects(self._fetch_subprojects) # load_selection() # @@ -885,6 +886,8 @@ class Stream: # remove_dir (bool): Whether to remove the associated directory # def workspace_close(self, element_name, *, remove_dir): + self._assert_project("Unable to locate workspaces") + workspaces = self._context.get_workspaces() workspace = workspaces.get_workspace(element_name) @@ -913,6 +916,7 @@ class Stream: # soft (bool): Only set the workspace state to not prepared # def workspace_reset(self, targets, *, soft): + self._assert_project("Unable to locate workspaces") elements = self._load(targets, selection=_PipelineSelection.REDIRECT) @@ -953,6 +957,8 @@ class Stream: # True if there are any existing workspaces. # def workspace_exists(self, element_name=None): + self._assert_project("Unable to locate workspaces") + workspaces = self._context.get_workspaces() if element_name: workspace = workspaces.get_workspace(element_name) @@ -968,6 +974,8 @@ class Stream: # Serializes the workspaces and dumps them in YAML to stdout. # def workspace_list(self): + self._assert_project("Unable to locate workspaces") + workspaces = [] for element_name, workspace_ in self._context.get_workspaces().list(): workspace_detail = { @@ -1106,6 +1114,22 @@ class Stream: # Private Methods # ############################################################# + # _assert_project() + # + # Raises an assertion of a project was not loaded + # + # Args: + # message: The user facing error message, e.g. "Unable to load elements" + # + # Raises: + # A StreamError with reason "project-not-loaded" is raised if no project was loaded + # + def _assert_project(self, message: str) -> None: + if not self._project: + raise StreamError( + message, detail="No project.conf or active workspace was located", reason="project-not-loaded" + ) + # _fetch_subprojects() # # Fetch subprojects as part of the project and element loading process. @@ -1210,7 +1234,12 @@ class Stream: targets, valid_artifact_names=valid_artifact_names ) - self._project.load_context.set_rewritable(rewritable) + # We need a project in order to load elements + if element_names: + self._assert_project("Unable to load elements: {}".format(", ".join(element_names))) + + if self._project: + self._project.load_context.set_rewritable(rewritable) # Load elements and except elements if element_names: @@ -1493,7 +1522,11 @@ class Stream: def _resolve_elements(self, targets): with self._context.messenger.simple_task("Resolving cached state", silent_nested=True) as task: # We need to go through the project to access the loader - if task: + # + # FIXME: We need to calculate the total elements to resolve differently so that + # it can include artifact elements + # + if task and self._project: task.set_maximum_progress(self._project.loader.loaded) # XXX: Now that Element._update_state() can trigger recursive update_state calls @@ -1845,22 +1878,23 @@ class Stream: element_targets = initial_targets # Expand globs for elements - all_elements = [] - element_path_length = len(self._project.element_path) + 1 - for dirpath, _, filenames in os.walk(self._project.element_path): - for filename in filenames: - if filename.endswith(".bst"): - element_path = os.path.join(dirpath, filename) - element_path = element_path[element_path_length:] # Strip out the element_path - all_elements.append(element_path) - - for glob in globs: - matched = False - for element_path in utils.glob(all_elements, glob): - element_targets.append(element_path) - matched = True - if matched: - globs[glob] = globs[glob] + 1 + if self._project: + all_elements = [] + element_path_length = len(self._project.element_path) + 1 + for dirpath, _, filenames in os.walk(self._project.element_path): + for filename in filenames: + if filename.endswith(".bst"): + element_path = os.path.join(dirpath, filename) + element_path = element_path[element_path_length:] # Strip out the element_path + all_elements.append(element_path) + + for glob in globs: + matched = False + for element_path in utils.glob(all_elements, glob): + element_targets.append(element_path) + matched = True + if matched: + globs[glob] = globs[glob] + 1 # Expand globs for artifact names if valid_artifact_names: diff --git a/src/buildstream/_workspaces.py b/src/buildstream/_workspaces.py index e51be08..ebca148 100644 --- a/src/buildstream/_workspaces.py +++ b/src/buildstream/_workspaces.py @@ -313,10 +313,16 @@ class Workspace: class Workspaces: def __init__(self, toplevel_project, workspace_project_cache): self._toplevel_project = toplevel_project - self._bst_directory = os.path.join(toplevel_project.directory, ".bst") - self._workspaces = self._load_config() self._workspace_project_cache = workspace_project_cache + # A project without a directory can happen + if toplevel_project.directory: + self._bst_directory = os.path.join(toplevel_project.directory, ".bst") + self._workspaces = self._load_config() + else: + self._bst_directory = None + self._workspaces = {} + # list() # # Generator function to enumerate workspaces. diff --git a/tests/format/project.py b/tests/format/project.py index d3de672..6e06176 100644 --- a/tests/format/project.py +++ b/tests/format/project.py @@ -15,10 +15,11 @@ DATA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "project") @pytest.mark.datafiles(os.path.join(DATA_DIR)) -def test_missing_project_conf(cli, datafiles): [email protected]("args", [["workspace", "list"], ["show", "pony.bst"]], ids=["list-workspace", "show-element"]) +def test_missing_project_conf(cli, datafiles, args): project = str(datafiles) - result = cli.run(project=project, args=["workspace", "list"]) - result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.MISSING_PROJECT_CONF) + result = cli.run(project=project, args=args) + result.assert_main_error(ErrorDomain.STREAM, "project-not-loaded") @pytest.mark.datafiles(os.path.join(DATA_DIR))
