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



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



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]