[BuildStream] Breaking change: changing '--force or fail' to y/n prompts

[Angelos Evripiotis <angelos evripiotis gmail com>]

Hello list,

I'm proposing a change that will make bst's behaviour around destructive

actions more consistent, moving toward the "confirmation prompt" approach and

away from the '--force or fail' approach. The proposal is a combo of both.

For the record, I strongly prefer the '--force or fail' but it seems the

philosophy of the project is 'confirmation prompt', so I'm respecting that.

Please let me know if you disagree :)

Executive summmary

------------------

Commands that show 'Are you sure?' prompts will gain '--assume-yes' and

'--force' arguments. They will not automatically proceed when non-interactive,

this is a breaking change.

Commands that have '--force' arguments will also gain '--assume-yes', which

mean the same thing. They will change to prompt by default, with

'<<destructive action>>? (yes/no/y/n)'.

We will also introduce '--assume-no', and '--require-confirmation' where it

makes sense.

If a user responds with 'no' to a prompt, the loop will skip the current item

and continue with the remaining items. '--assume-no' will behave in the same

way. This is a breaking change, as the current behaviour is to abort

immediately. This new behaviour is more complex, but allows unattended scripts

to do more without intervention. It only affects 'workspace open'.

In later work we may introduce '--assume-abort', '(abort/a)' in the prompt, and

an 'abort' option in buildstream.conf.

Option names rationale

----------------------

When we introduce confirmation prompts, there will be three kinds of behaviour:

1. Assume the user wants the destructive action. (as in `--force`)

2. Assume the user wants to skip the destructive action and continue any loop,

eventually exit with error if any destructive actions were skipped.

3. Ask the user (y/n).

We will be removing a fourth kind of behaviour:

4. Assume the user doesn't want the destructive action and abort immediately

with an error.

It feels ambiguous to me whether `--no-force` would be asking for (2), (3), or

(4); so I haven't put that forward. It's a shame because '--no-...' is

something of a common pattern and also specially supported by click.

I really like `--force` but haven't found good matching names for the other

things, so I'm proposing this combination:

- `--assume-yes`, and `--force`, and `--yes`, maybe `-y`, maybe `-f` for (1). A

little redundancy isn't a bad thing if it's clear what it means.

- `--assume-no`, `--no` for (2).

- For completeness `--require-confirmation`, `--ask` for (3); which is the new

default in all cases.

I considered this but decided against this combination:

- Only `--force`, maybe `-f` for (1).

- `--fail`, for (4).

- `--require-confirmation`, `--ask` for (3), which is the new default.

I also decided against this combination:

- `--no-safety` for (1).

- `--safety` for (4).

- `--require-confirmation`, `--ask` for (3), which is the new default.

We may later want to also add '--assume-abort' for (4) for scriptability

anyway. That wouldn't be a breaking change and can be tackled in separate

work.

It's expected that --assume-no and --assume-abort will result in a helpful

error message with the command necessary to resume, e.g. something like:

$ bst workspace open --assume-no {1..100}.bst

...

Assumed 'no' to overwriting some existing workspaces,

you can force those with:

bst workspace open --force 1.bst 5.bst 10.bst

$ echo $?

1

cli changes for 'are you sure?' prompts

---------------------------------------

We have two 'are you sure?' prompts:

- Every time you run 'bst workspace close --remove-dir'.

- Every time you run 'bst workspace reset'.

These commands will gain '--assume-yes' and '--force' parameters, which

override the prompts with a 'yes' answer.

These commands will also gain '--require-confirmation' parameters, which

overrides anything in buildstream.conf to present a confirmation prompt.

They will not get '--assume-no', because it would always fail.

If non-interactive and confirmation is required, these commands will change to

fail by default. This is a breaking change, which importantly makes

non-interactive consistent in that it's safe by default.

cli changes for '--force'-able commands

---------------------------------------

We have four '--force'-able commands:

- bst init: allow overwriting an existing project.conf.

- bst checkout: allow files to be overwritten.

- bst workspace open: The workspace will be created even if the directory in

which it will be created is not empty or if a workspace for that element

already exists.

- bst source-bundle: overwrite an existing tarball.

We will add an '--assume-no' option. This is so that scripts can be more

readable and don't have to resort to the more opaque '--no-interactive' to be

non-destructive and be sure they won't hang on a prompt.

In addition to '--force', there will be '--assume-yes' for symmetry with

'--assume-no'.

If interactive and '--assume-...' is not specified, these commands will change

to ask the user for confirmation of the destructive action, in the case of

exceptional events that can be forced. This can be changed back in

buildstream.conf.

These commands will also gain '--require-confirmation' parameters, which

overrides anything in buildstream.conf to present a confirmation prompt.

If non-interactive and confirmation is required, these commands will abort

immediately by default, like option (4) above. This is the current behaviour

for non-interactive.

builstream.conf changes

-----------------------

The 'are you sure?' prompts already have buildstream.conf options:

#

# 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 --remove-dir'

# removing a workspace directory, potentially losing changes.

#

# ask - Ask the user if they are sure.

# yes - Always remove, without asking.

#

really-workspace-close-remove-dir: ask

# Whether to really proceed with 'bst workspace reset' doing a hard

# reset of a workspace, potentially losing changes.

#

# ask - Ask the user if they are sure.

# yes - Always hard reset, without asking.

#

really-workspace-reset-hard: ask

We would add these ones:

# 'bst init': whether to overwrite an existing project.conf.

#

# ask - Ask the user if they want to.

# yes - Always overwrite, without asking.

# no - Never overwrite, unless overridden with '--force'.

#

bst-init-overwrite: ask

# 'bst checkout': whether to allow files to be overwritten.

#

# ask - Ask the user if they want to.

# yes - Always overwrite, without asking.

# no - Never overwrite, unless overridden with '--force'.

#

bst-checkout-overwrite: ask

# 'bst workspace open': whether to create the workspace even if the

# directory in which it will be created is not empty or if a workspace

# for that element already exists.

#

# ask - Ask the user if they want to.

# yes - Always overwrite, without asking.

# no - Never overwrite, unless overridden with '--force'.

#

bst-workspace-open-overwrite: ask

# 'bst source-bundle': whether to overwrite an existing tarball.

#

# ask - Ask the user if they want to.

# yes - Always overwrite, without asking.

# no - Never overwrite, unless overridden with '--force'.

#

bst-source-bundle-overwrite: ask

In later work, we might also add 'abort' options.

I don't think we will need 'ask-assume-yes', 'ask-assume-no', etc.

The proposed meaning for 'ask' is 'ask-assume-abort'. This means that

'--no-interactive' in combination with '--ask' is the same as '--assume-abort'.

---

Hopefully you'll prefer to stick with the simpler '--force or fail',

and instead remove the all 'are you sure?' prompts, let me know!

Cheers,

Angelos

P.S.

Note that this all springs out of this merge-request conversation:

https://gitlab.com/BuildStream/buildstream/merge_requests/957#note_121310675

P.P.S.

Note that there is a separate proposal to remove the 'auto-init' functionality,

which also has a prompt. Therefore that is not considered here.

https://mail.gnome.org/archives/buildstream-list/2018-December/msg00082.html

Also note that the --force parameter to source-bundle doesn't appear to be

hooked up, so I'd probably fix that while I'm there:

https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_stream.py#L744