From 95ce8bf3fa06091c398cd0226e81c8d77ad679ca Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Sat, 29 Jan 2022 19:54:15 +0100 Subject: [PATCH] meson: Allow directory options outside of prefix This bring us in line with Autotools and CMake and it is useful for platforms like Nix, which install projects into multiple independent prefixes. As a consequence, `get_option` might return absolute paths for some directory options, if a directory outside of prefix is passed. This is technically a backwards incompatible change but its effect should be minimal, thanks to widespread use of `join_paths`/`/` operator and pkg-config generator module. It should only cause an issue when a path were constructed by concatenating the value of directory path option. Also remove a comment about commonpath since we do not use that since . Fixes: https://github.com/mesonbuild/meson/issues/2561 --- docs/markdown/Builtin-options.md | 4 ++- .../snippets/dir_options_outside_prefix.md | 9 ++++++ docs/yaml/functions/_build_target_base.yaml | 8 ++--- docs/yaml/functions/get_option.yaml | 18 +++++------- mesonbuild/coredata.py | 29 +++++++------------ .../meson.build | 6 ---- .../38 libdir must be inside prefix/test.json | 10 ------- unittests/allplatformstests.py | 14 ++++----- 8 files changed, 41 insertions(+), 57 deletions(-) create mode 100644 docs/markdown/snippets/dir_options_outside_prefix.md delete mode 100644 test cases/failing/38 libdir must be inside prefix/meson.build delete mode 100644 test cases/failing/38 libdir must be inside prefix/test.json diff --git a/docs/markdown/Builtin-options.md b/docs/markdown/Builtin-options.md index 613b8b89b523..db3c3e879664 100644 --- a/docs/markdown/Builtin-options.md +++ b/docs/markdown/Builtin-options.md @@ -23,13 +23,15 @@ For legacy reasons `--warnlevel` is the cli argument for the They can also be edited after setup using `meson configure -Doption=value`. -Installation options are all relative to the prefix, except: +Installation options are usually relative to the prefix but it should +not be relied on, since they can be absolute paths in the following cases: * When the prefix is `/usr`: `sysconfdir` defaults to `/etc`, `localstatedir` defaults to `/var`, and `sharedstatedir` defaults to `/var/lib` * When the prefix is `/usr/local`: `localstatedir` defaults to `/var/local`, and `sharedstatedir` defaults to `/var/local/lib` +* When an absolute path outside of prefix is provided by the user/distributor. ### Directories diff --git a/docs/markdown/snippets/dir_options_outside_prefix.md b/docs/markdown/snippets/dir_options_outside_prefix.md new file mode 100644 index 000000000000..7427ea9c9d2f --- /dev/null +++ b/docs/markdown/snippets/dir_options_outside_prefix.md @@ -0,0 +1,9 @@ +It is now allowed to set directory options to paths outside of prefix. +This bring us in line with Autotools and CMake and it is useful for +platforms like Nix, which install projects into multiple independent prefixes. + +As a consequence, `get_option` might return absolute paths for some +directory options, if a directory outside of prefix is passed. This +is technically a backwards incompatible change but its effect +should be minimal, thanks to widespread use of `join_paths`/ +`/` operator and pkg-config generator module. diff --git a/docs/yaml/functions/_build_target_base.yaml b/docs/yaml/functions/_build_target_base.yaml index 62424b67a99a..09c76b9750ed 100644 --- a/docs/yaml/functions/_build_target_base.yaml +++ b/docs/yaml/functions/_build_target_base.yaml @@ -161,10 +161,10 @@ kwargs: install_dir: type: str description: | - override install directory for this file. The value is - relative to the `prefix` specified. F.ex, if you want to install - plugins into a subdir, you'd use something like this: `install_dir : - get_option('libdir') / 'projectname-1.0'`. + override install directory for this file. If the value is a relative path, + it will be considered relative the `prefix` option. + F.ex, if you want to install plugins into a subdir, you'd use something + like this: `install_dir : get_option('libdir') / 'projectname-1.0'`. install_mode: type: list[str | int] diff --git a/docs/yaml/functions/get_option.yaml b/docs/yaml/functions/get_option.yaml index 0bf0042b36ac..893a7ac6a59d 100644 --- a/docs/yaml/functions/get_option.yaml +++ b/docs/yaml/functions/get_option.yaml @@ -5,16 +5,14 @@ description: | specified in the positional argument. Note that the value returned for built-in options that end in `dir` - such as `bindir` and `libdir` is always a path relative to (and - inside) the `prefix`. - - The only exceptions are: `sysconfdir`, `localstatedir`, and - `sharedstatedir` which will return the value passed during - configuration as-is, which may be absolute, or relative to `prefix`. - [`install_dir` arguments](Installing.md) handles that as expected, but - if you need the absolute path to one of these e.g. to use in a define - etc., you should use `get_option('prefix') / - get_option('localstatedir')` + such as `bindir` and `libdir` is usually a path relative to (and + inside) the `prefix` but you should not rely on that, as it can also + be an absolute path [in some cases](Builtin-options.md#Universal options). + [`install_dir` arguments](Installing.md) handle that as expected + but if you need an absolute path, e.g. to use in a define etc., + you should use path concatenation operator like this: + `get_option('prefix') / get_option('localstatedir')`. + Never manually join paths as if they were strings. For options of type `feature` a [[@feature]] option object diff --git a/mesonbuild/coredata.py b/mesonbuild/coredata.py index 2bb89a25b4b4..f200771433e4 100644 --- a/mesonbuild/coredata.py +++ b/mesonbuild/coredata.py @@ -549,14 +549,13 @@ def sanitize_prefix(self, prefix): def sanitize_dir_option_value(self, prefix: str, option: OptionKey, value: T.Any) -> T.Any: ''' - If the option is an installation directory option and the value is an - absolute path, check that it resides within prefix and return the value - as a path relative to the prefix. + If the option is an installation directory option, the value is an + absolute path and resides within prefix, return the value + as a path relative to the prefix. Otherwise, return it as is. - This way everyone can do f.ex, get_option('libdir') and be sure to get - the library directory relative to prefix. - - .as_posix() keeps the posix-like file separators Meson uses. + This way everyone can do f.ex, get_option('libdir') and usually get + the library directory relative to prefix, even though it really + should not be relied upon. ''' try: value = PurePath(value) @@ -564,21 +563,13 @@ def sanitize_dir_option_value(self, prefix: str, option: OptionKey, value: T.Any return value if option.name.endswith('dir') and value.is_absolute() and \ option not in BULITIN_DIR_NOPREFIX_OPTIONS: - # Value must be a subdir of the prefix - # commonpath will always return a path in the native format, so we - # must use pathlib.PurePath to do the same conversion before - # comparing. - msg = ('The value of the \'{!s}\' option is \'{!s}\' which must be a ' - 'subdir of the prefix {!r}.\nNote that if you pass a ' - 'relative path, it is assumed to be a subdir of prefix.') - # os.path.commonpath doesn't understand case-insensitive filesystems, - # but PurePath().relative_to() does. try: + # Try to relativize the path. value = value.relative_to(prefix) except ValueError: - raise MesonException(msg.format(option, value, prefix)) - if '..' in str(value): - raise MesonException(msg.format(option, value, prefix)) + # Path is not relative, let’s keep it as is. + pass + # .as_posix() keeps the posix-like file separators Meson uses. return value.as_posix() def init_builtins(self, subproject: str) -> None: diff --git a/test cases/failing/38 libdir must be inside prefix/meson.build b/test cases/failing/38 libdir must be inside prefix/meson.build deleted file mode 100644 index 4cce7f81c553..000000000000 --- a/test cases/failing/38 libdir must be inside prefix/meson.build +++ /dev/null @@ -1,6 +0,0 @@ -project('libdir prefix', 'c', - default_options : ['libdir=/opt/lib']) - -if host_machine.system() == 'windows' - error('MESON_SKIP_TEST: this test does not work on Windows since /foo is not absolute') -endif \ No newline at end of file diff --git a/test cases/failing/38 libdir must be inside prefix/test.json b/test cases/failing/38 libdir must be inside prefix/test.json deleted file mode 100644 index d9256d1a247a..000000000000 --- a/test cases/failing/38 libdir must be inside prefix/test.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "do_not_set_opts": [ - "libdir" - ], - "stdout": [ - { - "line": "test cases/failing/38 libdir must be inside prefix/meson.build:1:0: ERROR: The value of the 'libdir' option is '/opt/lib' which must be a subdir of the prefix '/usr'." - } - ] -} diff --git a/unittests/allplatformstests.py b/unittests/allplatformstests.py index 51ca7a033b6d..e67a5cedc57c 100644 --- a/unittests/allplatformstests.py +++ b/unittests/allplatformstests.py @@ -213,9 +213,9 @@ def test_absolute_prefix_libdir(self): elif opt['name'] == 'libdir': self.assertEqual(libdir, opt['value']) - def test_libdir_must_be_inside_prefix(self): + def test_libdir_can_be_outside_prefix(self): ''' - Tests that libdir is forced to be inside prefix no matter how it is set. + Tests that libdir is allowed to be outside prefix. Must be a unit test for obvious reasons. ''' testdir = os.path.join(self.common_test_dir, '1 trivial') @@ -226,19 +226,19 @@ def test_libdir_must_be_inside_prefix(self): args = ['--prefix', '/opt', '--libdir', '/opt/lib32'] self.init(testdir, extra_args=args) self.wipe() - # libdir not being inside prefix is not ok + # libdir not being inside prefix is ok too if is_windows(): args = ['--prefix', 'x:/usr', '--libdir', 'x:/opt/lib32'] else: args = ['--prefix', '/usr', '--libdir', '/opt/lib32'] - self.assertRaises(subprocess.CalledProcessError, self.init, testdir, extra_args=args) + self.init(testdir, extra_args=args) self.wipe() - # libdir must be inside prefix even when set via mesonconf + # libdir can be outside prefix even when set via mesonconf self.init(testdir) if is_windows(): - self.assertRaises(subprocess.CalledProcessError, self.setconf, '-Dlibdir=x:/opt', False) + self.setconf('-Dlibdir=x:/opt', will_build=False) else: - self.assertRaises(subprocess.CalledProcessError, self.setconf, '-Dlibdir=/opt', False) + self.setconf('-Dlibdir=/opt', will_build=False) def test_prefix_dependent_defaults(self): '''