install modes should not apply sticky bit to files

This is generally a bad idea, e.g. it causes OSError on freebsd.

It also gets ignored by solaris and thus causes unittest failures.

The proper solution is to simply reject any attempt to set this, and log a
warning.

The install_emptydir function does apply the mode as well, and since it
is a directory it actually does something. This is the only place where
we don't reset the mode.

Although install_subdir also installs directories, and in theory it
could set the mode as well, that would be a new feature. Also it doesn't
provide much granularity and has mixed semantics with files. Better to
let people use install_emptydir + install_subdir.

Fixes #5902

4 files changed, 54 insertions, 12 deletions

diff --git a/docs/markdown/snippets/deprecated_install_mode_sticky.md b/docs/markdown/snippets/deprecated_install_mode_sticky.md
new file mode 100644
index 0000000..3168df6
--- /dev/null
+++ b/docs/markdown/snippets/deprecated_install_mode_sticky.md
@@ -0,0 +1,14 @@
+## various `install_*` functions no longer handle the sticky bit
+
+It is not possible to portably grant the sticky bit to a file, and where
+possible, it doesn't do anything. It is not expected that any users are using
+this functionality.
+
+Variously:
+- on Linux, it has no meaningful effect
+- on Solaris, attempting to set the permission bit is silently ignored by the OS
+- on FreeBSD, attempting to set the permission bit is an error
+
+Attempting to set this permission bit in the `install_mode:` kwarg to any
+function other than [[install_emptydir]] will now result in a warning, and the
+permission bit being ignored.
diff --git a/mesonbuild/interpreter/interpreter.py b/mesonbuild/interpreter/interpreter.py
index 131f073..0521125 100644
--- a/mesonbuild/interpreter/interpreter.py
+++ b/mesonbuild/interpreter/interpreter.py
@@ -1894,6 +1894,7 @@ class Interpreter(InterpreterBase, HoldableObject):
                            kwargs: 'kwargs.CustomTarget') -> build.CustomTarget:
         if kwargs['depfile'] and ('@BASENAME@' in kwargs['depfile'] or '@PLAINNAME@' in kwargs['depfile']):
             FeatureNew.single_use('substitutions in custom_target depfile', '0.47.0', self.subproject, location=node)
+        install_mode = self._warn_kwarg_install_mode_sticky(kwargs['install_mode'])
 
         # Don't mutate the kwargs
 
@@ -1976,7 +1977,7 @@ class Interpreter(InterpreterBase, HoldableObject):
             feed=kwargs['feed'],
             install=kwargs['install'],
             install_dir=kwargs['install_dir'],
-            install_mode=kwargs['install_mode'],
+            install_mode=install_mode,
             install_tag=kwargs['install_tag'],
             backend=self.backend)
         self.add_target(tg.name, tg)
@@ -2129,6 +2130,7 @@ class Interpreter(InterpreterBase, HoldableObject):
     def func_install_headers(self, node: mparser.BaseNode,
                              args: T.Tuple[T.List['mesonlib.FileOrString']],
                              kwargs: 'kwargs.FuncInstallHeaders') -> build.Headers:
+        install_mode = self._warn_kwarg_install_mode_sticky(kwargs['install_mode'])
         source_files = self.source_strings_to_files(args[0])
         install_subdir = kwargs['subdir']
         if install_subdir is not None:
@@ -2150,7 +2152,7 @@ class Interpreter(InterpreterBase, HoldableObject):
 
         for childdir in dirs:
             h = build.Headers(dirs[childdir], os.path.join(install_subdir, childdir), kwargs['install_dir'],
-                              kwargs['install_mode'], self.subproject)
+                              install_mode, self.subproject)
             ret_headers.append(h)
             self.build.headers.append(h)
 
@@ -2166,6 +2168,7 @@ class Interpreter(InterpreterBase, HoldableObject):
     def func_install_man(self, node: mparser.BaseNode,
                          args: T.Tuple[T.List['mesonlib.FileOrString']],
                          kwargs: 'kwargs.FuncInstallMan') -> build.Man:
+        install_mode = self._warn_kwarg_install_mode_sticky(kwargs['install_mode'])
         # We just need to narrow this, because the input is limited to files and
         # Strings as inputs, so only Files will be returned
         sources = self.source_strings_to_files(args[0])
@@ -2177,7 +2180,7 @@ class Interpreter(InterpreterBase, HoldableObject):
             if not 1 <= num <= 9:
                 raise InvalidArguments('Man file must have a file extension of a number between 1 and 9')
 
-        m = build.Man(sources, kwargs['install_dir'], kwargs['install_mode'],
+        m = build.Man(sources, kwargs['install_dir'], install_mode,
                       self.subproject, kwargs['locale'])
         self.build.man.append(m)
 
@@ -2321,6 +2324,21 @@ class Interpreter(InterpreterBase, HoldableObject):
                                    'permissions arg to be a string or false')
         return FileMode(*install_mode)
 
+
+    # This is either ignored on basically any OS nowadays, or silently gets
+    # ignored (Solaris) or triggers an "illegal operation" error (FreeBSD).
+    # It was likely added "because it exists", but should never be used. In
+    # theory it is useful for directories, but we never apply modes to
+    # directories other than in install_emptydir.
+    def _warn_kwarg_install_mode_sticky(self, mode: FileMode) -> None:
+        if mode.perms > 0 and mode.perms & stat.S_ISVTX:
+            mlog.deprecation('install_mode with the sticky bit on a file does not do anything and will '
+                             f'be ignored since Meson 0.64.0', location=self.current_node)
+            perms = stat.filemode(mode.perms - stat.S_ISVTX)[1:]
+            return FileMode(perms, mode.owner, mode.group)
+        else:
+            return mode
+
     @typed_pos_args('install_data', varargs=(str, mesonlib.File))
     @typed_kwargs(
         'install_data',
@@ -2342,7 +2360,8 @@ class Interpreter(InterpreterBase, HoldableObject):
                     '"rename" and "sources" argument lists must be the same length if "rename" is given. '
                     f'Rename has {len(rename)} elements and sources has {len(sources)}.')
 
-        return self.install_data_impl(sources, kwargs['install_dir'], kwargs['install_mode'],
+        install_mode = self._warn_kwarg_install_mode_sticky(kwargs['install_mode'])
+        return self.install_data_impl(sources, kwargs['install_dir'], install_mode,
                                       rename, kwargs['install_tag'],
                                       preserve_path=kwargs['preserve_path'])
 
@@ -2398,12 +2417,13 @@ class Interpreter(InterpreterBase, HoldableObject):
             FeatureNew.single_use('install_subdir with empty directory', '0.47.0', self.subproject, location=node)
             FeatureDeprecated.single_use('install_subdir with empty directory', '0.60.0', self.subproject,
                                          'It worked by accident and is buggy. Use install_emptydir instead.', node)
+        install_mode = self._warn_kwarg_install_mode_sticky(kwargs['install_mode'])
 
         idir = build.InstallDir(
             self.subdir,
             args[0],
             kwargs['install_dir'],
-            kwargs['install_mode'],
+            install_mode,
             exclude,
             kwargs['strip_directory'],
             self.subproject,
@@ -2469,6 +2489,8 @@ class Interpreter(InterpreterBase, HoldableObject):
         if kwargs['capture'] and not kwargs['command']:
             raise InvalidArguments('configure_file: "capture" keyword requires "command" keyword.')
 
+        install_mode = self._warn_kwarg_install_mode_sticky(kwargs['install_mode'])
+
         fmt = kwargs['format']
         output_format = kwargs['output_format']
         depfile = kwargs['depfile']
@@ -2594,7 +2616,6 @@ class Interpreter(InterpreterBase, HoldableObject):
             if isinstance(idir_name, P_OBJ.OptionString):
                 idir_name = idir_name.optname
             cfile = mesonlib.File.from_built_file(ofile_path, ofile_fname)
-            install_mode = kwargs['install_mode']
             install_tag = kwargs['install_tag']
             self.build.data.append(build.Data([cfile], idir, idir_name, install_mode, self.subproject,
                                               install_tag=install_tag, data_type='configure'))
diff --git a/test cases/common/190 install_mode/test.json b/test cases/common/190 install_mode/test.json
index 3614dbc..a58caa1 100644
--- a/test cases/common/190 install_mode/test.json
+++ b/test cases/common/190 install_mode/test.json
@@ -11,5 +11,12 @@
     {"type": "file", "file": "usr/share/sub2/stub"},
     {"type": "file", "file": "usr/subdir/data.dat"}
   ],
-  "do_not_set_opts": ["libdir"]
+  "do_not_set_opts": ["libdir"],
+  "stdout": [
+    {
+      "line": ".* DEPRECATION: install_mode with the sticky bit on a file",
+      "match": "re",
+      "count": 3
+    }
+  ]
 }
diff --git a/unittests/linuxliketests.py b/unittests/linuxliketests.py
index 76721aa..99adeac 100644
--- a/unittests/linuxliketests.py
+++ b/unittests/linuxliketests.py
@@ -615,7 +615,7 @@ class LinuxlikeTests(BasePlatformTests):
 
         f = os.path.join(self.installdir, 'etc', 'etcfile.dat')
         found_mode = stat.filemode(os.stat(f).st_mode)
-        want_mode = 'rw------T'
+        want_mode = 'rw-------'
         self.assertEqual(want_mode, found_mode[1:])
 
         f = os.path.join(self.installdir, 'usr', 'bin', 'runscript.sh')
@@ -650,7 +650,7 @@ class LinuxlikeTests(BasePlatformTests):
         f = os.path.join(self.installdir, 'usr', 'share', 'sub1', 'second.dat')
         statf = os.stat(f)
         found_mode = stat.filemode(statf.st_mode)
-        want_mode = 'rwxr-x--t'
+        want_mode = 'rwxr-x--x'
         self.assertEqual(want_mode, found_mode[1:])
         if os.getuid() == 0:
             # The chown failed nonfatally if we're not root
@@ -673,15 +673,15 @@ class LinuxlikeTests(BasePlatformTests):
                 ('bin/trivialprog', '-rwxr-sr-x'),
                 ('include', 'drwxr-x---'),
                 ('include/config.h', '-rw-rwSr--'),
-                ('include/rootdir.h', '-r--r--r-T'),
+                ('include/rootdir.h', '-r--r--r--'),
                 ('lib', 'drwxr-x---'),
                 ('lib/libstat.a', '-rw---Sr--'),
                 ('share', 'drwxr-x---'),
                 ('share/man', 'drwxr-x---'),
                 ('share/man/man1', 'drwxr-x---'),
-                ('share/man/man1/foo.1', '-r--r--r-T'),
+                ('share/man/man1/foo.1', '-r--r--r--'),
                 ('share/sub1', 'drwxr-x---'),
-                ('share/sub1/second.dat', '-rwxr-x--t'),
+                ('share/sub1/second.dat', '-rwxr-x--x'),
                 ('subdir', 'drwxr-x---'),
                 ('subdir/data.dat', '-rw-rwSr--'),
         ]: