Merge pull request #4030 from 3v1n0/broken-symlink-install-with-mode-fix

Fix broken symlinks install when using install_umask or install_mode

5 files changed, 51 insertions, 12 deletions

diff --git a/mesonbuild/minstall.py b/mesonbuild/minstall.py
index f08a6b3..748f06b 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)
@@ -79,13 +79,38 @@ 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)
+    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 = 0o777 if is_executable(path, follow_symlinks=False) 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))
@@ -98,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))
@@ -116,7 +141,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))
diff --git a/run_unittests.py b/run_unittests.py
index ab71882..411fd61 100755
--- a/run_unittests.py
+++ b/run_unittests.py
@@ -3831,32 +3831,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 00fb231..724d954 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 d06371f..18a2948 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 0000000..e69de29
--- /dev/null
+++ b/test cases/common/199 install_mode/sub2/stub