https://github.com/python/cpython/commit/8377cd4fcd0d51d86834c9b0518d29aac3b... commit: 8377cd4fcd0d51d86834c9b0518d29aac3b49e18 branch: master author: Anthony Sottile <asottile@umich.edu> committer: Giampaolo Rodola <g.rodola@gmail.com> date: 2019-02-25T23:32:27+01:00 summary: Clean up code which checked presence of os.{stat,lstat,chmod} (#11643) files: A Misc/NEWS.d/next/Library/2019-01-21-13-56-55.bpo-35802.6633PE.rst M Lib/dbm/dumb.py M Lib/fileinput.py M Lib/shutil.py M Lib/tarfile.py M Lib/tempfile.py M Lib/test/libregrtest/runtest.py M Lib/test/pickletester.py M Lib/test/test_compileall.py M Lib/test/test_dbm_dumb.py M Lib/test/test_fileinput.py M Lib/test/test_mailbox.py M Lib/test/test_mmap.py M Lib/test/test_os.py M Lib/test/test_posix.py M Lib/test/test_shutil.py M Lib/test/test_tempfile.py M setup.py diff --git a/Lib/dbm/dumb.py b/Lib/dbm/dumb.py index 6cef72a49acc..864ad371ec95 100644 --- a/Lib/dbm/dumb.py +++ b/Lib/dbm/dumb.py @@ -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 diff --git a/Lib/fileinput.py b/Lib/fileinput.py index a7f8df37aada..4a71cc5ff318 100644 --- a/Lib/fileinput.py +++ b/Lib/fileinput.py @@ -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 diff --git a/Lib/shutil.py b/Lib/shutil.py index 29e7564622e1..1f98a348d7e6 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 ba3e95f281df..bb09d10925b2 100755 --- a/Lib/tarfile.py +++ b/Lib/tarfile.py @@ -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) @@ -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. diff --git a/Lib/tempfile.py b/Lib/tempfile.py index e6fb3c8e9ad8..a66d6f3750cb 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -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: diff --git a/Lib/test/libregrtest/runtest.py b/Lib/test/libregrtest/runtest.py index dc2abf237bc0..4f218b769f98 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 293393fe3713..8f687c49e690 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -1568,11 +1568,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) diff --git a/Lib/test/test_compileall.py b/Lib/test/test_compileall.py index 2e2552303f8d..8e584602de36 100644 --- a/Lib/test/test_compileall.py +++ b/Lib/test/test_compileall.py @@ -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.""" diff --git a/Lib/test/test_dbm_dumb.py b/Lib/test/test_dbm_dumb.py index e8e827e5f5bc..a971359d33ff 100644 --- a/Lib/test/test_dbm_dumb.py +++ b/Lib/test/test_dbm_dumb.py @@ -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) @@ -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') diff --git a/Lib/test/test_fileinput.py b/Lib/test/test_fileinput.py index c5d722e58f59..3857401ca60f 100644 --- a/Lib/test/test_fileinput.py +++ b/Lib/test/test_fileinput.py @@ -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.""" diff --git a/Lib/test/test_mailbox.py b/Lib/test/test_mailbox.py index a75c8cb00e80..0995b1e386d0 100644 --- a/Lib/test/test_mailbox.py +++ b/Lib/test/test_mailbox.py @@ -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) @@ -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. @@ -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 246fdf01f9cd..3e6ecfc95660 100644 --- a/Lib/test/test_mmap.py +++ b/Lib/test/test_mmap.py @@ -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+") @@ -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. @@ -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. diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 2cfcebcc8648..2340b848d9b7 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -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) diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index e980dd014cd8..fe21b21b3385 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -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))) @@ -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) @@ -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): diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index 86f4dc97c3a1..ceafaeda1dc2 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -262,7 +262,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, @@ -337,7 +336,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() @@ -1022,14 +1020,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 diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index e5098d2eea6b..3c0b9a74e5c8 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -8,6 +8,7 @@ import re import warnings import contextlib +import stat import weakref from unittest import mock @@ -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') @@ -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 @@ -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 @@ -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: 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 000000000000..8b73d2bd5851 --- /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 4a01a8f45ac4..33106549458c 100644 --- a/setup.py +++ b/setup.py @@ -2266,7 +2266,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: @@ -2277,16 +2276,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)