Merge pull request #2780 from enkore/fix/broken-filters

fix reversed archive ordering with --last

src/borg/archiver.py

@@ -44,7 +44,7 @@ from .crypto.key import key_creator, tam_required_file, tam_required, RepoKey, P
 from .crypto.keymanager import KeyManager
 from .helpers import EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR
 from .helpers import Error, NoManifestError, set_ec
-from .helpers import location_validator, archivename_validator, ChunkerParams
+from .helpers import positive_int_validator, location_validator, archivename_validator, ChunkerParams
 from .helpers import PrefixSpec, SortBySpec, HUMAN_SORT_KEYS
 from .helpers import BaseFormatter, ItemFormatter, ArchiveFormatter
 from .helpers import format_timedelta, format_file_size, parse_file_size, format_archive
@@ -2394,9 +2394,9 @@ class Archiver:
 
             if first_last:
                 group = filters_group.add_mutually_exclusive_group()
-                group.add_argument('--first', metavar='N', dest='first', default=0, type=int,
+                group.add_argument('--first', metavar='N', dest='first', default=0, type=positive_int_validator,
                                    help='consider first N archives after other filters were applied')
-                group.add_argument('--last', metavar='N', dest='last', default=0, type=int,
+                group.add_argument('--last', metavar='N', dest='last', default=0, type=positive_int_validator,
                                    help='consider last N archives after other filters were applied')
 
         parser = argparse.ArgumentParser(prog=self.prog, description='Borg - Deduplicated Backups',

src/borg/helpers.py

@@ -219,11 +219,14 @@ class Archives(abc.MutableMapping):
         name = safe_encode(name)
         del self._archives[name]
 
-    def list(self, sort_by=(), reverse=False, glob=None, first=None, last=None):
+    def list(self, *, glob=None, sort_by=(), first=None, last=None, reverse=False):
         """
-        Inexpensive Archive.list_archives replacement if we just need .name, .id, .ts
-        Returns list of borg.helpers.ArchiveInfo instances.
-        sort_by can be a list of sort keys, they are applied in reverse order.
+        Return list of ArchiveInfo instances according to the parameters.
+
+        First match *glob*, then *sort_by*. Apply *first* and *last* filters,
+        and possibly *reverse* the list.
+
+        *sort_by* is a list of sort keys applied in reverse order.
         """
         if isinstance(sort_by, (str, bytes)):
             raise TypeError('sort_by must be a sequence of str')
@@ -231,10 +234,13 @@ class Archives(abc.MutableMapping):
         archives = [x for x in self.values() if regex.match(x.name) is not None]
         for sortkey in reversed(sort_by):
             archives.sort(key=attrgetter(sortkey))
-        if reverse or last:
+        if first:
+            archives = archives[:first]
+        elif last:
+            archives = archives[max(len(archives) - last, 0):]
+        if reverse:
             archives.reverse()
-        n = first or last or len(archives)
-        return archives[:n]
+        return archives
 
     def list_considering(self, args):
         """
@@ -397,6 +403,14 @@ class Manifest:
         self.repository.put(self.MANIFEST_ID, self.key.encrypt(data))
 
 
+def positive_int_validator(value):
+    """argparse type for positive integers"""
+    int_value = int(value)
+    if int_value <= 0:
+        raise argparse.ArgumentTypeError('A positive integer is required: %s' % value)
+    return int_value
+
+
 def interval(s):
     """Convert a string representing a valid interval to a number of hours."""
     multiplier = {'H': 1, 'd': 24, 'w': 24 * 7, 'm': 24 * 31, 'y': 24 * 365}