From 8377cd4fcd0d51d86834c9b0518d29aac3b49e18 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 25 Feb 2019 14:32:27 -0800 Subject: Clean up code which checked presence of os.{stat,lstat,chmod} (#11643) --- Lib/dbm/dumb.py | 3 +-- Lib/fileinput.py | 3 +-- Lib/shutil.py | 4 +--- Lib/tarfile.py | 14 ++++++-------- Lib/tempfile.py | 12 +----------- Lib/test/libregrtest/runtest.py | 6 ++---- Lib/test/pickletester.py | 9 ++++----- Lib/test/test_compileall.py | 1 - Lib/test/test_dbm_dumb.py | 2 -- Lib/test/test_fileinput.py | 1 - Lib/test/test_mailbox.py | 3 --- Lib/test/test_mmap.py | 3 --- Lib/test/test_os.py | 1 - Lib/test/test_posix.py | 6 +----- Lib/test/test_shutil.py | 4 ---- Lib/test/test_tempfile.py | 12 ++---------- .../next/Library/2019-01-21-13-56-55.bpo-35802.6633PE.rst | 2 ++ setup.py | 5 ----- 18 files changed, 21 insertions(+), 70 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2019-01-21-13-56-55.bpo-35802.6633PE.rst diff --git a/Lib/dbm/dumb.py b/Lib/dbm/dumb.py index 6cef72a..864ad37 100644 --- a/Lib/dbm/dumb.py +++ b/Lib/dbm/dumb.py @@ -278,8 +278,7 @@ class _Database(collections.abc.MutableMapping): __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 diff --git a/Lib/fileinput.py b/Lib/fileinput.py index a7f8df3..4a71cc5 100644 --- a/Lib/fileinput.py +++ b/Lib/fileinput.py @@ -357,8 +357,7 @@ class FileInput: 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 diff --git a/Lib/shutil.py b/Lib/shutil.py index 29e7564..1f98a34 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -290,10 +290,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)) diff --git a/Lib/tarfile.py b/Lib/tarfile.py index ba3e95f..bb09d10 100755 --- a/Lib/tarfile.py +++ b/Lib/tarfile.py @@ -1787,10 +1787,9 @@ class TarFile(object): 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) @@ -2235,11 +2234,10 @@ class TarFile(object): 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. diff --git a/Lib/tempfile.py b/Lib/tempfile.py index e6fb3c8..a66d6f3 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -70,20 +70,10 @@ template = "tmp" _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: diff --git a/Lib/test/libregrtest/runtest.py b/Lib/test/libregrtest/runtest.py index dc2abf2..4f218b7 100644 --- a/Lib/test/libregrtest/runtest.py +++ b/Lib/test/libregrtest/runtest.py @@ -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 " diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index 293393f..8f687c4 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -1568,11 +1568,10 @@ class AbstractPickleTests(unittest.TestCase): 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) diff --git a/Lib/test/test_compileall.py b/Lib/test/test_compileall.py index 2e25523..8e58460 100644 --- a/Lib/test/test_compileall.py +++ b/Lib/test/test_compileall.py @@ -57,7 +57,6 @@ class CompileallTestsBase: 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.""" diff --git a/Lib/test/test_dbm_dumb.py b/Lib/test/test_dbm_dumb.py index e8e827e..a971359 100644 --- a/Lib/test/test_dbm_dumb.py +++ b/Lib/test/test_dbm_dumb.py @@ -40,7 +40,6 @@ class DumbDBMTestCase(unittest.TestCase): 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) @@ -275,7 +274,6 @@ class DumbDBMTestCase(unittest.TestCase): "'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') diff --git a/Lib/test/test_fileinput.py b/Lib/test/test_fileinput.py index c5d722e..3857401 100644 --- a/Lib/test/test_fileinput.py +++ b/Lib/test/test_fileinput.py @@ -428,7 +428,6 @@ class FileInputTests(BaseTests, unittest.TestCase): 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.""" diff --git a/Lib/test/test_mailbox.py b/Lib/test/test_mailbox.py index a75c8cb..0995b1e 100644 --- a/Lib/test/test_mailbox.py +++ b/Lib/test/test_mailbox.py @@ -864,7 +864,6 @@ class TestMaildir(TestMailbox, unittest.TestCase): 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) @@ -878,7 +877,6 @@ class TestMaildir(TestMailbox, unittest.TestCase): 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. @@ -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. diff --git a/Lib/test/test_mmap.py b/Lib/test/test_mmap.py index 246fdf0..3e6ecfc 100644 --- a/Lib/test/test_mmap.py +++ b/Lib/test/test_mmap.py @@ -317,7 +317,6 @@ class MmapTests(unittest.TestCase): 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+") @@ -332,7 +331,6 @@ class MmapTests(unittest.TestCase): 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. @@ -345,7 +343,6 @@ class MmapTests(unittest.TestCase): 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. diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 2cfcebc..2340b84 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -238,7 +238,6 @@ class StatAttributeTests(unittest.TestCase): 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) diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index e980dd0..fe21b21 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -606,8 +606,6 @@ class PosixTester(unittest.TestCase): 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))) @@ -658,7 +656,6 @@ class PosixTester(unittest.TestCase): 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) @@ -755,8 +752,7 @@ class PosixTester(unittest.TestCase): # 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): diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index 86f4dc9..ceafaed 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -262,7 +262,6 @@ class TestShutil(unittest.TestCase): 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, @@ -337,7 +336,6 @@ class TestShutil(unittest.TestCase): 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() @@ -1022,14 +1020,12 @@ class TestShutil(unittest.TestCase): 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 diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index e5098d2..3c0b9a7 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -8,6 +8,7 @@ import sys import re import warnings import contextlib +import stat import weakref from unittest import mock @@ -16,12 +17,6 @@ from test import support 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') @@ -433,7 +428,6 @@ class TestMkstempInner(TestBadTempdir, BaseTestCase): 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 @@ -738,7 +732,6 @@ class TestMkdtemp(TestBadTempdir, BaseTestCase): finally: os.rmdir(dir) - @unittest.skipUnless(has_stat, 'os.stat not available') def test_mode(self): # mkdtemp creates directories with the proper mode @@ -1221,8 +1214,7 @@ class TestSpooledTemporaryFile(BaseTestCase): 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: diff --git a/Misc/NEWS.d/next/Library/2019-01-21-13-56-55.bpo-35802.6633PE.rst b/Misc/NEWS.d/next/Library/2019-01-21-13-56-55.bpo-35802.6633PE.rst new file mode 100644 index 0000000..8b73d2b --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-01-21-13-56-55.bpo-35802.6633PE.rst @@ -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. diff --git a/setup.py b/setup.py index 4a01a8f..3310654 100644 --- a/setup.py +++ b/setup.py @@ -2266,7 +2266,6 @@ class PyBuildInstallLib(install_lib): 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: @@ -2277,16 +2276,12 @@ class PyBuildInstallLib(install_lib): 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) -- cgit v0.12