You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by ro...@apache.org on 2020/12/29 13:34:31 UTC
[buildstream] 01/05: BREAK: make destructive action scripts
consistent
This is an automated email from the ASF dual-hosted git repository.
root pushed a commit to branch aevri/safe_noninteractive
in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit d7c8f739a7b4be102d6d8aac846f1e7b54048f03
Author: Angelos Evripiotis <je...@bloomberg.net>
AuthorDate: Fri Nov 16 14:48:17 2018 +0000
BREAK: make destructive action scripts consistent
Make sure that BuildStream doesn't behave differently about destructive
things just because it's in non-interactive mode, e.g. being run from a
misconfigured workflow helper script.
This is a breaking change, as the behaviour will not be the same for
folks that have already scripted the use of these commands.
By default, assume that users opt-out of destructive actions if we
are unable to ask them. For the currently affected commands, this means
aborting completely.
This seems less surprising than doing the destructive action instead.
Also, for folks that find the new behaviour surprising, at least they
don't lose work.
Introduce an '--assume-yes' option to affected commands to override this
behaviour.
If a command aborts for this reason, print a message suggesting the use
of '--assume-yes'. Don't mention the corresponding 'prompt.*' options
that also control this behaviour - this would hinder portability of
scripts between users with different settings.
This affects:
- bst workspace close --remove-dir
- bst workspace reset
Closes #744, #726.
---
NEWS | 9 +++++++++
buildstream/_frontend/cli.py | 30 ++++++++++++++++++++++--------
tests/examples/developing.py | 6 ++++--
tests/examples/junctions.py | 5 +++--
tests/frontend/cross_junction_workspace.py | 4 ++--
tests/frontend/workspace.py | 21 +++++++++++----------
tests/plugins/filter.py | 2 +-
7 files changed, 52 insertions(+), 25 deletions(-)
diff --git a/NEWS b/NEWS
index 16759e1..01569fa 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,12 @@ buildstream 1.3.1
make changes to their .bst files if they are expecting these environment
variables to be set.
+ o BREAKING CHANGE: When non-interactive, `bst workspace` commands `reset` and
+ `close --remove-dir` will abort unless told not to do so. This is to avoid
+ surprises around destructive actions. The new `--assume-yes` option to
+ those commands is the preferred way to do this for scripts. The new
+ `prompt.really-workspace-*` options in buildstream.conf is another way.
+
o Failed builds are included in the cache as well.
`bst checkout` will provide anything in `%{install-root}`.
A build including cached fails will cause any dependant elements
@@ -51,6 +57,9 @@ buildstream 1.3.1
certain confirmation prompts, e.g. double-checking that the user meant to
run the command as typed.
+ o The `bst workspace` commands `close` and `reset` learned an `--assume-yes`
+ option, for supressing prompts.
+
o Due to the element `build tree` being cached in the respective artifact their
size in some cases has significantly increased. In *most* cases the build trees
are not utilised when building targets, as such by default bst 'pull' & 'build'
diff --git a/buildstream/_frontend/cli.py b/buildstream/_frontend/cli.py
index b1b4e03..8375de6 100644
--- a/buildstream/_frontend/cli.py
+++ b/buildstream/_frontend/cli.py
@@ -734,10 +734,12 @@ def workspace_open(app, no_checkout, force, track_, directory, elements):
help="Remove the path that contains the closed workspace")
@click.option('--all', '-a', 'all_', default=False, is_flag=True,
help="Close all open workspaces")
+@click.option('--assume-yes', '-y', default=False, is_flag=True,
+ help="Assume 'yes' to confirmation of destructive changes")
@click.argument('elements', nargs=-1,
type=click.Path(readable=False))
@click.pass_obj
-def workspace_close(app, remove_dir, all_, elements):
+def workspace_close(app, remove_dir, all_, assume_yes, elements):
"""Close a workspace"""
if not (all_ or elements):
@@ -764,9 +766,14 @@ def workspace_close(app, remove_dir, all_, elements):
if nonexisting:
raise AppError("Workspace does not exist", detail="\n".join(nonexisting))
- if app.interactive and remove_dir and app.context.prompt_workspace_close_remove_dir:
- if not click.confirm('This will remove all your changes, are you sure?'):
- click.echo('Aborting', err=True)
+ if remove_dir and not assume_yes and app.context.prompt_workspace_close_remove_dir:
+ if app.interactive:
+ if not click.confirm('This will remove all your changes, are you sure?'):
+ click.echo('Aborting', err=True)
+ sys.exit(-1)
+ else:
+ click.echo("Aborted destructive non-interactive action.", err=True)
+ click.echo("Please use the '--assume-yes' option to override.", err=True)
sys.exit(-1)
for element_name in elements:
@@ -783,10 +790,12 @@ def workspace_close(app, remove_dir, all_, elements):
help="Track and fetch the latest source before resetting")
@click.option('--all', '-a', 'all_', default=False, is_flag=True,
help="Reset all open workspaces")
+@click.option('--assume-yes', '-y', default=False, is_flag=True,
+ help="Assume 'yes' to confirmation of destructive changes")
@click.argument('elements', nargs=-1,
type=click.Path(readable=False))
@click.pass_obj
-def workspace_reset(app, soft, track_, all_, elements):
+def workspace_reset(app, soft, track_, all_, assume_yes, elements):
"""Reset a workspace to its original state"""
# Check that the workspaces in question exist
@@ -798,9 +807,14 @@ def workspace_reset(app, soft, track_, all_, elements):
if all_ and not app.stream.workspace_exists():
raise AppError("No open workspaces to reset")
- if app.interactive and not soft and app.context.prompt_workspace_reset_hard:
- if not click.confirm('This will remove all your changes, are you sure?'):
- click.echo('Aborting', err=True)
+ if not soft and not assume_yes and app.context.prompt_workspace_reset_hard:
+ if app.interactive:
+ if not click.confirm('This will remove all your changes, are you sure?'):
+ click.echo('Aborting', err=True)
+ sys.exit(-1)
+ else:
+ click.echo("Aborted destructive non-interactive action.", err=True)
+ click.echo("Please use the '--assume-yes' option to override.", err=True)
sys.exit(-1)
if all_:
diff --git a/tests/examples/developing.py b/tests/examples/developing.py
index 4bb7076..7254794 100644
--- a/tests/examples/developing.py
+++ b/tests/examples/developing.py
@@ -65,7 +65,8 @@ def test_open_workspace(cli, tmpdir, datafiles):
result = cli.run(project=project, args=['workspace', 'list'])
result.assert_success()
- result = cli.run(project=project, args=['workspace', 'close', '--remove-dir', 'hello.bst'])
+ result = cli.run(
+ project=project, args=['workspace', 'close', '--remove-dir', '--assume-yes', 'hello.bst'])
result.assert_success()
@@ -95,5 +96,6 @@ def test_make_change_in_workspace(cli, tmpdir, datafiles):
result.assert_success()
assert result.output == 'Hello World\nWe can use workspaces!\n'
- result = cli.run(project=project, args=['workspace', 'close', '--remove-dir', 'hello.bst'])
+ result = cli.run(
+ project=project, args=['workspace', 'close', '--remove-dir', '--assume-yes', 'hello.bst'])
result.assert_success()
diff --git a/tests/examples/junctions.py b/tests/examples/junctions.py
index 97c622b..ddcada1 100644
--- a/tests/examples/junctions.py
+++ b/tests/examples/junctions.py
@@ -51,6 +51,7 @@ def test_open_cross_junction_workspace(cli, tmpdir, datafiles):
args=['workspace', 'open', '--directory', workspace_dir, 'hello-junction.bst:hello.bst'])
result.assert_success()
- result = cli.run(project=project,
- args=['workspace', 'close', '--remove-dir', 'hello-junction.bst:hello.bst'])
+ result = cli.run(
+ project=project,
+ args=['workspace', 'close', '--remove-dir', '--assume-yes', 'hello-junction.bst:hello.bst'])
result.assert_success()
diff --git a/tests/frontend/cross_junction_workspace.py b/tests/frontend/cross_junction_workspace.py
index fb2b34c..e265c9e 100644
--- a/tests/frontend/cross_junction_workspace.py
+++ b/tests/frontend/cross_junction_workspace.py
@@ -82,7 +82,7 @@ def test_close_cross_junction(cli, tmpdir):
project, workspace = open_cross_junction(cli, tmpdir)
element = 'sub.bst:data.bst'
- args = ['workspace', 'close', '--remove-dir', element]
+ args = ['workspace', 'close', '--remove-dir', '--assume-yes', element]
result = cli.run(project=project, args=args)
result.assert_success()
@@ -101,7 +101,7 @@ def test_close_cross_junction(cli, tmpdir):
def test_close_all_cross_junction(cli, tmpdir):
project, workspace = open_cross_junction(cli, tmpdir)
- args = ['workspace', 'close', '--remove-dir', '--all']
+ args = ['workspace', 'close', '--remove-dir', '--assume-yes', '--all']
result = cli.run(project=project, args=args)
result.assert_success()
diff --git a/tests/frontend/workspace.py b/tests/frontend/workspace.py
index bc928ad..03ae262 100644
--- a/tests/frontend/workspace.py
+++ b/tests/frontend/workspace.py
@@ -393,7 +393,7 @@ def test_close(cli, tmpdir, datafiles, kind):
# Close the workspace
result = cli.run(project=project, args=[
- 'workspace', 'close', '--remove-dir', element_name
+ 'workspace', 'close', '--remove-dir', '--assume-yes', element_name
])
result.assert_success()
@@ -413,7 +413,7 @@ def test_close_external_after_move_project(cli, tmpdir, datafiles):
# Close the workspace
result = cli.run(project=moved_dir, args=[
- 'workspace', 'close', '--remove-dir', element_name
+ 'workspace', 'close', '--remove-dir', '--assume-yes', element_name
])
result.assert_success()
@@ -433,7 +433,7 @@ def test_close_internal_after_move_project(cli, tmpdir, datafiles):
# Close the workspace
result = cli.run(project=moved_dir, args=[
- 'workspace', 'close', '--remove-dir', element_name
+ 'workspace', 'close', '--remove-dir', '--assume-yes', element_name
])
result.assert_success()
@@ -471,7 +471,7 @@ def test_close_nonexistant_element(cli, tmpdir, datafiles):
# Close the workspace
result = cli.run(project=project, args=[
- 'workspace', 'close', '--remove-dir', element_name
+ 'workspace', 'close', '--remove-dir', '--assume-yes', element_name
])
result.assert_success()
@@ -490,7 +490,7 @@ def test_close_multiple(cli, tmpdir, datafiles):
# Close the workspaces
result = cli.run(project=project, args=[
- 'workspace', 'close', '--remove-dir', alpha, beta
+ 'workspace', 'close', '--remove-dir', '--assume-yes', alpha, beta
])
result.assert_success()
@@ -510,7 +510,7 @@ def test_close_all(cli, tmpdir, datafiles):
# Close the workspaces
result = cli.run(project=project, args=[
- 'workspace', 'close', '--remove-dir', '--all'
+ 'workspace', 'close', '--remove-dir', '--assume-yes', '--all'
])
result.assert_success()
@@ -533,7 +533,7 @@ def test_reset(cli, tmpdir, datafiles):
# Now reset the open workspace, this should have the
# effect of reverting our changes.
result = cli.run(project=project, args=[
- 'workspace', 'reset', element_name
+ 'workspace', 'reset', '--assume-yes', element_name
])
result.assert_success()
assert os.path.exists(os.path.join(workspace, 'usr', 'bin', 'hello'))
@@ -559,7 +559,7 @@ def test_reset_multiple(cli, tmpdir, datafiles):
# Now reset the open workspaces, this should have the
# effect of reverting our changes.
result = cli.run(project=project, args=[
- 'workspace', 'reset', alpha, beta,
+ 'workspace', 'reset', '--assume-yes', alpha, beta,
])
result.assert_success()
assert os.path.exists(os.path.join(workspace_alpha, 'usr', 'bin', 'hello'))
@@ -585,7 +585,7 @@ def test_reset_all(cli, tmpdir, datafiles):
# Now reset the open workspace, this should have the
# effect of reverting our changes.
result = cli.run(project=project, args=[
- 'workspace', 'reset', '--all'
+ 'workspace', 'reset', '--assume-yes', '--all'
])
result.assert_success()
assert os.path.exists(os.path.join(workspace_alpha, 'usr', 'bin', 'hello'))
@@ -947,7 +947,8 @@ def test_list_supported_workspace(cli, tmpdir, datafiles, workspace_cfg, expecte
# Make a change to the workspaces file
result = cli.run(project=project, args=['workspace', 'open', '--directory', workspace, element_name])
result.assert_success()
- result = cli.run(project=project, args=['workspace', 'close', '--remove-dir', element_name])
+ result = cli.run(
+ project=project, args=['workspace', 'close', '--remove-dir', '--assume-yes', element_name])
result.assert_success()
# Check that workspace config is converted correctly if necessary
diff --git a/tests/plugins/filter.py b/tests/plugins/filter.py
index 6dae085..0a7fa1e 100644
--- a/tests/plugins/filter.py
+++ b/tests/plugins/filter.py
@@ -172,7 +172,7 @@ def test_filter_workspace_reset(datafiles, cli, tmpdir):
src = os.path.join(workspace_dir, "foo")
dst = os.path.join(workspace_dir, "quux")
shutil.copyfile(src, dst)
- result = cli.run(project=project, args=['workspace', 'reset', 'deps-permitted.bst'])
+ result = cli.run(project=project, args=['workspace', 'reset', '--assume-yes', 'deps-permitted.bst'])
result.assert_success()
result = cli.run(project=project, args=['build', 'output-orphans.bst'])
result.assert_success()