Issue #11014: Make 'filter' argument in tarfile.Tarfile.add() into a

keyword-only argument.  The preceding positional argument was deprecated,
so it made no sense to add filter as a positional argument.

(Patch reviewed by Brian Curtin and Anthony Long.)

diff --git a/Doc/library/tarfile.rst b/Doc/library/tarfile.rst
index d7fbf39ee7d8ab3443d02a651ceb79d355e950cb..d578a79421375fe5b1aee2b864bdb0dfdc527121 100644 (file)
--- a/Doc/library/tarfile.rst
+++ b/Doc/library/tarfile.rst
@@ -370,19 +370,20 @@ be finalized; only the internally used file object will be closed. See the
       and :meth:`close`, and also supports iteration over its lines.
 
 
-.. method:: TarFile.add(name, arcname=None, recursive=True, exclude=None, filter=None)
-
-   Add the file *name* to the archive. *name* may be any type of file (directory,
-   fifo, symbolic link, etc.). If given, *arcname* specifies an alternative name
-   for the file in the archive. Directories are added recursively by default. This
-   can be avoided by setting *recursive* to :const:`False`. If *exclude* is given,
-   it must be a function that takes one filename argument and returns a boolean
-   value. Depending on this value the respective file is either excluded
-   (:const:`True`) or added (:const:`False`). If *filter* is specified it must
-   be a function that takes a :class:`TarInfo` object argument and returns the
-   changed :class:`TarInfo` object. If it instead returns :const:`None` the :class:`TarInfo`
-   object will be excluded from the archive. See :ref:`tar-examples` for an
-   example.
+.. method:: TarFile.add(name, arcname=None, recursive=True, exclude=None, *, filter=None)
+
+   Add the file *name* to the archive. *name* may be any type of file
+   (directory, fifo, symbolic link, etc.). If given, *arcname* specifies an
+   alternative name for the file in the archive. Directories are added
+   recursively by default. This can be avoided by setting *recursive* to
+   :const:`False`. If *exclude* is given, it must be a function that takes one
+   filename argument and returns a boolean value. Depending on this value the
+   respective file is either excluded (:const:`True`) or added
+   (:const:`False`). If *filter* is specified it must be a keyword argument.  It
+   should be a function that takes a :class:`TarInfo` object argument and
+   returns the changed :class:`TarInfo` object. If it instead returns
+   :const:`None` the :class:`TarInfo` object will be excluded from the
+   archive. See :ref:`tar-examples` for an example.
 
    .. versionchanged:: 3.2
       Added the *filter* parameter.

diff --git a/Lib/tarfile.py b/Lib/tarfile.py
index fd898c74a1b9722f1c44ba967f436c21ce1cf5b5..e3747e9c79c3551b06969db5fb88858b594ad2d5 100644 (file)
--- a/Lib/tarfile.py
+++ b/Lib/tarfile.py
@@ -2025,7 +2025,7 @@ class TarFile(object):
                     print("link to", tarinfo.linkname, end=' ')
             print()
 
-    def add(self, name, arcname=None, recursive=True, exclude=None, filter=None):
+    def add(self, name, arcname=None, recursive=True, exclude=None, *, filter=None):
         """Add the file `name' to the archive. `name' may be any type of file
            (directory, fifo, symbolic link, etc.). If given, `arcname'
            specifies an alternative name for the file in the archive.
@@ -2082,7 +2082,7 @@ class TarFile(object):
             if recursive:
                 for f in os.listdir(name):
                     self.add(os.path.join(name, f), os.path.join(arcname, f),
-                            recursive, exclude, filter)
+                            recursive, exclude, filter=filter)
 
         else:
             self.addfile(tarinfo)

diff --git a/Lib/test/test_tarfile.py b/Lib/test/test_tarfile.py
index ff02c693330d2c0a1d0febb3b7630cac96299101..94ef61c0ce701f11a118ef70af259660f3f04901 100644 (file)
--- a/Lib/test/test_tarfile.py
+++ b/Lib/test/test_tarfile.py
@@ -919,6 +919,10 @@ class WriteTest(WriteTestBase):
             finally:
                 tar.close()
 
+            # Verify that filter is a keyword-only argument
+            with self.assertRaises(TypeError):
+                tar.add(tempdir, "empty_dir", True, None, filter)
+
             tar = tarfile.open(tmpname, "r")
             try:
                 for tarinfo in tar:

diff --git a/Misc/NEWS b/Misc/NEWS
index 56552055ca5f5df30631d29d0e5f3e97a687a9ae..f2e37b52a51ce6d9238c159a39d65c2fabba2fe7 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -16,6 +16,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #11014: Make 'filter' argument in tarfile.Tarfile.add() into a
+  keyword-only argument.  The preceding positional argument was deprecated,
+  so it made no sense to add filter as a positional argument.
+
 - Issue #11004: Repaired edge case in deque.count().
 
 - Issue #10974: IDLE no longer crashes if its recent files list includes files