[PATCH xserver] meson: Fix module_dir configuration

[Thierry Reding]

On Mon, Apr 02, 2018 at 02:31:20PM -0700, Aaron Plattner wrote:
> meson.build has code to set the module_dir variable to
> ${libdir}/xorg/modules if the module_dir option string is empty.
> However, this has several problems:
> 
> 1. The variable is only used for an unused @moduledir@ substitution in
>    the man page. The rule for xorg-server.pc uses option('module_dir')
>    directly instead.
> 2. The 'module_dir' option has a default value of 'xorg/modules' so the
>    above rule doesn't do anything by default.
> 3. The xorg-server.pc rule uses ${exec_prefix}/option('module_dir'), so
>    the effect of #2 is that the default moduledir is different between
>    autoconf and meson. E.g. if ${prefix} is /X, then you get
> 
>      autoconf: moduledir=/X/lib/xorg/modules
>      meson:    moduledir=/X/xorg/modules

Ugh... you're right. I was setting --libexecdir ${prefix}/lib in my
scripts, which is why I wasn't seeing the above inconsistency.

> Fix this by using the module_dir variable when generating
> xorg-server.pc, and by removing the default value for the module_dir
> option.
> 
> Signed-off-by: Aaron Plattner <aplattner at nvidia.com>
> ---
>  meson.build       | 2 +-
>  meson_options.txt | 3 +--
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 277534093b94..3e3f808b2d7a 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -595,7 +595,7 @@ if build_xorg
>      sdkconfig.set('libdir', join_paths('${exec_prefix}', get_option('libdir')))
>      sdkconfig.set('includedir', join_paths('${prefix}', get_option('includedir')))
>      sdkconfig.set('datarootdir', join_paths('${prefix}', get_option('datadir')))
> -    sdkconfig.set('moduledir', join_paths('${exec_prefix}', get_option('module_dir')))
> +    sdkconfig.set('moduledir', join_paths('${exec_prefix}', module_dir))

This would still give us an inconsistent path if the user passed in some
other, relative directory for module_dir, right? So if they passed:

	--module-dir foo/xorg/modules

they'd get /usr/foo/xorg/modules in the pkg-config files, but the server
would actually look for it in /usr/lib/foo/xorg/modules.

>      sdkconfig.set('sdkdir', join_paths('${prefix}', get_option('includedir'), 'xorg'))
>      sdkconfig.set('sysconfigdir', join_paths('${datarootdir}', 'X11/xorg.conf.d'))
>  
> diff --git a/meson_options.txt b/meson_options.txt
> index 5c7be0e26ce5..4cf8349ba9e5 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -19,8 +19,7 @@ option('builder_addr', type: 'string', description: 'Builder address', value: 'x
>  option('builder_string', type: 'string', description: 'Additional builder string')
>  
>  option('log_dir', type: 'string')
> -option('module_dir', type: 'string', value: 'xorg/modules',
> -       description: 'X.Org modules directory')
> +option('module_dir', type: 'string', description: 'X.Org modules directory')

It seems somewhat backwards to me to avoid the feature of assigning a
default value for an option and was totally surprising to me because I
didn't go look for a default assignment in meson.build, so I completely
missed it.

Why don't we do the following:

	1) define that module_dir is either absolute or relative to
	   ${libdir}

	2) keep the default in the option declaration

	3) change the module_dir variable to always be made up of the
	   libdir and module_dir options joined

For 3), the Meson documentation specifies that if any of the arguments
to join_paths() is an absolute path, all arguments before it are
dropped, so it automatically deals with the case where users specify an
absolute path.

Something like the below squashed into your patch.

Thierry

--- >8 ---
diff --git a/meson.build b/meson.build
index 3e3f808b2d7a..33e2f6d88b1a 100644
--- a/meson.build
+++ b/meson.build
@@ -267,10 +267,7 @@ if log_dir == ''
     log_dir = join_paths(get_option('prefix'), get_option('localstatedir'), 'log')
 endif
 
-module_dir = get_option('module_dir')
-if module_dir == ''
-    module_dir = join_paths(get_option('libdir'), 'xorg/modules')
-endif
+module_dir = join_paths(get_option('libdir'), get_option('module_dir'))
 
 if glamor_option == 'auto'
     build_glamor = build_xorg or build_xwayland
@@ -510,7 +507,7 @@ manpage_config.set('XKB_DFLT_LAYOUT', get_option('xkb_default_layout'))
 manpage_config.set('XKB_DFLT_VARIANT', get_option('xkb_default_variant'))
 manpage_config.set('XKB_DFLT_OPTIONS', get_option('xkb_default_options'))
 manpage_config.set('bundle_id_prefix', '...')
-manpage_config.set('modulepath', join_paths(get_option('prefix'), module_dir))
+manpage_config.set('modulepath', module_dir)
 # wtf doesn't this work
 # manpage_config.set('suid_wrapper_dir', join_paths(get_option('prefix'), libexecdir))
 manpage_config.set('suid_wrapper_dir', join_paths(get_option('prefix'), 'libexec'))
diff --git a/meson_options.txt b/meson_options.txt
index 4cf8349ba9e5..1c8775c3fdaa 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -19,7 +19,8 @@ option('builder_addr', type: 'string', description: 'Builder address', value: 'x
 option('builder_string', type: 'string', description: 'Additional builder string')
 
 option('log_dir', type: 'string')
-option('module_dir', type: 'string', description: 'X.Org modules directory')
+option('module_dir', type: 'string', value: 'xorg/modules',
+       description: 'X.Org modules directory (absolute or relative to the directory specified by the libdir option)')
 option('default_font_path', type: 'string')
 
 option('glx', type: 'boolean', value: true)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.x.org/archives/xorg-devel/attachments/20180403/631098c5/attachment.sig>