[PATCH 0/5] miscellaneous build mods (part 2)

[PATCH 0/5] miscellaneous build mods (part 2)

[Ramsay Jones]

This series (part 2) continues the miscellaneous changes to the make,
meson and autoconf build systems. I am sending this part a little
earlier than I expected, so there will now be a part 3!

The reason for the early posting is to try and avoid an regression in
the autoconf build system (see patch #5). Hopefully, we still have time
in this cycle to get patch #5 included.

This series is based on commit 6f84262c44 ("The eleventh batch", 2025-05-05)

I did a test merge to 'next'@629a3ecd64 without issue, but 'seen'@71cfd25022
had a small conflict with commit 1a2929c851 ("meson: allow customize perl
installation path", 2025-04-24) on the 'dd/meson-perl-custom-path' branch.
(I note that a new version of that patch was posted to the list today).

The fixup looks like:

    diff --cc perl/Git/SVN/Memoize/meson.build
    index 4c589b30c3,8c2e80d2d2..d6209dc3bf
    --- a/perl/Git/SVN/Memoize/meson.build
    +++ b/perl/Git/SVN/Memoize/meson.build
    @@@ -3,6 -3,6 +3,6 @@@ test_dependencies += custom_target
        output: 'YAML.pm',
        command: generate_perl_command,
        install: true,
    -   install_dir: perllibdir / 'Git/SVN',
     -  install_dir: get_option('datadir') / 'perl5/Git/SVN/Memoize',
    ++  install_dir: perllibdir / 'Git/SVN/Memoize',
        depends: [git_version_file],
      )

ATB,
Ramsay Jones


Ramsay Jones (5):
  meson.build: quote the GITWEBDIR build configuration
  meson: correct install location of YAML.pm
  meson: correct path to system config/attribute files
  meson.build: correct setting of GIT_EXEC_PATH
  configure.ac: upgrade to a compilation check for sysinfo

 configure.ac                     | 25 ++++++++++++++++++++++---
 meson.build                      | 28 +++++++++++++++++++++++-----
 meson_options.txt                |  4 ++--
 perl/Git/SVN/Memoize/meson.build |  2 +-
 4 files changed, 48 insertions(+), 11 deletions(-)

-- 
2.49.0

[PATCH 1/5] meson.build: quote the GITWEBDIR build configuration

[Ramsay Jones]

The build configuration options with (non-empty) values, for example
filesystem paths potentially containing spaces, have been set using
the '.set_quoted()' method. However, the GITWEBDIR value has been
set using the '.set()' method instead. In order to correctly quote
the GITWEBDIR value, replace the '.set()' method with '.set_quoted()'.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 270ce933d0..48f31157a0 100644
--- a/meson.build
+++ b/meson.build
@@ -739,7 +739,7 @@ build_options_config.set('GIT_TEST_OPTS', '')
 build_options_config.set('GIT_TEST_PERL_FATAL_WARNINGS', '')
 build_options_config.set_quoted('GIT_TEST_UTF8_LOCALE', get_option('test_utf8_locale'))
 build_options_config.set_quoted('LOCALEDIR', fs.as_posix(get_option('prefix') / get_option('localedir')))
-build_options_config.set('GITWEBDIR', fs.as_posix(get_option('prefix') / get_option('datadir') / 'gitweb'))
+build_options_config.set_quoted('GITWEBDIR', fs.as_posix(get_option('prefix') / get_option('datadir') / 'gitweb'))
 
 if get_option('sane_tool_path').length() != 0
   sane_tool_path = (host_machine.system() == 'windows' ? ';' : ':').join(get_option('sane_tool_path'))
-- 
2.49.0

[PATCH 2/5] meson: correct install location of YAML.pm

[Ramsay Jones]

When executing an 'meson install' the YAML.pm file is incorrectly
placed in the <prefix>/share/perl5/Git/SVN directory. The YAML.pm
file should be placed in a 'Memoize' subdirectory instead. In order
to correct the location, update the 'install_dir' of the relevant
target in the 'perl/Git/SVN/Memoize/meson.build' file.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 perl/Git/SVN/Memoize/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Git/SVN/Memoize/meson.build b/perl/Git/SVN/Memoize/meson.build
index 233ec670d7..8c2e80d2d2 100644
--- a/perl/Git/SVN/Memoize/meson.build
+++ b/perl/Git/SVN/Memoize/meson.build
@@ -3,6 +3,6 @@ test_dependencies += custom_target(
   output: 'YAML.pm',
   command: generate_perl_command,
   install: true,
-  install_dir: get_option('datadir') / 'perl5/Git/SVN',
+  install_dir: get_option('datadir') / 'perl5/Git/SVN/Memoize',
   depends: [git_version_file],
 )
-- 
2.49.0

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

[Ramsay Jones]

The path to the system-wide config and attributes files are not being
set correctly in the meson build. Unless explicitly overridden on the
command line during setup, the 'gitconfig' and 'gitattributes' options
are defaulting to absolute paths in the '/etc' system directory. This
is only appropriate if the <prefix> is set specifically to '/usr'.

The directory in which these files are placed is generally referred to
as the 'system configuration directory' or 'sysconfdir' for short. When
the prefix is '/usr' then the sysconfdir is usually set to '/etc', but
any other value for prefix results in the relative directory value 'etc'
instead. (eg if prefix is '/usr/local', then the 'etc' relative value
results in a system configuration directory of '/usr/local/etc'). When
setting the 'sysconfdir' builtin option value, the meson system uses
exactly this algorithm, so we can use get_option('sysconfdir') directly
when setting the (non-overridden) build variables.

In order to allow for overriding from the command line, remove the
default values specified for the 'gitconfig' and 'gitattributes' options
in the 'meson_options.txt' file. This allows the user to specify any
pathname for those options, while being able to test for the unset
(empty) value. An absolute pathname will be used unchanged and a relative
pathname will be appended to '<prefix>/'. These values are then used to
set the 'ETC_GITCONFIG' and 'ETC_GITATTRIBUTES' build variables which are,
in turn, passed to the compiler as '-D' arguments.

When the 'gitconfig' or 'gitattributes' options are not used, then use
the built-in 'sysconfdir' and set the ETC_GITCONFIG build variable to
the string "<sysconfdir>/gitconfig". Similarly, set ETC_ATTRIBUTES to
"<sysconfdir>/gitattributes".

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 meson.build       | 14 ++++++++++++--
 meson_options.txt |  4 ++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index 48f31157a0..106cb17612 100644
--- a/meson.build
+++ b/meson.build
@@ -757,8 +757,6 @@ endif
 libgit_c_args = [
   '-DBINDIR="' + get_option('bindir') + '"',
   '-DDEFAULT_GIT_TEMPLATE_DIR="' + get_option('datadir') / 'git-core/templates' + '"',
-  '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
-  '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
   '-DFALLBACK_RUNTIME_PREFIX="' + get_option('prefix') + '"',
   '-DGIT_HOST_CPU="' + host_machine.cpu_family() + '"',
   '-DGIT_HTML_PATH="' + get_option('datadir') / 'doc/git-doc"',
@@ -769,6 +767,18 @@ 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
 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..4d78d4c7ac 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',
   description: 'Path to the global git configuration file.')
-option('gitattributes', type: 'string', value: '/etc/gitattributes',
+option('gitattributes', type: 'string',
   description: 'Path to the global git attributes file.')
 option('pager_environment', type: 'string', value: 'LESS=FRX LV=-c',
   description: 'Environment used when spawning the pager')
-- 
2.49.0

[PATCH 4/5] meson.build: correct setting of GIT_EXEC_PATH

[Ramsay Jones]

For the non-'runtime prefix' case, the meson build sets the GIT_EXEC_PATH
build variable to an absolute path equivalent to <prefix>/libexec/git-core.
In comparison, the default make build sets it to a relative path equivalent
to 'libexec/git-core'. Indeed, the make build requires the use of some
means outside of the Makefile (eg. config.mak[.*] or the command-line)
to set GIT_EXEC_PATH to anything other than 'libexec/git-core'.

For example, the make invocation:

  $ make gitexecdir=/some/other/bin all install

will build git with GIT_EXEC_PATH set to '/some/other/bin' and install
the 'library' executables to that location. However, without setting the
'gitexecdir' make variable, irrespective of the 'runtime prefix' setting,
the GIT_EXEC_PATH is always set to 'libexec/git-core'.

The meson built-in 'libexecdir' option can be used to provide a similar
configurability. The default value for the option is 'libexec'. Attempting
to set the option to '' on the command-line, will reset it to the '.'
string, presumably to ensure a relative path value.

This commit allows the meson build, similar to the above, to configure the
project like:

  $ meson setup --buildtype=debugoptimized -Dprefix=$HOME -Dpcre2=disabled \
      -Dlibexecdir=/some/other/bin build

so that the GIT_EXEC_PATH is set to '/some/other/bin'. Absent the
-Dlibexecdir argument, the GIT_EXEC_PATH is set to 'libexec/git-core'.

In order to correct the value of GIT_EXEC_PATH, default the value to the
static string value 'libexec/git-core', and only override if the value
of the 'libexecdir' option has a value different to 'libexec' or '.'.
Also, like the Makefile, add a check for an absolute path when the
runtime prefix option is true (and if so, error out).

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 meson.build | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 106cb17612..0101a8179e 100644
--- a/meson.build
+++ b/meson.build
@@ -1590,10 +1590,19 @@ else
   error('Unsupported CSPRNG backend: ' + csprng_backend)
 endif
 
+git_exec_path = 'libexec/git-core'
+libexec = get_option('libexecdir')
+if libexec != 'libexec' and libexec != '.'
+  git_exec_path = libexec
+endif
+
 if get_option('runtime_prefix')
   libgit_c_args += '-DRUNTIME_PREFIX'
   build_options_config.set('RUNTIME_PREFIX', 'true')
-  git_exec_path = get_option('libexecdir') / 'git-core'
+
+  if git_exec_path.startswith('/')
+    error('runtime_prefix requires a relative libexecdir not:', libexec)
+  endif
 
   if compiler.has_header('mach-o/dyld.h')
     libgit_c_args += '-DHAVE_NS_GET_EXECUTABLE_PATH'
@@ -1630,7 +1639,6 @@ if get_option('runtime_prefix')
   endif
 else
   build_options_config.set('RUNTIME_PREFIX', 'false')
-  git_exec_path = get_option('prefix') / get_option('libexecdir') / 'git-core'
 endif
 libgit_c_args += '-DGIT_EXEC_PATH="' + git_exec_path + '"'
 
-- 
2.49.0

[PATCH 5/5] configure.ac: upgrade to a compilation check for sysinfo

[Ramsay Jones]

Commit f5e3c6c57d ("meson: do a full usage-based compile check for
sysinfo", 2025-04-25) updated the 'sysinfo()' check, as part of the
meson build, due to the failure of the check on Solaris. Prior to
that commit, the meson build only checked the availability of the
'<sys/sysinfo.h>' header file. On Solaris, both the header and the
'sysinfo()' function exist, but are completely unrelated to the same
function on Linux (and cygwin).

Commit 50dec7c566 ("config.mak.uname: add sysinfo() configuration for
cygwin", 2025-04-17) added a similar 'sysinfo()' check to the autoconf
build. This check looked for the 'sysinfo()' function itself, rather
than just the header, but it will fail (incorrectly set HAVE_SYSINFO)
for the same reason.

In order to correctly identify the 'sysinfo()' function we require as
part of 'git-gc' (used in the 'total_ram() function), we also upgrade
to a compilation check, in a similar way to the meson commit. Note that
since commit c9a51775a3 ("builtin/gc.c: correct RAM calculation when
using sysinfo", 2025-04-17) both the 'totalram' and 'mem_unit' fields
of the 'struct sysinfo' are used, so the new check includes both of
those fields in the compile check.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 configure.ac | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index d7e0503f1e..f6caab919a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1069,9 +1069,28 @@ GIT_CONF_SUBST([CHARSET_LIB])
 
 #
 # Define HAVE_SYSINFO=YesPlease if sysinfo is available.
-GIT_CHECK_FUNC(sysinfo,
-	[HAVE_SYSINFO=YesPlease],
-	[HAVE_SYSINFO=])
+#
+AC_DEFUN([HAVE_SYSINFO_SRC], [
+AC_LANG_PROGRAM([[
+#include <stdint.h>
+#include <sys/sysinfo.h>
+]], [[
+struct sysinfo si;
+uint64_t t = 0;
+if (!sysinfo(&si)) {
+	t = si.totalram;
+	if (si.mem_unit > 1)
+		t *= (uint64_t)si.mem_unit;
+}
+return t;
+]])])
+
+AC_MSG_CHECKING([for sysinfo])
+AC_COMPILE_IFELSE([HAVE_SYSINFO_SRC],
+	[AC_MSG_RESULT([yes])
+	HAVE_SYSINFO=YesPlease],
+	[AC_MSG_RESULT([no])
+	HAVE_SYSINFO=])
 GIT_CONF_SUBST([HAVE_SYSINFO])
 
 #
-- 
2.49.0

Re: [PATCH 0/5] miscellaneous build mods (part 2)

[Ramsay Jones]

Hmm, I seem to have misunderstood the '--chain-reply-to' documentation
on the 'git send-email' man page! :( Sorry about that.

[I had intended a shallow threading].

On 08/05/2025 17:44, Ramsay Jones wrote:
> This series (part 2) continues the miscellaneous changes to the make,
> meson and autoconf build systems. I am sending this part a little
> earlier than I expected, so there will now be a part 3!
> 
> The reason for the early posting is to try and avoid an regression in
> the autoconf build system (see patch #5). Hopefully, we still have time
> in this cycle to get patch #5 included.

Note that I cannot test the new autoconf check in patch #5 (I don't have
access to a Solaris system). I _think_ it will correctly unset HAVE_SYSINFO
on Solaris, but I cannot confirm that. (I can only test on Linux and cygwin).

> 
> This series is based on commit 6f84262c44 ("The eleventh batch", 2025-05-05)
> 
> I did a test merge to 'next'@629a3ecd64 without issue, but 'seen'@71cfd25022
> had a small conflict with commit 1a2929c851 ("meson: allow customize perl
> installation path", 2025-04-24) on the 'dd/meson-perl-custom-path' branch.
> (I note that a new version of that patch was posted to the list today).

Note that it is patch #2 "meson: correct install location of YAML.pm" that
conflicts with the 'seen' branch.

> 
> The fixup looks like:
> 
>     diff --cc perl/Git/SVN/Memoize/meson.build
>     index 4c589b30c3,8c2e80d2d2..d6209dc3bf
>     --- a/perl/Git/SVN/Memoize/meson.build
>     +++ b/perl/Git/SVN/Memoize/meson.build
>     @@@ -3,6 -3,6 +3,6 @@@ test_dependencies += custom_target
>         output: 'YAML.pm',
>         command: generate_perl_command,
>         install: true,
>     -   install_dir: perllibdir / 'Git/SVN',
>      -  install_dir: get_option('datadir') / 'perl5/Git/SVN/Memoize',
>     ++  install_dir: perllibdir / 'Git/SVN/Memoize',
>         depends: [git_version_file],
>       )
> 
> ATB,
> Ramsay Jones
> 
> 
> Ramsay Jones (5):
>   meson.build: quote the GITWEBDIR build configuration
>   meson: correct install location of YAML.pm
>   meson: correct path to system config/attribute files
>   meson.build: correct setting of GIT_EXEC_PATH
>   configure.ac: upgrade to a compilation check for sysinfo
> 
>  configure.ac                     | 25 ++++++++++++++++++++++---
>  meson.build                      | 28 +++++++++++++++++++++++-----
>  meson_options.txt                |  4 ++--
>  perl/Git/SVN/Memoize/meson.build |  2 +-
>  4 files changed, 48 insertions(+), 11 deletions(-)
> 

Re: [PATCH 5/5] configure.ac: upgrade to a compilation check for sysinfo

[Eli Schwartz]

[-- Attachment #1.1: Type: text/plain, Size: 3852 bytes --]

On 5/8/25 12:44 PM, Ramsay Jones wrote:
> Commit f5e3c6c57d ("meson: do a full usage-based compile check for
> sysinfo", 2025-04-25) updated the 'sysinfo()' check, as part of the
> meson build, due to the failure of the check on Solaris. Prior to
> that commit, the meson build only checked the availability of the
> '<sys/sysinfo.h>' header file. On Solaris, both the header and the
> 'sysinfo()' function exist, but are completely unrelated to the same
> function on Linux (and cygwin).
> 
> Commit 50dec7c566 ("config.mak.uname: add sysinfo() configuration for
> cygwin", 2025-04-17) added a similar 'sysinfo()' check to the autoconf
> build. This check looked for the 'sysinfo()' function itself, rather
> than just the header, but it will fail (incorrectly set HAVE_SYSINFO)
> for the same reason.
> 
> In order to correctly identify the 'sysinfo()' function we require as
> part of 'git-gc' (used in the 'total_ram() function), we also upgrade
> to a compilation check, in a similar way to the meson commit. Note that
> since commit c9a51775a3 ("builtin/gc.c: correct RAM calculation when
> using sysinfo", 2025-04-17) both the 'totalram' and 'mem_unit' fields
> of the 'struct sysinfo' are used, so the new check includes both of
> those fields in the compile check.

and

> Note that I cannot test the new autoconf check in patch #5 (I don't have
> access to a Solaris system). I _think_ it will correctly unset HAVE_SYSINFO
> on Solaris, but I cannot confirm that. (I can only test on Linux and cygwin).


Well, I can confirm this results in the detection being correctly
changed on Solaris 11.3 and stop reporting sysinfo as available during
./configure, so this has my ACK on technical grounds. That being said,
in the original meson thread, there was this review:


On 4/22/25 3:31 AM, Patrick Steinhardt wrote:
> On Mon, Apr 21, 2025 at 01:51:46PM -0400, Eli Schwartz wrote:
>> It is deprecated and removed in SUS v3 / POSIX 2001, so various systems
>> may not include it. Solaris, in particular, carefully refrains from
>> defining it except inside of a maze of `#ifdef` to make sure you have
>> kept your nose clean and only used it in code that *targets* SUS v2 or
>> earlier.
>>
>> config.mak.uname defines this automatically, though only for QNX.
> 
> Ah, interesting. I mostly went by our autoconf infrastructure when
> converting the checks, which didn't have a check for `getpagesize()`
> either. We might want to teach autoconf to check for this function while
> at it.
> 
> In all honesty though, I rather hope that we're soon in a state where we
> can just drop autoconf altogether in favor of Meson. The only two
> blockers I'm aware of are wiring up git-gui and gitk. The former project
> has already been adapted upstream, the latter is still in review. But
> once those have landed, we should be ready to mark Meson as stable and
> then we can start deprecating autoconf unless there are good reasons not
> to do so.


So you are indeed teaching autoconf to check for this function, but
should we also ask whether it's worth continued maintenance of autoconf?
It was/is not clear to me who the stakeholders are for the autoconf support.

On the one hand, it exists so maybe it should be fixed when we know it
has issues.

On the other hand, it sounds like this patch (and commit 50dec7c566
"config.mak.uname: add sysinfo() configuration for cygwin") only modify
autoconf out of a sense of duty, rather than finding autoconf useful.
What does it say about the autoconf support if the people finding bugs
in it don't even use it, but only discovered the bug while working on a
different build system they do use and depend on (config.mak.uname, or
meson.build, both count here). Who *is* using it? Apparently not Solaris
users?


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

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

[Junio C Hamano]

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

> -  '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
> -  '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
> ...
> +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

Just out of curiosity (because this cannot be a regression, since
the original removed one used the same constructs).

I am guessing from the presence of double quote around the value
that these strings are not directly used to invoke the compiler
without involving any shell (in other words, you wouldn't want these
quotes if you are shoving these strings in argv[] yourself to feed
execv()).

How does the above, and get_option() in particular, cope with a
pathname that has letters with special meanings to the shell when
they appear inside double-quote pair (like backquote or backslash or
even a dollar sign)?  On the Makefile side we give VAR_SQ for a raw
variable VAR and use the latter to write something like

        -DETC_GITATTRIBUTES='"$(ETC_GITATTRIBUTES_SQ)"'

to make sure we won't be broken by them.  Is Meson giving us an
equivalent to us for free by simply using get_option() here?

Thanks.

Re: [PATCH 4/5] meson.build: correct setting of GIT_EXEC_PATH

[Junio C Hamano]

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

>  if get_option('runtime_prefix')
>    libgit_c_args += '-DRUNTIME_PREFIX'
>    build_options_config.set('RUNTIME_PREFIX', 'true')
> -  git_exec_path = get_option('libexecdir') / 'git-core'
> +
> +  if git_exec_path.startswith('/')
> +    error('runtime_prefix requires a relative libexecdir not:', libexec)
> +  endif

Certainly nice to see these settings taught to be more careful.

Thanks.

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

[Eli Schwartz]

[-- Attachment #1.1: Type: text/plain, Size: 4170 bytes --]

On 5/8/25 5:48 PM, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
>> -  '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
>> -  '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
>> ...
>> +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
> 
> Just out of curiosity (because this cannot be a regression, since
> the original removed one used the same constructs).
> 
> I am guessing from the presence of double quote around the value
> that these strings are not directly used to invoke the compiler
> without involving any shell (in other words, you wouldn't want these
> quotes if you are shoving these strings in argv[] yourself to feed
> execv()).
> 
> How does the above, and get_option() in particular, cope with a
> pathname that has letters with special meanings to the shell when
> they appear inside double-quote pair (like backquote or backslash or
> even a dollar sign)?  On the Makefile side we give VAR_SQ for a raw
> variable VAR and use the latter to write something like
> 
>         -DETC_GITATTRIBUTES='"$(ETC_GITATTRIBUTES_SQ)"'
> 
> to make sure we won't be broken by them.  Is Meson giving us an
> equivalent to us for free by simply using get_option() here?
> 
> Thanks.


Meson uses strong implicit typing. The object type of '"' is a string
with value of double-quote-char, and the type of libgit_c_args is
"array". It's not a result of get_option() here, except inasmuch as
get_option() returned a string type.

Meson guarantees that arrays of strings e.g.

libgit_c_args = [
    '-Dfoo=string containing the $ (mighty dollar)',
    '-Dbar=string containing the ` soft-deprecated',
    '-Doopsies=string containing the \' (you read that right)',
]


are passed to the shell in a manner suitable for reinterpretation as an
argv array, notwithstanding other concerns (e.g. I believe there's some
mingw hack regarding doubling backslash escapes so they don't get
eaten). And of course this is conditional on the idea that it is
possible to reliably pass arguments on the Windows command line, which
doesn't have the notion of an array.

So, given that meson takes care of this, the actual value of the -D
define will be

string containing the $ (mighty dollar)


etc.

e.g. here is the ninja output:


[1/3] ccache cc -Ifoo.p -I. -I.. -fdiagnostics-color=always
-D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -O0 -g '-Dfoo=string
containing the $ (mighty dollar)' '-Dbar=string containing the `
soft-deprecated' '-Doopsies=string containing the '"'"' (you read that
right)' -MD -MQ foo.p/foo.c.o -MF foo.p/foo.c.o.d -o foo.p/foo.c.o -c
../foo.c
<command-line>: warning: missing terminating ' character

Notice that the array contained a single quote using a meson string type
backslash escape, but the generated command line chose to shell-escape
it as ' ... '"'"' .... '



In this case, the Makefile does:

-DETC_GITATTRIBUTES='"$(ETC_GITATTRIBUTES_SQ)"'


and if I understand correctly the _SQ is to handle single quotes in the
directory name:


-DETC_GITATTRIBUTES='"/etc/git'\''s attribute file"'


Or in meson,

libgit_c_args += [
    '-DETC_GITATTRIBUTES="/etc/git\'s attribute file"',
]


compiles as:

[1/3] ccache cc -Ifoo.p -I. -I.. -fdiagnostics-color=always
-D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -O0 -g '-Dfoo=string
containing the $ (mighty dollar)' '-Dbar=string containing the `
soft-deprecated' '-Doopsies=string containing the '"'"' (you read that
right)' '-DETC_GITATTRIBUTES="/etc/git'"'"'s attribute file"' -MD -MQ
foo.p/foo.c.o -MF foo.p/foo.c.o.d -o foo.p/foo.c.o -c ../foo.c


Meson has refrained from backslashes again:

ccache cc '-DETC_GITATTRIBUTES="/etc/git'"'"'s attribute file"'

Double quotes are part of the define value, single quote gets de-escaped
via the sequence:

'"'"'

instead of the sequence

'\''


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH 5/5] configure.ac: upgrade to a compilation check for sysinfo

[Ramsay Jones]

On 08/05/2025 22:07, Eli Schwartz wrote:
> On 5/8/25 12:44 PM, Ramsay Jones wrote:
[snip]
>> In order to correctly identify the 'sysinfo()' function we require as
>> part of 'git-gc' (used in the 'total_ram() function), we also upgrade
>> to a compilation check, in a similar way to the meson commit. Note that
>> since commit c9a51775a3 ("builtin/gc.c: correct RAM calculation when
>> using sysinfo", 2025-04-17) both the 'totalram' and 'mem_unit' fields
>> of the 'struct sysinfo' are used, so the new check includes both of
>> those fields in the compile check.
> 
> and
> 
>> Note that I cannot test the new autoconf check in patch #5 (I don't have
>> access to a Solaris system). I _think_ it will correctly unset HAVE_SYSINFO
>> on Solaris, but I cannot confirm that. (I can only test on Linux and cygwin).
> 
> 
> Well, I can confirm this results in the detection being correctly
> changed on Solaris 11.3 and stop reporting sysinfo as available during
> ./configure, so this has my ACK on technical grounds.

Thank you very much for testing this patch, much appreciated!

[snip]

> 
> So you are indeed teaching autoconf to check for this function, but
> should we also ask whether it's worth continued maintenance of autoconf?
> It was/is not clear to me who the stakeholders are for the autoconf support.

Hmm, someone posted a list of people using autoconf somewhat recently
to the mailing-list ... I don't have it to hand, but cygwin was one
of the projects using it.

> On the one hand, it exists so maybe it should be fixed when we know it
> has issues.

Yes, exactly.

> On the other hand, it sounds like this patch (and commit 50dec7c566
> "config.mak.uname: add sysinfo() configuration for cygwin") only modify
> autoconf out of a sense of duty, rather than finding autoconf useful.

Hmm, I am not convinced (yet) that meson is all that useful either. ;)
 
> What does it say about the autoconf support if the people finding bugs
> in it don't even use it, but only discovered the bug while working on a
> different build system they do use and depend on (config.mak.uname, or
> meson.build, both count here).

I am trying very hard not to express a view on this debate. :)

[well, except that I find CMake to be absolutely awful!]

Thanks!

ATB,
Ramsay Jones

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

[Ramsay Jones]

On 08/05/2025 22:48, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
>> -  '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
>> -  '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
>> ...
>> +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
> 
> Just out of curiosity (because this cannot be a regression, since
> the original removed one used the same constructs).
> 
> I am guessing from the presence of double quote around the value
> that these strings are not directly used to invoke the compiler
> without involving any shell (in other words, you wouldn't want these
> quotes if you are shoving these strings in argv[] yourself to feed
> execv()).
> 
> How does the above, and get_option() in particular, cope with a
> pathname that has letters with special meanings to the shell when
> they appear inside double-quote pair (like backquote or backslash or
> even a dollar sign)?  On the Makefile side we give VAR_SQ for a raw
> variable VAR and use the latter to write something like
> 
>         -DETC_GITATTRIBUTES='"$(ETC_GITATTRIBUTES_SQ)"'
> 
> to make sure we won't be broken by them.  Is Meson giving us an
> equivalent to us for free by simply using get_option() here?

Err ... no, meson is somewhat broken here, for example see [0].
To be fair, it could simply be that I am too dumb to understand
what meson is doing (and I haven't really studied the documentation
or the code).

Hopefully, someone who knows meson will be able to answer your question.

[0] https://lore.kernel.org/git/a5795bfa-cc02-4c9a-b7d2-4924a94cd0db@ramsayjones.plus.com/

Thanks.

ATB,
Ramsay Jones

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

[Junio C Hamano]

Eli Schwartz <eschwartz@gentoo.org> writes:

> Meson guarantees that arrays of strings e.g.
>
> libgit_c_args = [
>     '-Dfoo=string containing the $ (mighty dollar)',
>     '-Dbar=string containing the ` soft-deprecated',
>     '-Doopsies=string containing the \' (you read that right)',
> ]
>
>
> are passed to the shell in a manner suitable for reinterpretation as an
> argv array,...

Very nice.  Just I wanted to hear from a system that we are trying
to adopt and promote ;-)

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

[Patrick Steinhardt]

On Thu, May 08, 2025 at 05:44:37PM +0100, Ramsay Jones wrote:
> diff --git a/meson.build b/meson.build
> index 48f31157a0..106cb17612 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -757,8 +757,6 @@ endif
>  libgit_c_args = [
>    '-DBINDIR="' + get_option('bindir') + '"',
>    '-DDEFAULT_GIT_TEMPLATE_DIR="' + get_option('datadir') / 'git-core/templates' + '"',
> -  '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
> -  '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
>    '-DFALLBACK_RUNTIME_PREFIX="' + get_option('prefix') + '"',
>    '-DGIT_HOST_CPU="' + host_machine.cpu_family() + '"',
>    '-DGIT_HTML_PATH="' + get_option('datadir') / 'doc/git-doc"',
> @@ -769,6 +767,18 @@ 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"'

Instead of `get_option('sysconfdir') + '/gitconfig'` you can say
`get_option('sysconfdir') / 'gitconfig'`. It's a bit pointless in this
case and not really needed, but '/' has some special magic for handling
absolute and relative paths.

> +endif
>  editor_opt = get_option('default_editor')
>  if editor_opt != '' and editor_opt != 'vi'
>    libgit_c_args += '-DDEFAULT_EDITOR="' + editor_opt + '"'

Nit: let's maybe add an empty newline after each of these blocks to make
it a bit easier to see where handling for each specific option stops.

> diff --git a/meson_options.txt b/meson_options.txt
> index 8547c0eb47..4d78d4c7ac 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',
>    description: 'Path to the global git configuration file.')
> -option('gitattributes', type: 'string', value: '/etc/gitattributes',
> +option('gitattributes', type: 'string',
>    description: 'Path to the global git attributes file.')
>  option('pager_environment', type: 'string', value: 'LESS=FRX LV=-c',
>    description: 'Environment used when spawning the pager')

Makes sense. Should we maybe document the default values here now that
they aren't immediately obvious anymore?

Patrick

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

[Ramsay Jones]

On 09/05/2025 09:51, Patrick Steinhardt wrote:
> On Thu, May 08, 2025 at 05:44:37PM +0100, Ramsay Jones wrote:
>> diff --git a/meson.build b/meson.build
>> index 48f31157a0..106cb17612 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -757,8 +757,6 @@ endif
>>  libgit_c_args = [
>>    '-DBINDIR="' + get_option('bindir') + '"',
>>    '-DDEFAULT_GIT_TEMPLATE_DIR="' + get_option('datadir') / 'git-core/templates' + '"',
>> -  '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
>> -  '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
>>    '-DFALLBACK_RUNTIME_PREFIX="' + get_option('prefix') + '"',
>>    '-DGIT_HOST_CPU="' + host_machine.cpu_family() + '"',
>>    '-DGIT_HTML_PATH="' + get_option('datadir') / 'doc/git-doc"',
>> @@ -769,6 +767,18 @@ 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"'
> 
> Instead of `get_option('sysconfdir') + '/gitconfig'` you can say
> `get_option('sysconfdir') / 'gitconfig'`. It's a bit pointless in this
> case and not really needed, but '/' has some special magic for handling
> absolute and relative paths.

OK, TIL. I just looked this up ([0]) and I am not sure such 'magic' is
always a good thing. ;)

[0] https://mesonbuild.com/Syntax.html#string-path-building

>> +endif
>>  editor_opt = get_option('default_editor')
>>  if editor_opt != '' and editor_opt != 'vi'
>>    libgit_c_args += '-DDEFAULT_EDITOR="' + editor_opt + '"'
> 
> Nit: let's maybe add an empty newline after each of these blocks to make
> it a bit easier to see where handling for each specific option stops.

OK, will do.

>> diff --git a/meson_options.txt b/meson_options.txt
>> index 8547c0eb47..4d78d4c7ac 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',
>>    description: 'Path to the global git configuration file.')
>> -option('gitattributes', type: 'string', value: '/etc/gitattributes',
>> +option('gitattributes', type: 'string',
>>    description: 'Path to the global git attributes file.')
>>  option('pager_environment', type: 'string', value: 'LESS=FRX LV=-c',
>>    description: 'Environment used when spawning the pager')
> 
> Makes sense. Should we maybe document the default values here now that
> they aren't immediately obvious anymore?

good idea.

Thanks!

ATB,
Ramsay Jones

[PATCH v2 0/5] miscellaneous build mods (part 2)

[Ramsay Jones]

Changes in v2:

Patch #3 is the only one changed (as a result of Patrick's review [0]):

 - add some blank lines to make the option handling blocks
   easier to see.
 - add a comment to 'gitconfig' and 'gitattributes' options
   to indicate the default values.

Note: The indicated defaults for the 'gitconfig' and 'gitattributes'
are only valid when the 'prefix' option is defaulted (or not /usr).
Indicating the 'correct' value when -Dprefix=/usr in the comment
would consume too much space. Is this acceptable, or is it too
confusing/misleading?

Also, thanks to Eli for testing patch #5 on Solaris and confirming
that it fixes the regression [1].

Note that I did a test merge to master@38af977b81 without issue and
to next@889b7c5bd8 and seen@d8088176ab. The conflict which showed
up in v1 against the 'seen' branch now appears in 'next' and has
the same resolution as v1.

A range-diff against v1 is given below.

[0] https://lore.kernel.org/git/aB3CDOljn9zJsVwt@pks.im/
[1] https://lore.kernel.org/git/9baad29d-a5bf-443d-98a1-36d7020e5835@gentoo.org/

v1 cover letter follows:

This series (part 2) continues the miscellaneous changes to the make,
meson and autoconf build systems. I am sending this part a little
earlier than I expected, so there will now be a part 3!

The reason for the early posting is to try and avoid an regression in
the autoconf build system (see patch #5). Hopefully, we still have time
in this cycle to get patch #5 included.

This series is based on commit 6f84262c44 ("The eleventh batch", 2025-05-05)

I did a test merge to 'next'@629a3ecd64 without issue, but 'seen'@71cfd25022
had a small conflict with commit 1a2929c851 ("meson: allow customize perl
installation path", 2025-04-24) on the 'dd/meson-perl-custom-path' branch.
(I note that a new version of that patch was posted to the list today).

The fixup looks like:

    diff --cc perl/Git/SVN/Memoize/meson.build
    index 4c589b30c3,8c2e80d2d2..d6209dc3bf
    --- a/perl/Git/SVN/Memoize/meson.build
    +++ b/perl/Git/SVN/Memoize/meson.build
    @@@ -3,6 -3,6 +3,6 @@@ test_dependencies += custom_target
        output: 'YAML.pm',
        command: generate_perl_command,
        install: true,
    -   install_dir: perllibdir / 'Git/SVN',
     -  install_dir: get_option('datadir') / 'perl5/Git/SVN/Memoize',
    ++  install_dir: perllibdir / 'Git/SVN/Memoize',
        depends: [git_version_file],
      )

ATB,
Ramsay Jones


Ramsay Jones (5):
  meson.build: quote the GITWEBDIR build configuration
  meson: correct install location of YAML.pm
  meson: correct path to system config/attribute files
  meson.build: correct setting of GIT_EXEC_PATH
  configure.ac: upgrade to a compilation check for sysinfo

 configure.ac                     | 25 ++++++++++++++++++++++---
 meson.build                      | 30 +++++++++++++++++++++++++-----
 meson_options.txt                |  4 ++--
 perl/Git/SVN/Memoize/meson.build |  2 +-
 4 files changed, 50 insertions(+), 11 deletions(-)

range-diff against v1:

1:  d5847ef791 = 1:  1f3d34eafd meson.build: quote the GITWEBDIR build configuration
2:  49e2ec6fd2 = 2:  1b7751914a meson: correct install location of YAML.pm
3:  fece809f11 ! 3:  a385bbed83 meson: correct path to system config/attribute files
    @@ meson.build: libgit_c_args = [
     +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
    ++
      editor_opt = get_option('default_editor')
      if editor_opt != '' and editor_opt != 'vi'
        libgit_c_args += '-DDEFAULT_EDITOR="' + editor_opt + '"'
    @@ meson_options.txt: option('default_pager', type: 'string', value: 'less',
      option('default_editor', type: 'string', value: 'vi',
        description: 'Fall-back editor.')
     -option('gitconfig', type: 'string', value: '/etc/gitconfig',
    -+option('gitconfig', type: 'string',
    ++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',
    ++option('gitattributes', type: 'string', # default 'etc/gitattributes'
        description: 'Path to the global git attributes file.')
      option('pager_environment', type: 'string', value: 'LESS=FRX LV=-c',
        description: 'Environment used when spawning the pager')
4:  d49afaedf3 = 4:  0d00951475 meson.build: correct setting of GIT_EXEC_PATH
5:  69848e557f = 5:  150e4110d2 configure.ac: upgrade to a compilation check for sysinfo
-- 
2.49.0

[PATCH v2 1/5] meson.build: quote the GITWEBDIR build configuration

[Ramsay Jones]

The build configuration options with (non-empty) values, for example
filesystem paths potentially containing spaces, have been set using
the '.set_quoted()' method. However, the GITWEBDIR value has been
set using the '.set()' method instead. In order to correctly quote
the GITWEBDIR value, replace the '.set()' method with '.set_quoted()'.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 270ce933d0..48f31157a0 100644
--- a/meson.build
+++ b/meson.build
@@ -739,7 +739,7 @@ build_options_config.set('GIT_TEST_OPTS', '')
 build_options_config.set('GIT_TEST_PERL_FATAL_WARNINGS', '')
 build_options_config.set_quoted('GIT_TEST_UTF8_LOCALE', get_option('test_utf8_locale'))
 build_options_config.set_quoted('LOCALEDIR', fs.as_posix(get_option('prefix') / get_option('localedir')))
-build_options_config.set('GITWEBDIR', fs.as_posix(get_option('prefix') / get_option('datadir') / 'gitweb'))
+build_options_config.set_quoted('GITWEBDIR', fs.as_posix(get_option('prefix') / get_option('datadir') / 'gitweb'))
 
 if get_option('sane_tool_path').length() != 0
   sane_tool_path = (host_machine.system() == 'windows' ? ';' : ':').join(get_option('sane_tool_path'))
-- 
2.49.0

[PATCH v2 2/5] meson: correct install location of YAML.pm

[Ramsay Jones]

When executing an 'meson install' the YAML.pm file is incorrectly
placed in the <prefix>/share/perl5/Git/SVN directory. The YAML.pm
file should be placed in a 'Memoize' subdirectory instead. In order
to correct the location, update the 'install_dir' of the relevant
target in the 'perl/Git/SVN/Memoize/meson.build' file.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 perl/Git/SVN/Memoize/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Git/SVN/Memoize/meson.build b/perl/Git/SVN/Memoize/meson.build
index 233ec670d7..8c2e80d2d2 100644
--- a/perl/Git/SVN/Memoize/meson.build
+++ b/perl/Git/SVN/Memoize/meson.build
@@ -3,6 +3,6 @@ test_dependencies += custom_target(
   output: 'YAML.pm',
   command: generate_perl_command,
   install: true,
-  install_dir: get_option('datadir') / 'perl5/Git/SVN',
+  install_dir: get_option('datadir') / 'perl5/Git/SVN/Memoize',
   depends: [git_version_file],
 )
-- 
2.49.0

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

[Ramsay Jones]

The path to the system-wide config and attributes files are not being
set correctly in the meson build. Unless explicitly overridden on the
command line during setup, the 'gitconfig' and 'gitattributes' options
are defaulting to absolute paths in the '/etc' system directory. This
is only appropriate if the <prefix> is set specifically to '/usr'.

The directory in which these files are placed is generally referred to
as the 'system configuration directory' or 'sysconfdir' for short. When
the prefix is '/usr' then the sysconfdir is usually set to '/etc', but
any other value for prefix results in the relative directory value 'etc'
instead. (eg if prefix is '/usr/local', then the 'etc' relative value
results in a system configuration directory of '/usr/local/etc'). When
setting the 'sysconfdir' builtin option value, the meson system uses
exactly this algorithm, so we can use get_option('sysconfdir') directly
when setting the (non-overridden) build variables.

In order to allow for overriding from the command line, remove the
default values specified for the 'gitconfig' and 'gitattributes' options
in the 'meson_options.txt' file. This allows the user to specify any
pathname for those options, while being able to test for the unset
(empty) value. An absolute pathname will be used unchanged and a relative
pathname will be appended to '<prefix>/'. These values are then used to
set the 'ETC_GITCONFIG' and 'ETC_GITATTRIBUTES' build variables which are,
in turn, passed to the compiler as '-D' arguments.

When the 'gitconfig' or 'gitattributes' options are not used, then use
the built-in 'sysconfdir' and set the ETC_GITCONFIG build variable to
the string "<sysconfdir>/gitconfig". Similarly, set ETC_ATTRIBUTES to
"<sysconfdir>/gitattributes".

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 meson.build       | 16 ++++++++++++++--
 meson_options.txt |  4 ++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index 48f31157a0..7f811030bd 100644
--- a/meson.build
+++ b/meson.build
@@ -757,8 +757,6 @@ endif
 libgit_c_args = [
   '-DBINDIR="' + get_option('bindir') + '"',
   '-DDEFAULT_GIT_TEMPLATE_DIR="' + get_option('datadir') / 'git-core/templates' + '"',
-  '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
-  '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
   '-DFALLBACK_RUNTIME_PREFIX="' + get_option('prefix') + '"',
   '-DGIT_HOST_CPU="' + host_machine.cpu_family() + '"',
   '-DGIT_HTML_PATH="' + get_option('datadir') / 'doc/git-doc"',
@@ -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
+
 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.')
 option('pager_environment', type: 'string', value: 'LESS=FRX LV=-c',
   description: 'Environment used when spawning the pager')
-- 
2.49.0

[PATCH v2 4/5] meson.build: correct setting of GIT_EXEC_PATH

[Ramsay Jones]

For the non-'runtime prefix' case, the meson build sets the GIT_EXEC_PATH
build variable to an absolute path equivalent to <prefix>/libexec/git-core.
In comparison, the default make build sets it to a relative path equivalent
to 'libexec/git-core'. Indeed, the make build requires the use of some
means outside of the Makefile (eg. config.mak[.*] or the command-line)
to set GIT_EXEC_PATH to anything other than 'libexec/git-core'.

For example, the make invocation:

  $ make gitexecdir=/some/other/bin all install

will build git with GIT_EXEC_PATH set to '/some/other/bin' and install
the 'library' executables to that location. However, without setting the
'gitexecdir' make variable, irrespective of the 'runtime prefix' setting,
the GIT_EXEC_PATH is always set to 'libexec/git-core'.

The meson built-in 'libexecdir' option can be used to provide a similar
configurability. The default value for the option is 'libexec'. Attempting
to set the option to '' on the command-line, will reset it to the '.'
string, presumably to ensure a relative path value.

This commit allows the meson build, similar to the above, to configure the
project like:

  $ meson setup --buildtype=debugoptimized -Dprefix=$HOME -Dpcre2=disabled \
      -Dlibexecdir=/some/other/bin build

so that the GIT_EXEC_PATH is set to '/some/other/bin'. Absent the
-Dlibexecdir argument, the GIT_EXEC_PATH is set to 'libexec/git-core'.

In order to correct the value of GIT_EXEC_PATH, default the value to the
static string value 'libexec/git-core', and only override if the value
of the 'libexecdir' option has a value different to 'libexec' or '.'.
Also, like the Makefile, add a check for an absolute path when the
runtime prefix option is true (and if so, error out).

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 meson.build | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 7f811030bd..28276e5305 100644
--- a/meson.build
+++ b/meson.build
@@ -1592,10 +1592,19 @@ else
   error('Unsupported CSPRNG backend: ' + csprng_backend)
 endif
 
+git_exec_path = 'libexec/git-core'
+libexec = get_option('libexecdir')
+if libexec != 'libexec' and libexec != '.'
+  git_exec_path = libexec
+endif
+
 if get_option('runtime_prefix')
   libgit_c_args += '-DRUNTIME_PREFIX'
   build_options_config.set('RUNTIME_PREFIX', 'true')
-  git_exec_path = get_option('libexecdir') / 'git-core'
+
+  if git_exec_path.startswith('/')
+    error('runtime_prefix requires a relative libexecdir not:', libexec)
+  endif
 
   if compiler.has_header('mach-o/dyld.h')
     libgit_c_args += '-DHAVE_NS_GET_EXECUTABLE_PATH'
@@ -1632,7 +1641,6 @@ if get_option('runtime_prefix')
   endif
 else
   build_options_config.set('RUNTIME_PREFIX', 'false')
-  git_exec_path = get_option('prefix') / get_option('libexecdir') / 'git-core'
 endif
 libgit_c_args += '-DGIT_EXEC_PATH="' + git_exec_path + '"'
 
-- 
2.49.0

[PATCH v2 5/5] configure.ac: upgrade to a compilation check for sysinfo

[Ramsay Jones]

Commit f5e3c6c57d ("meson: do a full usage-based compile check for
sysinfo", 2025-04-25) updated the 'sysinfo()' check, as part of the
meson build, due to the failure of the check on Solaris. Prior to
that commit, the meson build only checked the availability of the
'<sys/sysinfo.h>' header file. On Solaris, both the header and the
'sysinfo()' function exist, but are completely unrelated to the same
function on Linux (and cygwin).

Commit 50dec7c566 ("config.mak.uname: add sysinfo() configuration for
cygwin", 2025-04-17) added a similar 'sysinfo()' check to the autoconf
build. This check looked for the 'sysinfo()' function itself, rather
that just the header, but it will fail (incorrectly set HAVE_SYSINFO)
for the same reason.

In order to correctly identify the 'sysinfo()' function we require as
part of 'git-gc' (used in the 'total_ram() function), we also upgrade
to a compilation check, in a similar way to the meson commit. Note that
since commit c9a51775a3 ("builtin/gc.c: correct RAM calculation when
using sysinfo", 2025-04-17) both the 'totalram' and 'mem_unit' fields
of the 'struct sysinfo' are used, so the new check includes both of
those fields in the compile check.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 configure.ac | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index d7e0503f1e..f6caab919a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1069,9 +1069,28 @@ GIT_CONF_SUBST([CHARSET_LIB])
 
 #
 # Define HAVE_SYSINFO=YesPlease if sysinfo is available.
-GIT_CHECK_FUNC(sysinfo,
-	[HAVE_SYSINFO=YesPlease],
-	[HAVE_SYSINFO=])
+#
+AC_DEFUN([HAVE_SYSINFO_SRC], [
+AC_LANG_PROGRAM([[
+#include <stdint.h>
+#include <sys/sysinfo.h>
+]], [[
+struct sysinfo si;
+uint64_t t = 0;
+if (!sysinfo(&si)) {
+	t = si.totalram;
+	if (si.mem_unit > 1)
+		t *= (uint64_t)si.mem_unit;
+}
+return t;
+]])])
+
+AC_MSG_CHECKING([for sysinfo])
+AC_COMPILE_IFELSE([HAVE_SYSINFO_SRC],
+	[AC_MSG_RESULT([yes])
+	HAVE_SYSINFO=YesPlease],
+	[AC_MSG_RESULT([no])
+	HAVE_SYSINFO=])
 GIT_CONF_SUBST([HAVE_SYSINFO])
 
 #
-- 
2.49.0

Re: [PATCH v2 0/5] miscellaneous build mods (part 2)

[Junio C Hamano]

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

> Changes in v2:
>
> Patch #3 is the only one changed (as a result of Patrick's review [0]):
>
>  - add some blank lines to make the option handling blocks
>    easier to see.
>  - add a comment to 'gitconfig' and 'gitattributes' options
>    to indicate the default values.
>
> Note: The indicated defaults for the 'gitconfig' and 'gitattributes'
> are only valid when the 'prefix' option is defaulted (or not /usr).
> Indicating the 'correct' value when -Dprefix=/usr in the comment
> would consume too much space. Is this acceptable, or is it too
> confusing/misleading?
>
> Also, thanks to Eli for testing patch #5 on Solaris and confirming
> that it fixes the regression [1].

Yeah, thanks, all.

> A range-diff against v1 is given below.
> ...
> 3:  fece809f11 ! 3:  a385bbed83 meson: correct path to system config/attribute files
>     @@ meson.build: libgit_c_args = [
> ...
>         description: 'Environment used when spawning the pager')
> 4:  d49afaedf3 = 4:  0d00951475 meson.build: correct setting of GIT_EXEC_PATH
> 5:  69848e557f = 5:  150e4110d2 configure.ac: upgrade to a compilation check for sysinfo

Hmph, For #5 I am seeing this difference:

    @@ Commit message
         Commit 50dec7c566 ("config.mak.uname: add sysinfo() configuration for
         cygwin", 2025-04-17) added a similar 'sysinfo()' check to the autoconf
         build. This check looked for the 'sysinfo()' function itself, rather
    -    than just the header, but it will fail (incorrectly set HAVE_SYSINFO)
    +    that just the header, but it will fail (incorrectly set HAVE_SYSINFO)
         for the same reason.
     
         In order to correctly identify the 'sysinfo()' function we require as

The original comes from what was posted in the first iteration, and
somehow the change is not showing in your range-diff, which is a bit
disturbing.

I think for now I'll just amend the log message of #5 back to what
was in the previous round locally.

Thanks.

Re: [PATCH v2 0/5] miscellaneous build mods (part 2)

[Ramsay Jones]

On 13/05/2025 21:13, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
>> Changes in v2:
>>
>> Patch #3 is the only one changed (as a result of Patrick's review [0]):
>>
>>  - add some blank lines to make the option handling blocks
>>    easier to see.
>>  - add a comment to 'gitconfig' and 'gitattributes' options
>>    to indicate the default values.
>>
>> Note: The indicated defaults for the 'gitconfig' and 'gitattributes'
>> are only valid when the 'prefix' option is defaulted (or not /usr).
>> Indicating the 'correct' value when -Dprefix=/usr in the comment
>> would consume too much space. Is this acceptable, or is it too
>> confusing/misleading?
>>
>> Also, thanks to Eli for testing patch #5 on Solaris and confirming
>> that it fixes the regression [1].
> 
> Yeah, thanks, all.
> 
>> A range-diff against v1 is given below.
>> ...
>> 3:  fece809f11 ! 3:  a385bbed83 meson: correct path to system config/attribute files
>>     @@ meson.build: libgit_c_args = [
>> ...
>>         description: 'Environment used when spawning the pager')
>> 4:  d49afaedf3 = 4:  0d00951475 meson.build: correct setting of GIT_EXEC_PATH
>> 5:  69848e557f = 5:  150e4110d2 configure.ac: upgrade to a compilation check for sysinfo
> 
> Hmph, For #5 I am seeing this difference:
> 
>     @@ Commit message
>          Commit 50dec7c566 ("config.mak.uname: add sysinfo() configuration for
>          cygwin", 2025-04-17) added a similar 'sysinfo()' check to the autoconf
>          build. This check looked for the 'sysinfo()' function itself, rather
>     -    than just the header, but it will fail (incorrectly set HAVE_SYSINFO)
>     +    that just the header, but it will fail (incorrectly set HAVE_SYSINFO)
>          for the same reason.
>      
>          In order to correctly identify the 'sysinfo()' function we require as
> 
> The original comes from what was posted in the first iteration, and
> somehow the change is not showing in your range-diff, which is a bit
> disturbing.

Oops! yeah, I noticed the typo late in the last round and changed that
patch text directly. :)

I could have sworn that I made the same change to the commit message
as well, but ...

Sorry about that!

> I think for now I'll just amend the log message of #5 back to what
> was in the previous round locally.

Yes please! Thanks.

ATB,
Ramsay Jones

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

[Patrick Steinhardt]

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.

>  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

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

[Ramsay Jones]

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
  
  ...
  
    Project options    Current Value        Possible Values      Description      
    -----------------  -------------        ---------------      -----------      
  benchmark_large_repo                                           Large repository 
                                                                 to copy for the  
                                                                 performance      
                                                                 tests. Should be 
                                                                 at least the size
                                                                 of the Linux     
                                                                 repository.      

  ...

    gitattributes                                                Path to the      
                                                                 global git       
                                                                 attributes file. 
                                                                 (default: etc/git
                                                                 attributes)      
    gitconfig                                                    Path to the      
                                                                 global git       
                                                                 configuration    
                                                                 file. (default:  
                                                                 etc/gitconfig)   
  
  ...
  
  $ 

[Yes, I use 80 column terminals! :) ]

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).

[Well, it can be argued that eg. 'etc/gitconfig' is the default value for
the project option 'gitconfig', but that is just one input to determine
the actual path compiled into git (which _may_depend on the value of another
option ie. 'prefix')].

Having said that, the --sysconfdir default is shown as 'etc' (see above) and
that has (*is*) the same problem (ie it is '/etc' iff prefix is '/usr').

Also, looking through that list, other options which are similarly specified
to gitconfig/gitattributes don't have their 'default' noted in the description.
Why make an exception for these options?

Is this what you wanted to see? If so, then I can submit a v3 with the
above changes. Just let me know.

Thanks.

ATB,
Ramsay Jones


 

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

[Eli Schwartz]

[-- Attachment #1.1: Type: text/plain, Size: 9385 bytes --]

On 5/15/25 12:42 PM, Ramsay Jones wrote:
> 
> 
> 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).


Well, so-so.


eschwartz@acleverhostname ~/git/git $ meson configure .

meson.build:208:0: ERROR: None of values [] are supported by the C
compiler. Possible values for option "C_std" are ['none', 'c89', 'c99',
'c11', 'c17', 'c18', 'c2x', 'c23', 'gnu89', 'gnu99', 'gnu11', 'gnu17',
'gnu18', 'gnu2x', 'gnu23']


But also,

$ sed -i '/c_std=/d' meson.build && PAGER=cat COLUMNS=80 meson configure .

WARNING: The source directory instead of the build directory was specified.
WARNING: Only the default values for the project are printed.

Core properties:
  Source dir /home/eschwartz/git/git

Main project options:

  Core options       Default Value        Possible Values
Description
  --------------     -------------        ---------------
-----------
  auto_features      auto                 [enabled, disabled,  Override
value of
                                           auto]               all
'auto'
                                                               features


[...]


I'm not completely certain why this evaluates as an empty node:

```(meson.version().version_compare('>=1.3.0') ? 'gnu99,c11' : 'gnu99')
```


but it (meson configure) is part of the AST interpreter, not the runtime
one, which I know less about...


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

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

[Ramsay Jones]

On 15/05/2025 18:51, Eli Schwartz wrote:
> On 5/15/25 12:42 PM, Ramsay Jones wrote:
[snip]
> Well, so-so.
> 
> 
> eschwartz@acleverhostname ~/git/git $ meson configure .
> 
> meson.build:208:0: ERROR: None of values [] are supported by the C
> compiler. Possible values for option "C_std" are ['none', 'c89', 'c99',
> 'c11', 'c17', 'c18', 'c2x', 'c23', 'gnu89', 'gnu99', 'gnu11', 'gnu17',
> 'gnu18', 'gnu2x', 'gnu23']
> 
> 
> But also,
> 
> $ sed -i '/c_std=/d' meson.build && PAGER=cat COLUMNS=80 meson configure .
> 
> WARNING: The source directory instead of the build directory was specified.
> WARNING: Only the default values for the project are printed.
> 
> Core properties:
>   Source dir /home/eschwartz/git/git
> 
> Main project options:
> 
>   Core options       Default Value        Possible Values
> Description
>   --------------     -------------        ---------------
> -----------
>   auto_features      auto                 [enabled, disabled,  Override
> value of
>                                            auto]               all
> 'auto'
>                                                                features
> 
> 
> [...]
> 
> 
> I'm not completely certain why this evaluates as an empty node:
> 
> ```(meson.version().version_compare('>=1.3.0') ? 'gnu99,c11' : 'gnu99')
> ```
> 
> 
> but it (meson configure) is part of the AST interpreter, not the runtime
> one, which I know less about...


Ah, interesting. When it came time to test this patch, I spent about
half an hour trying to get 'meson configure' to tell me about the
project options, because I had a distant memory of having done it
before at one point ... :)

Hmm, maybe it was before commit 13cb20fc46 ("meson: fix compilation with
Visual Studio", 2025-01-22). If I (effectively) revert that commit, then
'meson configure' works fine for me (you don't even need the '.').

TIL. Thanks!

ATB,
Ramsay Jones

Re: [PATCH 5/5] configure.ac: upgrade to a compilation check for sysinfo

[Eli Schwartz]

[-- Attachment #1.1: Type: text/plain, Size: 1940 bytes --]

On 5/8/25 7:01 PM, Ramsay Jones wrote:
>> So you are indeed teaching autoconf to check for this function, but
>> should we also ask whether it's worth continued maintenance of autoconf?
>> It was/is not clear to me who the stakeholders are for the autoconf support.
> 
> Hmm, someone posted a list of people using autoconf somewhat recently
> to the mailing-list ... I don't have it to hand, but cygwin was one
> of the projects using it.


Thanks for the correction. This solves my major concern...


>> On the other hand, it sounds like this patch (and commit 50dec7c566
>> "config.mak.uname: add sysinfo() configuration for cygwin") only modify
>> autoconf out of a sense of duty, rather than finding autoconf useful.
> 
> Hmm, I am not convinced (yet) that meson is all that useful either. ;)
>  
>> What does it say about the autoconf support if the people finding bugs
>> in it don't even use it, but only discovered the bug while working on a
>> different build system they do use and depend on (config.mak.uname, or
>> meson.build, both count here).
> 
> I am trying very hard not to express a view on this debate. :)
> 
> [well, except that I find CMake to be absolutely awful!]



... because to my way of thinking, the ultimate proof of usefulness is
that people, well, *use* it. Gentoo uses Meson, ipso facto it is useful. :)

From my experience with the ./configure script it wasn't clear to *me*
that anyone did use it (and I did a non-exhaustive check of other
distributors). If cygwin uses it, it is useful and the question becomes
*whether* to (properly) deprecate it after evaluating the existing
options and balancing use cases with maintenance burden, rather than
"just let it rot until we finish discussing its likely deletion".


I'm still not volunteering to do the actual fixing of ./configure :D but
testing is easy if someone else wrote the fix...


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

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

[Patrick Steinhardt]

On Thu, May 15, 2025 at 05:42:00PM +0100, Ramsay Jones wrote:
> 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
> >>  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! :)

[snip]

> The only way I have found to display the project options to the user (after
> configuring the project) is using 'meson configure', thus:  
>   

Yup, that's how I typically do it.

[snip]
> Also, looking through that list, other options which are similarly specified
> to gitconfig/gitattributes don't have their 'default' noted in the description.
> Why make an exception for these options?

It's not so much about one being exceptional, it's rather that I didn't
think about it for the other options. Ideally, we'd document the default
for all values where we don't have a hardcoded one.

> Is this what you wanted to see? If so, then I can submit a v3 with the
> above changes. Just let me know.

Yup, the diff you had looks exactly like what I expected. Thanks!

Patrick

[PATCH v3 0/5] miscellaneous build mods (part 2)

[Ramsay Jones]

Changes in v3:

Patch #3 changed as a result of Patrick's review [*]:

 - use the '/' magic string operator when setting the
   ETC_GIT{CONFIG,ATTRIBUTES} build options.
 - add the default values to the 'description' fields
   for the 'gitconfig' and 'gitattributes' option
   definitions

Also, Junio, the range-diff below shows the typo fixup, but
you should find that it doesn't show for you this time. ;)

[yes, I did fix it up locally, but then overwrote it from
my cygwin repo ... Ahem! ;) ]

Again, I did a test merge to master@1a8a4971cc without issue and 
to next@0be31eac6b and seen@d36a872499. The conflict which showed
up in v1 against the 'seen' branch now appears in 'next' and has
the same resolution as v1.

A range-diff against v2 is given below.

[*] https://lore.kernel.org/git/aCbQ9OX2vqF82au8@pks.im/


Changes in v2:

Patch #3 is the only one changed (as a result of Patrick's review [0]):

 - add some blank lines to make the option handling blocks
   easier to see.
 - add a comment to 'gitconfig' and 'gitattributes' options
   to indicate the default values.

Note: The indicated defaults for the 'gitconfig' and 'gitattributes'
are only valid when the 'prefix' option is defaulted (or not /usr).
Indicating the 'correct' value when -Dprefix=/usr in the comment
would consume too much space. Is this acceptable, or is it too
confusing/misleading?

Also, thanks to Eli for testing patch #5 on Solaris and confirming
that it fixes the regression [1].

Note that I did a test merge to master@38af977b81 without issue and
to next@889b7c5bd8 and seen@d8088176ab. The conflict which showed
up in v1 against the 'seen' branch now appears in 'next' and has
the same resolution as v1.

A range-diff against v1 is given below.

[0] https://lore.kernel.org/git/aB3CDOljn9zJsVwt@pks.im/
[1] https://lore.kernel.org/git/9baad29d-a5bf-443d-98a1-36d7020e5835@gentoo.org/

v1 cover letter follows:

This series (part 2) continues the miscellaneous changes to the make,
meson and autoconf build systems. I am sending this part a little
earlier than I expected, so there will now be a part 3!

The reason for the early posting is to try and avoid an regression in
the autoconf build system (see patch #5). Hopefully, we still have time
in this cycle to get patch #5 included.

This series is based on commit 6f84262c44 ("The eleventh batch", 2025-05-05)

I did a test merge to 'next'@629a3ecd64 without issue, but 'seen'@71cfd25022
had a small conflict with commit 1a2929c851 ("meson: allow customize perl
installation path", 2025-04-24) on the 'dd/meson-perl-custom-path' branch.
(I note that a new version of that patch was posted to the list today).

The fixup looks like:

    diff --cc perl/Git/SVN/Memoize/meson.build
    index 4c589b30c3,8c2e80d2d2..d6209dc3bf
    --- a/perl/Git/SVN/Memoize/meson.build
    +++ b/perl/Git/SVN/Memoize/meson.build
    @@@ -3,6 -3,6 +3,6 @@@ test_dependencies += custom_target
        output: 'YAML.pm',
        command: generate_perl_command,
        install: true,
    -   install_dir: perllibdir / 'Git/SVN',
     -  install_dir: get_option('datadir') / 'perl5/Git/SVN/Memoize',
    ++  install_dir: perllibdir / 'Git/SVN/Memoize',
        depends: [git_version_file],
      )

ATB,
Ramsay Jones

Ramsay Jones (5):
  meson.build: quote the GITWEBDIR build configuration
  meson: correct install location of YAML.pm
  meson: correct path to system config/attribute files
  meson.build: correct setting of GIT_EXEC_PATH
  configure.ac: upgrade to a compilation check for sysinfo

 configure.ac                     | 25 ++++++++++++++++++++++---
 meson.build                      | 30 +++++++++++++++++++++++++-----
 meson_options.txt                |  8 ++++----
 perl/Git/SVN/Memoize/meson.build |  2 +-
 4 files changed, 52 insertions(+), 13 deletions(-)

range-diff against v2:

1:  1f3d34eafd = 1:  ac97fdc76e meson.build: quote the GITWEBDIR build configuration
2:  1b7751914a = 2:  181738d371 meson: correct install location of YAML.pm
3:  a385bbed83 ! 3:  1f4c74973c meson: correct path to system config/attribute files
    @@ meson.build: libgit_c_args = [
     +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')
    @@ meson_options.txt: option('default_pager', type: 'string', value: 'less',
      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.')
    +-  description: 'Path to the global git configuration file.')
     -option('gitattributes', type: 'string', value: '/etc/gitattributes',
    +-  description: 'Path to the global git attributes file.')
    ++option('gitconfig', type: 'string', # default 'etc/gitconfig'
    ++  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,
4:  0d00951475 = 4:  97ff1613f5 meson.build: correct setting of GIT_EXEC_PATH
5:  150e4110d2 ! 5:  983488fe4e configure.ac: upgrade to a compilation check for sysinfo
    @@ Commit message
         Commit 50dec7c566 ("config.mak.uname: add sysinfo() configuration for
         cygwin", 2025-04-17) added a similar 'sysinfo()' check to the autoconf
         build. This check looked for the 'sysinfo()' function itself, rather
    -    that just the header, but it will fail (incorrectly set HAVE_SYSINFO)
    +    than just the header, but it will fail (incorrectly set HAVE_SYSINFO)
         for the same reason.
     
         In order to correctly identify the 'sysinfo()' function we require as
-- 
2.49.0

[PATCH v3 1/5] meson.build: quote the GITWEBDIR build configuration

[Ramsay Jones]

The build configuration options with (non-empty) values, for example
filesystem paths potentially containing spaces, have been set using
the '.set_quoted()' method. However, the GITWEBDIR value has been
set using the '.set()' method instead. In order to correctly quote
the GITWEBDIR value, replace the '.set()' method with '.set_quoted()'.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 270ce933d0..48f31157a0 100644
--- a/meson.build
+++ b/meson.build
@@ -739,7 +739,7 @@ build_options_config.set('GIT_TEST_OPTS', '')
 build_options_config.set('GIT_TEST_PERL_FATAL_WARNINGS', '')
 build_options_config.set_quoted('GIT_TEST_UTF8_LOCALE', get_option('test_utf8_locale'))
 build_options_config.set_quoted('LOCALEDIR', fs.as_posix(get_option('prefix') / get_option('localedir')))
-build_options_config.set('GITWEBDIR', fs.as_posix(get_option('prefix') / get_option('datadir') / 'gitweb'))
+build_options_config.set_quoted('GITWEBDIR', fs.as_posix(get_option('prefix') / get_option('datadir') / 'gitweb'))
 
 if get_option('sane_tool_path').length() != 0
   sane_tool_path = (host_machine.system() == 'windows' ? ';' : ':').join(get_option('sane_tool_path'))
-- 
2.49.0

[PATCH v3 2/5] meson: correct install location of YAML.pm

[Ramsay Jones]

When executing an 'meson install' the YAML.pm file is incorrectly
placed in the <prefix>/share/perl5/Git/SVN directory. The YAML.pm
file should be placed in a 'Memoize' subdirectory instead. In order
to correct the location, update the 'install_dir' of the relevant
target in the 'perl/Git/SVN/Memoize/meson.build' file.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 perl/Git/SVN/Memoize/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Git/SVN/Memoize/meson.build b/perl/Git/SVN/Memoize/meson.build
index 233ec670d7..8c2e80d2d2 100644
--- a/perl/Git/SVN/Memoize/meson.build
+++ b/perl/Git/SVN/Memoize/meson.build
@@ -3,6 +3,6 @@ test_dependencies += custom_target(
   output: 'YAML.pm',
   command: generate_perl_command,
   install: true,
-  install_dir: get_option('datadir') / 'perl5/Git/SVN',
+  install_dir: get_option('datadir') / 'perl5/Git/SVN/Memoize',
   depends: [git_version_file],
 )
-- 
2.49.0

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

[Ramsay Jones]

The path to the system-wide config and attributes files are not being
set correctly in the meson build. Unless explicitly overridden on the
command line during setup, the 'gitconfig' and 'gitattributes' options
are defaulting to absolute paths in the '/etc' system directory. This
is only appropriate if the <prefix> is set specifically to '/usr'.

The directory in which these files are placed is generally referred to
as the 'system configuration directory' or 'sysconfdir' for short. When
the prefix is '/usr' then the sysconfdir is usually set to '/etc', but
any other value for prefix results in the relative directory value 'etc'
instead. (eg if prefix is '/usr/local', then the 'etc' relative value
results in a system configuration directory of '/usr/local/etc'). When
setting the 'sysconfdir' builtin option value, the meson system uses
exactly this algorithm, so we can use get_option('sysconfdir') directly
when setting the (non-overridden) build variables.

In order to allow for overriding from the command line, remove the
default values specified for the 'gitconfig' and 'gitattributes' options
in the 'meson_options.txt' file. This allows the user to specify any
pathname for those options, while being able to test for the unset
(empty) value. An absolute pathname will be used unchanged and a relative
pathname will be appended to '<prefix>/'. These values are then used to
set the 'ETC_GITCONFIG' and 'ETC_GITATTRIBUTES' build variables which are,
in turn, passed to the compiler as '-D' arguments.

When the 'gitconfig' or 'gitattributes' options are not used, then use
the built-in 'sysconfdir' and set the ETC_GITCONFIG build variable to
the string "<sysconfdir>/gitconfig". Similarly, set ETC_ATTRIBUTES to
"<sysconfdir>/gitattributes".

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 meson.build       | 16 ++++++++++++++--
 meson_options.txt |  8 ++++----
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/meson.build b/meson.build
index 48f31157a0..8e8f228a37 100644
--- a/meson.build
+++ b/meson.build
@@ -757,8 +757,6 @@ endif
 libgit_c_args = [
   '-DBINDIR="' + get_option('bindir') + '"',
   '-DDEFAULT_GIT_TEMPLATE_DIR="' + get_option('datadir') / 'git-core/templates' + '"',
-  '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
-  '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
   '-DFALLBACK_RUNTIME_PREFIX="' + get_option('prefix') + '"',
   '-DGIT_HOST_CPU="' + host_machine.cpu_family() + '"',
   '-DGIT_HTML_PATH="' + get_option('datadir') / 'doc/git-doc"',
@@ -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
+
 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..7a4b896f7e 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -3,10 +3,10 @@ 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',
-  description: 'Path to the global git configuration file.')
-option('gitattributes', type: 'string', value: '/etc/gitattributes',
-  description: 'Path to the global git attributes file.')
+option('gitconfig', type: 'string', # default 'etc/gitconfig'
+  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. (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,
-- 
2.49.0

[PATCH v3 4/5] meson.build: correct setting of GIT_EXEC_PATH

[Ramsay Jones]

For the non-'runtime prefix' case, the meson build sets the GIT_EXEC_PATH
build variable to an absolute path equivalent to <prefix>/libexec/git-core.
In comparison, the default make build sets it to a relative path equivalent
to 'libexec/git-core'. Indeed, the make build requires the use of some
means outside of the Makefile (eg. config.mak[.*] or the command-line)
to set GIT_EXEC_PATH to anything other than 'libexec/git-core'.

For example, the make invocation:

  $ make gitexecdir=/some/other/bin all install

will build git with GIT_EXEC_PATH set to '/some/other/bin' and install
the 'library' executables to that location. However, without setting the
'gitexecdir' make variable, irrespective of the 'runtime prefix' setting,
the GIT_EXEC_PATH is always set to 'libexec/git-core'.

The meson built-in 'libexecdir' option can be used to provide a similar
configurability. The default value for the option is 'libexec'. Attempting
to set the option to '' on the command-line, will reset it to the '.'
string, presumably to ensure a relative path value.

This commit allows the meson build, similar to the above, to configure the
project like:

  $ meson setup --buildtype=debugoptimized -Dprefix=$HOME -Dpcre2=disabled \
      -Dlibexecdir=/some/other/bin build

so that the GIT_EXEC_PATH is set to '/some/other/bin'. Absent the
-Dlibexecdir argument, the GIT_EXEC_PATH is set to 'libexec/git-core'.

In order to correct the value of GIT_EXEC_PATH, default the value to the
static string value 'libexec/git-core', and only override if the value
of the 'libexecdir' option has a value different to 'libexec' or '.'.
Also, like the Makefile, add a check for an absolute path when the
runtime prefix option is true (and if so, error out).

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 meson.build | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 8e8f228a37..bd14bc15a1 100644
--- a/meson.build
+++ b/meson.build
@@ -1592,10 +1592,19 @@ else
   error('Unsupported CSPRNG backend: ' + csprng_backend)
 endif
 
+git_exec_path = 'libexec/git-core'
+libexec = get_option('libexecdir')
+if libexec != 'libexec' and libexec != '.'
+  git_exec_path = libexec
+endif
+
 if get_option('runtime_prefix')
   libgit_c_args += '-DRUNTIME_PREFIX'
   build_options_config.set('RUNTIME_PREFIX', 'true')
-  git_exec_path = get_option('libexecdir') / 'git-core'
+
+  if git_exec_path.startswith('/')
+    error('runtime_prefix requires a relative libexecdir not:', libexec)
+  endif
 
   if compiler.has_header('mach-o/dyld.h')
     libgit_c_args += '-DHAVE_NS_GET_EXECUTABLE_PATH'
@@ -1632,7 +1641,6 @@ if get_option('runtime_prefix')
   endif
 else
   build_options_config.set('RUNTIME_PREFIX', 'false')
-  git_exec_path = get_option('prefix') / get_option('libexecdir') / 'git-core'
 endif
 libgit_c_args += '-DGIT_EXEC_PATH="' + git_exec_path + '"'
 
-- 
2.49.0

[PATCH v3 5/5] configure.ac: upgrade to a compilation check for sysinfo

[Ramsay Jones]

Commit f5e3c6c57d ("meson: do a full usage-based compile check for
sysinfo", 2025-04-25) updated the 'sysinfo()' check, as part of the
meson build, due to the failure of the check on Solaris. Prior to
that commit, the meson build only checked the availability of the
'<sys/sysinfo.h>' header file. On Solaris, both the header and the
'sysinfo()' function exist, but are completely unrelated to the same
function on Linux (and cygwin).

Commit 50dec7c566 ("config.mak.uname: add sysinfo() configuration for
cygwin", 2025-04-17) added a similar 'sysinfo()' check to the autoconf
build. This check looked for the 'sysinfo()' function itself, rather
than just the header, but it will fail (incorrectly set HAVE_SYSINFO)
for the same reason.

In order to correctly identify the 'sysinfo()' function we require as
part of 'git-gc' (used in the 'total_ram() function), we also upgrade
to a compilation check, in a similar way to the meson commit. Note that
since commit c9a51775a3 ("builtin/gc.c: correct RAM calculation when
using sysinfo", 2025-04-17) both the 'totalram' and 'mem_unit' fields
of the 'struct sysinfo' are used, so the new check includes both of
those fields in the compile check.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 configure.ac | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index d7e0503f1e..f6caab919a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1069,9 +1069,28 @@ GIT_CONF_SUBST([CHARSET_LIB])
 
 #
 # Define HAVE_SYSINFO=YesPlease if sysinfo is available.
-GIT_CHECK_FUNC(sysinfo,
-	[HAVE_SYSINFO=YesPlease],
-	[HAVE_SYSINFO=])
+#
+AC_DEFUN([HAVE_SYSINFO_SRC], [
+AC_LANG_PROGRAM([[
+#include <stdint.h>
+#include <sys/sysinfo.h>
+]], [[
+struct sysinfo si;
+uint64_t t = 0;
+if (!sysinfo(&si)) {
+	t = si.totalram;
+	if (si.mem_unit > 1)
+		t *= (uint64_t)si.mem_unit;
+}
+return t;
+]])])
+
+AC_MSG_CHECKING([for sysinfo])
+AC_COMPILE_IFELSE([HAVE_SYSINFO_SRC],
+	[AC_MSG_RESULT([yes])
+	HAVE_SYSINFO=YesPlease],
+	[AC_MSG_RESULT([no])
+	HAVE_SYSINFO=])
 GIT_CONF_SUBST([HAVE_SYSINFO])
 
 #
-- 
2.49.0

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

[Patrick Steinhardt]

On Fri, May 16, 2025 at 07:48:41PM +0100, Ramsay Jones wrote:
> diff --git a/meson_options.txt b/meson_options.txt
> index 8547c0eb47..7a4b896f7e 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -3,10 +3,10 @@ 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',
> -  description: 'Path to the global git configuration file.')
> -option('gitattributes', type: 'string', value: '/etc/gitattributes',
> -  description: 'Path to the global git attributes file.')
> +option('gitconfig', type: 'string', # default 'etc/gitconfig'
> +  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. (default: etc/gitattributes)')

Sorry to be nitpicky, but the default information is duplicated. We now
have it once as comment and once as description. Other than that this
version looks great to me, thanks!

Patrick

[PATCH v4 0/5] miscellaneous build mods (part 2)

[Ramsay Jones]

Changes in v4:

Patch #3 changed as a result of Patricks review [+]:

 - remove comment showing the default option values for
   'gitattributes' and 'gitconfig', since this now appears
   in the description field (ie remove duplicate info).

Again, I did a test merge to master@cb96e1697a, next@a128411c76
and seen@df1b4f9cf9. The conflict which showed up in v1 against
the 'seen' branch now appears in the 'master' branch and has the
same resolution as v1.

Note: the test-suite passes for both 'make test' and 'meson test'
on Linux and 'meson test' on cygwin. (I have already had to wait
3.5 hours for 'meson test' to run on cygwin, so I will run the
'make test' later [it takes 6+ hours] and let you know if it fails.
Given that the only change from last time is the removal of a comment
in the 'meson_options.txt' file, the chances of 'make test' failing
seem to be rather remote - famous last words! ;) ).

A range-diff against v3 is given below.

[+] https://lore.kernel.org/git/aCrekcz6onTFgEWw@pks.im/

Changes in v3:

Patch #3 changed as a result of Patrick's review [*]:

 - use the '/' magic string operator when setting the
   ETC_GIT{CONFIG,ATTRIBUTES} build options.
 - add the default values to the 'description' fields
   for the 'gitconfig' and 'gitattributes' option
   definitions

Also, Junio, the range-diff below shows the typo fixup, but
you should find that it doesn't show for you this time. ;)

[yes, I did fix it up locally, but then overwrote it from
my cygwin repo ... Ahem! ;) ]

Again, I did a test merge to master@1a8a4971cc without issue and 
to next@0be31eac6b and seen@d36a872499. The conflict which showed
up in v1 against the 'seen' branch now appears in 'next' and has
the same resolution as v1.

A range-diff against v2 is given below.

[*] https://lore.kernel.org/git/aCbQ9OX2vqF82au8@pks.im/


Changes in v2:

Patch #3 is the only one changed (as a result of Patrick's review [0]):

 - add some blank lines to make the option handling blocks
   easier to see.
 - add a comment to 'gitconfig' and 'gitattributes' options
   to indicate the default values.

Note: The indicated defaults for the 'gitconfig' and 'gitattributes'
are only valid when the 'prefix' option is defaulted (or not /usr).
Indicating the 'correct' value when -Dprefix=/usr in the comment
would consume too much space. Is this acceptable, or is it too
confusing/misleading?

Also, thanks to Eli for testing patch #5 on Solaris and confirming
that it fixes the regression [1].

Note that I did a test merge to master@38af977b81 without issue and
to next@889b7c5bd8 and seen@d8088176ab. The conflict which showed
up in v1 against the 'seen' branch now appears in 'next' and has
the same resolution as v1.

A range-diff against v1 is given below.

[0] https://lore.kernel.org/git/aB3CDOljn9zJsVwt@pks.im/
[1] https://lore.kernel.org/git/9baad29d-a5bf-443d-98a1-36d7020e5835@gentoo.org/

v1 cover letter follows:

This series (part 2) continues the miscellaneous changes to the make,
meson and autoconf build systems. I am sending this part a little
earlier than I expected, so there will now be a part 3!

The reason for the early posting is to try and avoid an regression in
the autoconf build system (see patch #5). Hopefully, we still have time
in this cycle to get patch #5 included.

This series is based on commit 6f84262c44 ("The eleventh batch", 2025-05-05)

I did a test merge to 'next'@629a3ecd64 without issue, but 'seen'@71cfd25022
had a small conflict with commit 1a2929c851 ("meson: allow customize perl
installation path", 2025-04-24) on the 'dd/meson-perl-custom-path' branch.
(I note that a new version of that patch was posted to the list today).

The fixup looks like:

    diff --cc perl/Git/SVN/Memoize/meson.build
    index 4c589b30c3,8c2e80d2d2..d6209dc3bf
    --- a/perl/Git/SVN/Memoize/meson.build
    +++ b/perl/Git/SVN/Memoize/meson.build
    @@@ -3,6 -3,6 +3,6 @@@ test_dependencies += custom_target
        output: 'YAML.pm',
        command: generate_perl_command,
        install: true,
    -   install_dir: perllibdir / 'Git/SVN',
     -  install_dir: get_option('datadir') / 'perl5/Git/SVN/Memoize',
    ++  install_dir: perllibdir / 'Git/SVN/Memoize',
        depends: [git_version_file],
      )

ATB,
Ramsay Jones

Ramsay Jones (5):
  meson.build: quote the GITWEBDIR build configuration
  meson: correct install location of YAML.pm
  meson: correct path to system config/attribute files
  meson.build: correct setting of GIT_EXEC_PATH
  configure.ac: upgrade to a compilation check for sysinfo

 configure.ac                     | 25 ++++++++++++++++++++++---
 meson.build                      | 30 +++++++++++++++++++++++++-----
 meson_options.txt                |  8 ++++----
 perl/Git/SVN/Memoize/meson.build |  2 +-
 4 files changed, 52 insertions(+), 13 deletions(-)

range-diff against v3:

1:  ac97fdc76e = 1:  247bc82c01 meson.build: quote the GITWEBDIR build configuration
2:  181738d371 = 2:  a87f683a95 meson: correct install location of YAML.pm
3:  1f4c74973c ! 3:  40ea59efca meson: correct path to system config/attribute files
    @@ meson_options.txt: option('default_pager', type: 'string', value: 'less',
     -  description: 'Path to the global git configuration file.')
     -option('gitattributes', type: 'string', value: '/etc/gitattributes',
     -  description: 'Path to the global git attributes file.')
    -+option('gitconfig', type: 'string', # default 'etc/gitconfig'
    ++option('gitconfig', type: 'string',
     +  description: 'Path to the global git configuration file. (default: etc/gitconfig)')
    -+option('gitattributes', type: 'string', # default 'etc/gitattributes'
    ++option('gitattributes', type: 'string',
     +  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')
4:  97ff1613f5 = 4:  a7d2f1e132 meson.build: correct setting of GIT_EXEC_PATH
5:  983488fe4e = 5:  a45b9dc650 configure.ac: upgrade to a compilation check for sysinfo

-- 
2.49.0

[PATCH v4 1/5] meson.build: quote the GITWEBDIR build configuration

[Ramsay Jones]

The build configuration options with (non-empty) values, for example
filesystem paths potentially containing spaces, have been set using
the '.set_quoted()' method. However, the GITWEBDIR value has been
set using the '.set()' method instead. In order to correctly quote
the GITWEBDIR value, replace the '.set()' method with '.set_quoted()'.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 270ce933d0..48f31157a0 100644
--- a/meson.build
+++ b/meson.build
@@ -739,7 +739,7 @@ build_options_config.set('GIT_TEST_OPTS', '')
 build_options_config.set('GIT_TEST_PERL_FATAL_WARNINGS', '')
 build_options_config.set_quoted('GIT_TEST_UTF8_LOCALE', get_option('test_utf8_locale'))
 build_options_config.set_quoted('LOCALEDIR', fs.as_posix(get_option('prefix') / get_option('localedir')))
-build_options_config.set('GITWEBDIR', fs.as_posix(get_option('prefix') / get_option('datadir') / 'gitweb'))
+build_options_config.set_quoted('GITWEBDIR', fs.as_posix(get_option('prefix') / get_option('datadir') / 'gitweb'))
 
 if get_option('sane_tool_path').length() != 0
   sane_tool_path = (host_machine.system() == 'windows' ? ';' : ':').join(get_option('sane_tool_path'))
-- 
2.49.0

[PATCH v4 2/5] meson: correct install location of YAML.pm

[Ramsay Jones]

When executing an 'meson install' the YAML.pm file is incorrectly
placed in the <prefix>/share/perl5/Git/SVN directory. The YAML.pm
file should be placed in a 'Memoize' subdirectory instead. In order
to correct the location, update the 'install_dir' of the relevant
target in the 'perl/Git/SVN/Memoize/meson.build' file.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 perl/Git/SVN/Memoize/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/perl/Git/SVN/Memoize/meson.build b/perl/Git/SVN/Memoize/meson.build
index 233ec670d7..8c2e80d2d2 100644
--- a/perl/Git/SVN/Memoize/meson.build
+++ b/perl/Git/SVN/Memoize/meson.build
@@ -3,6 +3,6 @@ test_dependencies += custom_target(
   output: 'YAML.pm',
   command: generate_perl_command,
   install: true,
-  install_dir: get_option('datadir') / 'perl5/Git/SVN',
+  install_dir: get_option('datadir') / 'perl5/Git/SVN/Memoize',
   depends: [git_version_file],
 )
-- 
2.49.0

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

[Ramsay Jones]

The path to the system-wide config and attributes files are not being
set correctly in the meson build. Unless explicitly overridden on the
command line during setup, the 'gitconfig' and 'gitattributes' options
are defaulting to absolute paths in the '/etc' system directory. This
is only appropriate if the <prefix> is set specifically to '/usr'.

The directory in which these files are placed is generally referred to
as the 'system configuration directory' or 'sysconfdir' for short. When
the prefix is '/usr' then the sysconfdir is usually set to '/etc', but
any other value for prefix results in the relative directory value 'etc'
instead. (eg if prefix is '/usr/local', then the 'etc' relative value
results in a system configuration directory of '/usr/local/etc'). When
setting the 'sysconfdir' builtin option value, the meson system uses
exactly this algorithm, so we can use get_option('sysconfdir') directly
when setting the (non-overridden) build variables.

In order to allow for overriding from the command line, remove the
default values specified for the 'gitconfig' and 'gitattributes' options
in the 'meson_options.txt' file. This allows the user to specify any
pathname for those options, while being able to test for the unset
(empty) value. An absolute pathname will be used unchanged and a relative
pathname will be appended to '<prefix>/'. These values are then used to
set the 'ETC_GITCONFIG' and 'ETC_GITATTRIBUTES' build variables which are,
in turn, passed to the compiler as '-D' arguments.

When the 'gitconfig' or 'gitattributes' options are not used, then use
the built-in 'sysconfdir' and set the ETC_GITCONFIG build variable to
the string "<sysconfdir>/gitconfig". Similarly, set ETC_ATTRIBUTES to
"<sysconfdir>/gitattributes".

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 meson.build       | 16 ++++++++++++++--
 meson_options.txt |  8 ++++----
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/meson.build b/meson.build
index 48f31157a0..8e8f228a37 100644
--- a/meson.build
+++ b/meson.build
@@ -757,8 +757,6 @@ endif
 libgit_c_args = [
   '-DBINDIR="' + get_option('bindir') + '"',
   '-DDEFAULT_GIT_TEMPLATE_DIR="' + get_option('datadir') / 'git-core/templates' + '"',
-  '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
-  '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
   '-DFALLBACK_RUNTIME_PREFIX="' + get_option('prefix') + '"',
   '-DGIT_HOST_CPU="' + host_machine.cpu_family() + '"',
   '-DGIT_HTML_PATH="' + get_option('datadir') / 'doc/git-doc"',
@@ -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
+
 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..5afbf8ec00 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -3,10 +3,10 @@ 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',
-  description: 'Path to the global git configuration file.')
-option('gitattributes', type: 'string', value: '/etc/gitattributes',
-  description: 'Path to the global git attributes file.')
+option('gitconfig', type: 'string',
+  description: 'Path to the global git configuration file. (default: etc/gitconfig)')
+option('gitattributes', type: 'string',
+  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,
-- 
2.49.0

[PATCH v4 4/5] meson.build: correct setting of GIT_EXEC_PATH

[Ramsay Jones]

For the non-'runtime prefix' case, the meson build sets the GIT_EXEC_PATH
build variable to an absolute path equivalent to <prefix>/libexec/git-core.
In comparison, the default make build sets it to a relative path equivalent
to 'libexec/git-core'. Indeed, the make build requires the use of some
means outside of the Makefile (eg. config.mak[.*] or the command-line)
to set GIT_EXEC_PATH to anything other than 'libexec/git-core'.

For example, the make invocation:

  $ make gitexecdir=/some/other/bin all install

will build git with GIT_EXEC_PATH set to '/some/other/bin' and install
the 'library' executables to that location. However, without setting the
'gitexecdir' make variable, irrespective of the 'runtime prefix' setting,
the GIT_EXEC_PATH is always set to 'libexec/git-core'.

The meson built-in 'libexecdir' option can be used to provide a similar
configurability. The default value for the option is 'libexec'. Attempting
to set the option to '' on the command-line, will reset it to the '.'
string, presumably to ensure a relative path value.

This commit allows the meson build, similar to the above, to configure the
project like:

  $ meson setup --buildtype=debugoptimized -Dprefix=$HOME -Dpcre2=disabled \
      -Dlibexecdir=/some/other/bin build

so that the GIT_EXEC_PATH is set to '/some/other/bin'. Absent the
-Dlibexecdir argument, the GIT_EXEC_PATH is set to 'libexec/git-core'.

In order to correct the value of GIT_EXEC_PATH, default the value to the
static string value 'libexec/git-core', and only override if the value
of the 'libexecdir' option has a value different to 'libexec' or '.'.
Also, like the Makefile, add a check for an absolute path when the
runtime prefix option is true (and if so, error out).

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 meson.build | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 8e8f228a37..bd14bc15a1 100644
--- a/meson.build
+++ b/meson.build
@@ -1592,10 +1592,19 @@ else
   error('Unsupported CSPRNG backend: ' + csprng_backend)
 endif
 
+git_exec_path = 'libexec/git-core'
+libexec = get_option('libexecdir')
+if libexec != 'libexec' and libexec != '.'
+  git_exec_path = libexec
+endif
+
 if get_option('runtime_prefix')
   libgit_c_args += '-DRUNTIME_PREFIX'
   build_options_config.set('RUNTIME_PREFIX', 'true')
-  git_exec_path = get_option('libexecdir') / 'git-core'
+
+  if git_exec_path.startswith('/')
+    error('runtime_prefix requires a relative libexecdir not:', libexec)
+  endif
 
   if compiler.has_header('mach-o/dyld.h')
     libgit_c_args += '-DHAVE_NS_GET_EXECUTABLE_PATH'
@@ -1632,7 +1641,6 @@ if get_option('runtime_prefix')
   endif
 else
   build_options_config.set('RUNTIME_PREFIX', 'false')
-  git_exec_path = get_option('prefix') / get_option('libexecdir') / 'git-core'
 endif
 libgit_c_args += '-DGIT_EXEC_PATH="' + git_exec_path + '"'
 
-- 
2.49.0

[PATCH v4 5/5] configure.ac: upgrade to a compilation check for sysinfo

[Ramsay Jones]

Commit f5e3c6c57d ("meson: do a full usage-based compile check for
sysinfo", 2025-04-25) updated the 'sysinfo()' check, as part of the
meson build, due to the failure of the check on Solaris. Prior to
that commit, the meson build only checked the availability of the
'<sys/sysinfo.h>' header file. On Solaris, both the header and the
'sysinfo()' function exist, but are completely unrelated to the same
function on Linux (and cygwin).

Commit 50dec7c566 ("config.mak.uname: add sysinfo() configuration for
cygwin", 2025-04-17) added a similar 'sysinfo()' check to the autoconf
build. This check looked for the 'sysinfo()' function itself, rather
than just the header, but it will fail (incorrectly set HAVE_SYSINFO)
for the same reason.

In order to correctly identify the 'sysinfo()' function we require as
part of 'git-gc' (used in the 'total_ram() function), we also upgrade
to a compilation check, in a similar way to the meson commit. Note that
since commit c9a51775a3 ("builtin/gc.c: correct RAM calculation when
using sysinfo", 2025-04-17) both the 'totalram' and 'mem_unit' fields
of the 'struct sysinfo' are used, so the new check includes both of
those fields in the compile check.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 configure.ac | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index d7e0503f1e..f6caab919a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1069,9 +1069,28 @@ GIT_CONF_SUBST([CHARSET_LIB])
 
 #
 # Define HAVE_SYSINFO=YesPlease if sysinfo is available.
-GIT_CHECK_FUNC(sysinfo,
-	[HAVE_SYSINFO=YesPlease],
-	[HAVE_SYSINFO=])
+#
+AC_DEFUN([HAVE_SYSINFO_SRC], [
+AC_LANG_PROGRAM([[
+#include <stdint.h>
+#include <sys/sysinfo.h>
+]], [[
+struct sysinfo si;
+uint64_t t = 0;
+if (!sysinfo(&si)) {
+	t = si.totalram;
+	if (si.mem_unit > 1)
+		t *= (uint64_t)si.mem_unit;
+}
+return t;
+]])])
+
+AC_MSG_CHECKING([for sysinfo])
+AC_COMPILE_IFELSE([HAVE_SYSINFO_SRC],
+	[AC_MSG_RESULT([yes])
+	HAVE_SYSINFO=YesPlease],
+	[AC_MSG_RESULT([no])
+	HAVE_SYSINFO=])
 GIT_CONF_SUBST([HAVE_SYSINFO])
 
 #
-- 
2.49.0

Re: [PATCH v4 0/5] miscellaneous build mods (part 2)

[Junio C Hamano]

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

> Again, I did a test merge to master@cb96e1697a, next@a128411c76
> and seen@df1b4f9cf9. The conflict which showed up in v1 against
> the 'seen' branch now appears in the 'master' branch and has the
> same resolution as v1.

Thanks.

> A range-diff against v3 is given below.
>
> [+] https://lore.kernel.org/git/aCrekcz6onTFgEWw@pks.im/
>
> Changes in v3:
>
> Patch #3 changed as a result of Patrick's review [*]:
>
>  - use the '/' magic string operator when setting the
>    ETC_GIT{CONFIG,ATTRIBUTES} build options.
>  - add the default values to the 'description' fields
>    for the 'gitconfig' and 'gitattributes' option
>    definitions
>
> Also, Junio, the range-diff below shows the typo fixup, but
> you should find that it doesn't show for you this time. ;)

OK.  And changes in v4 are just that two redundant comments in the
option definitions are removed, which makes sense looking at the
discussion from the sidelines.

Will queue.  Are we done with this series by now?

> range-diff against v3:
>
> 1:  ac97fdc76e = 1:  247bc82c01 meson.build: quote the GITWEBDIR build configuration
> 2:  181738d371 = 2:  a87f683a95 meson: correct install location of YAML.pm
> 3:  1f4c74973c ! 3:  40ea59efca meson: correct path to system config/attribute files
>     @@ meson_options.txt: option('default_pager', type: 'string', value: 'less',
>      -  description: 'Path to the global git configuration file.')
>      -option('gitattributes', type: 'string', value: '/etc/gitattributes',
>      -  description: 'Path to the global git attributes file.')
>     -+option('gitconfig', type: 'string', # default 'etc/gitconfig'
>     ++option('gitconfig', type: 'string',
>      +  description: 'Path to the global git configuration file. (default: etc/gitconfig)')
>     -+option('gitattributes', type: 'string', # default 'etc/gitattributes'
>     ++option('gitattributes', type: 'string',
>      +  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')
> 4:  97ff1613f5 = 4:  a7d2f1e132 meson.build: correct setting of GIT_EXEC_PATH
> 5:  983488fe4e = 5:  a45b9dc650 configure.ac: upgrade to a compilation check for sysinfo

Re: [PATCH v4 0/5] miscellaneous build mods (part 2)

[Patrick Steinhardt]

On Mon, May 19, 2025 at 11:48:01AM -0700, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
> > Again, I did a test merge to master@cb96e1697a, next@a128411c76
> > and seen@df1b4f9cf9. The conflict which showed up in v1 against
> > the 'seen' branch now appears in the 'master' branch and has the
> > same resolution as v1.
> 
> Thanks.
> 
> > A range-diff against v3 is given below.
> >
> > [+] https://lore.kernel.org/git/aCrekcz6onTFgEWw@pks.im/
> >
> > Changes in v3:
> >
> > Patch #3 changed as a result of Patrick's review [*]:
> >
> >  - use the '/' magic string operator when setting the
> >    ETC_GIT{CONFIG,ATTRIBUTES} build options.
> >  - add the default values to the 'description' fields
> >    for the 'gitconfig' and 'gitattributes' option
> >    definitions
> >
> > Also, Junio, the range-diff below shows the typo fixup, but
> > you should find that it doesn't show for you this time. ;)
> 
> OK.  And changes in v4 are just that two redundant comments in the
> option definitions are removed, which makes sense looking at the
> discussion from the sidelines.
> 
> Will queue.  Are we done with this series by now?

Yup, the range diff looks as expected, so this is ready to go from my
point of view. Thanks!

Patrick

Re: [PATCH v4 0/5] miscellaneous build mods (part 2)

[Ramsay Jones]

On 19/05/2025 20:08, Patrick Steinhardt wrote:
> On Mon, May 19, 2025 at 11:48:01AM -0700, Junio C Hamano wrote:
>> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>>
>>> Again, I did a test merge to master@cb96e1697a, next@a128411c76
>>> and seen@df1b4f9cf9. The conflict which showed up in v1 against
>>> the 'seen' branch now appears in the 'master' branch and has the
>>> same resolution as v1.
>>
>> Thanks.
>>
>>> A range-diff against v3 is given below.
>>>
>>> [+] https://lore.kernel.org/git/aCrekcz6onTFgEWw@pks.im/
>>>
>>> Changes in v3:
>>>
>>> Patch #3 changed as a result of Patrick's review [*]:
>>>
>>>  - use the '/' magic string operator when setting the
>>>    ETC_GIT{CONFIG,ATTRIBUTES} build options.
>>>  - add the default values to the 'description' fields
>>>    for the 'gitconfig' and 'gitattributes' option
>>>    definitions
>>>
>>> Also, Junio, the range-diff below shows the typo fixup, but
>>> you should find that it doesn't show for you this time. ;)
>>
>> OK.  And changes in v4 are just that two redundant comments in the
>> option definitions are removed, which makes sense looking at the
>> discussion from the sidelines.
>>
>> Will queue.  Are we done with this series by now?
> 
> Yup, the range diff looks as expected, so this is ready to go from my
> point of view. Thanks!

Yes, I think we're good to go.

Also, the 'make test' on cygwin just finished and passed without issue! :)

Thanks.

ATB,
Ramsay Jones

Re: [PATCH v4 0/5] miscellaneous build mods (part 2)

[Junio C Hamano]

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

>>> Will queue.  Are we done with this series by now?
>> 
>> Yup, the range diff looks as expected, so this is ready to go from my
>> point of view. Thanks!
>
> Yes, I think we're good to go.
>
> Also, the 'make test' on cygwin just finished and passed without issue! :)
>
> Thanks.

Thank you, all!

Re: [PATCH v2 4/5] meson.build: correct setting of GIT_EXEC_PATH

[irecca.kun]

This completely breaks meson builds with 'libexecdir' set. Git can not find any helper tools because 'git_exec_path' is set incorrectly.

It supposed to be:

--- meson.build
+++ meson.build
@@ -1597,7 +1597,7 @@
  git_exec_path = 'libexec/git-core'
  libexec = get_option('libexecdir')
  if libexec != 'libexec' and libexec != '.'
-  git_exec_path = libexec
+  git_exec_path = libexec / 'git-core'
  endif
  
  if get_option('runtime_prefix')