From 9f87d5d95545c29a1242a05c4db9ea7253a57ab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 16 Aug 2018 04:05:28 +0200 Subject: [PATCH 1/4] unittests: test_install_subdir_symlinks_with_default_umask Add test to verify the installation of broken symlinks when a default_umask is set. Reusing the same code of other test, thus sharing the actual test code in a single function. --- run_unittests.py | 22 ++++++++++++++----- .../199 install_mode/installed_files.txt | 1 + .../common/199 install_mode/meson.build | 3 +++ test cases/common/199 install_mode/sub2/stub | 0 4 files changed, 20 insertions(+), 6 deletions(-) create mode 100644 test cases/common/199 install_mode/sub2/stub diff --git a/run_unittests.py b/run_unittests.py index d768028ae..6d74549ff 100755 --- a/run_unittests.py +++ b/run_unittests.py @@ -3779,32 +3779,42 @@ endian = 'little' # Ensure that the otool output does not contain self.installdir self.assertNotRegex(out, self.installdir + '.*dylib ') - def test_install_subdir_symlinks(self): + def install_subdir_invalid_symlinks(self, testdir, subdir_path): ''' Test that installation of broken symlinks works fine. https://github.com/mesonbuild/meson/issues/3914 ''' - testdir = os.path.join(self.common_test_dir, '66 install subdir') - subdir = os.path.join(testdir, 'sub/sub1') + testdir = os.path.join(self.common_test_dir, testdir) + subdir = os.path.join(testdir, subdir_path) curdir = os.getcwd() os.chdir(subdir) # Can't distribute broken symlinks in the source tree because it breaks # the creation of zipapps. Create it dynamically and run the test by # hand. src = '../../nonexistent.txt' - os.symlink(src, 'test.txt') + os.symlink(src, 'invalid-symlink.txt') try: self.init(testdir) self.build() self.install() - link = os.path.join(self.installdir, 'usr', 'share', 'sub1', 'test.txt') + install_path = subdir_path.split(os.path.sep)[-1] + link = os.path.join(self.installdir, 'usr', 'share', install_path, 'invalid-symlink.txt') self.assertTrue(os.path.islink(link), msg=link) self.assertEqual(src, os.readlink(link)) self.assertFalse(os.path.isfile(link), msg=link) finally: - os.remove(os.path.join(subdir, 'test.txt')) + os.remove(os.path.join(subdir, 'invalid-symlink.txt')) os.chdir(curdir) + def test_install_subdir_symlinks(self): + self.install_subdir_invalid_symlinks('66 install subdir', os.path.join('sub', 'sub1')) + + def test_install_subdir_symlinks_with_default_umask(self): + self.install_subdir_invalid_symlinks('199 install_mode', 'sub2') + + def test_install_subdir_symlinks_with_default_umask_and_mode(self): + self.install_subdir_invalid_symlinks('199 install_mode', 'sub1') + class LinuxCrossArmTests(BasePlatformTests): ''' diff --git a/test cases/common/199 install_mode/installed_files.txt b/test cases/common/199 install_mode/installed_files.txt index 00fb231e7..724d9546e 100644 --- a/test cases/common/199 install_mode/installed_files.txt +++ b/test cases/common/199 install_mode/installed_files.txt @@ -5,4 +5,5 @@ usr/include/rootdir.h usr/libtest/libstat.a usr/share/man/man1/foo.1.gz usr/share/sub1/second.dat +usr/share/sub2/stub usr/subdir/data.dat diff --git a/test cases/common/199 install_mode/meson.build b/test cases/common/199 install_mode/meson.build index d06371f84..18a29483f 100644 --- a/test cases/common/199 install_mode/meson.build +++ b/test cases/common/199 install_mode/meson.build @@ -11,6 +11,9 @@ install_subdir('sub1', install_dir : 'share', install_mode : ['rwxr-x--t', 'root']) +install_subdir('sub2', + install_dir : 'share') + # test install_mode in configure_file conf = configuration_data() conf.set('var', 'mystring') diff --git a/test cases/common/199 install_mode/sub2/stub b/test cases/common/199 install_mode/sub2/stub new file mode 100644 index 000000000..e69de29bb From 2d010727ed6657cb53d5043032417e0a9035e117 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 16 Aug 2018 04:12:29 +0200 Subject: [PATCH 2/4] minstall: never try to set chmod on symlinks It's only supported by few platforms when the linked file exists, while it would cause an error otherwise. In any case just implement this via an helper set_chmod function that will handle the case where follow_symlinks is not supported by the platform and will just not set any mod for the link itself (as it would otherwise apply to the linked file). Fixes #3914 --- mesonbuild/minstall.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/mesonbuild/minstall.py b/mesonbuild/minstall.py index f08a6b361..5ac1279cc 100644 --- a/mesonbuild/minstall.py +++ b/mesonbuild/minstall.py @@ -79,13 +79,20 @@ def append_to_log(lf, line): lf.write('\n') lf.flush() +def set_chmod(path, mode, dir_fd=None, follow_symlinks=True): + try: + os.chmod(path, mode, dir_fd=dir_fd, follow_symlinks=follow_symlinks) + except (NotImplementedError, OSError, SystemError) as e: + if not os.path.islink(path): + os.chmod(path, mode, dir_fd=dir_fd) + def sanitize_permissions(path, umask): if umask is None: return new_perms = 0o777 if is_executable(path) else 0o666 new_perms &= ~umask try: - os.chmod(path, new_perms) + set_chmod(path, new_perms, follow_symlinks=False) except PermissionError as e: msg = '{!r}: Unable to set permissions {!r}: {}, ignoring...' print(msg.format(path, new_perms, e.strerror)) @@ -116,7 +123,7 @@ def set_mode(path, mode, default_umask): # NOTE: On Windows you can set read/write perms; the rest are ignored if mode.perms_s is not None: try: - os.chmod(path, mode.perms) + set_chmod(path, mode.perms, follow_symlinks=False) except PermissionError as e: msg = '{!r}: Unable to set permissions {!r}: {}, ignoring...' print(msg.format(path, mode.perms_s, e.strerror)) From 5de2a7910aafb39940789f5dbed244c230624917 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 16 Aug 2018 04:19:16 +0200 Subject: [PATCH 3/4] minstall: never follow symlinks when setting ownership Since we're supposed to call this for each installed path, we only should go through what we've installed and not what this point to, as it might be outside our scope or not existent. To do this, since shutil.chown doesn't expose the follow_symlink that os.chown has, we can temporarily replace os.chown with a lambda that acutually passes all the values as we want them, and then restore it to the built-in functions. Not the nicest way, but fixes the issue without having to reimplement what shutil does. Fixes #3914 --- mesonbuild/minstall.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/mesonbuild/minstall.py b/mesonbuild/minstall.py index 5ac1279cc..3ae1199e5 100644 --- a/mesonbuild/minstall.py +++ b/mesonbuild/minstall.py @@ -79,6 +79,24 @@ def append_to_log(lf, line): lf.write('\n') lf.flush() +def set_chown(path, user=None, group=None, dir_fd=None, follow_symlinks=True): + # shutil.chown will call os.chown without passing all the parameters + # and particularly follow_symlinks, thus we replace it temporary + # with a lambda with all the parameters so that follow_symlinks will + # be actually passed properly. + # Not nice, but better than actually rewriting shutil.chown until + # this python bug is fixed: https://bugs.python.org/issue18108 + real_os_chown = os.chown + try: + os.chown = lambda p, u, g: real_os_chown(p, u, g, + dir_fd=dir_fd, + follow_symlinks=follow_symlinks) + shutil.chown(path, user, group) + except: + raise + finally: + os.chown = real_os_chown + def set_chmod(path, mode, dir_fd=None, follow_symlinks=True): try: os.chmod(path, mode, dir_fd=dir_fd, follow_symlinks=follow_symlinks) @@ -105,7 +123,7 @@ def set_mode(path, mode, default_umask): # No chown() on Windows, and must set one of owner/group if not is_windows() and (mode.owner or mode.group) is not None: try: - shutil.chown(path, mode.owner, mode.group) + set_chown(path, mode.owner, mode.group, follow_symlinks=False) except PermissionError as e: msg = '{!r}: Unable to set owner {!r} and group {!r}: {}, ignoring...' print(msg.format(path, mode.owner, mode.group, e.strerror)) From abf65c92af298683fa9ffa5e360ce6c836a1d2fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 17 Aug 2018 04:02:23 +0200 Subject: [PATCH 4/4] minstall: use follow_symlinks to check executable This could happen when setting an default install mode but with broken symlinks. Fixes #3914 --- mesonbuild/minstall.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mesonbuild/minstall.py b/mesonbuild/minstall.py index 3ae1199e5..748f06baa 100644 --- a/mesonbuild/minstall.py +++ b/mesonbuild/minstall.py @@ -69,9 +69,9 @@ class DirMaker: for d in self.dirs: append_to_log(self.lf, d) -def is_executable(path): +def is_executable(path, follow_symlinks=False): '''Checks whether any of the "x" bits are set in the source file mode.''' - return bool(os.stat(path).st_mode & 0o111) + return bool(os.stat(path, follow_symlinks=follow_symlinks).st_mode & 0o111) def append_to_log(lf, line): lf.write(line) @@ -107,7 +107,7 @@ def set_chmod(path, mode, dir_fd=None, follow_symlinks=True): def sanitize_permissions(path, umask): if umask is None: return - new_perms = 0o777 if is_executable(path) else 0o666 + new_perms = 0o777 if is_executable(path, follow_symlinks=False) else 0o666 new_perms &= ~umask try: set_chmod(path, new_perms, follow_symlinks=False)