[Notes] [Git][BuildStream/buildstream][aevri/safe_noninteractive] BREAK: make destructive action scripts consistent

[Angelos Evripiotis <gitlab mg gitlab com>]

Title: GitLab

Angelos Evripiotis pushed to branch aevri/safe_noninteractive at BuildStream / buildstream

Commits:

• 5d1a33e1
  by Angelos Evripiotis at 2018-11-16T18:30:17Z

  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.
  

6 changed files:

• buildstream/_frontend/cli.py
• tests/examples/developing.py
• tests/examples/junctions.py
• tests/frontend/cross_junction_workspace.py
• tests/frontend/workspace.py
• tests/plugins/filter.py

Changes:

buildstream/_frontend/cli.py

---

...	...	@@ -713,10 +713,12 @@ def workspace_open(app, no_checkout, force, track_, element, directory):
713	713	help="Remove the path that contains the closed workspace")
714	714	@click.option('--all', '-a', 'all_', default=False, is_flag=True,
715	715	help="Close all open workspaces")
	716	+@click.option('--assume-yes', '-y', default=False, is_flag=True,
	717	+ help="Assume 'yes' to confirmation of destructive changes")
716	718	@click.argument('elements', nargs=-1,
717	719	type=click.Path(readable=False))
718	720	@click.pass_obj
719		-def workspace_close(app, remove_dir, all_, elements):
	721	+def workspace_close(app, remove_dir, all_, assume_yes, elements):
720	722	"""Close a workspace"""
721	723
722	724	if not (all_ or elements):
...	...	@@ -743,9 +745,15 @@ def workspace_close(app, remove_dir, all_, elements):
743	745	if nonexisting:
744	746	raise AppError("Workspace does not exist", detail="\n".join(nonexisting))
745	747
746		- if app.interactive and remove_dir and app.context.prompt_workspace_close_remove_dir:
747		- if not click.confirm('This will remove all your changes, are you sure?'):
748		- click.echo('Aborting', err=True)
	748	+ if remove_dir and not assume_yes and app.context.prompt_workspace_close_remove_dir:
	749	+ if app.interactive:
	750	+ if not click.confirm('This will remove all your changes, are you sure?'):
	751	+ click.echo('Aborting', err=True)
	752	+ sys.exit(-1)
	753	+ else:
	754	+ click.echo(
	755	+ "Aborting since non-interactive and destructive action requested.", err=True)
	756	+ click.echo("Please use the '--assume-yes' option to override.", err=True)
749	757	sys.exit(-1)
750	758
751	759	for element_name in elements:
...	...	@@ -762,10 +770,12 @@ def workspace_close(app, remove_dir, all_, elements):
762	770	help="Track and fetch the latest source before resetting")
763	771	@click.option('--all', '-a', 'all_', default=False, is_flag=True,
764	772	help="Reset all open workspaces")
	773	+@click.option('--assume-yes', '-y', default=False, is_flag=True,
	774	+ help="Assume 'yes' to confirmation of destructive changes")
765	775	@click.argument('elements', nargs=-1,
766	776	type=click.Path(readable=False))
767	777	@click.pass_obj
768		-def workspace_reset(app, soft, track_, all_, elements):
	778	+def workspace_reset(app, soft, track_, all_, assume_yes, elements):
769	779	"""Reset a workspace to its original state"""
770	780
771	781	# Check that the workspaces in question exist
...	...	@@ -777,9 +787,15 @@ def workspace_reset(app, soft, track_, all_, elements):
777	787	if all_ and not app.stream.workspace_exists():
778	788	raise AppError("No open workspaces to reset")
779	789
780		- if app.interactive and not soft and app.context.prompt_workspace_reset_hard:
781		- if not click.confirm('This will remove all your changes, are you sure?'):
782		- click.echo('Aborting', err=True)
	790	+ if not soft and not assume_yes and app.context.prompt_workspace_reset_hard:
	791	+ if app.interactive:
	792	+ if not click.confirm('This will remove all your changes, are you sure?'):
	793	+ click.echo('Aborting', err=True)
	794	+ sys.exit(-1)
	795	+ else:
	796	+ click.echo(
	797	+ "Aborting since non-interactive and destructive action requested.", err=True)
	798	+ click.echo("Please use the '--assume-yes' option to override.", err=True)
783	799	sys.exit(-1)
784	800
785	801	if all_:

tests/examples/developing.py

---

...	...	@@ -61,7 +61,8 @@ def test_open_workspace(cli, tmpdir, datafiles):
61	61	result = cli.run(project=project, args=['workspace', 'list'])
62	62	result.assert_success()
63	63
64		- result = cli.run(project=project, args=['workspace', 'close', '--remove-dir', 'hello.bst'])
	64	+ result = cli.run(
	65	+ project=project, args=['workspace', 'close', '--remove-dir', '--assume-yes', 'hello.bst'])
65	66	result.assert_success()
66	67
67	68
...	...	@@ -89,5 +90,6 @@ def test_make_change_in_workspace(cli, tmpdir, datafiles):
89	90	result.assert_success()
90	91	assert result.output == 'Hello World\nWe can use workspaces!\n'
91	92
92		- result = cli.run(project=project, args=['workspace', 'close', '--remove-dir', 'hello.bst'])
	93	+ result = cli.run(
	94	+ project=project, args=['workspace', 'close', '--remove-dir', '--assume-yes', 'hello.bst'])
93	95	result.assert_success()

tests/examples/junctions.py

---

...	...	@@ -47,6 +47,7 @@ def test_open_cross_junction_workspace(cli, tmpdir, datafiles):
47	47	args=['workspace', 'open', 'hello-junction.bst:hello.bst', workspace_dir])
48	48	result.assert_success()
49	49
50		- result = cli.run(project=project,
51		- args=['workspace', 'close', '--remove-dir', 'hello-junction.bst:hello.bst'])
	50	+ result = cli.run(
	51	+ project=project,
	52	+ args=['workspace', 'close', '--remove-dir', '--assume-yes', 'hello-junction.bst:hello.bst'])
52	53	result.assert_success()

tests/frontend/cross_junction_workspace.py

---

...	...	@@ -82,7 +82,7 @@ def test_close_cross_junction(cli, tmpdir):
82	82	project, workspace = open_cross_junction(cli, tmpdir)
83	83
84	84	element = 'sub.bst:data.bst'
85		- args = ['workspace', 'close', '--remove-dir', element]
	85	+ args = ['workspace', 'close', '--remove-dir', '--assume-yes', element]
86	86	result = cli.run(project=project, args=args)
87	87	result.assert_success()
88	88
...	...	@@ -101,7 +101,7 @@ def test_close_cross_junction(cli, tmpdir):
101	101	def test_close_all_cross_junction(cli, tmpdir):
102	102	project, workspace = open_cross_junction(cli, tmpdir)
103	103
104		- args = ['workspace', 'close', '--remove-dir', '--all']
	104	+ args = ['workspace', 'close', '--remove-dir', '--assume-yes', '--all']
105	105	result = cli.run(project=project, args=args)
106	106	result.assert_success()
107	107

tests/frontend/workspace.py

---

...	...	@@ -214,7 +214,7 @@ def test_close(cli, tmpdir, datafiles, kind):
214	214
215	215	# Close the workspace
216	216	result = cli.run(project=project, args=[
217		- 'workspace', 'close', '--remove-dir', element_name
	217	+ 'workspace', 'close', '--remove-dir', '--assume-yes', element_name
218	218	])
219	219	result.assert_success()
220	220
...	...	@@ -234,7 +234,7 @@ def test_close_external_after_move_project(cli, tmpdir, datafiles):
234	234
235	235	# Close the workspace
236	236	result = cli.run(project=moved_dir, args=[
237		- 'workspace', 'close', '--remove-dir', element_name
	237	+ 'workspace', 'close', '--remove-dir', '--assume-yes', element_name
238	238	])
239	239	result.assert_success()
240	240
...	...	@@ -254,7 +254,7 @@ def test_close_internal_after_move_project(cli, tmpdir, datafiles):
254	254
255	255	# Close the workspace
256	256	result = cli.run(project=moved_dir, args=[
257		- 'workspace', 'close', '--remove-dir', element_name
	257	+ 'workspace', 'close', '--remove-dir', '--assume-yes', element_name
258	258	])
259	259	result.assert_success()
260	260
...	...	@@ -292,7 +292,7 @@ def test_close_nonexistant_element(cli, tmpdir, datafiles):
292	292
293	293	# Close the workspace
294	294	result = cli.run(project=project, args=[
295		- 'workspace', 'close', '--remove-dir', element_name
	295	+ 'workspace', 'close', '--remove-dir', '--assume-yes', element_name
296	296	])
297	297	result.assert_success()
298	298
...	...	@@ -311,7 +311,7 @@ def test_close_multiple(cli, tmpdir, datafiles):
311	311
312	312	# Close the workspaces
313	313	result = cli.run(project=project, args=[
314		- 'workspace', 'close', '--remove-dir', alpha, beta
	314	+ 'workspace', 'close', '--remove-dir', '--assume-yes', alpha, beta
315	315	])
316	316	result.assert_success()
317	317
...	...	@@ -331,7 +331,7 @@ def test_close_all(cli, tmpdir, datafiles):
331	331
332	332	# Close the workspaces
333	333	result = cli.run(project=project, args=[
334		- 'workspace', 'close', '--remove-dir', '--all'
	334	+ 'workspace', 'close', '--remove-dir', '--assume-yes', '--all'
335	335	])
336	336	result.assert_success()
337	337
...	...	@@ -354,7 +354,7 @@ def test_reset(cli, tmpdir, datafiles):
354	354	# Now reset the open workspace, this should have the
355	355	# effect of reverting our changes.
356	356	result = cli.run(project=project, args=[
357		- 'workspace', 'reset', element_name
	357	+ 'workspace', 'reset', '--assume-yes', element_name
358	358	])
359	359	result.assert_success()
360	360	assert os.path.exists(os.path.join(workspace, 'usr', 'bin', 'hello'))
...	...	@@ -380,7 +380,7 @@ def test_reset_multiple(cli, tmpdir, datafiles):
380	380	# Now reset the open workspaces, this should have the
381	381	# effect of reverting our changes.
382	382	result = cli.run(project=project, args=[
383		- 'workspace', 'reset', alpha, beta,
	383	+ 'workspace', 'reset', '--assume-yes', alpha, beta,
384	384	])
385	385	result.assert_success()
386	386	assert os.path.exists(os.path.join(workspace_alpha, 'usr', 'bin', 'hello'))
...	...	@@ -406,7 +406,7 @@ def test_reset_all(cli, tmpdir, datafiles):
406	406	# Now reset the open workspace, this should have the
407	407	# effect of reverting our changes.
408	408	result = cli.run(project=project, args=[
409		- 'workspace', 'reset', '--all'
	409	+ 'workspace', 'reset', '--assume-yes', '--all'
410	410	])
411	411	result.assert_success()
412	412	assert os.path.exists(os.path.join(workspace_alpha, 'usr', 'bin', 'hello'))
...	...	@@ -768,7 +768,8 @@ def test_list_supported_workspace(cli, tmpdir, datafiles, workspace_cfg, expecte
768	768	# Make a change to the workspaces file
769	769	result = cli.run(project=project, args=['workspace', 'open', element_name, workspace])
770	770	result.assert_success()
771		- result = cli.run(project=project, args=['workspace', 'close', '--remove-dir', element_name])
	771	+ result = cli.run(
	772	+ project=project, args=['workspace', 'close', '--remove-dir', '--assume-yes', element_name])
772	773	result.assert_success()
773	774
774	775	# Check that workspace config is converted correctly if necessary

tests/plugins/filter.py

---

...	...	@@ -163,7 +163,7 @@ def test_filter_workspace_reset(datafiles, cli, tmpdir):
163	163	src = os.path.join(workspace_dir, "foo")
164	164	dst = os.path.join(workspace_dir, "quux")
165	165	shutil.copyfile(src, dst)
166		- result = cli.run(project=project, args=['workspace', 'reset', 'deps-permitted.bst'])
	166	+ result = cli.run(project=project, args=['workspace', 'reset', '--assume-yes', 'deps-permitted.bst'])
167	167	result.assert_success()
168	168	result = cli.run(project=project, args=['build', 'output-orphans.bst'])
169	169	result.assert_success()

—
View it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.