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

> 
> 
> On 14/05/2025 05:36, Patrick Steinhardt wrote:
>> On Tue, May 13, 2025 at 08:17:24PM +0100, Ramsay Jones wrote:
>>> 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.
> 
> OK, if I need to re-roll, I will fix this up. (but see below)
> 
>>>  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.
> 
> Hmm, but how do you get the description! :)
> 
> 
> I applied the following patch on top:
> 
>   diff --git a/meson.build b/meson.build
>   index 28276e5305..bd14bc15a1 100644
>   --- a/meson.build
>   +++ b/meson.build
>   @@ -771,14 +771,14 @@ 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"'
>   +  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"'
>   +  libgit_c_args += '-DETC_GITCONFIG="' + get_option('sysconfdir') / 'gitconfig"'
>    endif
>    
>    editor_opt = get_option('default_editor')
>   diff --git a/meson_options.txt b/meson_options.txt
>   index ff877e67ce..7a4b896f7e 100644
>   --- a/meson_options.txt
>   +++ b/meson_options.txt
>   @@ -4,9 +4,9 @@ option('default_pager', type: 'string', value: 'less',
>    option('default_editor', type: 'string', value: 'vi',
>      description: 'Fall-back editor.')
>    option('gitconfig', type: 'string', # default 'etc/gitconfig'
>   -  description: 'Path to the global git configuration file.')
>   +  description: 'Path to the global git configuration file. (default: etc/gitconfig)')
>    option('gitattributes', type: 'string', # default 'etc/gitattributes'
>   -  description: 'Path to the global git attributes file.')
>   +  description: 'Path to the global git attributes file. (default: etc/gitattributes)')
>    option('pager_environment', type: 'string', value: 'LESS=FRX LV=-c',
>      description: 'Environment used when spawning the pager')
>    option('perl_cpan_fallback', type: 'boolean', value: true,
> ----
> 
> So, the addition of the '(default: <value>)' to the description field is
> intended to mimic the setup help text for the built-in meson options:
> 
>   $ meson help setup
>   usage: meson setup [-h] [--prefix PREFIX] [--bindir BINDIR] [--datadir DATADIR]
>   
>   ...
>   
>   options:
>     -h, --help                            show this help message and exit
>     --prefix PREFIX                       Installation prefix (default:
>                                           /usr/local).
>     --bindir BINDIR                       Executable directory (default: bin).
>     --datadir DATADIR                     Data file directory (default: share).
>     --includedir INCLUDEDIR               Header file directory (default:
>                                           include).
>     --infodir INFODIR                     Info page directory (default:
>                                           share/info).
>     --libdir LIBDIR                       Library directory (default:
>                                           lib/x86_64-linux-gnu).
>     --licensedir LICENSEDIR               Licenses directory (default: ).
>     --libexecdir LIBEXECDIR               Library executable directory (default:
>                                           libexec).
>     --localedir LOCALEDIR                 Locale data directory (default:
>                                           share/locale).
>     --localstatedir LOCALSTATEDIR         Localstate data directory (default:
>                                           var).
>     --mandir MANDIR                       Manual page directory (default:
>                                           share/man).
>     --sbindir SBINDIR                     System executable directory (default:
>                                           sbin).
>     --sharedstatedir SHAREDSTATEDIR       Architecture-independent data directory
>                                           (default: com).
>     --sysconfdir SYSCONFDIR               Sysconf data directory (default: etc).
>   
>   ...
>   
>   $ 
> 
> Indeed, there appears to be no way to display the project specific options
> to the user *before* configuring a build directory. 
> 
>   $ pwd
>   /home/ramsay/git
>   $ meson introspect --buildoptions
>   Current directory is not a meson build directory.
>   Please specify a valid build dir or change the working directory to it.
>   $ 
> 
> Note that I don't recommend 'meson introspect --buildoptions' as a means
> for the user to inspect the available options, but it does allow me to
> check that the description field looks correct:
> 
>   $ meson introspect --buildoptions build | jq | grep gitconfig
>       "name": "gitconfig",
>       "description": "Path to the global git configuration file. (default: etc/gitconfig)"
>   $ meson introspect --buildoptions build | jq | grep gitattributes
>       "name": "gitattributes",
>       "description": "Path to the global git attributes file. (default: etc/gitattributes)"
>   $ 
> 
> The only way I have found to display the project options to the user (after
> configuring the project) is using 'meson configure', thus:  
>   
>   $ meson configure build
>   
>   ...
> 
> Note that this display shows the *current* value, not the default value, and
> (once again) in this case there really isn't a default value! ;) (iff prefix
> is exactly '/usr', then the 'default' is eg. '/etc/gitconfig').
> 
> Of course, the current value would be the default value unless you have
> set the value on the command-line (of which you would presumably be aware).