Skip to content

bpo-35802: Clean up code which checked presence of os.{stat,lstat,chmod} #11643

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Lib/dbm/dumb.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,7 @@ def close(self):
__del__ = close

def _chmod(self, file):
if hasattr(self._os, 'chmod'):
self._os.chmod(file, self._mode)
self._os.chmod(file, self._mode)

def __enter__(self):
return self
Expand Down
3 changes: 1 addition & 2 deletions Lib/fileinput.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,8 +357,7 @@ def _readline(self):
fd = os.open(self._filename, mode, perm)
self._output = os.fdopen(fd, "w")
try:
if hasattr(os, 'chmod'):
os.chmod(self._filename, perm)
os.chmod(self._filename, perm)
except OSError:
pass
self._savestdout = sys.stdout
Expand Down
4 changes: 1 addition & 3 deletions Lib/shutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,8 @@ def copymode(src, dst, *, follow_symlinks=True):
stat_func, chmod_func = os.lstat, os.lchmod
else:
return
elif hasattr(os, 'chmod'):
stat_func, chmod_func = _stat, os.chmod
else:
return
stat_func, chmod_func = _stat, os.chmod

st = stat_func(src)
chmod_func(dst, stat.S_IMODE(st.st_mode))
Expand Down
14 changes: 6 additions & 8 deletions Lib/tarfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -1787,10 +1787,9 @@ def gettarinfo(self, name=None, arcname=None, fileobj=None):
tarinfo = self.tarinfo()
tarinfo.tarfile = self # Not needed

# Use os.stat or os.lstat, depending on platform
# and if symlinks shall be resolved.
# Use os.stat or os.lstat, depending on if symlinks shall be resolved.
if fileobj is None:
if hasattr(os, "lstat") and not self.dereference:
if not self.dereference:
statres = os.lstat(name)
else:
statres = os.stat(name)
Expand Down Expand Up @@ -2235,11 +2234,10 @@ def chown(self, tarinfo, targetpath, numeric_owner):
def chmod(self, tarinfo, targetpath):
"""Set file permissions of targetpath according to tarinfo.
"""
if hasattr(os, 'chmod'):
try:
os.chmod(targetpath, tarinfo.mode)
except OSError:
raise ExtractError("could not change mode")
try:
os.chmod(targetpath, tarinfo.mode)
except OSError:
raise ExtractError("could not change mode")

def utime(self, tarinfo, targetpath):
"""Set modification time of targetpath according to tarinfo.
Expand Down
12 changes: 1 addition & 11 deletions Lib/tempfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,20 +70,10 @@

_once_lock = _allocate_lock()

if hasattr(_os, "lstat"):
_stat = _os.lstat
elif hasattr(_os, "stat"):
_stat = _os.stat
else:
# Fallback. All we need is something that raises OSError if the
# file doesn't exist.
def _stat(fn):
fd = _os.open(fn, _os.O_RDONLY)
_os.close(fd)

def _exists(fn):
try:
_stat(fn)
_os.lstat(fn)
except OSError:
return False
else:
Expand Down
6 changes: 2 additions & 4 deletions Lib/test/libregrtest/runtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,8 @@ def cleanup_test_droppings(testname, verbose):
if verbose:
print("%r left behind %s %r" % (testname, kind, name))
try:
# if we have chmod, fix possible permissions problems
# that might prevent cleanup
if (hasattr(os, 'chmod')):
os.chmod(name, stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO)
# fix possible permissions problems that might prevent cleanup
os.chmod(name, stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO)
nuker(name)
except Exception as msg:
print(("%r left behind %s %r and it couldn't be "
Expand Down
9 changes: 4 additions & 5 deletions Lib/test/pickletester.py
Original file line number Diff line number Diff line change
Expand Up @@ -1503,11 +1503,10 @@ def test_structseq(self):
s = self.dumps(t, proto)
u = self.loads(s)
self.assert_is_copy(t, u)
if hasattr(os, "stat"):
t = os.stat(os.curdir)
s = self.dumps(t, proto)
u = self.loads(s)
self.assert_is_copy(t, u)
t = os.stat(os.curdir)
s = self.dumps(t, proto)
u = self.loads(s)
self.assert_is_copy(t, u)
if hasattr(os, "statvfs"):
t = os.statvfs(os.curdir)
s = self.dumps(t, proto)
Expand Down
1 change: 0 additions & 1 deletion Lib/test/test_compileall.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ def timestamp_metadata(self):
compare = struct.pack('<4sll', importlib.util.MAGIC_NUMBER, 0, mtime)
return data, compare

@unittest.skipUnless(hasattr(os, 'stat'), 'test needs os.stat()')
def recreation_check(self, metadata):
"""Check that compileall recreates bytecode when the new metadata is
used."""
Expand Down
2 changes: 0 additions & 2 deletions Lib/test/test_dbm_dumb.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ def test_dumbdbm_creation(self):
f.close()

@unittest.skipUnless(hasattr(os, 'umask'), 'test needs os.umask()')
@unittest.skipUnless(hasattr(os, 'chmod'), 'test needs os.chmod()')
def test_dumbdbm_creation_mode(self):
try:
old_umask = os.umask(0o002)
Expand Down Expand Up @@ -275,7 +274,6 @@ def test_invalid_flag(self):
"'r', 'w', 'c', or 'n'"):
dumbdbm.open(_fname, flag)

@unittest.skipUnless(hasattr(os, 'chmod'), 'test needs os.chmod()')
def test_readonly_files(self):
with support.temp_dir() as dir:
fname = os.path.join(dir, 'db')
Expand Down
1 change: 0 additions & 1 deletion Lib/test/test_fileinput.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,6 @@ def test_readline_os_fstat_raises_OSError(self):
self.assertTrue(os_fstat_replacement.invoked,
"os.fstat() was not invoked")

@unittest.skipIf(not hasattr(os, "chmod"), "os.chmod does not exist")
def test_readline_os_chmod_raises_OSError(self):
"""Tests invoking FileInput.readline() when os.chmod() raises OSError.
This exception should be silently discarded."""
Expand Down
3 changes: 0 additions & 3 deletions Lib/test/test_mailbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,6 @@ def test_directory_in_folder (self):
pass

@unittest.skipUnless(hasattr(os, 'umask'), 'test needs os.umask()')
@unittest.skipUnless(hasattr(os, 'stat'), 'test needs os.stat()')
def test_file_permissions(self):
# Verify that message files are created without execute permissions
msg = mailbox.MaildirMessage(self._template % 0)
Expand All @@ -878,7 +877,6 @@ def test_file_permissions(self):
self.assertFalse(mode & 0o111)

@unittest.skipUnless(hasattr(os, 'umask'), 'test needs os.umask()')
@unittest.skipUnless(hasattr(os, 'stat'), 'test needs os.stat()')
def test_folder_file_perms(self):
# From bug #3228, we want to verify that the file created inside a Maildir
# subfolder isn't marked as executable.
Expand Down Expand Up @@ -1120,7 +1118,6 @@ class TestMbox(_TestMboxMMDF, unittest.TestCase):
_factory = lambda self, path, factory=None: mailbox.mbox(path, factory)

@unittest.skipUnless(hasattr(os, 'umask'), 'test needs os.umask()')
@unittest.skipUnless(hasattr(os, 'stat'), 'test needs os.stat()')
def test_file_perms(self):
# From bug #3228, we want to verify that the mailbox file isn't executable,
# even if the umask is set to something that would leave executable bits set.
Expand Down
3 changes: 0 additions & 3 deletions Lib/test/test_mmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,6 @@ def test_double_close(self):
mf.close()
f.close()

@unittest.skipUnless(hasattr(os, "stat"), "needs os.stat()")
def test_entire_file(self):
# test mapping of entire file by passing 0 for map length
f = open(TESTFN, "wb+")
Expand All @@ -332,7 +331,6 @@ def test_entire_file(self):
mf.close()
f.close()

@unittest.skipUnless(hasattr(os, "stat"), "needs os.stat()")
def test_length_0_offset(self):
# Issue #10916: test mapping of remainder of file by passing 0 for
# map length with an offset doesn't cause a segfault.
Expand All @@ -345,7 +343,6 @@ def test_length_0_offset(self):
with mmap.mmap(f.fileno(), 0, offset=65536, access=mmap.ACCESS_READ) as mf:
self.assertRaises(IndexError, mf.__getitem__, 80000)

@unittest.skipUnless(hasattr(os, "stat"), "needs os.stat()")
def test_length_0_large_offset(self):
# Issue #10959: test mapping of a file by passing 0 for
# map length with a large offset doesn't cause a segfault.
Expand Down
1 change: 0 additions & 1 deletion Lib/test/test_os.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ def setUp(self):
self.addCleanup(support.unlink, self.fname)
create_file(self.fname, b"ABC")

@unittest.skipUnless(hasattr(os, 'stat'), 'test needs os.stat()')
def check_stat_attributes(self, fname):
result = os.stat(fname)

Expand Down
6 changes: 1 addition & 5 deletions Lib/test/test_posix.py
Original file line number Diff line number Diff line change
Expand Up @@ -606,8 +606,6 @@ def test_fstat(self):
finally:
fp.close()

@unittest.skipUnless(hasattr(posix, 'stat'),
'test needs posix.stat()')
def test_stat(self):
self.assertTrue(posix.stat(support.TESTFN))
self.assertTrue(posix.stat(os.fsencode(support.TESTFN)))
Expand Down Expand Up @@ -658,7 +656,6 @@ def test_mknod(self):
except OSError as e:
self.assertIn(e.errno, (errno.EPERM, errno.EINVAL, errno.EACCES))

@unittest.skipUnless(hasattr(posix, 'stat'), 'test needs posix.stat()')
@unittest.skipUnless(hasattr(posix, 'makedev'), 'test needs posix.makedev()')
def test_makedev(self):
st = posix.stat(support.TESTFN)
Expand Down Expand Up @@ -755,8 +752,7 @@ def test_chown(self):

# re-create the file
support.create_empty_file(support.TESTFN)
self._test_all_chown_common(posix.chown, support.TESTFN,
getattr(posix, 'stat', None))
self._test_all_chown_common(posix.chown, support.TESTFN, posix.stat)

@unittest.skipUnless(hasattr(posix, 'fchown'), "test needs os.fchown()")
def test_fchown(self):
Expand Down
4 changes: 0 additions & 4 deletions Lib/test/test_shutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ def onerror(*args):
self.assertIn(errors[1][2][1].filename, possible_args)


@unittest.skipUnless(hasattr(os, 'chmod'), 'requires os.chmod()')
@unittest.skipIf(sys.platform[:6] == 'cygwin',
"This test can't be run on Cygwin (issue #1071513).")
@unittest.skipIf(hasattr(os, 'geteuid') and os.geteuid() == 0,
Expand Down Expand Up @@ -325,7 +324,6 @@ def raiser(fn, *args, **kwargs):
finally:
os.lstat = orig_lstat

@unittest.skipUnless(hasattr(os, 'chmod'), 'requires os.chmod')
@support.skip_unless_symlink
def test_copymode_follow_symlinks(self):
tmp_dir = self.mkdtemp()
Expand Down Expand Up @@ -1010,14 +1008,12 @@ def _copy_file(self, method):
file2 = os.path.join(tmpdir2, fname)
return (file1, file2)

@unittest.skipUnless(hasattr(os, 'chmod'), 'requires os.chmod')
def test_copy(self):
# Ensure that the copied file exists and has the same mode bits.
file1, file2 = self._copy_file(shutil.copy)
self.assertTrue(os.path.exists(file2))
self.assertEqual(os.stat(file1).st_mode, os.stat(file2).st_mode)

@unittest.skipUnless(hasattr(os, 'chmod'), 'requires os.chmod')
@unittest.skipUnless(hasattr(os, 'utime'), 'requires os.utime')
def test_copy2(self):
# Ensure that the copied file exists and has the same mode and
Expand Down
12 changes: 2 additions & 10 deletions Lib/test/test_tempfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import re
import warnings
import contextlib
import stat
import weakref
from unittest import mock

Expand All @@ -16,12 +17,6 @@
from test.support import script_helper


if hasattr(os, 'stat'):
import stat
has_stat = 1
else:
has_stat = 0

has_textmode = (tempfile._text_openflags != tempfile._bin_openflags)
has_spawnl = hasattr(os, 'spawnl')

Expand Down Expand Up @@ -433,7 +428,6 @@ def test_choose_directory(self):
finally:
os.rmdir(dir)

@unittest.skipUnless(has_stat, 'os.stat not available')
def test_file_mode(self):
# _mkstemp_inner creates files with the proper mode

Expand Down Expand Up @@ -738,7 +732,6 @@ def test_choose_directory(self):
finally:
os.rmdir(dir)

@unittest.skipUnless(has_stat, 'os.stat not available')
def test_mode(self):
# mkdtemp creates directories with the proper mode

Expand Down Expand Up @@ -1221,8 +1214,7 @@ def test_truncate_with_size_parameter(self):
f.write(b'abcdefg\n')
f.truncate(20)
self.assertTrue(f._rolled)
if has_stat:
self.assertEqual(os.fstat(f.fileno()).st_size, 20)
self.assertEqual(os.fstat(f.fileno()).st_size, 20)


if tempfile.NamedTemporaryFile is not tempfile.TemporaryFile:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Clean up code which checked presence of ``os.stat`` / ``os.lstat`` /
``os.chmod`` which are always present. Patch by Anthony Sottile.
5 changes: 0 additions & 5 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -2254,7 +2254,6 @@ def install(self):
return outfiles

def set_file_modes(self, files, defaultMode, sharedLibMode):
if not self.is_chmod_supported(): return
if not files: return

for filename in files:
Expand All @@ -2265,16 +2264,12 @@ def set_file_modes(self, files, defaultMode, sharedLibMode):
if not self.dry_run: os.chmod(filename, mode)

def set_dir_modes(self, dirname, mode):
if not self.is_chmod_supported(): return
for dirpath, dirnames, fnames in os.walk(dirname):
if os.path.islink(dirpath):
continue
log.info("changing mode of %s to %o", dirpath, mode)
if not self.dry_run: os.chmod(dirpath, mode)

def is_chmod_supported(self):
return hasattr(os, 'chmod')

class PyBuildScripts(build_scripts):
def copy_scripts(self):
outfiles, updated_files = build_scripts.copy_scripts(self)
Expand Down