gh-74696: Pass root_dir to custom archivers which support it

serhiy-storchaka · Oct 5, 2022 · Jun 22, 2022 · Jun 25, 2022 · Sep 29, 2022 · Sep 29, 2022
commit e5ebb8d3036197337639bc52935ec0a14698e0eb
@@ -575,7 +575,8 @@ provided.  They rely on the :mod:`zipfile` and :mod:`tarfile` modules.
    .. note::
 
       This function is not thread-safe when custom archivers registered
-      with :func:`register_archive_format` are used.  In this case it
+      with :func:`register_archive_format` does not support the *root_dir*
+      argument.  In this case it
       temporarily changes the current working directory of the process
       to perform archiving.
 
@@ -614,12 +615,21 @@ provided.  They rely on the :mod:`zipfile` and :mod:`tarfile` modules.
    Further arguments are passed as keyword arguments: *owner*, *group*,
    *dry_run* and *logger* (as passed in :func:`make_archive`).
 
+   If *function* has the *supports_root_dir* attribute set to ``True``,
+   the *root_dir* argument is passed as a keyword argument.
+   Otherwise the current working directory of the process is temporarily
+   changed before calling *function*.
+   In this case :func:`make_archive` is not thread-safe.
+
    If given, *extra_args* is a sequence of ``(name, value)`` pairs that will be
    used as extra keywords arguments when the archiver callable is used.
 
    *description* is used by :func:`get_archive_formats` which returns the
    list of archivers.  Defaults to an empty string.
 
+   .. versionchanged:: 3.12
+      Added support of functions supporting the *root_dir* argument.
+
 
 .. function:: unregister_archive_format(name)
 

@@ -97,6 +97,15 @@ os
   for a process with :func:`os.pidfd_open` in non-blocking mode.
   (Contributed by Kumar Aditya in :gh:`93312`.)
 
+shutil
+------
+
+* :func:`shutil.make_archive` now passes the *root_dir* argument to custom
+  archivers which support it.
+  In this case it no longer temporarily changes the current working directory
+  of the process to perform archiving.
+  (Contributed by Serhiy Storchaka in :gh:`74696`.)
+
 
 Optimizations
 =============

@@ -1023,28 +1023,30 @@ def _make_zipfile(base_name, base_dir, verbose=0, dry_run=0,
         zip_filename = os.path.abspath(zip_filename)
     return zip_filename
 
+_make_tarball.supports_root_dir = True
+_make_zipfile.supports_root_dir = True
+
 # Maps the name of the archive format to a tuple containing:
 # * the archiving function
 # * extra keyword arguments
 # * description
-# * does it support the root_dir argument?
 _ARCHIVE_FORMATS = {
     'tar':   (_make_tarball, [('compress', None)],
-              "uncompressed tar file", True),
+              "uncompressed tar file"),
 }
 
 if _ZLIB_SUPPORTED:
     _ARCHIVE_FORMATS['gztar'] = (_make_tarball, [('compress', 'gzip')],
-                                "gzip'ed tar-file", True)
-    _ARCHIVE_FORMATS['zip'] = (_make_zipfile, [], "ZIP file", True)
+                                "gzip'ed tar-file")
+    _ARCHIVE_FORMATS['zip'] = (_make_zipfile, [], "ZIP file")
 
 if _BZ2_SUPPORTED:
     _ARCHIVE_FORMATS['bztar'] = (_make_tarball, [('compress', 'bzip2')],
-                                "bzip2'ed tar-file", True)
+                                "bzip2'ed tar-file")
 
 if _LZMA_SUPPORTED:
     _ARCHIVE_FORMATS['xztar'] = (_make_tarball, [('compress', 'xz')],
-                                "xz'ed tar-file", True)
+                                "xz'ed tar-file")
 
 def get_archive_formats():
     """Returns a list of supported formats for archiving and unarchiving.
@@ -1075,7 +1077,7 @@ def register_archive_format(name, function, extra_args=None, description=''):
         if not isinstance(element, (tuple, list)) or len(element) !=2:
             raise TypeError('extra_args elements are : (arg_name, value)')
 
-    _ARCHIVE_FORMATS[name] = (function, extra_args, description, False)
+    _ARCHIVE_FORMATS[name] = (function, extra_args, description)
 
 def unregister_archive_format(name):
     del _ARCHIVE_FORMATS[name]
@@ -1114,10 +1116,10 @@ def make_archive(base_name, format, root_dir=None, base_dir=None, verbose=0,
     if base_dir is None:
         base_dir = os.curdir
 
-    support_root_dir = format_info[3]
+    supports_root_dir = getattr(func, 'supports_root_dir', False)
     save_cwd = None
     if root_dir is not None:
-        if support_root_dir:
+        if supports_root_dir:
             kwargs['root_dir'] = root_dir
         else:
             save_cwd = os.getcwd()

@@ -1572,26 +1572,62 @@ def test_tarfile_root_owner(self):
 
     def test_make_archive_cwd(self):
         current_dir = os.getcwd()
-        root_dir = self.mkdtemp()
-        def _breaks(*args, **kw):
+        def archiver(base_name, base_dir, **kw):
+            self.assertNotIn('root_dir', kw)
+            if root_dir is None:
+                self.assertEqual(base_name, 'basename')
+                self.assertEqual(os.getcwd(), current_dir)
+            else:
+                self.assertEqual(base_name, os.path.join(current_dir, 'basename'))
+                self.assertEqual(os.getcwd(), root_dir)
             raise RuntimeError()
         dirs = []
         def _chdir(path):
             dirs.append(path)
             orig_chdir(path)
 
-        register_archive_format('xxx', _breaks, [], 'xxx file')
+        register_archive_format('xxx', archiver, [], 'xxx file')
         try:
+            root_dir = None
             with support.swap_attr(os, 'chdir', _chdir) as orig_chdir:
-                try:
-                    make_archive('xxx', 'xxx', root_dir=root_dir)
-                except Exception:
-                    pass
+                with self.assertRaises(RuntimeError):
+                    make_archive('basename', 'xxx')
+            self.assertEqual(os.getcwd(), current_dir)
+            self.assertEqual(dirs, [])
+
+            root_dir = self.mkdtemp()
+            with support.swap_attr(os, 'chdir', _chdir) as orig_chdir:
+                with self.assertRaises(RuntimeError):
+                    make_archive('basename', 'xxx', root_dir=root_dir)
             self.assertEqual(os.getcwd(), current_dir)
             self.assertEqual(dirs, [root_dir, current_dir])
         finally:
             unregister_archive_format('xxx')
 
+    def test_make_archive_cwd_supports_root_dir(self):
+        current_dir = os.getcwd()
+        root_dir = self.mkdtemp()
+        def archiver(base_name, base_dir, **kw):
+            self.assertEqual(base_name, 'basename')
+            self.assertEqual(kw['root_dir'], root_dir)
+            self.assertEqual(os.getcwd(), current_dir)
+            raise RuntimeError()
+        archiver.supports_root_dir = True
+        dirs = []
+        def _chdir(path):
+            dirs.append(path)
+            orig_chdir(path)
+
+        register_archive_format('xxx', archiver, [], 'xxx file')
+        try:
+            with support.swap_attr(os, 'chdir', _chdir) as orig_chdir:
+                with self.assertRaises(RuntimeError):
+                    make_archive('basename', 'xxx', root_dir=root_dir)
+            self.assertEqual(os.getcwd(), current_dir)
+            self.assertEqual(dirs, [])
+        finally:
+            unregister_archive_format('xxx')
+
     def test_make_tarfile_in_curdir(self):
         # Issue #21280
         root_dir = self.mkdtemp()

diff --git a/Misc/NEWS.d/next/Library/2022-06-25-09-12-23.gh-issue-74696.fxC9ua.rst b/Misc/NEWS.d/next/Library/2022-06-25-09-12-23.gh-issue-74696.fxC9ua.rst
@@ -0,0 +1,2 @@
+:func:`shutil.make_archive` now passes the *root_dir* argument to custom
+archivers which support it.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		:func:`shutil.make_archive` now passes the root_dir argument to custom
		archivers which support it.