Re: [PATCH v2 3/5] meson: correct path to system config/attribute files

From: Patrick Steinhardt <ps@pks.im>
Date: May 14, 2025, 04:36 UTC
Message-ID: <aCQdznsNfaIeSPDI@pks.im>
In-Reply-To: <20250513191739.1513460-4-ramsay@ramsayjones.plus.com>

On Tue, May 13, 2025 at 08:17:24PM +0100, Ramsay Jones wrote:

Show 21 quoted lines

> diff --git a/meson.build b/meson.build
> index 48f31157a0..7f811030bd 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -769,6 +767,20 @@ libgit_c_args = [
>    '-DSHELL_PATH="' + fs.as_posix(target_shell.full_path()) + '"',
>  ]
>  
> +system_attributes = get_option('gitattributes')
> +if system_attributes != ''
> +  libgit_c_args += '-DETC_GITATTRIBUTES="' + system_attributes + '"'
> +else
> +  libgit_c_args += '-DETC_GITATTRIBUTES="' + get_option('sysconfdir') + '/gitattributes"'
> +endif
> +
> +system_config = get_option('gitconfig')
> +if system_config != ''
> +  libgit_c_args += '-DETC_GITCONFIG="' + system_config + '"'
> +else
> +  libgit_c_args += '-DETC_GITCONFIG="' + get_option('sysconfdir') + '/gitconfig"'
> +endif

Nit: I still think that we should use `get_option('sysconfdir') /
'gitattributes'`, with the slash instead of a plus, mostly because it is
more idiomatic and reads better. But that alone doesn't warrant a
reroll.

Show 17 quoted lines

>  editor_opt = get_option('default_editor')
>  if editor_opt != '' and editor_opt != 'vi'
>    libgit_c_args += '-DDEFAULT_EDITOR="' + editor_opt + '"'
> diff --git a/meson_options.txt b/meson_options.txt
> index 8547c0eb47..ff877e67ce 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -3,9 +3,9 @@ option('default_pager', type: 'string', value: 'less',
>    description: 'Fall-back pager.')
>  option('default_editor', type: 'string', value: 'vi',
>    description: 'Fall-back editor.')
> -option('gitconfig', type: 'string', value: '/etc/gitconfig',
> +option('gitconfig', type: 'string', # default 'etc/gitconfig'
>    description: 'Path to the global git configuration file.')
> -option('gitattributes', type: 'string', value: '/etc/gitattributes',
> +option('gitattributes', type: 'string', # default 'etc/gitattributes'
>    description: 'Path to the global git attributes file.')

I'd prefer if we documented the default value in the description. Otherwise it is impossible to discover it without having a look at the sources.

Other than that the range-diff looks as expected, thanks!

Patrick