8000 gh-74696: Do not change the current working directory in shutil.make_… · miss-islington/cpython@fda4b2f · GitHub
[go: up one dir, main page]
Skip to content

Commit fda4b2f

Browse files
serhiy-storchakamerwokambv
authored
pythongh-74696: Do not change the current working directory in shutil.make_archive() if possible (pythonGH-93160)
It is no longer changed when create a zip or tar archive. It is still changed for custom archivers registered with shutil.register_archive_format() if root_dir is not None. Co-authored-by: Éric <merwok@netwok.org> Co-authored-by: Łukasz Langa <lukasz@langa.pl>
1 parent f805d37 commit fda4b2f

File tree

4 files changed

+106
-52
lines changed

4 files changed

+106
-52
lines changed

‎Doc/library/shutil.rst

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,12 +574,18 @@ provided. They rely on the :mod:`zipfile` and :mod:`tarfile` modules.
574574

575575
.. note::
576576

577-
This function is not thread-safe.
577+
This function is not thread-safe when custom archivers registered
578+
with :func:`register_archive_format` are used. In this case it
579+
temporarily changes the current working directory of the process
580+
to perform archiving.
578581

579582
.. versionchanged:: 3.8
580583
The modern pax (POSIX.1-2001) format is now used instead of
581584
the legacy GNU format for archives created with ``format="tar"``.
582585

586+
.. versionchanged:: 3.10.6
587+
This function is now made thread-safe during creation of standard
588+
``.zip`` and tar archives.
583589

584590
.. function:: get_archive_formats()
585591

‎Lib/shutil.py

Lines changed: 65 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -897,7 +897,7 @@ def _get_uid(name):
897897
return None
898898

899899
def _make_tarball(base_name, base_dir, compress="gzip", verbose=0, dry_run=0,
900-
owner=None, group=None, logger=None):
900+
owner=None, group=None, logger=None, root_dir=None):
901901
"""Create a (possibly compressed) tar file from all the files under
902902
'base_dir'.
903903
@@ -954,14 +954,20 @@ def _set_uid_gid(tarinfo):
954954

955955
if not dry_run:
956956
tar = tarfile.open(archive_name, 'w|%s' % tar_compression)
957+
arcname = base_dir
958+
if root_dir is not None:
959+
base_dir = os.path.join(root_dir, base_dir)
957960
try:
958-
tar.add(base_dir, filter=_set_uid_gid)
961+
tar.add(base_dir, arcname, filter=_set_uid_gid)
959962
finally:
960963
tar.close()
961964

965+
if root_dir is not None:
966+
archive_name = os.path.abspath(archive_name)
962967
return archive_name
963968

964-
def _make_zipfile(base_name, base_dir, verbose=0, dry_run=0, logger=None):
969+
def _make_zipfile(base_name, base_dir, verbose=0, dry_run=0,
970+
logger=None, owner=None, group=None, root_dir=None):
965971
"""Create a zip file from all the files under 'base_dir'.
966972
967973 3262
The output zip file will be named 'base_name' + ".zip". Returns the
@@ -985,42 +991,60 @@ def _make_zipfile(base_name, base_dir, verbose=0, dry_run=0, logger=None):
985991
if not dry_run:
986992
with zipfile.ZipFile(zip_filename, "w",
987993
compression=zipfile.ZIP_DEFLATED) as zf:
988-
path = os.path.normpath(base_dir)
989-
if path != os.curdir:
990-
zf.write(path, path)
994+
arcname = os.path.normpath(base_dir)
995+
if root_dir is not None:
996+
base_dir = os.path.join(root_dir, base_dir)
997+
base_dir = os.path.normpath(base_dir)
998+
if arcname != os.curdir:
999+
zf.write(base_dir, arcname)
9911000
if logger is not None:
992-
logger.info("adding '%s'", path)
1001+
logger.info("adding '%s'", base_dir)
9931002
for dirpath, dirnames, filenames in os.walk(base_dir):
1003+
arcdirpath = dirpath
1004+
if root_dir is not None:
1005+
arcdirpath = os.path.relpath(arcdirpath, root_dir)
1006+
arcdirpath = os.path.normpath(arcdirpath)
9941007
for name in sorted(dirnames):
995-
path = os.path.normpath(os.path.join(dirpath, name))
996-
zf.write(path, path)
1008+
path = os.path.join(dirpath, name)
1009+
arcname = os.path.join(arcdirpath, name)
1010+
zf.write(path, arcname)
9971011
if logger is not None:
9981012
logger.info("adding '%s'", path)
9991013
for name in filenames:
1000-
path = os.path.normpath(os.path.join(dirpath, name))
1014+
path = os.path.join(dirpath, name)
1015+
path = os.path.normpath(path)
10011016
if os.path.isfile(path):
1002-
zf.write(path, path)
1017+
arcname = os.path.join(arcdirpath, name)
1018+
zf.write(path, arcname)
10031019
if logger is not None:
10041020
logger.info("adding '%s'", path)
10051021

1022+
if root_dir is not None:
1023+
zip_filename = os.path.abspath(zip_filename)
10061024
return zip_filename
10071025

1026+
# Maps the name of the archive format to a tuple containing:
1027+
# * the archiving function
1028+
# * extra keyword arguments
1029+
# * description
1030+
# * does it support the root_dir argument?
10081031
_ARCHIVE_FORMATS = {
1009-
'tar': (_make_tarball, [('compress', None)], "uncompressed tar file"),
1032+
'tar': (_make_tarball, [('compress', None)],
1033+
"uncompressed tar file", True),
10101034
}
10111035

10121036
if _ZLIB_SUPPORTED:
10131037
_ARCHIVE_FORMATS['gztar'] = (_make_tarball, [('compress', 'gzip')],
1014-
"gzip'ed tar-file")
1015-
_ARCHIVE_FORMATS['zip'] = (_make_zipfile, [], "ZIP file")
1038+
"gzip'ed tar-file", True)
1039+
_ARCHIVE_FORMATS['zip'] = (_make_zipfile, [], "ZIP file", True)
10161040

10171041
if _BZ2_SUPPORTED:
10181042
_ARCHIVE_FORMATS['bztar'] = (_make_tarball, [('compress', 'bzip2')],
1019-
"bzip2'ed tar-file")
1043+
"bzip2'ed tar-file", True)
10201044

10211045
if _LZMA_SUPPORTED:
10221046
_ARCHIVE_FORMATS['xztar'] = (_make_tarball, [('compress', 'xz')],
1023-
"xz'ed tar-file")
1047+
"xz'ed tar-file", True)
10241048

10251049
def get_archive_formats():
10261050
"""Returns a list of supported formats for archiving and unarchiving.
@@ -1051,7 +1075,7 @@ def register_archive_format(name, function, extra_args=None, description=''):
10511075
if not isinstance(element, (tuple, list)) or len(element) !=2:
10521076
raise TypeError('extra_args elements are : (arg_name, value)')
10531077

1054-
_ARCHIVE_FORMATS[name] = (function, extra_args, description)
1078+
_ARCHIVE_FORMATS[name] = (function, extra_args, description, False)
10551079

10561080
def unregister_archive_format(name):
10571081
del _ARCHIVE_FORMATS[name]
@@ -1075,36 +1099,38 @@ def make_archive(base_name, format, root_dir=None, base_dir=None, verbose=0,
10751099
uses the current owner and group.
10761100
"""
10771101
sys.audit("shutil.make_archive", base_name, format, root_dir, base_dir)
1078-
save_cwd = os.getcwd()
1079-
if root_dir is not None:
1080-
if logger is not None:
1081-
logger.debug("changing into '%s'", root_dir)
1082-
base_name = os.path.abspath(base_name)
1083-
if not dry_run:
1084-
os.chdir(root_dir)
1085-
1086-
if base_dir is None:
1087-
base_dir = os.curdir
1088-
1089-
kwargs = {'dry_run': dry_run, 'logger': logger}
1090-
10911102
try:
10921103
format_info = _ARCHIVE_FORMATS[format]
10931104
except KeyError:
10941105
raise ValueError("unknown archive format '%s'" % format) from None
10951106

1107+
kwargs = {'dry_run': dry_run, 'logger': logger,
1108+
'owner': owner, 'group': group}
1109+
10961110
func = format_info[0]
10971111
for arg, val in format_info[1]:
10981112
kwargs[arg] = val
10991113

1100-
if format != 'zip':
1101-
kwargs['owner'] = owner
1102-
kwargs['group'] = group
1114+
if base_dir is None:
1115+
base_dir = os.curdir
1116+
1117+
support_root_dir = format_info[3]
1118+
save_cwd = None
1119+
if root_dir is not None:
1120+
if support_root_dir:
1121+
kwargs['root_dir'] = root_dir
1122+
else:
1123+
save_cwd = os.getcwd()
1124+
if logger is not None:
1125+
logger.debug("changing into '%s'", root_dir)
1126+
base_name = os.path.abspath(base_name)
1127+
if not dry_run:
1128+
os.chdir(root_dir)
11031129

11041130
try:
11051131
filename = func(base_name, base_dir, **kwargs)
11061132
finally:
1107-
if root_dir is not None:
1133+
if save_cwd is not None:
11081134
if logger is not None:
11091135
logger.debug("changing back to '%s'", save_cwd)
11101136
os.chdir(save_cwd)
@@ -1217,6 +1243,11 @@ def _unpack_tarfile(filename, extract_dir):
12171243
finally:
12181244
tarobj.close()
12191245

1246+
# Maps the name of the unpack format to a tuple containing:
1247+
# * extensions
1248+
# * the unpacking function
1249+
# * extra keyword arguments
1250+
# * description
12201251
_UNPACK_FORMATS = {
12211252
'tar': (['.tar'], _unpack_tarfile, [], "uncompressed tar file"),
12221253
'zip': (['.zip'], _unpack_zipfile, [], "ZIP file"),

‎Lib/test/test_shutil.py

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@
5151
except ImportError:
5252
_winapi = None
5353

54+
no_chdir = unittest.mock.patch('os.chdir',
55+
side_effect=AssertionError("shouldn't call os.chdir()"))
56+
5457
def _fake_rename(*args, **kwargs):
5558
# Pretend the destination path is on a different filesyste 4D1F m.
5659
raise OSError(getattr(errno, 'EXDEV', 18), "Invalid cross-device link")
@@ -1342,7 +1345,7 @@ def test_make_tarball(self):
13421345
work_dir = os.path.dirname(tmpdir2)
13431346
rel_base_name = os.path.join(os.path.basename(tmpdir2), 'archive')
13441347

1345-
with os_helper.change_cwd(work_dir):
1348+
with os_helper.change_cwd(work_dir), no_chdir:
13461349
base_name = os.path.abspath(rel_base_name)
13471350
tarball = make_archive(rel_base_name, 'gztar', root_dir, '.')
13481351

@@ -1356,7 +1359,7 @@ def test_make_tarball(self):
13561359
'./file1', './file2', './sub/file3'])
13571360

13581361
# trying an uncompressed one
1359-
with os_helper.change_cwd(work_dir):
1362+
with os_helper.change_cwd(work_dir), no_chdir:
13601363
tarball = make_archive(rel_base_name, 'tar', root_dir, '.')
13611364
self.assertEqual(tarball, base_name + '.tar')
13621365
self.assertTrue(os.path.isfile(tarball))
@@ -1392,7 +1395,8 @@ def _create_files(self, base_dir='dist'):
13921395
def test_tarfile_vs_tar(self):
13931396
root_dir, base_dir = self._create_files()
13941397
base_name = os.path.join(self.mkdtemp(), 'archive')
1395-
tarball = make_archive(base_name, 'gztar', root_dir, base_dir)
1398+
with no_chdir:
1399+
tarball = make_archive(base_name, 'gztar', root_dir, base_dir)
13961400

13971401
# check if the compressed tarball was created
13981402
self.assertEqual(tarball, base_name + '.tar.gz')
@@ -1409,13 +1413,15 @@ def test_tarfile_vs_tar(self):
14091413
self.assertEqual(self._tarinfo(tarball), self._tarinfo(tarball2))
14101414

14111415
# trying an uncompressed one
1412-
tarball = make_archive(base_name, 'tar', root_dir, base_dir)
1416+
with no_chdir:
1417+
tarball = make_archive(base_name, 'tar', root_dir, base_dir)
14131418
self.assertEqual(tarball, base_name + '.tar')
14141419
self.assertTrue(os.path.isfile(tarball))
14151420

14161421
# now for a dry_run
1417-
tarball = make_archive(base_name, 'tar', root_dir, base_dir,
1418-
dry_run=True)
1422+
with no_chdir:
1423+
tarball = make_archive(base_name, 'tar', root_dir, base_dir,
1424+
dry_run=True)
14191425
self.assertEqual(tarball, base_name + '.tar')
14201426
self.assertTrue(os.path.isfile(tarball))
14211427

@@ -1431,7 +1437,7 @@ def test_make_zipfile(self):
14311437
work_dir = os.path.dirname(tmpdir2)
14321438
rel_base_name = os.path.join(os.path.basename(tmpdir2), 'archive')
14331439

1434-
with os_helper.change_cwd(work_dir):
1440+
with os_helper.change_cwd(work_dir), no_chdir:
14351441
base_name = os.path.abspath(rel_base_name)
14361442
res = make_archive(rel_base_name, 'zip', root_dir)
14371443

@@ -1444,7 +1450,7 @@ def test_make_zipfile(self):
14441450
'dist/file1', 'dist/file2', 'dist/sub/file3',
14451451
'outer'])
14461452

1447-
with os_helper.change_cwd(work_dir):
1453+
with os_helper.change_cwd(work_dir), no_chdir:
14481454
base_name = os.path.abspath(rel_base_name)
14491455
res = make_archive(rel_base_name, 'zip', root_dir, base_dir)
14501456

@@ -1462,7 +1468,8 @@ def test_make_zipfile(self):
14621468
def test_zipfile_vs_zip(self):
14631469
root_dir, base_dir = self._create_files()
14641470
base_name = os.path.join(self.mkdtemp(), 'archive')
1465-
archive = make_archive(base_name, 'zip', root_dir, base_dir)
1471+
with no_chdir:
1472+
archive = make_archive(base_name, 'zip', root_dir, base_dir)
14661473

14671474
# check if ZIP file was created
14681475
self.assertEqual(archive, base_name + '.zip')
@@ -1488,7 +1495,8 @@ def test_zipfile_vs_zip(self):
14881495
def test_unzip_zipfile(self):
14891496
root_dir, base_dir = self._create_files()
14901497
base_name = os.path.join(self.mkdtemp(), 'archive')
1491-
archive = make_archive(base_name, 'zip', root_dir, base_dir)
1498+
with no_chdir:
1499+
archive = make_archive(base_name, 'zip', root_dir, base_dir)
14921500

14931501
# check if ZIP file was created
14941502
self.assertEqual(archive, base_name + '.zip')
@@ -1546,7 +1554,7 @@ def test_tarfile_root_owner(self):
15461554
base_name = os.path.join(self.mkdtemp(), 'archive')
15471555
group = grp.getgrgid(0)[0]
15481556
owner = pwd.getpwuid(0)[0]
1549-
with os_helper.change_cwd(root_dir):
1557+
with os_helper.change_cwd(root_dir), no_chdir:
15501558
archive_name = make_archive(base_name, 'gztar', root_dir, 'dist',
15511559
owner=owner, group=group)
15521560

@@ -1564,31 +1572,38 @@ def test_tarfile_root_owner(self):
15641572

15651573
def test_make_archive_cwd(self):
15661574
current_dir = os.getcwd()
1575+
root_dir = self.mkdtemp()
15671576
def _breaks(*args, **kw):
15681577
raise RuntimeError()
1578+
dirs = []
1579+
def _chdir(path):
1580+
dirs.append(path)
1581+
orig_chdir(path)
15691582

15701583
register_archive_format('xxx', _breaks, [], 'xxx file')
15711584
try:
1572-
try:
1573-
make_archive('xxx', 'xxx', root_dir=self.mkdtemp())
1574-
except Exception:
1575-
pass
1585+
with support.swap_attr(os, 'chdir', _chdir) as orig_chdir:
1586+
try:
1587+
make_archive('xxx', 'xxx', root_dir=root_dir)
1588+
except Exception:
1589+
pass
15761590
self.assertEqual(os.getcwd(), current_dir)
1591+
self.assertEqual(dirs, [root_dir, current_dir])
15771592
finally:
15781593
unregister_archive_format('xxx')
15791594

15801595
def test_make_tarfile_in_curdir(self):
15811596
# Issue #21280
15821597
root_dir = self.mkdtemp()
1583-
with os_helper.change_cwd(root_dir):
1598+
with os_helper.change_cwd(root_dir), no_chdir:
15841599
self.assertEqual(make_archive('test', 'tar'), 'test.tar')
15851600
self.assertTrue(os.path.isfile('test.tar'))
15861601

15871602
@support.requires_zlib()
15881603
def test_make_zipfile_in_curdir(self):
15891604
# Issue #21280
15901605
root_dir = self.mkdtemp()
1591-
with os_helper.change_cwd(root_dir):
1606+
with os_helper.change_cwd(root_dir), no_chdir:
15921607
self.assertEqual(make_archive('test', 'zip'), 'test.zip')
15931608
self.assertTrue(os.path.isfile('test.zip'))
15941609

‎Misc/NEWS.d/next/Library/2022-05-24-11-19-04.gh-issue-74696.-cnf-A.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
:func:`shutil.make_archive` no longer temporarily changes the current
2+
working directory during creation of standard ``.zip`` or tar archives.

0 commit comments

Comments
 (0)
0