From b2dedee3c821415d05996dc6414254610bc14040 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sun, 20 Dec 2015 01:34:00 +0100 Subject: [PATCH] refactor yes(), cleanup env var semantics, fixes #355 refactorings: - introduced concept of default answer: if the answer string is in the defaultish sequence, the return value of yes() will be the default. e.g. if just pressing when asked on the console or if an empty string or "default" is in the environment variable for overriding. if an environment var has an invalid value and no retries are enabled: return default if retries are enabled, next retry won't use the env var again, but either ask via input(). - simplify: only one default - this should be a SAFE default as it is used in some special conditions like EOF or invalid input with retries disallowed. no isatty() magic, the "yes" shell command exists, so we could receive input even if it is not from a tty. - clean: separate retry flag from retry_msg --- borg/archiver.py | 8 ++-- borg/cache.py | 6 +-- borg/helpers.py | 86 +++++++++++++++++++------------------ borg/testsuite/archiver.py | 8 ++-- borg/testsuite/benchmark.py | 6 +-- borg/testsuite/helpers.py | 81 ++++++++++++++++++++-------------- docs/usage.rst | 18 +++++--- 7 files changed, 117 insertions(+), 96 deletions(-) diff --git a/borg/archiver.py b/borg/archiver.py index 69bda26a3f..5afb2f89df 100644 --- a/borg/archiver.py +++ b/borg/archiver.py @@ -105,8 +105,8 @@ def do_check(self, args): msg = ("'check --repair' is an experimental feature that might result in data loss." + "\n" + "Type 'YES' if you understand this and want to continue: ") - if not yes(msg, false_msg="Aborting.", default_notty=False, - env_var_override='BORG_CHECK_I_KNOW_WHAT_I_AM_DOING', truish=('YES', )): + if not yes(msg, false_msg="Aborting.", truish=('YES', ), + env_var_override='BORG_CHECK_I_KNOW_WHAT_I_AM_DOING'): return EXIT_ERROR if not args.archives_only: if not repository.check(repair=args.repair, save_space=args.save_space): @@ -374,8 +374,8 @@ def do_delete(self, args): msg.append(format_archive(archive_info)) msg.append("Type 'YES' if you understand this and want to continue: ") msg = '\n'.join(msg) - if not yes(msg, false_msg="Aborting.", default_notty=False, - env_var_override='BORG_DELETE_I_KNOW_WHAT_I_AM_DOING', truish=('YES', )): + if not yes(msg, false_msg="Aborting.", truish=('YES', ), + env_var_override='BORG_DELETE_I_KNOW_WHAT_I_AM_DOING'): self.exit_code = EXIT_ERROR return self.exit_code repository.destroy() diff --git a/borg/cache.py b/borg/cache.py index 707ad963b2..521b684627 100644 --- a/borg/cache.py +++ b/borg/cache.py @@ -54,8 +54,7 @@ def __init__(self, repository, key, manifest, path=None, sync=True, do_files=Fal msg = ("Warning: Attempting to access a previously unknown unencrypted repository!" + "\n" + "Do you want to continue? [yN] ") - if not yes(msg, false_msg="Aborting.", default_notty=False, - env_var_override='BORG_UNKNOWN_UNENCRYPTED_REPO_ACCESS_IS_OK'): + if not yes(msg, false_msg="Aborting.", env_var_override='BORG_UNKNOWN_UNENCRYPTED_REPO_ACCESS_IS_OK'): raise self.CacheInitAbortedError() self.create() self.open(lock_wait=lock_wait) @@ -64,8 +63,7 @@ def __init__(self, repository, key, manifest, path=None, sync=True, do_files=Fal msg = ("Warning: The repository at location {} was previously located at {}".format(repository._location.canonical_path(), self.previous_location) + "\n" + "Do you want to continue? [yN] ") - if not yes(msg, false_msg="Aborting.", default_notty=False, - env_var_override='BORG_RELOCATED_REPO_ACCESS_IS_OK'): + if not yes(msg, false_msg="Aborting.", env_var_override='BORG_RELOCATED_REPO_ACCESS_IS_OK'): raise self.RepositoryAccessAborted() if sync and self.manifest.id != self.manifest_id: diff --git a/borg/helpers.py b/borg/helpers.py index 3bb7cf5b2c..7957ee38d6 100644 --- a/borg/helpers.py +++ b/borg/helpers.py @@ -842,71 +842,69 @@ def is_slow_msgpack(): return msgpack.Packer is msgpack.fallback.Packer -def yes(msg=None, retry_msg=None, false_msg=None, true_msg=None, - default=False, default_notty=None, default_eof=None, - falsish=('No', 'no', 'N', 'n'), truish=('Yes', 'yes', 'Y', 'y'), - env_var_override=None, ifile=None, ofile=None, input=input): +FALSISH = ('No', 'NO', 'no', 'N', 'n', '0', ) +TRUISH = ('Yes', 'YES', 'yes', 'Y', 'y', '1', ) +DEFAULTISH = ('Default', 'DEFAULT', 'default', 'D', 'd', '', ) + +def yes(msg=None, false_msg=None, true_msg=None, default_msg=None, + retry_msg=None, invalid_msg=None, env_msg=None, + falsish=FALSISH, truish=TRUISH, defaultish=DEFAULTISH, + default=False, retry=True, env_var_override=None, ofile=None, input=input): """ Output (usually a question) and let user input an answer. - Qualifies the answer according to falsish and truish as True or False. + Qualifies the answer according to falsish, truish and defaultish as True, False or . If it didn't qualify and retry_msg is None (no retries wanted), return the default [which defaults to False]. Otherwise let user retry answering until answer is qualified. - If env_var_override is given and it is non-empty, counts as truish answer - and won't ask user for an answer. - If we don't have a tty as input and default_notty is not None, return its value. - Otherwise read input from non-tty and proceed as normal. - If EOF is received instead an input, return default_eof [or default, if not given]. + If env_var_override is given and this var is present in the environment, do not ask + the user, but just use the env var contents as answer as if it was typed in. + Otherwise read input from stdin and proceed as normal. + If EOF is received instead an input or an invalid input without retry possibility, + return default. :param msg: introducing message to output on ofile, no \n is added [None] :param retry_msg: retry message to output on ofile, no \n is added [None] - (also enforces retries instead of returning default) :param false_msg: message to output before returning False [None] :param true_msg: message to output before returning True [None] - :param default: default return value (empty answer is given) [False] - :param default_notty: if not None, return its value if no tty is connected [None] - :param default_eof: return value if EOF was read as answer [same as default] + :param default_msg: message to output before returning a [None] + :param invalid_msg: message to output after a invalid answer was given [None] + :param env_msg: message to output when using input from env_var_override [None], + needs to have 2 placeholders for answer and env var name, e.g.: "{} (from {})" :param falsish: sequence of answers qualifying as False :param truish: sequence of answers qualifying as True + :param defaultish: sequence of answers qualifying as + :param default: default return value (defaultish answer was given or no-answer condition) [False] + :param retry: if True and input is incorrect, retry. Otherwise return default. [True] :param env_var_override: environment variable name [None] - :param ifile: input stream [sys.stdin] (only for testing!) :param ofile: output stream [sys.stderr] :param input: input function [input from builtins] :return: boolean answer value, True or False """ - # note: we do not assign sys.stdin/stderr as defaults above, so they are + # note: we do not assign sys.stderr as default above, so it is # really evaluated NOW, not at function definition time. - if ifile is None: - ifile = sys.stdin if ofile is None: ofile = sys.stderr if default not in (True, False): raise ValueError("invalid default value, must be True or False") - if default_notty not in (None, True, False): - raise ValueError("invalid default_notty value, must be None, True or False") - if default_eof not in (None, True, False): - raise ValueError("invalid default_eof value, must be None, True or False") if msg: - print(msg, file=ofile, end='') - ofile.flush() - if env_var_override: - value = os.environ.get(env_var_override) - # currently, any non-empty value counts as truish - # TODO: change this so one can give y/n there? - if value: - value = bool(value) - value_str = truish[0] if value else falsish[0] - print("{} (from {})".format(value_str, env_var_override), file=ofile) - return value - if default_notty is not None and not ifile.isatty(): - # looks like ifile is not a terminal (but e.g. a pipe) - return default_notty + print(msg, file=ofile, end='', flush=True) while True: - try: - answer = input() # XXX how can we use ifile? - except EOFError: - return default_eof if default_eof is not None else default + answer = None + if env_var_override: + answer = os.environ.get(env_var_override) + if answer is not None and env_msg: + print(env_msg.format(answer, env_var_override), file=ofile) + if answer is None: + try: + answer = input() + except EOFError: + # avoid defaultish[0], defaultish could be empty + answer = truish[0] if default else falsish[0] + if answer in defaultish: + if default_msg: + print(default_msg, file=ofile) + return default if answer in truish: if true_msg: print(true_msg, file=ofile) @@ -915,11 +913,15 @@ def yes(msg=None, retry_msg=None, false_msg=None, true_msg=None, if false_msg: print(false_msg, file=ofile) return False - if retry_msg is None: - # no retries wanted, we just return the default + # if we get here, the answer was invalid + if invalid_msg: + print(invalid_msg, file=ofile) + if not retry: return default if retry_msg: print(retry_msg, file=ofile, end='', flush=True) + # in case we used an environment variable and it gave an invalid answer, do not use it again: + env_var_override = None class ProgressIndicatorPercent: diff --git a/borg/testsuite/archiver.py b/borg/testsuite/archiver.py index 90d85c056b..ea9d1a7272 100644 --- a/borg/testsuite/archiver.py +++ b/borg/testsuite/archiver.py @@ -136,7 +136,7 @@ def make_files(dir, count, size, rnd=True): data = os.urandom(size) f.write(data) - with environment_variable(BORG_CHECK_I_KNOW_WHAT_I_AM_DOING='1'): + with environment_variable(BORG_CHECK_I_KNOW_WHAT_I_AM_DOING='YES'): mount = DF_MOUNT assert os.path.exists(mount) repo = os.path.join(mount, 'repo') @@ -190,8 +190,8 @@ class ArchiverTestCaseBase(BaseTestCase): prefix = '' def setUp(self): - os.environ['BORG_CHECK_I_KNOW_WHAT_I_AM_DOING'] = '1' - os.environ['BORG_DELETE_I_KNOW_WHAT_I_AM_DOING'] = '1' + os.environ['BORG_CHECK_I_KNOW_WHAT_I_AM_DOING'] = 'YES' + os.environ['BORG_DELETE_I_KNOW_WHAT_I_AM_DOING'] = 'YES' os.environ['BORG_PASSPHRASE'] = 'waytooeasyonlyfortests' self.archiver = not self.FORK_DEFAULT and Archiver() or None self.tmpdir = tempfile.mkdtemp() @@ -330,7 +330,7 @@ def test_basic_functionality(self): item_count = 3 if has_lchflags else 4 # one file is UF_NODUMP self.assert_in('Number of files: %d' % item_count, info_output) shutil.rmtree(self.cache_path) - with environment_variable(BORG_UNKNOWN_UNENCRYPTED_REPO_ACCESS_IS_OK='1'): + with environment_variable(BORG_UNKNOWN_UNENCRYPTED_REPO_ACCESS_IS_OK='yes'): info_output2 = self.cmd('info', self.repository_location + '::test') def filter(output): diff --git a/borg/testsuite/benchmark.py b/borg/testsuite/benchmark.py index 6979fcfa97..cb7f9e90bd 100644 --- a/borg/testsuite/benchmark.py +++ b/borg/testsuite/benchmark.py @@ -16,9 +16,9 @@ @pytest.yield_fixture def repo_url(request, tmpdir): os.environ['BORG_PASSPHRASE'] = '123456' - os.environ['BORG_CHECK_I_KNOW_WHAT_I_AM_DOING'] = '1' - os.environ['BORG_DELETE_I_KNOW_WHAT_I_AM_DOING'] = '1' - os.environ['BORG_UNKNOWN_UNENCRYPTED_REPO_ACCESS_IS_OK'] = '1' + os.environ['BORG_CHECK_I_KNOW_WHAT_I_AM_DOING'] = 'YES' + os.environ['BORG_DELETE_I_KNOW_WHAT_I_AM_DOING'] = 'YES' + os.environ['BORG_UNKNOWN_UNENCRYPTED_REPO_ACCESS_IS_OK'] = 'yes' os.environ['BORG_KEYS_DIR'] = str(tmpdir.join('keys')) os.environ['BORG_CACHE_DIR'] = str(tmpdir.join('cache')) yield str(tmpdir.join('repository')) diff --git a/borg/testsuite/helpers.py b/borg/testsuite/helpers.py index cc4b3df38e..82b3f135bc 100644 --- a/borg/testsuite/helpers.py +++ b/borg/testsuite/helpers.py @@ -9,11 +9,11 @@ import msgpack import msgpack.fallback -from ..helpers import Location, format_file_size, format_timedelta, PathPrefixPattern, FnmatchPattern, make_path_safe, \ - prune_within, prune_split, get_cache_dir, Statistics, is_slow_msgpack, yes, RegexPattern, \ +from ..helpers import Location, format_file_size, format_timedelta, make_path_safe, \ + prune_within, prune_split, get_cache_dir, Statistics, is_slow_msgpack, yes, TRUISH, FALSISH, DEFAULTISH, \ StableDict, int_to_bigint, bigint_to_int, parse_timestamp, CompressionSpec, ChunkerParams, \ - ProgressIndicatorPercent, ProgressIndicatorEndless, load_excludes, parse_pattern, PatternMatcher, \ - ShellPattern + ProgressIndicatorPercent, ProgressIndicatorEndless, load_excludes, parse_pattern, \ + PatternMatcher, RegexPattern, PathPrefixPattern, FnmatchPattern, ShellPattern from . import BaseTestCase, environment_variable, FakeInputs @@ -691,20 +691,28 @@ def test_is_slow_msgpack(): assert not is_slow_msgpack() -def test_yes_simple(): - input = FakeInputs(['y', 'Y', 'yes', 'Yes', ]) - assert yes(input=input) - assert yes(input=input) - assert yes(input=input) - assert yes(input=input) - input = FakeInputs(['n', 'N', 'no', 'No', ]) - assert not yes(input=input) - assert not yes(input=input) - assert not yes(input=input) - assert not yes(input=input) +def test_yes_input(): + inputs = list(TRUISH) + input = FakeInputs(inputs) + for i in inputs: + assert yes(input=input) + inputs = list(FALSISH) + input = FakeInputs(inputs) + for i in inputs: + assert not yes(input=input) -def test_yes_custom(): +def test_yes_input_defaults(): + inputs = list(DEFAULTISH) + input = FakeInputs(inputs) + for i in inputs: + assert yes(default=True, input=input) + input = FakeInputs(inputs) + for i in inputs: + assert not yes(default=False, input=input) + + +def test_yes_input_custom(): input = FakeInputs(['YES', 'SURE', 'NOPE', ]) assert yes(truish=('YES', ), input=input) assert yes(truish=('SURE', ), input=input) @@ -712,11 +720,20 @@ def test_yes_custom(): def test_yes_env(): - input = FakeInputs(['n', 'n']) - with environment_variable(OVERRIDE_THIS='nonempty'): - assert yes(env_var_override='OVERRIDE_THIS', input=input) - with environment_variable(OVERRIDE_THIS=None): # env not set - assert not yes(env_var_override='OVERRIDE_THIS', input=input) + for value in TRUISH: + with environment_variable(OVERRIDE_THIS=value): + assert yes(env_var_override='OVERRIDE_THIS') + for value in FALSISH: + with environment_variable(OVERRIDE_THIS=value): + assert not yes(env_var_override='OVERRIDE_THIS') + + +def test_yes_env_default(): + for value in DEFAULTISH: + with environment_variable(OVERRIDE_THIS=value): + assert yes(env_var_override='OVERRIDE_THIS', default=True) + with environment_variable(OVERRIDE_THIS=value): + assert not yes(env_var_override='OVERRIDE_THIS', default=False) def test_yes_defaults(): @@ -728,27 +745,27 @@ def test_yes_defaults(): assert yes(default=True, input=input) assert yes(default=True, input=input) assert yes(default=True, input=input) - ifile = StringIO() - assert yes(default_notty=True, ifile=ifile) - assert not yes(default_notty=False, ifile=ifile) input = FakeInputs([]) - assert yes(default_eof=True, input=input) - assert not yes(default_eof=False, input=input) + assert yes(default=True, input=input) + assert not yes(default=False, input=input) with pytest.raises(ValueError): yes(default=None) - with pytest.raises(ValueError): - yes(default_notty='invalid') - with pytest.raises(ValueError): - yes(default_eof='invalid') def test_yes_retry(): - input = FakeInputs(['foo', 'bar', 'y', ]) + input = FakeInputs(['foo', 'bar', TRUISH[0], ]) assert yes(retry_msg='Retry: ', input=input) - input = FakeInputs(['foo', 'bar', 'N', ]) + input = FakeInputs(['foo', 'bar', FALSISH[0], ]) assert not yes(retry_msg='Retry: ', input=input) +def test_yes_no_retry(): + input = FakeInputs(['foo', 'bar', TRUISH[0], ]) + assert not yes(retry=False, default=False, input=input) + input = FakeInputs(['foo', 'bar', FALSISH[0], ]) + assert yes(retry=False, default=True, input=input) + + def test_yes_output(capfd): input = FakeInputs(['invalid', 'y', 'n']) assert yes(msg='intro-msg', false_msg='false-msg', true_msg='true-msg', retry_msg='retry-msg', input=input) diff --git a/docs/usage.rst b/docs/usage.rst index d6aad6a874..3094f403da 100644 --- a/docs/usage.rst +++ b/docs/usage.rst @@ -69,15 +69,19 @@ General: TMPDIR where temporary files are stored (might need a lot of temporary space for some operations) -Some "yes" sayers (if set, they automatically confirm that you really want to do X even if there is that warning): - BORG_UNKNOWN_UNENCRYPTED_REPO_ACCESS_IS_OK +Some automatic "answerers" (if set, they automatically answer confirmation questions): + BORG_UNKNOWN_UNENCRYPTED_REPO_ACCESS_IS_OK=no (or =yes) For "Warning: Attempting to access a previously unknown unencrypted repository" - BORG_RELOCATED_REPO_ACCESS_IS_OK + BORG_RELOCATED_REPO_ACCESS_IS_OK=no (or =yes) For "Warning: The repository at location ... was previously located at ..." - BORG_CHECK_I_KNOW_WHAT_I_AM_DOING - For "Warning: '``check --repair``' is an experimental feature that might result in data loss." - BORG_DELETE_I_KNOW_WHAT_I_AM_DOING - For "You requested to completely DELETE the repository *including* all archives it contains: " + BORG_CHECK_I_KNOW_WHAT_I_AM_DOING=NO (or =YES) + For "Warning: 'check --repair' is an experimental feature that might result in data loss." + BORG_DELETE_I_KNOW_WHAT_I_AM_DOING=NO (or =YES) + For "You requested to completely DELETE the repository *including* all archives it contains:" + + Note: answers are case sensitive. setting an invalid answer value might either give the default + answer or ask you interactively, depending on whether retries are allowed (they by default are + allowed). So please test your scripts interactively before making them a non-interactive script. Directories: BORG_KEYS_DIR