Skip to content

Commit 6870753

Browse files
committed
gh-86533: Clarify parent_mode umask docs and harden makedirs/mkdir tests
Address review feedback on the parent_mode addition: - Docs (os.makedirs, Path.mkdir): state that parent_mode, like mode, is combined with the process umask; it does not bypass it. - Mark the parameter as added in 3.15 (approved for the 3.15 backport) instead of "next", which on main would resolve to 3.16. - Replace the misleading "parent_mode overrides umask" test with one that verifies umask IS applied to parent_mode (0o777 + umask 0o022 -> 0o755); add the matching os.makedirs test. - Make MakedirTests cleanup name-agnostic and robust via os_helper.rmtree() instead of a hard-coded dir1/.../dir6 walk. - Align the new test_os skips with the pathlib tests (Emscripten/WASI/Android); restore the original 0o555 case in test_mode. - Use os_helper.temp_umask() in the pathlib tests instead of a manual os.umask() try/finally.
1 parent 73cc9eb commit 6870753

4 files changed

Lines changed: 68 additions & 44 deletions

File tree

Doc/library/os.rst

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2568,9 +2568,11 @@ features:
25682568
If *exist_ok* is ``False`` (the default), a :exc:`FileExistsError` is
25692569
raised if the target directory already exists.
25702570

2571-
If *parent_mode* is not ``None``, it will be used as the mode for any
2572-
newly-created, intermediate-level directories. Otherwise, intermediate
2573-
directories are created with the default permissions (respecting umask).
2571+
If *parent_mode* is not ``None``, it is used as the mode for any
2572+
newly-created, intermediate-level directories. Like *mode*, it is
2573+
combined with the process's umask value; see :ref:`the mkdir()
2574+
description <mkdir_modebits>`. Otherwise, intermediate directories are
2575+
created with the default mode, which is also subject to the umask.
25742576

25752577
.. note::
25762578

@@ -2598,7 +2600,7 @@ features:
25982600
The *mode* argument no longer affects the file permission bits of
25992601
newly created intermediate-level directories.
26002602

2601-
.. versionadded:: next
2603+
.. versionadded:: 3.15
26022604
The *parent_mode* parameter. To match the behavior from Python 3.6 and
26032605
earlier (where *mode* was applied to all created directories), pass
26042606
``parent_mode=mode``.

Doc/library/pathlib.rst

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1530,10 +1530,11 @@ Creating files and directories
15301530
as needed; they are created with the default permissions without taking
15311531
*mode* into account (mimicking the POSIX ``mkdir -p`` command).
15321532

1533-
If *parent_mode* is not ``None``, it will be used as the mode for any
1533+
If *parent_mode* is not ``None``, it is used as the mode for any
15341534
newly-created, intermediate-level directories when *parents* is true.
1535+
Like *mode*, it is combined with the process's ``umask`` value.
15351536
Otherwise, intermediate directories are created with the default
1536-
permissions (respecting umask).
1537+
permissions (also subject to the umask).
15371538

15381539
If *parents* is false (the default), a missing parent raises
15391540
:exc:`FileNotFoundError`.
@@ -1548,7 +1549,7 @@ Creating files and directories
15481549
.. versionchanged:: 3.5
15491550
The *exist_ok* parameter was added.
15501551

1551-
.. versionadded:: next
1552+
.. versionadded:: 3.15
15521553
The *parent_mode* parameter.
15531554

15541555

Lib/test/test_os/test_os.py

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2128,21 +2128,24 @@ def test_makedir(self):
21282128
def test_mode(self):
21292129
# Note: in some cases, the umask might already be 2 in which case this
21302130
# will pass even if os.umask is actually broken.
2131-
parent = os.path.join(os_helper.TESTFN, 'dir1')
2132-
path = os.path.join(parent, 'dir2')
21332131
with os_helper.temp_umask(0o002):
2134-
os.makedirs(path, 0o705)
2132+
base = os_helper.TESTFN
2133+
parent = os.path.join(base, 'dir1')
2134+
path = os.path.join(parent, 'dir2')
2135+
os.makedirs(path, 0o555)
21352136
self.assertTrue(os.path.exists(path))
21362137
self.assertTrue(os.path.isdir(path))
21372138
if os.name != 'nt':
2138-
# Leaf directory gets the specified mode
2139-
self.assertEqual(os.stat(path).st_mode & 0o777, 0o705)
2140-
# Parent directory uses default permissions (respecting umask)
2139+
self.assertEqual(os.stat(path).st_mode & 0o777, 0o555)
21412140
self.assertEqual(os.stat(parent).st_mode & 0o777, 0o775)
21422141

21432142
@unittest.skipIf(
2144-
support.is_wasi,
2145-
"WASI's umask is a stub."
2143+
support.is_emscripten or support.is_wasi,
2144+
"umask is not implemented on Emscripten/WASI."
2145+
)
2146+
@unittest.skipIf(
2147+
sys.platform == "android",
2148+
"Android filesystem may not honor requested permissions."
21462149
)
21472150
def test_mode_with_parent_mode(self):
21482151
# Test the parent_mode parameter
@@ -2160,8 +2163,12 @@ def test_mode_with_parent_mode(self):
21602163
self.assertEqual(os.stat(parent).st_mode & 0o777, 0o750)
21612164

21622165
@unittest.skipIf(
2163-
support.is_wasi,
2164-
"WASI's umask is a stub."
2166+
support.is_emscripten or support.is_wasi,
2167+
"umask is not implemented on Emscripten/WASI."
2168+
)
2169+
@unittest.skipIf(
2170+
sys.platform == "android",
2171+
"Android filesystem may not honor requested permissions."
21652172
)
21662173
def test_parent_mode_deep_hierarchy(self):
21672174
# Test parent_mode with deep directory hierarchy
@@ -2179,8 +2186,12 @@ def test_parent_mode_deep_hierarchy(self):
21792186
self.assertEqual(os.stat(base).st_mode & 0o777, 0o755)
21802187

21812188
@unittest.skipIf(
2182-
support.is_wasi,
2183-
"WASI's umask is a stub."
2189+
support.is_emscripten or support.is_wasi,
2190+
"umask is not implemented on Emscripten/WASI."
2191+
)
2192+
@unittest.skipIf(
2193+
sys.platform == "android",
2194+
"Android filesystem may not honor requested permissions."
21842195
)
21852196
def test_parent_mode_same_as_mode(self):
21862197
# Test emulating Python 3.6 behavior by setting parent_mode=mode
@@ -2194,6 +2205,28 @@ def test_parent_mode_same_as_mode(self):
21942205
self.assertEqual(os.stat(path).st_mode & 0o777, 0o705)
21952206
self.assertEqual(os.stat(parent).st_mode & 0o777, 0o705)
21962207

2208+
@unittest.skipIf(
2209+
support.is_emscripten or support.is_wasi,
2210+
"umask is not implemented on Emscripten/WASI."
2211+
)
2212+
@unittest.skipIf(
2213+
sys.platform == "android",
2214+
"Android filesystem may not honor requested permissions."
2215+
)
2216+
def test_parent_mode_combined_with_umask(self):
2217+
# parent_mode, like mode, is combined with the process umask; it does
2218+
# not bypass it.
2219+
parent = os.path.join(os_helper.TESTFN, 'dir1')
2220+
path = os.path.join(parent, 'dir2')
2221+
with os_helper.temp_umask(0o022):
2222+
os.makedirs(path, 0o777, parent_mode=0o777)
2223+
self.assertTrue(os.path.isdir(path))
2224+
if os.name != 'nt':
2225+
# 0o777 is masked down to 0o755 by the 0o022 umask, for both
2226+
# the leaf (mode) and the parent (parent_mode).
2227+
self.assertEqual(os.stat(path).st_mode & 0o777, 0o755)
2228+
self.assertEqual(os.stat(parent).st_mode & 0o777, 0o755)
2229+
21972230
@unittest.skipIf(
21982231
support.is_wasi,
21992232
"WASI's umask is a stub."
@@ -2267,15 +2300,9 @@ def test_win32_mkdir_700(self):
22672300
)
22682301

22692302
def tearDown(self):
2270-
path = os.path.join(os_helper.TESTFN, 'dir1', 'dir2', 'dir3',
2271-
'dir4', 'dir5', 'dir6')
2272-
# If the tests failed, the bottom-most directory ('../dir6')
2273-
# may not have been created, so we look for the outermost directory
2274-
# that exists.
2275-
while not os.path.exists(path) and path != os_helper.TESTFN:
2276-
path = os.path.dirname(path)
2277-
2278-
os.removedirs(path)
2303+
# Remove the whole tree regardless of which sub-directories a test
2304+
# created and regardless of their permission bits.
2305+
os_helper.rmtree(os_helper.TESTFN)
22792306

22802307

22812308
@unittest.skipUnless(hasattr(os, "chown"), "requires os.chown()")

Lib/test/test_pathlib/test_pathlib.py

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2505,16 +2505,13 @@ def test_mkdir_parents_umask(self):
25052505
p = self.cls(self.base, 'umasktest', 'child')
25062506
self.assertFalse(p.exists())
25072507
if os.name != 'nt':
2508-
old_mask = os.umask(0o002)
2509-
try:
2508+
with os_helper.temp_umask(0o002):
25102509
p.mkdir(0o755, parents=True)
25112510
self.assertTrue(p.exists())
25122511
# Leaf directory gets the specified mode
25132512
self.assertEqual(p.stat().st_mode & 0o777, 0o755)
25142513
# Parent directory respects umask (0o777 & ~0o002 = 0o775)
25152514
self.assertEqual(p.parent.stat().st_mode & 0o777, 0o775)
2516-
finally:
2517-
os.umask(old_mask)
25182515

25192516
@unittest.skipIf(
25202517
is_emscripten or is_wasi,
@@ -2569,22 +2566,19 @@ def test_mkdir_parent_mode_deep_hierarchy(self):
25692566
sys.platform == "android",
25702567
"Android filesystem may not honor requested permissions."
25712568
)
2572-
def test_mkdir_parent_mode_overrides_umask(self):
2573-
# Test that parent_mode overrides umask for parent directories
2574-
p = self.cls(self.base, 'overridetest', 'child')
2569+
def test_mkdir_parent_mode_combined_with_umask(self):
2570+
# parent_mode, like mode, is combined with the process umask; it does
2571+
# not bypass it.
2572+
p = self.cls(self.base, 'umaskPM', 'child')
25752573
self.assertFalse(p.exists())
25762574
if os.name != 'nt':
2577-
old_mask = os.umask(0o022) # Restrictive umask
2578-
try:
2579-
# parent_mode should override umask for parents
2580-
p.mkdir(0o755, parents=True, parent_mode=0o700)
2575+
with os_helper.temp_umask(0o022):
2576+
p.mkdir(0o777, parents=True, parent_mode=0o777)
25812577
self.assertTrue(p.exists())
2582-
# Leaf directory gets the specified mode
2578+
# 0o777 is masked down to 0o755 by the 0o022 umask, for both
2579+
# the leaf (mode) and the parent (parent_mode).
25832580
self.assertEqual(p.stat().st_mode & 0o777, 0o755)
2584-
# Parent directory gets parent_mode, not affected by umask
2585-
self.assertEqual(p.parent.stat().st_mode & 0o777, 0o700)
2586-
finally:
2587-
os.umask(old_mask)
2581+
self.assertEqual(p.parent.stat().st_mode & 0o777, 0o755)
25882582

25892583
@unittest.skipIf(
25902584
is_emscripten or is_wasi,

0 commit comments

Comments
 (0)