From c24f2971db15c8ef23fb569f9ee204ade643ec14 Mon Sep 17 00:00:00 2001 From: Angelos Evripiotis Date: Tue, 5 Feb 2019 15:26:58 +0000 Subject: userconfig: rm really-workspace-close-project-inaccessible Remove the need for the 'really-workspace-close-project-inaccessible' config option, as well as the option itself. As agreed on the mailing list [1], all the 'are you sure?' prompts on workspace reset and close were removed. While that discussion was going on, this new prompt and option was added. At the 2019 BuildStream Gathering, it was verbally agreed between myself and Tristan VB that we would also remove this instance. It was also agreed that we should have a notice to let the user know what they'd done, this was already in place if interactive. Moved it to be unconditional so that there's no difference in non-interactive behaviour. Made it output to stderr, as it's diagnostic meant for the user. Made it the last thing echo'd so it's next to the prompt - it's very relevant to what they type next. Added a test to make sure the text makes it to stderr in the appropriate case, and not in an inappropriate one. This is the last instance of any prompt configuration, so BuildStream can also forget all of that machinery. [1] https://mail.gnome.org/archives/buildstream-list/2018-December/msg00111.html --- buildstream/_context.py | 20 -------------------- buildstream/_frontend/cli.py | 18 +++++++++++------- buildstream/data/userconfig.yaml | 17 ----------------- tests/frontend/workspace.py | 2 ++ 4 files changed, 13 insertions(+), 44 deletions(-) diff --git a/buildstream/_context.py b/buildstream/_context.py index 6fe7c3f52..2fbf415fb 100644 --- a/buildstream/_context.py +++ b/buildstream/_context.py @@ -124,10 +124,6 @@ class Context(): # Whether or not to cache build trees on artifact creation self.cache_buildtrees = None - # Boolean, whether we double-check with the user that they meant to - # close the workspace when they're using it to access the project. - self.prompt_workspace_close_project_inaccessible = None - # Whether elements must be rebuilt when their dependencies have changed self._strict_build_plan = None @@ -248,22 +244,6 @@ class Context(): self.sched_pushers = _yaml.node_get(scheduler, int, 'pushers') self.sched_network_retries = _yaml.node_get(scheduler, int, 'network-retries') - # Load prompt preferences - # - # We convert string options to booleans here, so we can be both user - # and coder-friendly. The string options are worded to match the - # responses the user would give at the cli, for least surprise. The - # booleans are converted here because it's easiest to eyeball that the - # strings are right. - # - prompt = _yaml.node_get( - defaults, Mapping, 'prompt') - _yaml.node_validate(prompt, [ - 'really-workspace-close-project-inaccessible', - ]) - self.prompt_workspace_close_project_inaccessible = _node_get_option_str( - prompt, 'really-workspace-close-project-inaccessible', ['ask', 'yes']) == 'ask' - # Load per-projects overrides self._project_overrides = _yaml.node_get(defaults, Mapping, 'projects', default_value={}) diff --git a/buildstream/_frontend/cli.py b/buildstream/_frontend/cli.py index 401eda9b6..02ca52e85 100644 --- a/buildstream/_frontend/cli.py +++ b/buildstream/_frontend/cli.py @@ -814,6 +814,8 @@ def workspace_open(app, no_checkout, force, track_, directory, elements): def workspace_close(app, remove_dir, all_, elements): """Close a workspace""" + removed_required_element = False + with app.initialized(): if not (all_ or elements): # NOTE: I may need to revisit this when implementing multiple projects @@ -840,18 +842,20 @@ def workspace_close(app, remove_dir, all_, elements): for element_name in elements: if not app.stream.workspace_exists(element_name): nonexisting.append(element_name) - if (app.stream.workspace_is_required(element_name) and app.interactive and - app.context.prompt_workspace_close_project_inaccessible): - click.echo("Removing '{}' will prevent you from running " - "BuildStream commands from the current directory".format(element_name)) - if not click.confirm('Are you sure you want to close this workspace?'): - click.echo('Aborting', err=True) - sys.exit(-1) if nonexisting: raise AppError("Workspace does not exist", detail="\n".join(nonexisting)) for element_name in elements: app.stream.workspace_close(element_name, remove_dir=remove_dir) + if app.stream.workspace_is_required(element_name): + removed_required_element = True + + # This message is echo'd last, as it's most relevant to the next + # thing the user will type. + if removed_required_element: + click.echo( + "Removed '{}', therefore you can no longer run BuildStream " + "commands from the current directory.".format(element_name), err=True) ################################################################## diff --git a/buildstream/data/userconfig.yaml b/buildstream/data/userconfig.yaml index 0b4535cea..f17dac88c 100644 --- a/buildstream/data/userconfig.yaml +++ b/buildstream/data/userconfig.yaml @@ -111,20 +111,3 @@ logging: message-format: | [%{elapsed}][%{key}][%{element}] %{action} %{message} - -# -# Prompt overrides -# -# Here you can suppress 'are you sure?' and other kinds of prompts by supplying -# override values. Note that e.g. 'yes' and 'no' have the same meaning here as -# they do in the actual cli prompt. -# -prompt: - - # Whether to really proceed with 'bst workspace close' when doing so would - # stop them from running bst commands in this workspace. - # - # ask - Ask the user if they are sure. - # yes - Always close, without asking. - # - really-workspace-close-project-inaccessible: ask diff --git a/tests/frontend/workspace.py b/tests/frontend/workspace.py index f6d12e8bf..91004b9f4 100644 --- a/tests/frontend/workspace.py +++ b/tests/frontend/workspace.py @@ -1184,6 +1184,7 @@ def test_external_close_other(cli, datafiles, tmpdir_factory): result = cli.run(project=project, args=['-C', alpha_workspace, 'workspace', 'close', beta_element]) result.assert_success() + assert 'you can no longer run BuildStream' not in result.stderr @pytest.mark.datafiles(DATA_DIR) @@ -1199,6 +1200,7 @@ def test_external_close_self(cli, datafiles, tmpdir_factory, guess_element): result = cli.run(project=project, args=['-C', alpha_workspace, 'workspace', 'close'] + arg_elm) result.assert_success() + assert 'you can no longer run BuildStream' in result.stderr @pytest.mark.datafiles(DATA_DIR) -- cgit v1.2.1