On 19/03/2025 13:36, Patrick Steinhardt wrote:
> On Sat, Mar 15, 2025 at 02:46:59AM +0000, Ramsay Jones wrote:
>>
>> Some preprocessor -Defines have defaults sets in the source code when
>> they have not been provided to the C compiler. In this case, there is
>> no need to pass them on the command-line, unless the build requires a
>> non-standard value.
>>
>> The build variables for DEFAULT_EDITOR, DEFAULT_HELP_FORMAT along with
>> DEFAULT_PAGER have appropriate defaults ('vi', 'man' and 'less') set in
>> the code. Add the preprocessor -Defines to the 'libgit_c_args' only if
>> the values set with the corresponding 'options' are different to these
>> standard values.
>
> Hm. Does this really change anything though? The behaviour before and
> after this patch are exactly the same as far as I understand, and by
> explicitly handling the defaults we basically have to hard-code more
> assumptions. So in the current form I don't see that this patch adds
> much.
Hmm, I suppose it kinda depends on how you view it! :)
I have been looking at how the three build systems (well, mainly make
and meson) differ in various ways, in order to try and determine if
there are any significant differences and (most important) bugs.
Reducing the differences allows me to more clearly identify the bugs. ;)
In this case, the original author(s) had clearly intended that the
default values were included in the code, with the ability to override
the values from the command-line/environment only for 'non-standard' or
platform-specific uses. For example, on Windows and MINGW the
DEFAULT_HELP_FORMAT is html, which is specified in the 'config.mak.uname'
file. (I don't see this override in the meson build).
Also, the documentation (see git-var.adoc) has a statement of the compiled
in choice for the default pager and default editor, *only* if they are *not*
the standard values. I have a note, from several months ago, that says the
meson build does not pass the 'git-default-pager' and 'git-default-editor'
attributes to asciidoc. The make build only sets those attributes if the
DEFAULT_PAGER and DEFAULT_EDITOR variables are *defined* (and they should
*not* be defined to the 'standard' values or the docs would not read well).
(see git-var.adoc lines 49-51 and 67-69, Documentation/Makefile lines 239-242
and 244-247).
Also, I believe (ie I need to check) that the make build relies on the main
Makefile export-ing DEFAULT_EDITOR and DEFAULT_PAGER (see line #2923) to
make that work.
I haven't looked into all of that yet (it's one of the part #2 un-written
patches), and I don't yet know how those values get 'transmitted' to the
docs meson.build file.
Also, although I have found some meson documentation (https://mesonbuild.com/),
I haven't had the time to actually study it, so I have just used search to
try and get some answers (it seems my search-fu leaves a lot to be desired).
I was half expecting you to say something like 'hey, you don't do it like
that ... do this instead ...'. ;)
[I tried searching for a option 'is_defined()' or 'is_default()' method or
similar, but didn't find anything].
So, yes I think this patch has value. :)
ATB,
Ramsay Jones
