[PATCH 4/5] prune-older: test invalid invocations; fix bugs revealed

[Rob Browning]

We were catching a localtime() ValueError when we should have been
catching OverflowError, and optons.py's automagic integer conversion
broke the --keep-* value parsing for integer convertible values.

Signed-off-by: Rob Browning <r...@defaultvalue.org>
Tested-by: Rob Browning <r...@defaultvalue.org>
---
lib/bup/cmd/prune_older.py | 41 ++++++++++++++-----------
test/ext/test_prune_older.py | 59 ++++++++++++++++++++++++++++++++++--
2 files changed, 80 insertions(+), 20 deletions(-)

diff --git a/lib/bup/cmd/prune_older.py b/lib/bup/cmd/prune_older.py
index 08c6b687..6c221823 100644
--- a/lib/bup/cmd/prune_older.py
+++ b/lib/bup/cmd/prune_older.py
@@ -9,7 +9,7 @@ from bup import git, options
from bup.compat import argv_bytes
from bup.gc import bup_gc
from bup.helpers import die_if_errors, log, partition, period_as_secs
-from bup.io import byte_stream
+from bup.io import byte_stream, path_msg
from bup.repo import LocalRepo
from bup.rm import bup_rm
from bup.vfs import save_names_for_commit_utcs
@@ -89,10 +89,12 @@ def main(argv):
('dailies', opt.keep_dailies_for),
('monthlies', opt.keep_monthlies_for),
('yearlies', opt.keep_yearlies_for)):
- if extent:
+ if extent is not None:
+ if isinstance(extent, int): # bup.options...
+ extent = str(extent)
secs = period_as_secs(extent.encode('ascii'))
if not secs:
- o.fatal('%r is not a valid period' % extent)
+ o.fatal(f'{path_msg(extent)} is not a valid period')
period_start[period] = now - secs

if not period_start:
@@ -104,22 +106,25 @@ def main(argv):
epoch_ymd = strftime('%Y-%m-%d-%H%M%S', localtime(0))
for kind in ['all', 'dailies', 'monthlies', 'yearlies']:
period_utc = period_start[kind]
- if period_utc != float('inf'):
- if not (period_utc > float('-inf')):
- log(f'keeping all {kind}\n')
+ if period_utc == float('inf'):
+ continue
+ if period_utc == float('-inf'):
+ log(f'keeping all {kind}\n')
+ continue
+ try:
+ period_local = localtime(period_utc)
+ except OverflowError:
+ if period_utc < 0:
+ log('keeping %s since %d seconds before %s\n'
+ %(kind, abs(period_utc), epoch_ymd))
+ elif period_utc > 0:
+ log('keeping %s since %d seconds after %s\n'
+ %(kind, period_utc, epoch_ymd))
else:
- try:
- when = strftime('%Y-%m-%d-%H%M%S', localtime(period_utc))
- log('keeping ' + kind + ' since ' + when + '\n')
- except ValueError:
- if period_utc < 0:
- log('keeping %s since %d seconds before %s\n'
- %(kind, abs(period_utc), epoch_ymd))
- elif period_utc > 0:
- log('keeping %s since %d seconds after %s\n'
- %(kind, period_utc, epoch_ymd))
- else:
- log('keeping %s since %s\n' % (kind, epoch_ymd))
+ log('keeping %s since %s\n' % (kind, epoch_ymd))
+ else:
+ when = strftime('%Y-%m-%d-%H%M%S', period_local)
+ log(f'keeping {kind} since {when}\n')

git.check_repo_or_die()

diff --git a/test/ext/test_prune_older.py b/test/ext/test_prune_older.py
index 63290ded..46d28ab8 100644
--- a/test/ext/test_prune_older.py
+++ b/test/ext/test_prune_older.py
@@ -8,7 +8,7 @@ from shutil import copytree, rmtree
from subprocess import PIPE
from sys import stderr
from time import localtime, strftime, time
-import random, sys
+import random, re, sys

from buptest import exc as ex, exo
from wvpytest import wvpass, wvpasseq, wvpassne, wvstart
@@ -180,7 +180,7 @@ def check_pretend_intent(branch, pretend_out, all_utcs, dispositions):
actual = pretend_out.splitlines()
assert expected == actual

-def test_prune_older(tmpdir):
+def test_random_keeps(tmpdir):
environ[b'GIT_AUTHOR_NAME'] = b'bup test'
environ[b'GIT_COMMITTER_NAME'] = b'bup test'
environ[b'GIT_AUTHOR_EMAIL'] = b'bup@a425bc70a02811e49bdf73ee56450e6f'
@@ -265,3 +265,58 @@ def test_prune_older(tmpdir):
+ period_spec_to_period_args(spec) \
+ (b'main',))
check_prune_result(b'main', [utc for keep, utc in expected if keep])
+
+def test_argument_validation(tmpdir):
+ rxs = re.search
+ environ[b'BUP_DIR'] = tmpdir + b'/bup'
+ environ[b'GIT_DIR'] = tmpdir + b'/bup'
+ chdir(tmpdir)
+ ex([b'git', b'init', b'bup'])
+
+ def prune(*args):
+ return ex((bup.path.exe(), b'prune-older', b'-v', b'--no-gc',
+ '--pretend', b'--keep-all-for', b'forever', *args),
+ check=False, stderr=PIPE)
+
+ wvstart('no --unsafe')
+ cp = prune()
+ assert cp.rc
+ assert b'refusing to run dangerous, experimental command without --unsafe\n' in cp.err
+
+ wvstart('no --keep-*')
+ cp = ex((bup.path.exe(), b'prune-older', b'--unsafe', b'-v', b'--no-gc',
+ '--pretend'), check=False, stderr=PIPE)
+ assert cp.rc
+ assert b'\nerror: at least one keep argument is required\n' in cp.err
+
+ wvstart('--wrt non-integer')
+ cp = prune(b'--unsafe', b'--wrt', b'x', b'main')
+ assert cp.rc
+ assert b'\nerror: --wrt value x is not an integer\n' in cp.err
+
+ wvstart('invalid --keep-*-for value')
+ cp = prune(b'--unsafe', b'--keep-all-for', b'x y', b'main')
+ assert cp.rc
+ assert rxs(br"(?m)^error: 'x y' is not a valid period$", cp.err)
+
+ wvstart('invalid --keep-*-for value')
+ cp = prune(b'--unsafe', b'--keep-all-for', b'x y', b'main')
+ assert cp.rc
+ assert rxs(br"(?m)^error: 'x y' is not a valid period", cp.err)
+
+ wvstart('invalid --keep-*-for value (zero)')
+ cp = prune(b'--unsafe', b'--keep-all-for', b'0', b'main')
+ assert cp.rc
+ assert rxs(br'(?m)^error: 0 is not a valid period', cp.err)
+
+ wvstart('--keep-*-for value larger than localtime() range')
+ cp = prune(b'--unsafe', b'--keep-all-for',
+ b'100000000000000000000000000000000000000y', b'main')
+ assert cp.rc == 0
+ assert rxs(br'(?m)^keeping all since \d+ seconds before', cp.err)
+
+ wvstart('--keep-*-for value smaller than localtime() range')
+ cp = prune(b'--unsafe', b'--keep-all-for',
+ b'100000000000000000000000000000000000000y', b'main')
+ assert cp.rc == 0
+ assert rxs(br'(?m)^keeping all since \d+ seconds before', cp.err)
--
2.47.3