[PATCH 00/12] miscellaneous build mods (part 1)

[PATCH 00/12] miscellaneous build mods (part 1)

[Ramsay Jones]

This series contains some miscellaneous changes to the make, meson and
autoconf build systems. These are only the patches I have managed to
write a commit message for; I have some others with no commit message,
some need more testing and others are yet to be written. (So, there will
be a 'part #2' series later... :) ).

(I have notes from a couple of months ago - as an example, I have a note
that the meson build incorrectly sets ETC_GITATTRIBUTES and ETC_GITCONFIG
to e.g. '/etc/gitattributes' rather than 'etc/gitattributes').

This series is based on the master branch at commit 683c54c999 ("Git 2.49",
2025-03-14).

As a simple means for comparing the build products, we can compare the
symbols exported by git. So, starting at current master:

  $ git checkout master
  Switched to branch 'master'
  Your branch is up-to-date with 'origin/master'.
  $ git describe
  v2.49.0
  $ 

  $ make >out 2>&1
  $ 

  $ meson setup build/
  The Meson build system
  ...
  Found ninja-1.11.1 at /usr/bin/ninja
  $ 

  $ cd build
  $ meson compile
  INFO: autodetecting backend as ninja
  INFO: calculating backend command to run: /usr/bin/ninja
  [661/661] Copying file contrib/completion/git-completion.zsh
  $ cd ..
  $ 

  $ rm *.syms
  $ ./syms.pl git >git.syms 2>/dev/null
  $ ./syms.pl build/git >mgit.syms 2>/dev/null
  $ 

  $ diff git.syms mgit.syms
  88a89
  > U -- arc4random_buf@GLIBC_2.36
  98a100,101
  > U -- atoi@GLIBC_2.2.5
  > U -- atol@GLIBC_2.2.5
  195a199
  > U -- bsearch@GLIBC_2.2.5
  586,587d589
  < U -- dcgettext@GLIBC_2.2.5
  < U -- dcngettext@GLIBC_2.2.5
  746,747c748,749
  < B -- __environ@GLIBC_2.2.5
  < V -- environ@GLIBC_2.2.5
  ---
  > U -- __environ@GLIBC_2.2.5
  > U -- environ@GLIBC_2.2.5
  909c911
  < U -- __fprintf_chk@GLIBC_2.3.4
  ---
  > U -- fprintf@GLIBC_2.2.5
  914d915
  < U -- __fread_chk@GLIBC_2.7
  1007c1008
  < U -- getc@GLIBC_2.2.5
  ---
  > U -- getchar@GLIBC_2.2.5
  1018a1020
  > U -- getc_unlocked@GLIBC_2.2.5
  1107a1110
  > U -- gettext@GLIBC_2.2.5
  1253d1255
  < T -- gitstrlcpy
  1355a1358
  > U -- htons@GLIBC_2.2.5
  1666d1668
  < U -- __memcpy_chk@GLIBC_2.3.4
  1673d1674
  < U -- __memmove_chk@GLIBC_2.3.4
  1684d1684
  < U -- __memset_chk@GLIBC_2.3.4
  1737a1738
  > U -- ngettext@GLIBC_2.2.5
  1759a1761
  > U -- ntohs@GLIBC_2.2.5
  1819d1820
  < U -- __open64_2@GLIBC_2.7
  1988a1990,2007
  > U -- pcre2_code_free_8
  > U -- pcre2_compile_8
  > U -- pcre2_compile_context_create_8
  > U -- pcre2_compile_context_free_8
  > U -- pcre2_config_8
  > U -- pcre2_general_context_create_8
  > U -- pcre2_general_context_free_8
  > U -- pcre2_get_error_message_8
  > U -- pcre2_get_ovector_pointer_8
  > U -- pcre2_jit_compile_8
  > U -- pcre2_jit_match_8
  > U -- pcre2_maketables_8
  > U -- pcre2_maketables_free_8
  > U -- pcre2_match_8
  > U -- pcre2_match_data_create_from_pattern_8
  > U -- pcre2_match_data_free_8
  > U -- pcre2_pattern_info_8
  > U -- pcre2_set_character_tables_8
  2045c2064
  < U -- __printf_chk@GLIBC_2.3.4
  ---
  > U -- printf@GLIBC_2.2.5
  2080a2100
  > U -- pthread_equal@GLIBC_2.2.5
  2767c2787
  < U -- __snprintf_chk@GLIBC_2.3.4
  ---
  > U -- snprintf@GLIBC_2.2.5
  2925a2946
  > U -- strlcpy@GLIBC_2.38
  3185d3205
  < U -- __uflow@GLIBC_2.2.5
  3289,3290c3309,3311
  < U -- __vfprintf_chk@GLIBC_2.3.4
  < U -- __vsnprintf_chk@GLIBC_2.3.4
  ---
  > U -- vfprintf@GLIBC_2.2.5
  > U -- vprintf@GLIBC_2.2.5
  > U -- vsnprintf@GLIBC_2.2.5
  $ 

Looking at the above, you can see that quite a few differences relate to
symbols which differ due to the level of optimization during compilation.
This reflects, among other things, the different 'default' settings between
the Makefile and the meson build; e.g. prefix=$HOME vs. /usr/local, initial
CFLAGS='-g -O2 -Wall' vs. '-g -O0 -Wall'. Also, note that the make build
does not include the (optional) pcre2 library, even if it is installed on
the system.

[On cygwin the diff is slightly different, but also contains more symbols,
such as all symbols relating to the compat regex routines, clock_gettime,
getdelim, sysinfo and so on ... ;) ]

If we change the meson setup, thus:

  $ meson setup --optimization=2 -Dprefix=$HOME -Dpcre2=disabled build/

After we re-build and repeat the above symbol generation, we find:

  $ diff git.syms mgit.syms
  88a89
  > U -- arc4random_buf@GLIBC_2.36
  746,747c747,748
  < B -- __environ@GLIBC_2.2.5
  < V -- environ@GLIBC_2.2.5
  ---
  > U -- __environ@GLIBC_2.2.5
  > U -- environ@GLIBC_2.2.5
  1253d1253
  < T -- gitstrlcpy
  2925a2926
  > U -- strlcpy@GLIBC_2.38
  $ 

After applying these patches and adding settings to the config.mak file: 

  $ vim config.mak # add NO_STRLCPY and CSPRNG_METHOD
  $ 
  $ cat config.mak
  DEFAULT_TEST_TARGET=prove
  GIT_PROVE_OPTS='--timer'
  PYTHON_PATH=/usr/bin/python3
  GIT_TEST_HTTPD=false
  export GIT_TEST_HTTPD
  #DEVELOPER=1
  SPARSE_FLAGS += -D__STDC_NO_VLA__
  NO_STRLCPY=
  CSPRNG_METHOD=arc4random
  $ 

[Note that I commented out DEVELOPER=1. I'm not entirely sure why I did
that. :) Maybe it was one less variable? ]

  $ make >out 2>&1
  $ 

  $ meson setup --optimization=2 -Dprefix=$HOME -Dpcre2=disabled build/
  The Meson build system
  ...
    User defined options
      optimization: 2
      prefix      : /home/ramsay
      pcre2       : disabled
  
  Found ninja-1.11.1 at /usr/bin/ninja
  $ 

  $ cd build
  $ meson compile
  INFO: autodetecting backend as ninja
  INFO: calculating backend command to run: /usr/bin/ninja
  [661/661] Copying file contrib/completion/git-prompt.sh
  $ cd ..
  $ 

  $ rm *.syms
  $ ./syms.pl git >git.syms 2>/dev/null
  $ ./syms.pl build/git >mgit.syms 2>/dev/null
  $ 

  $ diff git.syms mgit.syms
  747,748c747,748
  < B -- __environ@GLIBC_2.2.5
  < V -- environ@GLIBC_2.2.5
  ---
  > U -- __environ@GLIBC_2.2.5
  > U -- environ@GLIBC_2.2.5
  $ 
 
On cygwin, this diff is now empty. ;)

The above diff seems to imply a difference in the linking of main() and
the C library startup routines (or something similar). However, I happened
to notice that the meson build included '-fPIC' in the build flags, so I
thought it was worth a punt:

  $ make CFLAGS='-g -O2 -Wall -fPIC' >out 2>&1

[with the advent of ASLR, I thought -fPIC/-fPIE was the default anyway!]

  $ diff git.syms mgit.syms
  2809,2811c2809,2811
  < U -- stderr@GLIBC_2.2.5
  < U -- stdin@GLIBC_2.2.5
  < U -- stdout@GLIBC_2.2.5
  ---
  > B -- stderr@GLIBC_2.2.5
  > B -- stdin@GLIBC_2.2.5
  > B -- stdout@GLIBC_2.2.5
  $ 

Hmm, so one step forward, one step back! (or is it 2 steps forward, 3 steps
back? ;) ). So, something for another day.

Note that both test suite runs ('make test' and 'meson test') pass all
tests (excluding the http, cvs, svn, and p4 tests) on Linux. (I am still
waiting for the v2.49.0 test run to finish on cygwin! :( ).

Also, patches #5 and #12 only add/modify a comment, so they could be
dropped, if people feel that they are not appropriate.

[Just in case you want to reproduce the above, I have appended the syms.pl
script below.]

ATB,
Ramsay Jones

Ramsay Jones (12):
  meson.build: remove -DCURL_DISABLE_TYPECHECK
  Makefile: only set some BASIC_CFLAGS when RUNTIME_PREFIX is set
  meson.build: only set build variables for non-default values
  Makefile: remove NEEDS_LIBRT build variable
  config.mak.uname: add a note about NO_STRLCPY for Linux
  config.mak.uname: only set NO_REGEX on cygwin for v1.7
  config.mak.uname: add HAVE_GETDELIM to the cygwin section
  config.mak.uname: add clock_gettime() to the cygwin build
  builtin/gc.c: correct RAM calculation when using sysinfo
  config.mak.uname: add sysinfo() configuration for cygwin
  config.mak.uname; add arc4random to the cygwin build
  config.mak.uname: add a note about CSPRNG_METHOD for Linux

 Makefile         | 45 +++++++++++++++++++++++----------------------
 builtin/gc.c     |  9 +++++++--
 config.mak.uname | 16 ++++++++++++----
 configure.ac     |  8 ++++++++
 meson.build      | 14 ++++++++++----
 5 files changed, 60 insertions(+), 32 deletions(-)

-- 
2.49.0

----------------- >8 ------------------ [syms.pl]
#!/usr/bin/perl -w

open(I, "-|", qw(nm -g), $ARGV[0]) or die;
while (<I>) {
	unless (/^[0-9a-f ]+ ([A-Z]) (\S*)$/) {
		print STDERR "? $_";
		next;
	}
	next if ($2 =~ /^\.refptr\./);
	print "$1 -- $2\n";
}
close I;

[PATCH v2 00/13] miscellaneous build mods (part 1)

[Ramsay Jones]

changes in v2:
--------------

- patch #3 has been split into two (new patches #3,4). The motivation
  for the original patch seems not to have been too obvious. The new
  patch #3 combines the suppressing the -Defines for standard default
  values for editor and pager, with the setting of the missing docs
  attributes. The new patch #4 combines the suppressing of the -Define
  for the default-help-format, with the overriding of the default value
  on the windows platform.

- patch #6 (new #7) updates the commit message along with skipping the
  'failing' test #12 in t7815-grep-binary.sh on cygwin. (With the cygwin
  platform regex library the '.' pattern matches a NUL character).

- patch #11 (new #12) fixes a simple typo in the patch subject.

- patch #12 (new #13) updates the commit message and makes a choice for
  the default CSPRNG_METHOD on Linux; 'getrandom' as advised by Brian
  here [0].

[0] https://lore.kernel.org/git/Z9YbJFJjtXNYnTzk@tapette.crustytoothpaste.net/

As before, this series is based on commit 683c54c999 ("Git 2.49", 2025-03-14).

Also, I did a test merge to 'master'@5b97a56fa0, 'next'@ff1f9ca942 and
'seen'@1b09671a54 and they all 'auto-merged' without issue.

As an experiment, I am using 'git sendemail' to send this series for the
first time (I have sent single patches before, but not a series), so if
anything goes awry I apologize in advance! :)

A range-diff is given below.


The original v1 cover-letter follows:
-------------------------------------

This series contains some miscellaneous changes to the make, meson and
autoconf build systems. These are only the patches I have managed to
write a commit message for; I have some others with no commit message,
some need more testing and others are yet to be written. (So, there will
be a 'part #2' series later... :) ).

(I have notes from a couple of months ago - as an example, I have a note
that the meson build incorrectly sets ETC_GITATTRIBUTES and ETC_GITCONFIG
to e.g. '/etc/gitattributes' rather than 'etc/gitattributes').

This series is based on the master branch at commit 683c54c999 ("Git 2.49",
2025-03-14).

As a simple means for comparing the build products, we can compare the
symbols exported by git. So, starting at current master:

  $ git checkout master
  Switched to branch 'master'
  Your branch is up-to-date with 'origin/master'.
  $ git describe
  v2.49.0
  $ 

  $ make >out 2>&1
  $ 

  $ meson setup build/
  The Meson build system
  ...
  Found ninja-1.11.1 at /usr/bin/ninja
  $ 

  $ cd build
  $ meson compile
  INFO: autodetecting backend as ninja
  INFO: calculating backend command to run: /usr/bin/ninja
  [661/661] Copying file contrib/completion/git-completion.zsh
  $ cd ..
  $ 

  $ rm *.syms
  $ ./syms.pl git >git.syms 2>/dev/null
  $ ./syms.pl build/git >mgit.syms 2>/dev/null
  $ 

  $ diff git.syms mgit.syms
  88a89
  > U -- arc4random_buf@GLIBC_2.36
  98a100,101
  > U -- atoi@GLIBC_2.2.5
  > U -- atol@GLIBC_2.2.5
  195a199
  > U -- bsearch@GLIBC_2.2.5
  586,587d589
  < U -- dcgettext@GLIBC_2.2.5
  < U -- dcngettext@GLIBC_2.2.5
  746,747c748,749
  < B -- __environ@GLIBC_2.2.5
  < V -- environ@GLIBC_2.2.5
  ---
  > U -- __environ@GLIBC_2.2.5
  > U -- environ@GLIBC_2.2.5
  909c911
  < U -- __fprintf_chk@GLIBC_2.3.4
  ---
  > U -- fprintf@GLIBC_2.2.5
  914d915
  < U -- __fread_chk@GLIBC_2.7
  1007c1008
  < U -- getc@GLIBC_2.2.5
  ---
  > U -- getchar@GLIBC_2.2.5
  1018a1020
  > U -- getc_unlocked@GLIBC_2.2.5
  1107a1110
  > U -- gettext@GLIBC_2.2.5
  1253d1255
  < T -- gitstrlcpy
  1355a1358
  > U -- htons@GLIBC_2.2.5
  1666d1668
  < U -- __memcpy_chk@GLIBC_2.3.4
  1673d1674
  < U -- __memmove_chk@GLIBC_2.3.4
  1684d1684
  < U -- __memset_chk@GLIBC_2.3.4
  1737a1738
  > U -- ngettext@GLIBC_2.2.5
  1759a1761
  > U -- ntohs@GLIBC_2.2.5
  1819d1820
  < U -- __open64_2@GLIBC_2.7
  1988a1990,2007
  > U -- pcre2_code_free_8
  > U -- pcre2_compile_8
  > U -- pcre2_compile_context_create_8
  > U -- pcre2_compile_context_free_8
  > U -- pcre2_config_8
  > U -- pcre2_general_context_create_8
  > U -- pcre2_general_context_free_8
  > U -- pcre2_get_error_message_8
  > U -- pcre2_get_ovector_pointer_8
  > U -- pcre2_jit_compile_8
  > U -- pcre2_jit_match_8
  > U -- pcre2_maketables_8
  > U -- pcre2_maketables_free_8
  > U -- pcre2_match_8
  > U -- pcre2_match_data_create_from_pattern_8
  > U -- pcre2_match_data_free_8
  > U -- pcre2_pattern_info_8
  > U -- pcre2_set_character_tables_8
  2045c2064
  < U -- __printf_chk@GLIBC_2.3.4
  ---
  > U -- printf@GLIBC_2.2.5
  2080a2100
  > U -- pthread_equal@GLIBC_2.2.5
  2767c2787
  < U -- __snprintf_chk@GLIBC_2.3.4
  ---
  > U -- snprintf@GLIBC_2.2.5
  2925a2946
  > U -- strlcpy@GLIBC_2.38
  3185d3205
  < U -- __uflow@GLIBC_2.2.5
  3289,3290c3309,3311
  < U -- __vfprintf_chk@GLIBC_2.3.4
  < U -- __vsnprintf_chk@GLIBC_2.3.4
  ---
  > U -- vfprintf@GLIBC_2.2.5
  > U -- vprintf@GLIBC_2.2.5
  > U -- vsnprintf@GLIBC_2.2.5
  $ 

Looking at the above, you can see that quite a few differences relate to
symbols which differ due to the level of optimization during compilation.
This reflects, among other things, the different 'default' settings between
the Makefile and the meson build; e.g. prefix=$HOME vs. /usr/local, initial
CFLAGS='-g -O2 -Wall' vs. '-g -O0 -Wall'. Also, note that the make build
does not include the (optional) pcre2 library, even if it is installed on
the system.

[On cygwin the diff is slightly different, but also contains more symbols,
such as all symbols relating to the compat regex routines, clock_gettime,
getdelim, sysinfo and so on ... ;) ]

If we change the meson setup, thus:

  $ meson setup --optimization=2 -Dprefix=$HOME -Dpcre2=disabled build/

After we re-build and repeat the above symbol generation, we find:

  $ diff git.syms mgit.syms
  88a89
  > U -- arc4random_buf@GLIBC_2.36
  746,747c747,748
  < B -- __environ@GLIBC_2.2.5
  < V -- environ@GLIBC_2.2.5
  ---
  > U -- __environ@GLIBC_2.2.5
  > U -- environ@GLIBC_2.2.5
  1253d1253
  < T -- gitstrlcpy
  2925a2926
  > U -- strlcpy@GLIBC_2.38
  $ 

After applying these patches and adding settings to the config.mak file: 

  $ vim config.mak # add NO_STRLCPY and CSPRNG_METHOD
  $ 
  $ cat config.mak
  DEFAULT_TEST_TARGET=prove
  GIT_PROVE_OPTS='--timer'
  PYTHON_PATH=/usr/bin/python3
  GIT_TEST_HTTPD=false
  export GIT_TEST_HTTPD
  #DEVELOPER=1
  SPARSE_FLAGS += -D__STDC_NO_VLA__
  NO_STRLCPY=
  CSPRNG_METHOD=arc4random
  $ 

[Note that I commented out DEVELOPER=1. I'm not entirely sure why I did
that. :) Maybe it was one less variable? ]

  $ make >out 2>&1
  $ 

  $ meson setup --optimization=2 -Dprefix=$HOME -Dpcre2=disabled build/
  The Meson build system
  ...
    User defined options
      optimization: 2
      prefix      : /home/ramsay
      pcre2       : disabled
  
  Found ninja-1.11.1 at /usr/bin/ninja
  $ 

  $ cd build
  $ meson compile
  INFO: autodetecting backend as ninja
  INFO: calculating backend command to run: /usr/bin/ninja
  [661/661] Copying file contrib/completion/git-prompt.sh
  $ cd ..
  $ 

  $ rm *.syms
  $ ./syms.pl git >git.syms 2>/dev/null
  $ ./syms.pl build/git >mgit.syms 2>/dev/null
  $ 

  $ diff git.syms mgit.syms
  747,748c747,748
  < B -- __environ@GLIBC_2.2.5
  < V -- environ@GLIBC_2.2.5
  ---
  > U -- __environ@GLIBC_2.2.5
  > U -- environ@GLIBC_2.2.5
  $ 
 
On cygwin, this diff is now empty. ;)

The above diff seems to imply a difference in the linking of main() and
the C library startup routines (or something similar). However, I happened
to notice that the meson build included '-fPIC' in the build flags, so I
thought it was worth a punt:

  $ make CFLAGS='-g -O2 -Wall -fPIC' >out 2>&1

[with the advent of ASLR, I thought -fPIC/-fPIE was the default anyway!]

  $ diff git.syms mgit.syms
  2809,2811c2809,2811
  < U -- stderr@GLIBC_2.2.5
  < U -- stdin@GLIBC_2.2.5
  < U -- stdout@GLIBC_2.2.5
  ---
  > B -- stderr@GLIBC_2.2.5
  > B -- stdin@GLIBC_2.2.5
  > B -- stdout@GLIBC_2.2.5
  $ 

Hmm, so one step forward, one step back! (or is it 2 steps forward, 3 steps
back? ;) ). So, something for another day.

Note that both test suite runs ('make test' and 'meson test') pass all
tests (excluding the http, cvs, svn, and p4 tests) on Linux. (I am still
waiting for the v2.49.0 test run to finish on cygwin! :( ).

Also, patches #5 and #12 only add/modify a comment, so they could be
dropped, if people feel that they are not appropriate.

[Just in case you want to reproduce the above, I have appended the syms.pl
script below.]

ATB,
Ramsay Jones

Ramsay Jones (13):
  meson.build: remove -DCURL_DISABLE_TYPECHECK
  Makefile: only set some BASIC_CFLAGS when RUNTIME_PREFIX is set
  meson.build: only set build variables for non-default values
  meson.build: set default help format to html on windows
  Makefile: remove NEEDS_LIBRT build variable
  config.mak.uname: add a note about NO_STRLCPY for Linux
  config.mak.uname: only set NO_REGEX on cygwin for v1.7
  config.mak.uname: add HAVE_GETDELIM to the cygwin section
  config.mak.uname: add clock_gettime() to the cygwin build
  builtin/gc.c: correct RAM calculation when using sysinfo
  config.mak.uname: add sysinfo() configuration for cygwin
  config.mak.uname: add arc4random to the cygwin build
  config.mak.uname: set CSPRNG_METHOD to getrandom on Linux

 Documentation/meson.build | 20 +++++++++++++++++
 Makefile                  | 45 ++++++++++++++++++++-------------------
 builtin/gc.c              |  9 ++++++--
 config.mak.uname          | 15 +++++++++----
 configure.ac              |  8 +++++++
 meson.build               | 27 +++++++++++++++++++----
 meson_options.txt         |  2 +-
 t/t7815-grep-binary.sh    |  2 +-
 8 files changed, 94 insertions(+), 34 deletions(-)

range-diff against v1:

 1:  50ce21363e =  1:  fe969deec0 meson.build: remove -DCURL_DISABLE_TYPECHECK
 2:  b7097a71b4 =  2:  8df541634a Makefile: only set some BASIC_CFLAGS when RUNTIME_PREFIX is set
 3:  85363624b2 <  -:  ---------- meson.build: only set build variables for non-default values
 -:  ---------- >  3:  280363cd56 meson.build: only set build variables for non-default values
 -:  ---------- >  4:  34349bf98c meson.build: set default help format to html on windows
 4:  4bda84bbf2 =  5:  e73c2c3219 Makefile: remove NEEDS_LIBRT build variable
 5:  45d570f6d4 =  6:  8c76671f0e config.mak.uname: add a note about NO_STRLCPY for Linux
 6:  45ef2d4268 !  7:  324bb21342 config.mak.uname: only set NO_REGEX on cygwin for v1.7
    @@ Commit message
         some of the tests (t4018 and t4034), which passed just fine with the
         compat library.
     
    -    After some time (may a year or two), the platform library had been
    +    After some time (maybe a year or two), the platform library had been
         updated (with an import from FreeBSD, I believe) and now passed the full
         test-suite. This would be about the time of the v1.7 -> v2.0 transition
         in 2015. I had a patch ready to send, but just didn't get around to
    @@ Commit message
         NO_REGEX variable to use the platform regex library functions. The new
         meson build system does likewise.
     
    +    The cygwin platform regex library, in addition to now passing the tests
    +    which formerly failed, now passes an 'test_expect_failure' test in the
    +    t7815-grep-binary test file. In particular, test #12 'git grep .fi a'
    +    which determines that the regex pattern '.' matches a NUL character.
    +    The commit f96e56733a ("grep: use REG_STARTEND for all matching if
    +    available", 2010-05-22) added the test in question, but it does not
    +    give any indication as to why the test was framed as an expected fail,
    +    rather than a 'positive' test that the 'git grep' command fails to
    +    match a NUL. Note that the previous test #11 was also originally
    +    marked in that commit as a 'test_expect_failure', but was flipped to
    +    an 'success' test in commit 7e36de5859 ("t/t7008-grep-binary.sh: un-TODO
    +    a test that needs REG_STARTEND", 2010-08-17).
    +
         In order to produce the same NO_REGEX configuration from autoconf, meson
         and make, modify config.mak.uname to only set NO_REGEX for cygwin v1.7.
    +    In addition, skip test t7815.12 on cygwin, by adding the !CYGWIN pre-
    +    requisite to the test header, which (among other things) removes an
    +    '...; please update test(s)' comment.
     
      ## config.mak.uname ##
     @@ config.mak.uname: ifeq ($(uname_O),Cygwin)
    @@ config.mak.uname: ifeq ($(uname_O),Cygwin)
              endif
      	HAVE_DEV_TTY = YesPlease
      	HAVE_ALLOCA_H = YesPlease
    +
    + ## t/t7815-grep-binary.sh ##
    +@@ t/t7815-grep-binary.sh: test_expect_success 'git grep ile a' '
    + 	git grep ile a
    + '
    + 
    +-test_expect_failure 'git grep .fi a' '
    ++test_expect_failure !CYGWIN 'git grep .fi a' '
    + 	git grep .fi a
    + '
    + 
 7:  aa3e298fde =  8:  0a3ce7e80a config.mak.uname: add HAVE_GETDELIM to the cygwin section
 8:  dc30aa5653 =  9:  3bd6e40cd2 config.mak.uname: add clock_gettime() to the cygwin build
 9:  1e92c641a6 = 10:  a2eb9ab117 builtin/gc.c: correct RAM calculation when using sysinfo
10:  6e208c53dd = 11:  e45b9df3a4 config.mak.uname: add sysinfo() configuration for cygwin
11:  990fde71bb ! 12:  422d4b2571 config.mak.uname; add arc4random to the cygwin build
    @@ Metadata
     Author: Ramsay Jones <ramsay@ramsayjones.plus.com>
     
      ## Commit message ##
    -    config.mak.uname; add arc4random to the cygwin build
    +    config.mak.uname: add arc4random to the cygwin build
     
         The arc4random_buf() function has been available in cygwin since
         about 2016 (somewhere in the v2.x branch). Set the CSPRNG_METHOD
12:  847ca6b16c ! 13:  7b133af24b config.mak.uname: add a note about CSPRNG_METHOD for Linux
    @@ Metadata
     Author: Ramsay Jones <ramsay@ramsayjones.plus.com>
     
      ## Commit message ##
    -    config.mak.uname: add a note about CSPRNG_METHOD for Linux
    +    config.mak.uname: set CSPRNG_METHOD to getrandom on Linux
     
         Commit 05cd988dce ("wrapper: add a helper to generate numbers from a
         CSPRNG", 2022-01-17) added a csprng_bytes() function which used one
    @@ Commit message
         Commit 05cd988dce did not set CSPRNG_METHOD in the Linux section of
         the config.mak.uname file, so it defaults to using '/dev/urandom' as
         the source of random bytes. The 'backend' values which could be used
    -    on Linux, in order of preference, are 'arc4random', 'getrandom' or
    -    'getentropy' ('openssl' is an option, but seems to be discouraged).
    +    on Linux are 'arc4random', 'getrandom' or 'getentropy' ('openssl' is
    +    an option, but seems to be discouraged).
     
         The arc4random routines (ar4random_buf() is the one actually used) were
         added to glibc in version 2.36, while both getrandom() and getentropy()
         were included in 2.25. So, some of the more up-to-date distributions of
    -    Linux (eg Debian 12, Ubuntu 24.04) would be able to use the preferred
    -    'arc4random' setting.
    +    Linux (eg Debian 12, Ubuntu 24.04) would be able to use the 'arc4random'
    +    setting. All currently supported distributions have glibc 2.25 or later
    +    (RHEL 8 has v2.28) and, therefore, have support for the 'getrandom' and
    +    'getentropy' settings.
    +
    +    The arc4random routines on the *BSDs (along with cygwin) implement the
    +    ChaCha20 stream cipher algorithm (see RFC8439) in userspace, rather than
    +    as a system call, and are thus somewhat faster (having avoided a context
    +    switch to the kernel). In contrast, on Linux all three functions are
    +    simple wrappers around the same kernel CSPRNG syscall.
     
         If the meson build system is used on a newer platform, then they will be
         configured to use 'arc4random', whereas the make build will currently
    -    default to using '/dev/urandom'. Add a note to the config.mak.uname file,
    -    in the Linux section, to prompt make users to override CSPRNG_METHOD in
    -    the config.mak file, if appropriate.
    +    default to using '/dev/urandom' on Linux. Since there is no advantage,
    +    in terms of performance, to the 'arc4random' setting, the 'getrandom'
    +    setting should be preferred from an availability perspective. (Also, the
    +    current uses of csprng_bytes() are not in any hot path).
    +
    +    In order to set an appropriate default, set the CSPRNG_METHOD build
    +    variable to 'getrandom' in the Linux section of the 'config.mak.uname'
    +    file.
     
      ## config.mak.uname ##
     @@ config.mak.uname: ifeq ($(uname_S),Linux)
      	HAVE_ALLOCA_H = YesPlease
      	# override in config.mak if you have glibc >= 2.38
      	NO_STRLCPY = YesPlease
    -+	# set to arc4random (in config.mak) if you have glibc >= 2.36
    -+	CSPRNG_METHOD =
    ++	CSPRNG_METHOD = getrandom
      	HAVE_PATHS_H = YesPlease
      	LIBC_CONTAINS_LIBINTL = YesPlease
      	HAVE_DEV_TTY = YesPlease
-- 
2.49.0

----------------- >8 ------------------ [syms.pl]
#!/usr/bin/perl -w

open(I, "-|", qw(nm -g), $ARGV[0]) or die;
while (<I>) {
	unless (/^[0-9a-f ]+ ([A-Z]) (\S*)$/) {
		print STDERR "? $_";
		next;
	}
	next if ($2 =~ /^\.refptr\./);
	print "$1 -- $2\n";
}
close I;

[PATCH v2 01/13] meson.build: remove -DCURL_DISABLE_TYPECHECK

[Ramsay Jones]

Commit 9371322a60 ("sparse: suppress some \"using sizeof on a function\"
warnings", 2013-10-06) used target-specific variable assignments to add
-DCURL_DISABLE_TYPECHECK to SPARSE_FLAGS for each of the files affected
by the "typecheck-gcc.h" warnings. (http-push.c, http.c, http-walker.c
and remote-curl.c).

These warnings are only issued by sparse, and not by gcc, so we do not
want to disable the 'type checking' for non-sparse targets. The meson
build does not provide any sparse targets, so there is no need to use
the CURL_DISABLE_TYPECHECK preprocessor flag with the c compiler.

In order to re-enable the curl 'type checking' in the meson build, remove
the assignment of -DCURL_DISABLE_TYPECHECK to libgit_c_args.

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

diff --git a/meson.build b/meson.build
index efe2871c9d..88a29fd043 100644
--- a/meson.build
+++ b/meson.build
@@ -966,7 +966,6 @@ if curl.found()
   # Most executables don't have to link against libcurl, but we still need its
   # include directories so that we can resolve LIBCURL_VERSION in "help.c".
   libgit_dependencies += curl.partial_dependency(includes: true)
-  libgit_c_args += '-DCURL_DISABLE_TYPECHECK'
   build_options_config.set('NO_CURL', '')
 else
   libgit_c_args += '-DNO_CURL'
-- 
2.49.0

[PATCH v2 02/13] Makefile: only set some BASIC_CFLAGS when RUNTIME_PREFIX is set

[Ramsay Jones]

Several build variables only have any meaning when the RUNTIME_PREFIX
variable has been set. In particular, the following build variables are
otherwise ignored:

    HAVE_BSD_KERN_PROC_SYSCTL
    PROCFS_EXECUTABLE_PATH
    HAVE_NS_GET_EXECUTABLE_PATH
    HAVE_ZOS_GET_EXECUTABLE_PATH
    HAVE_WPGMPTR

Make setting BASIC_CFLAGS, for each of these variables, conditional on
the RUNTIME_PREFIX being defined.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 Makefile | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/Makefile b/Makefile
index 7315507381..818dde32a9 100644
--- a/Makefile
+++ b/Makefile
@@ -2179,10 +2179,6 @@ ifdef HAVE_BSD_SYSCTL
 	BASIC_CFLAGS += -DHAVE_BSD_SYSCTL
 endif
 
-ifdef HAVE_BSD_KERN_PROC_SYSCTL
-	BASIC_CFLAGS += -DHAVE_BSD_KERN_PROC_SYSCTL
-endif
-
 ifdef HAVE_GETDELIM
 	BASIC_CFLAGS += -DHAVE_GETDELIM
 endif
@@ -2213,25 +2209,33 @@ ifneq ($(findstring openssl,$(CSPRNG_METHOD)),)
 	EXTLIBS += -lcrypto -lssl
 endif
 
-ifneq ($(PROCFS_EXECUTABLE_PATH),)
-	procfs_executable_path_SQ = $(subst ','\'',$(PROCFS_EXECUTABLE_PATH))
-	BASIC_CFLAGS += '-DPROCFS_EXECUTABLE_PATH="$(procfs_executable_path_SQ)"'
-endif
-
 ifndef HAVE_PLATFORM_PROCINFO
 	COMPAT_OBJS += compat/stub/procinfo.o
 endif
 
-ifdef HAVE_NS_GET_EXECUTABLE_PATH
-	BASIC_CFLAGS += -DHAVE_NS_GET_EXECUTABLE_PATH
-endif
+ifdef RUNTIME_PREFIX
 
-ifdef HAVE_ZOS_GET_EXECUTABLE_PATH
-        BASIC_CFLAGS += -DHAVE_ZOS_GET_EXECUTABLE_PATH
-endif
+        ifdef HAVE_BSD_KERN_PROC_SYSCTL
+		BASIC_CFLAGS += -DHAVE_BSD_KERN_PROC_SYSCTL
+        endif
+
+        ifneq ($(PROCFS_EXECUTABLE_PATH),)
+		pep_SQ = $(subst ','\'',$(PROCFS_EXECUTABLE_PATH))
+		BASIC_CFLAGS += '-DPROCFS_EXECUTABLE_PATH="$(pep_SQ)"'
+        endif
+
+        ifdef HAVE_NS_GET_EXECUTABLE_PATH
+		BASIC_CFLAGS += -DHAVE_NS_GET_EXECUTABLE_PATH
+        endif
+
+        ifdef HAVE_ZOS_GET_EXECUTABLE_PATH
+		BASIC_CFLAGS += -DHAVE_ZOS_GET_EXECUTABLE_PATH
+        endif
+
+        ifdef HAVE_WPGMPTR
+		BASIC_CFLAGS += -DHAVE_WPGMPTR
+        endif
 
-ifdef HAVE_WPGMPTR
-	BASIC_CFLAGS += -DHAVE_WPGMPTR
 endif
 
 ifdef FILENO_IS_A_MACRO
-- 
2.49.0

[PATCH v2 03/13] meson.build: only set build variables for non-default values

[Ramsay Jones]

Some preprocessor -Defines have defaults set in the source code when
they have not been provided to the C compiler. In this case, there is
no need to pass them on the command-line, unless the build requires a
non-standard value.

The build variables for DEFAULT_EDITOR and DEFAULT_PAGER have appropriate
defaults ('vi' and 'less') set in the code. Add the preprocessor -Defines
to the 'libgit_c_args' only if the values set with the corresponding
'options' are different to these standard values.

Also, the 'git-var' documentation contains some conditional text which
documents the chosen compiled in value, which would not read well for
the standard values. Similar to the above, only add the corresponding
'-a' attribute arguments to the 'asciidoc_common_options' variable, if
the values set in the 'options' are different to these standard values.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 Documentation/meson.build | 20 ++++++++++++++++++++
 meson.build               | 13 +++++++++++--
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/Documentation/meson.build b/Documentation/meson.build
index 594546d68b..1642b6e2a3 100644
--- a/Documentation/meson.build
+++ b/Documentation/meson.build
@@ -242,6 +242,16 @@ if docs_backend == 'asciidoc'
     '--attribute=build_dir=' + meson.current_build_dir(),
   ]
 
+  pager_opt = get_option('default_pager')
+  if pager_opt != '' and pager_opt != 'less'
+    asciidoc_common_options += '-agit-default-pager=' + pager_opt
+  endif
+
+  editor_opt = get_option('default_editor')
+  if editor_opt != '' and editor_opt != 'vi'
+    asciidoc_common_options += '-agit-default-editor=' + editor_opt
+  endif
+
   documentation_deps = [
     asciidoc_conf,
   ]
@@ -279,6 +289,16 @@ elif docs_backend == 'asciidoctor'
     '--require', 'asciidoctor-extensions',
   ]
 
+  pager_opt = get_option('default_pager')
+  if pager_opt != '' and pager_opt != 'less'
+    asciidoc_common_options += '-agit-default-pager=' + pager_opt
+  endif
+
+  editor_opt = get_option('default_editor')
+  if editor_opt != '' and editor_opt != 'vi'
+    asciidoc_common_options += '-agit-default-editor=' + editor_opt
+  endif
+
   documentation_deps = [
     asciidoctor_extensions,
   ]
diff --git a/meson.build b/meson.build
index 88a29fd043..efd0bd3319 100644
--- a/meson.build
+++ b/meson.build
@@ -693,10 +693,8 @@ endif
 # These variables are used for building libgit.a.
 libgit_c_args = [
   '-DBINDIR="' + get_option('bindir') + '"',
-  '-DDEFAULT_EDITOR="' + get_option('default_editor') + '"',
   '-DDEFAULT_GIT_TEMPLATE_DIR="' + get_option('datadir') / 'git-core/templates' + '"',
   '-DDEFAULT_HELP_FORMAT="' + get_option('default_help_format') + '"',
-  '-DDEFAULT_PAGER="' + get_option('default_pager') + '"',
   '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
   '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
   '-DFALLBACK_RUNTIME_PREFIX="' + get_option('prefix') + '"',
@@ -708,6 +706,17 @@ libgit_c_args = [
   '-DPAGER_ENV="' + get_option('pager_environment') + '"',
   '-DSHELL_PATH="' + fs.as_posix(shell.full_path()) + '"',
 ]
+
+editor_opt = get_option('default_editor')
+if editor_opt != '' and editor_opt != 'vi'
+  libgit_c_args += '-DDEFAULT_EDITOR="' + editor_opt + '"'
+endif
+
+pager_opt = get_option('default_pager')
+if pager_opt != '' and pager_opt != 'less'
+  libgit_c_args += '-DDEFAULT_PAGER="' + pager_opt + '"'
+endif
+
 libgit_include_directories = [ '.' ]
 libgit_dependencies = [ ]
 
-- 
2.49.0

[PATCH v2 04/13] meson.build: set default help format to html on windows

[Ramsay Jones]

The build variable DEFAULT_HELP_FORMAT has an appropriate default
('man') set in the code, so there is no need to pass the -Define on
the compiler command-line, unless the build requires a non-standard
value.

In addition, on windows the make build overrides the default help
format to 'html', rather than 'man', in the 'config.mak.uname' file.

In order to suppress the -Define on the C compiler command-line, only
add the -Define to the 'libgit_c_args' variable when the requested
value is not the standard 'man'. In order to override the default value
on windows, add a 'platform' value to the 'default_help_format' combo
option and set it as the default choice. When this option is set to
'platform', use the 'host_machine.system()' method call to determine the
appropriate default value for the host system.

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

diff --git a/meson.build b/meson.build
index efd0bd3319..8f8a258064 100644
--- a/meson.build
+++ b/meson.build
@@ -694,7 +694,6 @@ endif
 libgit_c_args = [
   '-DBINDIR="' + get_option('bindir') + '"',
   '-DDEFAULT_GIT_TEMPLATE_DIR="' + get_option('datadir') / 'git-core/templates' + '"',
-  '-DDEFAULT_HELP_FORMAT="' + get_option('default_help_format') + '"',
   '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
   '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
   '-DFALLBACK_RUNTIME_PREFIX="' + get_option('prefix') + '"',
@@ -717,6 +716,18 @@ if pager_opt != '' and pager_opt != 'less'
   libgit_c_args += '-DDEFAULT_PAGER="' + pager_opt + '"'
 endif
 
+help_format_opt = get_option('default_help_format')
+if help_format_opt == 'platform'
+  if host_machine.system() == 'windows'
+    help_format_opt = 'html'
+  else
+    help_format_opt = 'man'
+  endif
+endif
+if help_format_opt != 'man'
+    libgit_c_args += '-DDEFAULT_HELP_FORMAT="' + help_format_opt + '"'
+endif
+
 libgit_include_directories = [ '.' ]
 libgit_dependencies = [ ]
 
diff --git a/meson_options.txt b/meson_options.txt
index 78d172a740..8ac30a5223 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -95,7 +95,7 @@ option('highlight_bin', type: 'string', value: 'highlight')
 # Documentation.
 option('docs', type: 'array', choices: ['man', 'html'], value: [],
   description: 'Which documenattion formats to build and install.')
-option('default_help_format', type: 'combo', choices: ['man', 'html'], value: 'man',
+option('default_help_format', type: 'combo', choices: ['man', 'html', 'platform'], value: 'platform',
   description: 'Default format used when executing git-help(1).')
 option('docs_backend', type: 'combo', choices: ['asciidoc', 'asciidoctor', 'auto'], value: 'auto',
   description: 'Which backend to use to generate documentation.')
-- 
2.49.0

[PATCH v2 05/13] Makefile: remove NEEDS_LIBRT build variable

[Ramsay Jones]

Commit d19e3a5b21 ("Makefile: add NEEDS_LIBRT to optionally link with
librt", 2016-07-07) introduced the NEEDS_LIBRT build variable to
disassociate the HAVE_CLOCK_GETTIME variable with the unconditional
linking of the librt library. At one time, the clock_gettime() function
was not available as part of the libc library and (on some unix systems)
required linking with librt.

Commit 52fcec75ce ("config.mak.uname: define NEEDS_LIBRT under Linux, for
now", 2016-07-10) set the NEEDS_LIBRT variable in the Linux section of
the config.mak.uname file, since Debian 7 (wheezy) was one of the few
remaining distributions, with glibc 2.13, that required linking with
librt for clock_gettime(). Note that from glibc version 2.17, this is no
longer necessary.

Note that Debian 7.0 was released on May 4th, 2013 and benefited from
long term support until May 2018 when it went end-of-life. Since that
time, Linux distributions use a more up-to-date library, for example:

    Distribution   version  end of support

    Debian 8       2.19     30th June 2020
    RHEL   8       2.28     31st May  2024 *
    Ubuntu 16.04   2.23     30th Apr  2021

* paid 'Maintenance support' ends 31st May 2029

Since it is no longer required, remove NEEDS_LIBRT from the Makefile and
config.mak.uname.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 Makefile         | 7 -------
 config.mak.uname | 2 --
 2 files changed, 9 deletions(-)

diff --git a/Makefile b/Makefile
index 818dde32a9..8fa4d2664e 100644
--- a/Makefile
+++ b/Makefile
@@ -340,9 +340,6 @@ include shared.mak
 #
 # Define HAVE_SYNC_FILE_RANGE if your platform has sync_file_range.
 #
-# Define NEEDS_LIBRT if your platform requires linking with librt (glibc version
-# before 2.17) for clock_gettime and CLOCK_MONOTONIC.
-#
 # Define HAVE_BSD_SYSCTL if your platform has a BSD-compatible sysctl function.
 #
 # Define HAVE_GETDELIM if your system has the getdelim() function.
@@ -2171,10 +2168,6 @@ ifdef HAVE_SYNC_FILE_RANGE
 	BASIC_CFLAGS += -DHAVE_SYNC_FILE_RANGE
 endif
 
-ifdef NEEDS_LIBRT
-	EXTLIBS += -lrt
-endif
-
 ifdef HAVE_BSD_SYSCTL
 	BASIC_CFLAGS += -DHAVE_BSD_SYSCTL
 endif
diff --git a/config.mak.uname b/config.mak.uname
index b12d4e168a..88007c4f13 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -54,8 +54,6 @@ ifeq ($(uname_S),Linux)
 	HAVE_DEV_TTY = YesPlease
 	HAVE_CLOCK_GETTIME = YesPlease
 	HAVE_CLOCK_MONOTONIC = YesPlease
-	# -lrt is needed for clock_gettime on glibc <= 2.16
-	NEEDS_LIBRT = YesPlease
 	HAVE_SYNC_FILE_RANGE = YesPlease
 	HAVE_GETDELIM = YesPlease
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
-- 
2.49.0

[PATCH v2 06/13] config.mak.uname: add a note about NO_STRLCPY for Linux

[Ramsay Jones]

Commit 817151e61a ("Rename safe_strncpy() to strlcpy().", 2006-06-24)
added the NO_STRLCPY make variable to allow the conditional use of
the gitstrlcpy() compat function on those platforms which didn't
provide the 'standard' strlcpy() function.

Recently, in the summer of 2023, the strlcpy() and strlcat() functions
were added to the glibc library (v2.38), so some of the more up-to-date
Linux distributions no longer need to set NO_STRLCPY. For example, both
Ubuntu 24.04 LTS and RHEL 10 beta have glibc v2.39. However, several
distributions, which are still within their support window, have an
earlier version and must still use the 'compat' version of strlcpy().

If the meson or autoconf build systems are used on newer platforms, then
they will be configured to to use strlcpy() from glibc, whereas the make
build will always choose the 'compat' function instead. Add a note to
the config.mak.uname file, in the Linux section, to prompt make users to
override NO_STRLCPY in the config.mak file, if appropriate.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 config.mak.uname | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.mak.uname b/config.mak.uname
index 88007c4f13..ae6ba15586 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -48,6 +48,7 @@ ifeq ($(uname_S),OSF1)
 endif
 ifeq ($(uname_S),Linux)
 	HAVE_ALLOCA_H = YesPlease
+	# override in config.mak if you have glibc >= 2.38
 	NO_STRLCPY = YesPlease
 	HAVE_PATHS_H = YesPlease
 	LIBC_CONTAINS_LIBINTL = YesPlease
-- 
2.49.0

[PATCH v2 07/13] config.mak.uname: only set NO_REGEX on cygwin for v1.7

[Ramsay Jones]

Commit 92f63d2b05 ("Cygwin 1.7 needs compat/regex", 2013-07-19) set
the NO_REGEX build variable because the platform regex library failed
some of the tests (t4018 and t4034), which passed just fine with the
compat library.

After some time (maybe a year or two), the platform library had been
updated (with an import from FreeBSD, I believe) and now passed the full
test-suite. This would be about the time of the v1.7 -> v2.0 transition
in 2015. I had a patch ready to send, but just didn't get around to
submitting it to the list. At some point in the interim, the official
cygwin git package used the autoconf build system, which sets the
NO_REGEX variable to use the platform regex library functions. The new
meson build system does likewise.

The cygwin platform regex library, in addition to now passing the tests
which formerly failed, now passes an 'test_expect_failure' test in the
t7815-grep-binary test file. In particular, test #12 'git grep .fi a'
which determines that the regex pattern '.' matches a NUL character.
The commit f96e56733a ("grep: use REG_STARTEND for all matching if
available", 2010-05-22) added the test in question, but it does not
give any indication as to why the test was framed as an expected fail,
rather than a 'positive' test that the 'git grep' command fails to
match a NUL. Note that the previous test #11 was also originally
marked in that commit as a 'test_expect_failure', but was flipped to
an 'success' test in commit 7e36de5859 ("t/t7008-grep-binary.sh: un-TODO
a test that needs REG_STARTEND", 2010-08-17).

In order to produce the same NO_REGEX configuration from autoconf, meson
and make, modify config.mak.uname to only set NO_REGEX for cygwin v1.7.
In addition, skip test t7815.12 on cygwin, by adding the !CYGWIN pre-
requisite to the test header, which (among other things) removes an
'...; please update test(s)' comment.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 config.mak.uname       | 4 +++-
 t/t7815-grep-binary.sh | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/config.mak.uname b/config.mak.uname
index ae6ba15586..b6adce0bc4 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -245,7 +245,9 @@ ifeq ($(uname_O),Cygwin)
 		# Try commenting this out if you suspect MMAP is more efficient
 		NO_MMAP = YesPlease
         else
-		NO_REGEX = UnfortunatelyYes
+                ifeq ($(shell expr "$(uname_R)" : '1\.7\.'),4)
+		        NO_REGEX = UnfortunatelyYes
+                endif
         endif
 	HAVE_DEV_TTY = YesPlease
 	HAVE_ALLOCA_H = YesPlease
diff --git a/t/t7815-grep-binary.sh b/t/t7815-grep-binary.sh
index 90ebb64f46..b7000dfa32 100755
--- a/t/t7815-grep-binary.sh
+++ b/t/t7815-grep-binary.sh
@@ -63,7 +63,7 @@ test_expect_success 'git grep ile a' '
 	git grep ile a
 '
 
-test_expect_failure 'git grep .fi a' '
+test_expect_failure !CYGWIN 'git grep .fi a' '
 	git grep .fi a
 '
 
-- 
2.49.0

[PATCH v2 08/13] config.mak.uname: add HAVE_GETDELIM to the cygwin section

[Ramsay Jones]

Cygwin has provided the getdelim() function as far back as (at least)
2011. The autoconf and meson builds enable the use of this symbol.
In order to have the same configuration for autoconf, meson and make,
enable the HAVE_GETDELIM build variable in the cygwin section of the
config.mak.uname file.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 config.mak.uname | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.mak.uname b/config.mak.uname
index b6adce0bc4..1b3ba8a341 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -250,6 +250,7 @@ ifeq ($(uname_O),Cygwin)
                 endif
         endif
 	HAVE_DEV_TTY = YesPlease
+	HAVE_GETDELIM = YesPlease
 	HAVE_ALLOCA_H = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
-- 
2.49.0

[PATCH v2 09/13] config.mak.uname: add clock_gettime() to the cygwin build

[Ramsay Jones]

Cygwin supports the clock_gettime() function, along with the associated
CLOCK_MONOTONIC preprocessor symbol. The autoconf and meson builds both
enable the use of those symbols. In order to have the same configuration
for the make builds, add the HAVE_CLOCK_GETTIME and HAVE_CLOCK_MONOTONIC
build variables to the cygwin section of the config.mak.uname file.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 config.mak.uname | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/config.mak.uname b/config.mak.uname
index 1b3ba8a341..942550fb9e 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -251,6 +251,8 @@ ifeq ($(uname_O),Cygwin)
         endif
 	HAVE_DEV_TTY = YesPlease
 	HAVE_GETDELIM = YesPlease
+	HAVE_CLOCK_GETTIME=YesPlease
+	HAVE_CLOCK_MONOTONIC=YesPlease
 	HAVE_ALLOCA_H = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
-- 
2.49.0

[PATCH v2 10/13] builtin/gc.c: correct RAM calculation when using sysinfo

[Ramsay Jones]

The man page for sysinfo(2) on Linux states that (from v2.3.48) the
sizes of the memory and swap fields, of the returned structure, are
given as multiples of 'mem_unit' bytes. In earlier versions (prior to
v2.3.23 on i386 in particular), the 'mem_unit' field was not part of
the structure, and all sizes were measured in bytes. The man page does
not discuss the motivation for this change, but it is possible that the
change was intended for the, relatively rare, 32-bit platform with more
than 4GB of memory.

The total_ram() function makes the assumption that the 'totalram' field
of the 'struct sysinfo' is measured in bytes, or alternatively that the
'mem_unit' field is always equal to one. Having writen a program to call
the sysinfo() function and print the structure fields, it seems that, on
Linux x84_64 and i686 anyway, the 'mem_unit' field is indeed set to one
(note that the 32-bit system had only 2GB ram). However, cygwin also has
an sysinfo() implementation, which gives the following values:

  $ ./sysinfo
  uptime:      21381
  loads:       0, 0, 0
  total ram:   2074637
  free ram:    843237
  shared ram:  0
  buffer ram:  0
  total swap:  327680
  free swap:   306932
  procs:       15
  total high:  0
  free high:   0
  mem_unit:    4096

  total ram: 8497713152
  $

[This laptop has 8GB ram, so a little bit seems to be missing. ;) ]

Modify the total_ram() function to allow for the possibility that the
memory size is not specified in bytes (ie 'mem_unit' is greater than
one).

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 builtin/gc.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 99431fd467..cdcf1dc6e7 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -373,8 +373,13 @@ static uint64_t total_ram(void)
 #if defined(HAVE_SYSINFO)
 	struct sysinfo si;
 
-	if (!sysinfo(&si))
-		return si.totalram;
+	if (!sysinfo(&si)) {
+		uint64_t total = si.totalram;
+
+		if (si.mem_unit > 1)
+			total *= (uint64_t)si.mem_unit;
+		return total;
+	}
 #elif defined(HAVE_BSD_SYSCTL) && (defined(HW_MEMSIZE) || defined(HW_PHYSMEM))
 	int64_t physical_memory;
 	int mib[2];
-- 
2.49.0

[PATCH v2 11/13] config.mak.uname: add sysinfo() configuration for cygwin

[Ramsay Jones]

Although sysinfo() is a 'Linux only' function, cygwin provides an
implementation which appears to be functional. The assumption that
this function is Linux only is reflected in the way the HAVE_SYSINFO
build variable is handled by the Makefile and config.mak.uname.

Rework the setting of HAVE_SYSINFO in the Linux section of the system
specific config file, along with the corresponding setting of the
BASIC_CFLAGS in the Makefile. Add the setting of HAVE_SYSINFO to the
cygwin section of 'config.mak.uname'. While here, add a test for the
sysinfo() function to the autoconf build system.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 Makefile         | 4 ++++
 config.mak.uname | 3 ++-
 configure.ac     | 8 ++++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 8fa4d2664e..ff89f2b81f 100644
--- a/Makefile
+++ b/Makefile
@@ -2168,6 +2168,10 @@ ifdef HAVE_SYNC_FILE_RANGE
 	BASIC_CFLAGS += -DHAVE_SYNC_FILE_RANGE
 endif
 
+ifdef HAVE_SYSINFO
+	BASIC_CFLAGS += -DHAVE_SYSINFO
+endif
+
 ifdef HAVE_BSD_SYSCTL
 	BASIC_CFLAGS += -DHAVE_BSD_SYSCTL
 endif
diff --git a/config.mak.uname b/config.mak.uname
index 942550fb9e..0e29341056 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -58,7 +58,7 @@ ifeq ($(uname_S),Linux)
 	HAVE_SYNC_FILE_RANGE = YesPlease
 	HAVE_GETDELIM = YesPlease
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
-	BASIC_CFLAGS += -DHAVE_SYSINFO
+	HAVE_SYSINFO = YesPlease
 	PROCFS_EXECUTABLE_PATH = /proc/self/exe
 	HAVE_PLATFORM_PROCINFO = YesPlease
 	COMPAT_OBJS += compat/linux/procinfo.o
@@ -253,6 +253,7 @@ ifeq ($(uname_O),Cygwin)
 	HAVE_GETDELIM = YesPlease
 	HAVE_CLOCK_GETTIME=YesPlease
 	HAVE_CLOCK_MONOTONIC=YesPlease
+	HAVE_SYSINFO = YesPlease
 	HAVE_ALLOCA_H = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
diff --git a/configure.ac b/configure.ac
index 5923edc44a..d7e0503f1e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1066,6 +1066,14 @@ AC_CHECK_LIB([iconv], [locale_charset],
        [AC_CHECK_LIB([charset], [locale_charset],
                      [CHARSET_LIB=-lcharset])])
 GIT_CONF_SUBST([CHARSET_LIB])
+
+#
+# Define HAVE_SYSINFO=YesPlease if sysinfo is available.
+GIT_CHECK_FUNC(sysinfo,
+	[HAVE_SYSINFO=YesPlease],
+	[HAVE_SYSINFO=])
+GIT_CONF_SUBST([HAVE_SYSINFO])
+
 #
 # Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
 GIT_CHECK_FUNC(clock_gettime,
-- 
2.49.0

[PATCH v2 12/13] config.mak.uname: add arc4random to the cygwin build

[Ramsay Jones]

The arc4random_buf() function has been available in cygwin since
about 2016 (somewhere in the v2.x branch). Set the CSPRNG_METHOD
build variable to 'arc4random', in the cygwin section, to enable
the use of this cryptographically-secure pseudorandom number
function. Note that the autoconf and new meson builds also enable
this function.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 config.mak.uname | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.mak.uname b/config.mak.uname
index 0e29341056..4f6770a5f4 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -254,6 +254,7 @@ ifeq ($(uname_O),Cygwin)
 	HAVE_CLOCK_GETTIME=YesPlease
 	HAVE_CLOCK_MONOTONIC=YesPlease
 	HAVE_SYSINFO = YesPlease
+	CSPRNG_METHOD = arc4random
 	HAVE_ALLOCA_H = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
-- 
2.49.0

[PATCH v2 13/13] config.mak.uname: set CSPRNG_METHOD to getrandom on Linux

[Ramsay Jones]

Commit 05cd988dce ("wrapper: add a helper to generate numbers from a
CSPRNG", 2022-01-17) added a csprng_bytes() function which used one
of several interfaces to provide a source of cryptographically secure
pseudorandom numbers. The CSPRNG_METHOD make variable was provided to
determine the choice of available 'backends' for the source of random
bytes.

Commit 05cd988dce did not set CSPRNG_METHOD in the Linux section of
the config.mak.uname file, so it defaults to using '/dev/urandom' as
the source of random bytes. The 'backend' values which could be used
on Linux are 'arc4random', 'getrandom' or 'getentropy' ('openssl' is
an option, but seems to be discouraged).

The arc4random routines (ar4random_buf() is the one actually used) were
added to glibc in version 2.36, while both getrandom() and getentropy()
were included in 2.25. So, some of the more up-to-date distributions of
Linux (eg Debian 12, Ubuntu 24.04) would be able to use the 'arc4random'
setting. All currently supported distributions have glibc 2.25 or later
(RHEL 8 has v2.28) and, therefore, have support for the 'getrandom' and
'getentropy' settings.

The arc4random routines on the *BSDs (along with cygwin) implement the
ChaCha20 stream cipher algorithm (see RFC8439) in userspace, rather than
as a system call, and are thus somewhat faster (having avoided a context
switch to the kernel). In contrast, on Linux all three functions are
simple wrappers around the same kernel CSPRNG syscall.

If the meson build system is used on a newer platform, then they will be
configured to use 'arc4random', whereas the make build will currently
default to using '/dev/urandom' on Linux. Since there is no advantage,
in terms of performance, to the 'arc4random' setting, the 'getrandom'
setting should be preferred from an availability perspective. (Also, the
current uses of csprng_bytes() are not in any hot path).

In order to set an appropriate default, set the CSPRNG_METHOD build
variable to 'getrandom' in the Linux section of the 'config.mak.uname'
file.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 config.mak.uname | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.mak.uname b/config.mak.uname
index 4f6770a5f4..6ebe6dcf37 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -50,6 +50,7 @@ ifeq ($(uname_S),Linux)
 	HAVE_ALLOCA_H = YesPlease
 	# override in config.mak if you have glibc >= 2.38
 	NO_STRLCPY = YesPlease
+	CSPRNG_METHOD = getrandom
 	HAVE_PATHS_H = YesPlease
 	LIBC_CONTAINS_LIBINTL = YesPlease
 	HAVE_DEV_TTY = YesPlease
-- 
2.49.0

Re: [PATCH v2 03/13] meson.build: only set build variables for non-default values

[Ramsay Jones]

On 06/04/2025 20:38, Ramsay Jones wrote:
[snip]
> diff --git a/meson.build b/meson.build
> index 88a29fd043..efd0bd3319 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -693,10 +693,8 @@ endif
>  # These variables are used for building libgit.a.
>  libgit_c_args = [
>    '-DBINDIR="' + get_option('bindir') + '"',
> -  '-DDEFAULT_EDITOR="' + get_option('default_editor') + '"',
>    '-DDEFAULT_GIT_TEMPLATE_DIR="' + get_option('datadir') / 'git-core/templates' + '"',
>    '-DDEFAULT_HELP_FORMAT="' + get_option('default_help_format') + '"',
> -  '-DDEFAULT_PAGER="' + get_option('default_pager') + '"',
>    '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
>    '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
>    '-DFALLBACK_RUNTIME_PREFIX="' + get_option('prefix') + '"',
> @@ -708,6 +706,17 @@ libgit_c_args = [
>    '-DPAGER_ENV="' + get_option('pager_environment') + '"',
>    '-DSHELL_PATH="' + fs.as_posix(shell.full_path()) + '"',
>  ]
> +
> +editor_opt = get_option('default_editor')
> +if editor_opt != '' and editor_opt != 'vi'
> +  libgit_c_args += '-DDEFAULT_EDITOR="' + editor_opt + '"'
> +endif
> +
> +pager_opt = get_option('default_pager')
> +if pager_opt != '' and pager_opt != 'less'
> +  libgit_c_args += '-DDEFAULT_PAGER="' + pager_opt + '"'
> +endif
> +
>  libgit_include_directories = [ '.' ]
>  libgit_dependencies = [ ]
>  


It would be somewhat remiss of me to not mention here that this does not
work for any but the simplest of values! :( If you set a simple single
'bareword' like 'vim' or 'more' (even '~/bin/vi') then every thing works
just fine. However, if the value contains any of (at least) the following
characters: single quote, double quote or backslash, then things
stop working!

[I spent one whole evening (and a bit - always something else to 'try')
trying to 'fix' this problem, without success]

If you try an example that is given in the Makefile:312, then the make build:

  $ make V=1 DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork' all doc >m-out 2>&1

passes the folowing arguments to (respectively) gcc and asciidoc:

  -DDEFAULT_EDITOR='"\"C:\\Program Files\\Vim\\gvim.exe\" --nofork"'
  -a 'git-default-editor="C:\Program Files\Vim\gvim.exe" --nofork'

whereas, the meson build:

  $ meson setup --optimization=2 -Ddocs=man,html -Ddefault_editor='"C:\Program Files\Vim\gvim.exe" --nofork' -Ddefault_pager=more -Dprefix=$HOME -Dpcre2=disabled build/
  The Meson build system
  ...
   User defined options
      optimization  : 2
      prefix        : /home/ramsay
      default_editor: "C:\Program Files\Vim\gvim.exe" --nofork
      default_pager : more
      docs          : man,html
      pcre2         : disabled

  Found ninja-1.11.1 at /usr/bin/ninja
  $ 

similarly, passes the folowing arguments to (respectively) gcc and asciidoc:

  '-DDEFAULT_EDITOR=""C:\\Program Files\\Vim\\gvim.exe" --nofork"'
  '-agit-default-editor="C:/Program Files/Vim/gvim.exe" --nofork'

If you now attempt a 'meson compile' it will, of course, fail to compile
editor.c because the DEFAULT_EDITOR is the empty string ("") followed by
(C:\\Program Files\\Vim\\gvim.exe" --nofork").

Also, note that the directory seperators have changed from \\ to / in the
argument to asciidoc.

[Again, spelunking the docs for meson, it said that "if you want quotes, you
will have to do it yourself"! ;) ]

OK, so I couldn't come up with any incantation which would fix this issue.
I will have to admit defeat and ask someone who actually knows meson to
fix it. :(

Thanks.

ATB,
Ramsay Jones

Re: [PATCH v2 04/13] meson.build: set default help format to html on windows

[Ramsay Jones]

On 06/04/2025 20:38, Ramsay Jones wrote:
> The build variable DEFAULT_HELP_FORMAT has an appropriate default
> ('man') set in the code, so there is no need to pass the -Define on
> the compiler command-line, unless the build requires a non-standard
> value.
> 
> In addition, on windows the make build overrides the default help
> format to 'html', rather than 'man', in the 'config.mak.uname' file.
> 
> In order to suppress the -Define on the C compiler command-line, only
> add the -Define to the 'libgit_c_args' variable when the requested
> value is not the standard 'man'. In order to override the default value
> on windows, add a 'platform' value to the 'default_help_format' combo
> option and set it as the default choice. When this option is set to
> 'platform', use the 'host_machine.system()' method call to determine the
> appropriate default value for the host system.
> 
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
>  meson.build       | 13 ++++++++++++-
>  meson_options.txt |  2 +-
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index efd0bd3319..8f8a258064 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -694,7 +694,6 @@ endif
>  libgit_c_args = [
>    '-DBINDIR="' + get_option('bindir') + '"',
>    '-DDEFAULT_GIT_TEMPLATE_DIR="' + get_option('datadir') / 'git-core/templates' + '"',
> -  '-DDEFAULT_HELP_FORMAT="' + get_option('default_help_format') + '"',
>    '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
>    '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
>    '-DFALLBACK_RUNTIME_PREFIX="' + get_option('prefix') + '"',
> @@ -717,6 +716,18 @@ if pager_opt != '' and pager_opt != 'less'
>    libgit_c_args += '-DDEFAULT_PAGER="' + pager_opt + '"'
>  endif
>  
> +help_format_opt = get_option('default_help_format')
> +if help_format_opt == 'platform'
> +  if host_machine.system() == 'windows'
> +    help_format_opt = 'html'
> +  else
> +    help_format_opt = 'man'
> +  endif
> +endif
> +if help_format_opt != 'man'
> +    libgit_c_args += '-DDEFAULT_HELP_FORMAT="' + help_format_opt + '"'
> +endif
> +

Note that host_machine.system() classifies 'cygwin' to include cygwin and
MSYS2 and 'windows' as windows but not cygwin or MSYS2! ;) The make build
overrides the help format to 'html' for windows and MINGW{32,64}, so it is
not clear to me if the test above includes MINGW.

Also, in general cygwin != MSYS2 != MINGW{32,6} != Gfw flavour MSYS2, but
it appears meson does not allow you to obtain such specific system info
(for example, by executing uname directly).

[Just a note that the make build includes 'info' as a choice, but Patrick
made the decision earlier to drop that for the meson build. Also, there is
a 'pdf' make target (I think that made it once, years ago ...)]

Thanks.

ATB,
Ramsay Jones

Re: [PATCH v2 02/13] Makefile: only set some BASIC_CFLAGS when RUNTIME_PREFIX is set

[Patrick Steinhardt]

On Sun, Apr 06, 2025 at 08:38:28PM +0100, Ramsay Jones wrote:
> Several build variables only have any meaning when the RUNTIME_PREFIX
> variable has been set. In particular, the following build variables are
> otherwise ignored:
> 
>     HAVE_BSD_KERN_PROC_SYSCTL
>     PROCFS_EXECUTABLE_PATH
>     HAVE_NS_GET_EXECUTABLE_PATH
>     HAVE_ZOS_GET_EXECUTABLE_PATH
>     HAVE_WPGMPTR
> 
> Make setting BASIC_CFLAGS, for each of these variables, conditional on
> the RUNTIME_PREFIX being defined.

Makes sense. We already handle all of these conditionally in Meson, so
there's nothing else we need to do there.

Patrick

Re: [PATCH v2 03/13] meson.build: only set build variables for non-default values

[Patrick Steinhardt]

On Sun, Apr 06, 2025 at 08:49:54PM +0100, Ramsay Jones wrote:
> 
> 
> On 06/04/2025 20:38, Ramsay Jones wrote:
> [snip]
> > diff --git a/meson.build b/meson.build
> > index 88a29fd043..efd0bd3319 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -693,10 +693,8 @@ endif
> >  # These variables are used for building libgit.a.
> >  libgit_c_args = [
> >    '-DBINDIR="' + get_option('bindir') + '"',
> > -  '-DDEFAULT_EDITOR="' + get_option('default_editor') + '"',
> >    '-DDEFAULT_GIT_TEMPLATE_DIR="' + get_option('datadir') / 'git-core/templates' + '"',
> >    '-DDEFAULT_HELP_FORMAT="' + get_option('default_help_format') + '"',
> > -  '-DDEFAULT_PAGER="' + get_option('default_pager') + '"',
> >    '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
> >    '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
> >    '-DFALLBACK_RUNTIME_PREFIX="' + get_option('prefix') + '"',
> > @@ -708,6 +706,17 @@ libgit_c_args = [
> >    '-DPAGER_ENV="' + get_option('pager_environment') + '"',
> >    '-DSHELL_PATH="' + fs.as_posix(shell.full_path()) + '"',
> >  ]
> > +
> > +editor_opt = get_option('default_editor')
> > +if editor_opt != '' and editor_opt != 'vi'
> > +  libgit_c_args += '-DDEFAULT_EDITOR="' + editor_opt + '"'
> > +endif
> > +
> > +pager_opt = get_option('default_pager')
> > +if pager_opt != '' and pager_opt != 'less'
> > +  libgit_c_args += '-DDEFAULT_PAGER="' + pager_opt + '"'
> > +endif
> > +
> >  libgit_include_directories = [ '.' ]
> >  libgit_dependencies = [ ]
> >  
> 
> 
> It would be somewhat remiss of me to not mention here that this does not
> work for any but the simplest of values! :( If you set a simple single
> 'bareword' like 'vim' or 'more' (even '~/bin/vi') then every thing works
> just fine. However, if the value contains any of (at least) the following
> characters: single quote, double quote or backslash, then things
> stop working!
> 
> [I spent one whole evening (and a bit - always something else to 'try')
> trying to 'fix' this problem, without success]

Shouldn't it be possible to escape these values via `.replace()` [1]? I
suspect that you already tried, but wanted to ask anyway :)

Patrick

[1]: https://mesonbuild.com/Reference-manual_elementary_str.html#strreplace

Re: [PATCH v2 04/13] meson.build: set default help format to html on windows

[Patrick Steinhardt]

On Sun, Apr 06, 2025 at 09:16:58PM +0100, Ramsay Jones wrote:
> [Just a note that the make build includes 'info' as a choice, but Patrick
> made the decision earlier to drop that for the meson build. Also, there is
> a 'pdf' make target (I think that made it once, years ago ...)]

Yeah, I just didn't find any distros where either of these were
installed at all, so I didn't bother porting them over. Which doesn't
mean that it has to stay this way if there actually are any users, but
this might also be a good opportunity to gauge interest in some of the
more esoteric parts of our build infra and potentially drop them if
nobody complains.

Patrick

Re: [PATCH v2 07/13] config.mak.uname: only set NO_REGEX on cygwin for v1.7

[Patrick Steinhardt]

On Sun, Apr 06, 2025 at 08:38:33PM +0100, Ramsay Jones wrote:
> Commit 92f63d2b05 ("Cygwin 1.7 needs compat/regex", 2013-07-19) set
> the NO_REGEX build variable because the platform regex library failed
> some of the tests (t4018 and t4034), which passed just fine with the
> compat library.
> 
> After some time (maybe a year or two), the platform library had been
> updated (with an import from FreeBSD, I believe) and now passed the full
> test-suite. This would be about the time of the v1.7 -> v2.0 transition
> in 2015. I had a patch ready to send, but just didn't get around to
> submitting it to the list. At some point in the interim, the official
> cygwin git package used the autoconf build system, which sets the
> NO_REGEX variable to use the platform regex library functions. The new
> meson build system does likewise.
> 
> The cygwin platform regex library, in addition to now passing the tests
> which formerly failed, now passes an 'test_expect_failure' test in the
> t7815-grep-binary test file. In particular, test #12 'git grep .fi a'
> which determines that the regex pattern '.' matches a NUL character.
> The commit f96e56733a ("grep: use REG_STARTEND for all matching if
> available", 2010-05-22) added the test in question, but it does not
> give any indication as to why the test was framed as an expected fail,
> rather than a 'positive' test that the 'git grep' command fails to
> match a NUL. Note that the previous test #11 was also originally
> marked in that commit as a 'test_expect_failure', but was flipped to
> an 'success' test in commit 7e36de5859 ("t/t7008-grep-binary.sh: un-TODO
> a test that needs REG_STARTEND", 2010-08-17).
> 
> In order to produce the same NO_REGEX configuration from autoconf, meson
> and make, modify config.mak.uname to only set NO_REGEX for cygwin v1.7.
> In addition, skip test t7815.12 on cygwin, by adding the !CYGWIN pre-
> requisite to the test header, which (among other things) removes an
> '...; please update test(s)' comment.

Out of curiosity, because I really don't know any better: why do we have
to even care about such oldish Cygwin installations from more than 10
years ago? Wouldn't people generally update Cygwin every once in a while
to have recent packages? Or is there a good reason why we should
continue to support it?

Patrick

Re: [PATCH v2 09/13] config.mak.uname: add clock_gettime() to the cygwin build

[Patrick Steinhardt]

On Sun, Apr 06, 2025 at 08:38:35PM +0100, Ramsay Jones wrote:
> diff --git a/config.mak.uname b/config.mak.uname
> index 1b3ba8a341..942550fb9e 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -251,6 +251,8 @@ ifeq ($(uname_O),Cygwin)
>          endif
>  	HAVE_DEV_TTY = YesPlease
>  	HAVE_GETDELIM = YesPlease
> +	HAVE_CLOCK_GETTIME=YesPlease
> +	HAVE_CLOCK_MONOTONIC=YesPlease

Nit: let's also have spaces around the assignment operator.

>  	HAVE_ALLOCA_H = YesPlease
>  	NEEDS_LIBICONV = YesPlease
>  	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes

Patrick

Re: [PATCH v2 10/13] builtin/gc.c: correct RAM calculation when using sysinfo

[Patrick Steinhardt]

On Sun, Apr 06, 2025 at 08:38:36PM +0100, Ramsay Jones wrote:
> The man page for sysinfo(2) on Linux states that (from v2.3.48) the
> sizes of the memory and swap fields, of the returned structure, are
> given as multiples of 'mem_unit' bytes. In earlier versions (prior to
> v2.3.23 on i386 in particular), the 'mem_unit' field was not part of
> the structure, and all sizes were measured in bytes. The man page does
> not discuss the motivation for this change, but it is possible that the
> change was intended for the, relatively rare, 32-bit platform with more
> than 4GB of memory.
> 
> The total_ram() function makes the assumption that the 'totalram' field
> of the 'struct sysinfo' is measured in bytes, or alternatively that the
> 'mem_unit' field is always equal to one. Having writen a program to call
> the sysinfo() function and print the structure fields, it seems that, on
> Linux x84_64 and i686 anyway, the 'mem_unit' field is indeed set to one
> (note that the 32-bit system had only 2GB ram). However, cygwin also has
> an sysinfo() implementation, which gives the following values:
> 
>   $ ./sysinfo
>   uptime:      21381
>   loads:       0, 0, 0
>   total ram:   2074637
>   free ram:    843237
>   shared ram:  0
>   buffer ram:  0
>   total swap:  327680
>   free swap:   306932
>   procs:       15
>   total high:  0
>   free high:   0
>   mem_unit:    4096
> 
>   total ram: 8497713152
>   $
> 
> [This laptop has 8GB ram, so a little bit seems to be missing. ;) ]

Interesting. I can confirm that `mem_unit` is 1 on my system, so this
does not make a difference here. But my tests on Cygwin show the same
behaviour as on your system, so the patch looks reasonable to me.

> Modify the total_ram() function to allow for the possibility that the
> memory size is not specified in bytes (ie 'mem_unit' is greater than
> one).
> 
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
>  builtin/gc.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 99431fd467..cdcf1dc6e7 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -373,8 +373,13 @@ static uint64_t total_ram(void)
>  #if defined(HAVE_SYSINFO)
>  	struct sysinfo si;
>  
> -	if (!sysinfo(&si))
> -		return si.totalram;
> +	if (!sysinfo(&si)) {
> +		uint64_t total = si.totalram;
> +
> +		if (si.mem_unit > 1)
> +			total *= (uint64_t)si.mem_unit;
> +		return total;
> +	}

I expect that all systems have a proper value for `si.mem_unit` set so
that we could unconditionally multiplicate the fields with one another.
But it doesn't hurt either, so I don't mind the guarding clause.

Patrick

Re: [PATCH v2 11/13] config.mak.uname: add sysinfo() configuration for cygwin

[Patrick Steinhardt]

On Sun, Apr 06, 2025 at 08:38:37PM +0100, Ramsay Jones wrote:
> Although sysinfo() is a 'Linux only' function, cygwin provides an
> implementation which appears to be functional. The assumption that
> this function is Linux only is reflected in the way the HAVE_SYSINFO
> build variable is handled by the Makefile and config.mak.uname.
> 
> Rework the setting of HAVE_SYSINFO in the Linux section of the system
> specific config file, along with the corresponding setting of the
> BASIC_CFLAGS in the Makefile. Add the setting of HAVE_SYSINFO to the
> cygwin section of 'config.mak.uname'. While here, add a test for the
> sysinfo() function to the autoconf build system.

Makes sense. Meson already does this, so it doesn't need to be adjusted.

Patrick

Re: [-SPAM-] Re: [PATCH v2 03/13] meson.build: only set build variables for non-default values

[Ramsay Jones]

On 14/04/2025 08:54, Patrick Steinhardt wrote:
> On Sun, Apr 06, 2025 at 08:49:54PM +0100, Ramsay Jones wrote:
>>
>>
>> On 06/04/2025 20:38, Ramsay Jones wrote:
>> [snip]
>>> diff --git a/meson.build b/meson.build
>>> index 88a29fd043..efd0bd3319 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -693,10 +693,8 @@ endif
>>>  # These variables are used for building libgit.a.
>>>  libgit_c_args = [
>>>    '-DBINDIR="' + get_option('bindir') + '"',
>>> -  '-DDEFAULT_EDITOR="' + get_option('default_editor') + '"',
>>>    '-DDEFAULT_GIT_TEMPLATE_DIR="' + get_option('datadir') / 'git-core/templates' + '"',
>>>    '-DDEFAULT_HELP_FORMAT="' + get_option('default_help_format') + '"',
>>> -  '-DDEFAULT_PAGER="' + get_option('default_pager') + '"',
>>>    '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
>>>    '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
>>>    '-DFALLBACK_RUNTIME_PREFIX="' + get_option('prefix') + '"',
>>> @@ -708,6 +706,17 @@ libgit_c_args = [
>>>    '-DPAGER_ENV="' + get_option('pager_environment') + '"',
>>>    '-DSHELL_PATH="' + fs.as_posix(shell.full_path()) + '"',
>>>  ]
>>> +
>>> +editor_opt = get_option('default_editor')
>>> +if editor_opt != '' and editor_opt != 'vi'
>>> +  libgit_c_args += '-DDEFAULT_EDITOR="' + editor_opt + '"'
>>> +endif
>>> +
>>> +pager_opt = get_option('default_pager')
>>> +if pager_opt != '' and pager_opt != 'less'
>>> +  libgit_c_args += '-DDEFAULT_PAGER="' + pager_opt + '"'
>>> +endif
>>> +
>>>  libgit_include_directories = [ '.' ]
>>>  libgit_dependencies = [ ]
>>>  
>>
>>
>> It would be somewhat remiss of me to not mention here that this does not
>> work for any but the simplest of values! :( If you set a simple single
>> 'bareword' like 'vim' or 'more' (even '~/bin/vi') then every thing works
>> just fine. However, if the value contains any of (at least) the following
>> characters: single quote, double quote or backslash, then things
>> stop working!
>>
>> [I spent one whole evening (and a bit - always something else to 'try')
>> trying to 'fix' this problem, without success]
> 
> Shouldn't it be possible to escape these values via `.replace()` [1]? I
> suspect that you already tried, but wanted to ask anyway :)

Yep. :)

I still haven't studied the meson documentation, but when I searched
for variations of 'quotes', the results showed that '... if you want
quotes, you will have to do it yourself ...'. So, I eventually found
'.replace()' in the 'string operations' section of the docs and tried
to reproduce what the Makefile does (see #2382):


  ifdef DEFAULT_EDITOR
  DEFAULT_EDITOR_CQ = "$(subst ",\",$(subst \,\\,$(DEFAULT_EDITOR)))"
  DEFAULT_EDITOR_CQ_SQ = $(subst ','\'',$(DEFAULT_EDITOR_CQ))

  BASIC_CFLAGS += -DDEFAULT_EDITOR='$(DEFAULT_EDITOR_CQ_SQ)'
  endif

which I translated into (on top of these patches):

  diff --git a/meson.build b/meson.build
  index 8f8a258064..608d665fd3 100644
  --- a/meson.build
  +++ b/meson.build
  @@ -708,7 +708,11 @@ libgit_c_args = [
 
   editor_opt = get_option('default_editor')
   if editor_opt != '' and editor_opt != 'vi'
  -  libgit_c_args += '-DDEFAULT_EDITOR="' + editor_opt + '"'
  +  editor_opt = editor_opt.replace('\\', '\\\\')
  +  editor_opt = editor_opt.replace('"', '\"')
  +  editor_opt = '"' + editor_opt + '"'
  +  editor_opt = editor_opt.replace('\'', '\\\'')
  +  libgit_c_args += '-DDEFAULT_EDITOR=' + editor_opt
   endif
 
[Actually, I think the very first attempt had:

  libgit_c_args += '-DDEFAULT_EDITOR=\'' + editor_opt + '\''

but meson, for some reason, adds a set of ' around the whole
-D argument to gcc, so I got rid of them - but it still didn't
work!]

Along with many, many, *many* such permutations! (trying to debug
this is hard work, with no help from meson).

So, just a little earlier this evening I read an email from Karthik
([PATCH v2 3/4] meson: add support for 'hdr-check') in which he
mentioned a problem with backslashes and referenced a github issue
on the mesonbuild repo [0], which is worth a read. ;)

Sorry I couldn't fix this issue, but it seems to be (in part) an issue
with meson. (Of course the example I used, which is taken directly
from the Makefile, happens to be particularly good at demonstrating
the problem!)

In any event, I think the current patch is a strict improvement, even
if it may need to be updated at a later date. I hope you agree.

Thank you for taking the time to review this series. I think this patch
was the only review comment that required a response - please let me
know, if that is not the case!

Thanks!

ATB,
Ramsay Jones

[0]: https://github.com/mesonbuild/meson/issues/1564

Re: [-SPAM-] Re: [PATCH v2 07/13] config.mak.uname: only set NO_REGEX on cygwin for v1.7

[Ramsay Jones]

On 14/04/2025 08:55, Patrick Steinhardt wrote:
> On Sun, Apr 06, 2025 at 08:38:33PM +0100, Ramsay Jones wrote:
>> Commit 92f63d2b05 ("Cygwin 1.7 needs compat/regex", 2013-07-19) set
>> the NO_REGEX build variable because the platform regex library failed
>> some of the tests (t4018 and t4034), which passed just fine with the
>> compat library.
>>
>> After some time (maybe a year or two), the platform library had been
>> updated (with an import from FreeBSD, I believe) and now passed the full
>> test-suite. This would be about the time of the v1.7 -> v2.0 transition
>> in 2015. I had a patch ready to send, but just didn't get around to
>> submitting it to the list. At some point in the interim, the official
>> cygwin git package used the autoconf build system, which sets the
>> NO_REGEX variable to use the platform regex library functions. The new
>> meson build system does likewise.
>>
>> The cygwin platform regex library, in addition to now passing the tests
>> which formerly failed, now passes an 'test_expect_failure' test in the
>> t7815-grep-binary test file. In particular, test #12 'git grep .fi a'
>> which determines that the regex pattern '.' matches a NUL character.
>> The commit f96e56733a ("grep: use REG_STARTEND for all matching if
>> available", 2010-05-22) added the test in question, but it does not
>> give any indication as to why the test was framed as an expected fail,
>> rather than a 'positive' test that the 'git grep' command fails to
>> match a NUL. Note that the previous test #11 was also originally
>> marked in that commit as a 'test_expect_failure', but was flipped to
>> an 'success' test in commit 7e36de5859 ("t/t7008-grep-binary.sh: un-TODO
>> a test that needs REG_STARTEND", 2010-08-17).
>>
>> In order to produce the same NO_REGEX configuration from autoconf, meson
>> and make, modify config.mak.uname to only set NO_REGEX for cygwin v1.7.
>> In addition, skip test t7815.12 on cygwin, by adding the !CYGWIN pre-
>> requisite to the test header, which (among other things) removes an
>> '...; please update test(s)' comment.
> 
> Out of curiosity, because I really don't know any better: why do we have
> to even care about such oldish Cygwin installations from more than 10
> years ago? Wouldn't people generally update Cygwin every once in a while
> to have recent packages? Or is there a good reason why we should
> continue to support it?

Heh, as I said in response to Junio, I have a patch that removes all
of the config in the conditional, so that we would no longer support
any 'pre-v2.x' versions of cygwin[*]. I think that would be an entirely
reasonable thing to do, particularly as cygwin thinks of itself as
a 'rolling release' type distribution. ;)

However, I don't think it is my place to make that kind of decision
and I was leaving that patch until last. Hopefully, Adam will make
that call. :)

Thanks.

ATB,
Ramsay Jones

[*] Note, even *with* that config in place, I have no idea if pre-v2.x
versions would even compile!

Re: [-SPAM-] Re: [PATCH v2 09/13] config.mak.uname: add clock_gettime() to the cygwin build

[Ramsay Jones]

On 14/04/2025 08:55, Patrick Steinhardt wrote:
> On Sun, Apr 06, 2025 at 08:38:35PM +0100, Ramsay Jones wrote:
>> diff --git a/config.mak.uname b/config.mak.uname
>> index 1b3ba8a341..942550fb9e 100644
>> --- a/config.mak.uname
>> +++ b/config.mak.uname
>> @@ -251,6 +251,8 @@ ifeq ($(uname_O),Cygwin)
>>          endif
>>  	HAVE_DEV_TTY = YesPlease
>>  	HAVE_GETDELIM = YesPlease
>> +	HAVE_CLOCK_GETTIME=YesPlease
>> +	HAVE_CLOCK_MONOTONIC=YesPlease
> 
> Nit: let's also have spaces around the assignment operator.

Ah, yes, I forgot this one. If I need to re-roll, then I can
adjust this.

Thanks.

ATB,
Ramsay Jones

> 
>>  	HAVE_ALLOCA_H = YesPlease
>>  	NEEDS_LIBICONV = YesPlease
>>  	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
> 
> Patrick
> 

Re: [-SPAM-] Re: [PATCH v2 10/13] builtin/gc.c: correct RAM calculation when using sysinfo

[Ramsay Jones]

On 14/04/2025 08:55, Patrick Steinhardt wrote:
> On Sun, Apr 06, 2025 at 08:38:36PM +0100, Ramsay Jones wrote:
>> The man page for sysinfo(2) on Linux states that (from v2.3.48) the
>> sizes of the memory and swap fields, of the returned structure, are
>> given as multiples of 'mem_unit' bytes. In earlier versions (prior to
>> v2.3.23 on i386 in particular), the 'mem_unit' field was not part of
>> the structure, and all sizes were measured in bytes. The man page does
>> not discuss the motivation for this change, but it is possible that the
>> change was intended for the, relatively rare, 32-bit platform with more
>> than 4GB of memory.
>>
>> The total_ram() function makes the assumption that the 'totalram' field
>> of the 'struct sysinfo' is measured in bytes, or alternatively that the
>> 'mem_unit' field is always equal to one. Having writen a program to call
>> the sysinfo() function and print the structure fields, it seems that, on
>> Linux x84_64 and i686 anyway, the 'mem_unit' field is indeed set to one
>> (note that the 32-bit system had only 2GB ram). However, cygwin also has
>> an sysinfo() implementation, which gives the following values:
>>
>>   $ ./sysinfo
>>   uptime:      21381
>>   loads:       0, 0, 0
>>   total ram:   2074637
>>   free ram:    843237
>>   shared ram:  0
>>   buffer ram:  0
>>   total swap:  327680
>>   free swap:   306932
>>   procs:       15
>>   total high:  0
>>   free high:   0
>>   mem_unit:    4096
>>
>>   total ram: 8497713152
>>   $
>>
>> [This laptop has 8GB ram, so a little bit seems to be missing. ;) ]
> 
> Interesting. I can confirm that `mem_unit` is 1 on my system, so this
> does not make a difference here. But my tests on Cygwin show the same
> behaviour as on your system, so the patch looks reasonable to me.
> 
>> Modify the total_ram() function to allow for the possibility that the
>> memory size is not specified in bytes (ie 'mem_unit' is greater than
>> one).
>>
>> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
>> ---
>>  builtin/gc.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/builtin/gc.c b/builtin/gc.c
>> index 99431fd467..cdcf1dc6e7 100644
>> --- a/builtin/gc.c
>> +++ b/builtin/gc.c
>> @@ -373,8 +373,13 @@ static uint64_t total_ram(void)
>>  #if defined(HAVE_SYSINFO)
>>  	struct sysinfo si;
>>  
>> -	if (!sysinfo(&si))
>> -		return si.totalram;
>> +	if (!sysinfo(&si)) {
>> +		uint64_t total = si.totalram;
>> +
>> +		if (si.mem_unit > 1)
>> +			total *= (uint64_t)si.mem_unit;
>> +		return total;
>> +	}
> 
> I expect that all systems have a proper value for `si.mem_unit` set so
> that we could unconditionally multiplicate the fields with one another.
> But it doesn't hurt either, so I don't mind the guarding clause.

Hmm, you may well be right, but I prefer to be cautious here.

Thanks.

ATB,
Ramsay Jones

Re: [-SPAM-] Re: [PATCH v2 03/13] meson.build: only set build variables for non-default values

[Patrick Steinhardt]

On Mon, Apr 14, 2025 at 08:19:15PM +0100, Ramsay Jones wrote:
> 
> 
> On 14/04/2025 08:54, Patrick Steinhardt wrote:
> > On Sun, Apr 06, 2025 at 08:49:54PM +0100, Ramsay Jones wrote:
> >>
> >>
> >> On 06/04/2025 20:38, Ramsay Jones wrote:
> >> [snip]
> >>> diff --git a/meson.build b/meson.build
> >>> index 88a29fd043..efd0bd3319 100644
> >>> --- a/meson.build
> >>> +++ b/meson.build
> >>> @@ -693,10 +693,8 @@ endif
> >>>  # These variables are used for building libgit.a.
> >>>  libgit_c_args = [
> >>>    '-DBINDIR="' + get_option('bindir') + '"',
> >>> -  '-DDEFAULT_EDITOR="' + get_option('default_editor') + '"',
> >>>    '-DDEFAULT_GIT_TEMPLATE_DIR="' + get_option('datadir') / 'git-core/templates' + '"',
> >>>    '-DDEFAULT_HELP_FORMAT="' + get_option('default_help_format') + '"',
> >>> -  '-DDEFAULT_PAGER="' + get_option('default_pager') + '"',
> >>>    '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
> >>>    '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
> >>>    '-DFALLBACK_RUNTIME_PREFIX="' + get_option('prefix') + '"',
> >>> @@ -708,6 +706,17 @@ libgit_c_args = [
> >>>    '-DPAGER_ENV="' + get_option('pager_environment') + '"',
> >>>    '-DSHELL_PATH="' + fs.as_posix(shell.full_path()) + '"',
> >>>  ]
> >>> +
> >>> +editor_opt = get_option('default_editor')
> >>> +if editor_opt != '' and editor_opt != 'vi'
> >>> +  libgit_c_args += '-DDEFAULT_EDITOR="' + editor_opt + '"'
> >>> +endif
> >>> +
> >>> +pager_opt = get_option('default_pager')
> >>> +if pager_opt != '' and pager_opt != 'less'
> >>> +  libgit_c_args += '-DDEFAULT_PAGER="' + pager_opt + '"'
> >>> +endif
> >>> +
> >>>  libgit_include_directories = [ '.' ]
> >>>  libgit_dependencies = [ ]
> >>>  
> >>
> >>
> >> It would be somewhat remiss of me to not mention here that this does not
> >> work for any but the simplest of values! :( If you set a simple single
> >> 'bareword' like 'vim' or 'more' (even '~/bin/vi') then every thing works
> >> just fine. However, if the value contains any of (at least) the following
> >> characters: single quote, double quote or backslash, then things
> >> stop working!
> >>
> >> [I spent one whole evening (and a bit - always something else to 'try')
> >> trying to 'fix' this problem, without success]
> > 
> > Shouldn't it be possible to escape these values via `.replace()` [1]? I
> > suspect that you already tried, but wanted to ask anyway :)
> 
> Yep. :)
> 
> I still haven't studied the meson documentation, but when I searched
> for variations of 'quotes', the results showed that '... if you want
> quotes, you will have to do it yourself ...'. So, I eventually found
> '.replace()' in the 'string operations' section of the docs and tried
> to reproduce what the Makefile does (see #2382):
> 
> 
>   ifdef DEFAULT_EDITOR
>   DEFAULT_EDITOR_CQ = "$(subst ",\",$(subst \,\\,$(DEFAULT_EDITOR)))"
>   DEFAULT_EDITOR_CQ_SQ = $(subst ','\'',$(DEFAULT_EDITOR_CQ))
> 
>   BASIC_CFLAGS += -DDEFAULT_EDITOR='$(DEFAULT_EDITOR_CQ_SQ)'
>   endif
> 
> which I translated into (on top of these patches):
> 
>   diff --git a/meson.build b/meson.build
>   index 8f8a258064..608d665fd3 100644
>   --- a/meson.build
>   +++ b/meson.build
>   @@ -708,7 +708,11 @@ libgit_c_args = [
>  
>    editor_opt = get_option('default_editor')
>    if editor_opt != '' and editor_opt != 'vi'
>   -  libgit_c_args += '-DDEFAULT_EDITOR="' + editor_opt + '"'
>   +  editor_opt = editor_opt.replace('\\', '\\\\')
>   +  editor_opt = editor_opt.replace('"', '\"')
>   +  editor_opt = '"' + editor_opt + '"'
>   +  editor_opt = editor_opt.replace('\'', '\\\'')
>   +  libgit_c_args += '-DDEFAULT_EDITOR=' + editor_opt
>    endif
>  
> [Actually, I think the very first attempt had:
> 
>   libgit_c_args += '-DDEFAULT_EDITOR=\'' + editor_opt + '\''
> 
> but meson, for some reason, adds a set of ' around the whole
> -D argument to gcc, so I got rid of them - but it still didn't
> work!]
> 
> Along with many, many, *many* such permutations! (trying to debug
> this is hard work, with no help from meson).
> 
> So, just a little earlier this evening I read an email from Karthik
> ([PATCH v2 3/4] meson: add support for 'hdr-check') in which he
> mentioned a problem with backslashes and referenced a github issue
> on the mesonbuild repo [0], which is worth a read. ;)
> 
> Sorry I couldn't fix this issue, but it seems to be (in part) an issue
> with meson. (Of course the example I used, which is taken directly
> from the Makefile, happens to be particularly good at demonstrating
> the problem!)

Fair enough. Maybe I'll try to upstream a feature like this into Meson.
It would be nice to have a `.quoted()` method on `str`, and it shouldn't
be hard to do.

> In any event, I think the current patch is a strict improvement, even
> if it may need to be updated at a later date. I hope you agree.

Agreed.

> Thank you for taking the time to review this series. I think this patch
> was the only review comment that required a response - please let me
> know, if that is not the case!

Yup. The only other thing was missing spaces around assignment
operators, but that alone doesn't feel like it's worth a reroll.

Thanks!

Patrick

Re: [-SPAM-] Re: [PATCH v2 07/13] config.mak.uname: only set NO_REGEX on cygwin for v1.7

[Patrick Steinhardt]

On Mon, Apr 14, 2025 at 09:03:30PM +0100, Ramsay Jones wrote:
> On 14/04/2025 08:55, Patrick Steinhardt wrote:
> > On Sun, Apr 06, 2025 at 08:38:33PM +0100, Ramsay Jones wrote:
> >> Commit 92f63d2b05 ("Cygwin 1.7 needs compat/regex", 2013-07-19) set
> >> the NO_REGEX build variable because the platform regex library failed
> >> some of the tests (t4018 and t4034), which passed just fine with the
> >> compat library.
> >>
> >> After some time (maybe a year or two), the platform library had been
> >> updated (with an import from FreeBSD, I believe) and now passed the full
> >> test-suite. This would be about the time of the v1.7 -> v2.0 transition
> >> in 2015. I had a patch ready to send, but just didn't get around to
> >> submitting it to the list. At some point in the interim, the official
> >> cygwin git package used the autoconf build system, which sets the
> >> NO_REGEX variable to use the platform regex library functions. The new
> >> meson build system does likewise.
> >>
> >> The cygwin platform regex library, in addition to now passing the tests
> >> which formerly failed, now passes an 'test_expect_failure' test in the
> >> t7815-grep-binary test file. In particular, test #12 'git grep .fi a'
> >> which determines that the regex pattern '.' matches a NUL character.
> >> The commit f96e56733a ("grep: use REG_STARTEND for all matching if
> >> available", 2010-05-22) added the test in question, but it does not
> >> give any indication as to why the test was framed as an expected fail,
> >> rather than a 'positive' test that the 'git grep' command fails to
> >> match a NUL. Note that the previous test #11 was also originally
> >> marked in that commit as a 'test_expect_failure', but was flipped to
> >> an 'success' test in commit 7e36de5859 ("t/t7008-grep-binary.sh: un-TODO
> >> a test that needs REG_STARTEND", 2010-08-17).
> >>
> >> In order to produce the same NO_REGEX configuration from autoconf, meson
> >> and make, modify config.mak.uname to only set NO_REGEX for cygwin v1.7.
> >> In addition, skip test t7815.12 on cygwin, by adding the !CYGWIN pre-
> >> requisite to the test header, which (among other things) removes an
> >> '...; please update test(s)' comment.
> > 
> > Out of curiosity, because I really don't know any better: why do we have
> > to even care about such oldish Cygwin installations from more than 10
> > years ago? Wouldn't people generally update Cygwin every once in a while
> > to have recent packages? Or is there a good reason why we should
> > continue to support it?
> 
> Heh, as I said in response to Junio, I have a patch that removes all
> of the config in the conditional, so that we would no longer support
> any 'pre-v2.x' versions of cygwin[*]. I think that would be an entirely
> reasonable thing to do, particularly as cygwin thinks of itself as
> a 'rolling release' type distribution. ;)
> 
> However, I don't think it is my place to make that kind of decision
> and I was leaving that patch until last. Hopefully, Adam will make
> that call. :)

Makes sense, it's a bigger discussion indeed. I do think it would be
reasonable to drop pre-2.0 Cygwin, and we have recently become a bit
more aggressive in dropping support for ancient OS versions. But I'm
totally fine with not doing it now.

Patrick

Re: [-SPAM-] Re: [PATCH v2 07/13] config.mak.uname: only set NO_REGEX on cygwin for v1.7

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Mon, Apr 14, 2025 at 09:03:30PM +0100, Ramsay Jones wrote:
>> Heh, as I said in response to Junio, I have a patch that removes all
>> of the config in the conditional, so that we would no longer support
>> any 'pre-v2.x' versions of cygwin[*]. I think that would be an entirely
>> reasonable thing to do, particularly as cygwin thinks of itself as
>> a 'rolling release' type distribution. ;)
>> 
>> However, I don't think it is my place to make that kind of decision
>> and I was leaving that patch until last. Hopefully, Adam will make
>> that call. :)
>
> Makes sense, it's a bigger discussion indeed. I do think it would be
> reasonable to drop pre-2.0 Cygwin, and we have recently become a bit
> more aggressive in dropping support for ancient OS versions. But I'm
> totally fine with not doing it now.

Surely, and thanks.

If Adam declares that pre-2.0 is no longer relevant to us, I am fine
dropping support for it.  The fewer knobs to tweak we have, the
better.

I do not know what the best plan is for changes to update things
that supposedly help supporting pre-2.0 boxes, without us being able
to test, though.  I think I am OK with "this patchset I try to cover
everybody, but this and that systems are untested and untestable by
me" and I am also OK with "this patchset does not address this and
that systems at all, as there is no way to test them by me".

Thanks.

[PATCH v3 00/13] miscellaneous build mods (part 1)

[Ramsay Jones]

changes in v3:
--------------

- patch #9 adds some whitespace around the '=' sign in the
  assignment to HAVE_CLOCK_GETTIME and HAVE_CLOCK_MONOTONIC
  build variables

The range-diff below was generated with a 'creation-factor'
of 80%, otherwise patches after #9 are shown as complete
re-writes.

I have built and tested both the make and meson builds on Linux,
along with the make build and meson build and test on cygwin.
(A 'make test' on cygwin takes 6 hours, so I will test that
tomorrow and let you know if it fails! ;) Highly unlikely to
fail, since the v2 patches passed just fine).

changes in v2:
--------------

- patch #3 has been split into two (new patches #3,4). The motivation
  for the original patch seems not to have been too obvious. The new
  patch #3 combines the suppressing the -Defines for standard default
  values for editor and pager, with the setting of the missing docs
  attributes. The new patch #4 combines the suppressing of the -Define
  for the default-help-format, with the overriding of the default value
  on the windows platform.

- patch #6 (new #7) updates the commit message along with skipping the
  'failing' test #12 in t7815-grep-binary.sh on cygwin. (With the cygwin
  platform regex library the '.' pattern matches a NUL character).

- patch #11 (new #12) fixes a simple typo in the patch subject.

- patch #12 (new #13) updates the commit message and makes a choice for
  the default CSPRNG_METHOD on Linux; 'getrandom' as advised by Brian
  here [0].

[0] https://lore.kernel.org/git/Z9YbJFJjtXNYnTzk@tapette.crustytoothpaste.net/

As before, this series is based on commit 683c54c999 ("Git 2.49", 2025-03-14).

Also, I did a test merge to 'master'@5b97a56fa0, 'next'@ff1f9ca942 and
'seen'@1b09671a54 and they all 'auto-merged' without issue.

As an experiment, I am using 'git sendemail' to send this series for the
first time (I have sent single patches before, but not a series), so if
anything goes awry I apologize in advance! :)

A range-diff is given below.


The original v1 cover-letter follows:
-------------------------------------

This series contains some miscellaneous changes to the make, meson and
autoconf build systems. These are only the patches I have managed to
write a commit message for; I have some others with no commit message,
some need more testing and others are yet to be written. (So, there will
be a 'part #2' series later... :) ).

(I have notes from a couple of months ago - as an example, I have a note
that the meson build incorrectly sets ETC_GITATTRIBUTES and ETC_GITCONFIG
to e.g. '/etc/gitattributes' rather than 'etc/gitattributes').

This series is based on the master branch at commit 683c54c999 ("Git 2.49",
2025-03-14).

As a simple means for comparing the build products, we can compare the
symbols exported by git. So, starting at current master:

  $ git checkout master
  Switched to branch 'master'
  Your branch is up-to-date with 'origin/master'.
  $ git describe
  v2.49.0
  $ 

  $ make >out 2>&1
  $ 

  $ meson setup build/
  The Meson build system
  ...
  Found ninja-1.11.1 at /usr/bin/ninja
  $ 

  $ cd build
  $ meson compile
  INFO: autodetecting backend as ninja
  INFO: calculating backend command to run: /usr/bin/ninja
  [661/661] Copying file contrib/completion/git-completion.zsh
  $ cd ..
  $ 

  $ rm *.syms
  $ ./syms.pl git >git.syms 2>/dev/null
  $ ./syms.pl build/git >mgit.syms 2>/dev/null
  $ 

  $ diff git.syms mgit.syms
  88a89
  > U -- arc4random_buf@GLIBC_2.36
  98a100,101
  > U -- atoi@GLIBC_2.2.5
  > U -- atol@GLIBC_2.2.5
  195a199
  > U -- bsearch@GLIBC_2.2.5
  586,587d589
  < U -- dcgettext@GLIBC_2.2.5
  < U -- dcngettext@GLIBC_2.2.5
  746,747c748,749
  < B -- __environ@GLIBC_2.2.5
  < V -- environ@GLIBC_2.2.5
  ---
  > U -- __environ@GLIBC_2.2.5
  > U -- environ@GLIBC_2.2.5
  909c911
  < U -- __fprintf_chk@GLIBC_2.3.4
  ---
  > U -- fprintf@GLIBC_2.2.5
  914d915
  < U -- __fread_chk@GLIBC_2.7
  1007c1008
  < U -- getc@GLIBC_2.2.5
  ---
  > U -- getchar@GLIBC_2.2.5
  1018a1020
  > U -- getc_unlocked@GLIBC_2.2.5
  1107a1110
  > U -- gettext@GLIBC_2.2.5
  1253d1255
  < T -- gitstrlcpy
  1355a1358
  > U -- htons@GLIBC_2.2.5
  1666d1668
  < U -- __memcpy_chk@GLIBC_2.3.4
  1673d1674
  < U -- __memmove_chk@GLIBC_2.3.4
  1684d1684
  < U -- __memset_chk@GLIBC_2.3.4
  1737a1738
  > U -- ngettext@GLIBC_2.2.5
  1759a1761
  > U -- ntohs@GLIBC_2.2.5
  1819d1820
  < U -- __open64_2@GLIBC_2.7
  1988a1990,2007
  > U -- pcre2_code_free_8
  > U -- pcre2_compile_8
  > U -- pcre2_compile_context_create_8
  > U -- pcre2_compile_context_free_8
  > U -- pcre2_config_8
  > U -- pcre2_general_context_create_8
  > U -- pcre2_general_context_free_8
  > U -- pcre2_get_error_message_8
  > U -- pcre2_get_ovector_pointer_8
  > U -- pcre2_jit_compile_8
  > U -- pcre2_jit_match_8
  > U -- pcre2_maketables_8
  > U -- pcre2_maketables_free_8
  > U -- pcre2_match_8
  > U -- pcre2_match_data_create_from_pattern_8
  > U -- pcre2_match_data_free_8
  > U -- pcre2_pattern_info_8
  > U -- pcre2_set_character_tables_8
  2045c2064
  < U -- __printf_chk@GLIBC_2.3.4
  ---
  > U -- printf@GLIBC_2.2.5
  2080a2100
  > U -- pthread_equal@GLIBC_2.2.5
  2767c2787
  < U -- __snprintf_chk@GLIBC_2.3.4
  ---
  > U -- snprintf@GLIBC_2.2.5
  2925a2946
  > U -- strlcpy@GLIBC_2.38
  3185d3205
  < U -- __uflow@GLIBC_2.2.5
  3289,3290c3309,3311
  < U -- __vfprintf_chk@GLIBC_2.3.4
  < U -- __vsnprintf_chk@GLIBC_2.3.4
  ---
  > U -- vfprintf@GLIBC_2.2.5
  > U -- vprintf@GLIBC_2.2.5
  > U -- vsnprintf@GLIBC_2.2.5
  $ 

Looking at the above, you can see that quite a few differences relate to
symbols which differ due to the level of optimization during compilation.
This reflects, among other things, the different 'default' settings between
the Makefile and the meson build; e.g. prefix=$HOME vs. /usr/local, initial
CFLAGS='-g -O2 -Wall' vs. '-g -O0 -Wall'. Also, note that the make build
does not include the (optional) pcre2 library, even if it is installed on
the system.

[On cygwin the diff is slightly different, but also contains more symbols,
such as all symbols relating to the compat regex routines, clock_gettime,
getdelim, sysinfo and so on ... ;) ]

If we change the meson setup, thus:

  $ meson setup --optimization=2 -Dprefix=$HOME -Dpcre2=disabled build/

After we re-build and repeat the above symbol generation, we find:

  $ diff git.syms mgit.syms
  88a89
  > U -- arc4random_buf@GLIBC_2.36
  746,747c747,748
  < B -- __environ@GLIBC_2.2.5
  < V -- environ@GLIBC_2.2.5
  ---
  > U -- __environ@GLIBC_2.2.5
  > U -- environ@GLIBC_2.2.5
  1253d1253
  < T -- gitstrlcpy
  2925a2926
  > U -- strlcpy@GLIBC_2.38
  $ 

After applying these patches and adding settings to the config.mak file: 

  $ vim config.mak # add NO_STRLCPY and CSPRNG_METHOD
  $ 
  $ cat config.mak
  DEFAULT_TEST_TARGET=prove
  GIT_PROVE_OPTS='--timer'
  PYTHON_PATH=/usr/bin/python3
  GIT_TEST_HTTPD=false
  export GIT_TEST_HTTPD
  #DEVELOPER=1
  SPARSE_FLAGS += -D__STDC_NO_VLA__
  NO_STRLCPY=
  CSPRNG_METHOD=arc4random
  $ 

[Note that I commented out DEVELOPER=1. I'm not entirely sure why I did
that. :) Maybe it was one less variable? ]

  $ make >out 2>&1
  $ 

  $ meson setup --optimization=2 -Dprefix=$HOME -Dpcre2=disabled build/
  The Meson build system
  ...
    User defined options
      optimization: 2
      prefix      : /home/ramsay
      pcre2       : disabled
  
  Found ninja-1.11.1 at /usr/bin/ninja
  $ 

  $ cd build
  $ meson compile
  INFO: autodetecting backend as ninja
  INFO: calculating backend command to run: /usr/bin/ninja
  [661/661] Copying file contrib/completion/git-prompt.sh
  $ cd ..
  $ 

  $ rm *.syms
  $ ./syms.pl git >git.syms 2>/dev/null
  $ ./syms.pl build/git >mgit.syms 2>/dev/null
  $ 

  $ diff git.syms mgit.syms
  747,748c747,748
  < B -- __environ@GLIBC_2.2.5
  < V -- environ@GLIBC_2.2.5
  ---
  > U -- __environ@GLIBC_2.2.5
  > U -- environ@GLIBC_2.2.5
  $ 
 
On cygwin, this diff is now empty. ;)

The above diff seems to imply a difference in the linking of main() and
the C library startup routines (or something similar). However, I happened
to notice that the meson build included '-fPIC' in the build flags, so I
thought it was worth a punt:

  $ make CFLAGS='-g -O2 -Wall -fPIC' >out 2>&1

[with the advent of ASLR, I thought -fPIC/-fPIE was the default anyway!]

  $ diff git.syms mgit.syms
  2809,2811c2809,2811
  < U -- stderr@GLIBC_2.2.5
  < U -- stdin@GLIBC_2.2.5
  < U -- stdout@GLIBC_2.2.5
  ---
  > B -- stderr@GLIBC_2.2.5
  > B -- stdin@GLIBC_2.2.5
  > B -- stdout@GLIBC_2.2.5
  $ 

Hmm, so one step forward, one step back! (or is it 2 steps forward, 3 steps
back? ;) ). So, something for another day.

Note that both test suite runs ('make test' and 'meson test') pass all
tests (excluding the http, cvs, svn, and p4 tests) on Linux. (I am still
waiting for the v2.49.0 test run to finish on cygwin! :( ).

Also, patches #5 and #12 only add/modify a comment, so they could be
dropped, if people feel that they are not appropriate.

[Just in case you want to reproduce the above, I have appended the syms.pl
script below.]

ATB,
Ramsay Jones

Ramsay Jones (13):
  meson.build: remove -DCURL_DISABLE_TYPECHECK
  Makefile: only set some BASIC_CFLAGS when RUNTIME_PREFIX is set
  meson.build: only set build variables for non-default values
  meson.build: set default help format to html on windows
  Makefile: remove NEEDS_LIBRT build variable
  config.mak.uname: add a note about NO_STRLCPY for Linux
  config.mak.uname: only set NO_REGEX on cygwin for v1.7
  config.mak.uname: add HAVE_GETDELIM to the cygwin section
  config.mak.uname: add clock_gettime() to the cygwin build
  builtin/gc.c: correct RAM calculation when using sysinfo
  config.mak.uname: add sysinfo() configuration for cygwin
  config.mak.uname: add arc4random to the cygwin build
  config.mak.uname: set CSPRNG_METHOD to getrandom on Linux

 Documentation/meson.build | 20 +++++++++++++++++
 Makefile                  | 45 ++++++++++++++++++++-------------------
 builtin/gc.c              |  9 ++++++--
 config.mak.uname          | 15 +++++++++----
 configure.ac              |  8 +++++++
 meson.build               | 27 +++++++++++++++++++----
 meson_options.txt         |  2 +-
 t/t7815-grep-binary.sh    |  2 +-
 8 files changed, 94 insertions(+), 34 deletions(-)


range-diff between v2 and v3:

 1:  fe969deec0 =  1:  62173d3c48 meson.build: remove -DCURL_DISABLE_TYPECHECK
 2:  8df541634a =  2:  cded8cb86c Makefile: only set some BASIC_CFLAGS when RUNTIME_PREFIX is set
 3:  280363cd56 =  3:  f6a5e46876 meson.build: only set build variables for non-default values
 4:  34349bf98c =  4:  a39952e29c meson.build: set default help format to html on windows
 5:  e73c2c3219 =  5:  9d4f2736cd Makefile: remove NEEDS_LIBRT build variable
 6:  8c76671f0e =  6:  619bee80a3 config.mak.uname: add a note about NO_STRLCPY for Linux
 7:  324bb21342 =  7:  b0909d3eba config.mak.uname: only set NO_REGEX on cygwin for v1.7
 8:  0a3ce7e80a =  8:  b61bf9ea2e config.mak.uname: add HAVE_GETDELIM to the cygwin section
 9:  3bd6e40cd2 !  9:  b2fcec5b9b config.mak.uname: add clock_gettime() to the cygwin build
    @@ config.mak.uname: ifeq ($(uname_O),Cygwin)
              endif
      	HAVE_DEV_TTY = YesPlease
      	HAVE_GETDELIM = YesPlease
    -+	HAVE_CLOCK_GETTIME=YesPlease
    -+	HAVE_CLOCK_MONOTONIC=YesPlease
    ++	HAVE_CLOCK_GETTIME = YesPlease
    ++	HAVE_CLOCK_MONOTONIC = YesPlease
      	HAVE_ALLOCA_H = YesPlease
      	NEEDS_LIBICONV = YesPlease
      	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
10:  a2eb9ab117 = 10:  7d62945b26 builtin/gc.c: correct RAM calculation when using sysinfo
11:  e45b9df3a4 ! 11:  eff0c2a1a2 config.mak.uname: add sysinfo() configuration for cygwin
    @@ config.mak.uname: ifeq ($(uname_S),Linux)
      	COMPAT_OBJS += compat/linux/procinfo.o
     @@ config.mak.uname: ifeq ($(uname_O),Cygwin)
      	HAVE_GETDELIM = YesPlease
    - 	HAVE_CLOCK_GETTIME=YesPlease
    - 	HAVE_CLOCK_MONOTONIC=YesPlease
    + 	HAVE_CLOCK_GETTIME = YesPlease
    + 	HAVE_CLOCK_MONOTONIC = YesPlease
     +	HAVE_SYSINFO = YesPlease
      	HAVE_ALLOCA_H = YesPlease
      	NEEDS_LIBICONV = YesPlease
12:  422d4b2571 ! 12:  5f8bf7862f config.mak.uname: add arc4random to the cygwin build
    @@ Commit message
     
      ## config.mak.uname ##
     @@ config.mak.uname: ifeq ($(uname_O),Cygwin)
    - 	HAVE_CLOCK_GETTIME=YesPlease
    - 	HAVE_CLOCK_MONOTONIC=YesPlease
    + 	HAVE_CLOCK_GETTIME = YesPlease
    + 	HAVE_CLOCK_MONOTONIC = YesPlease
      	HAVE_SYSINFO = YesPlease
     +	CSPRNG_METHOD = arc4random
      	HAVE_ALLOCA_H = YesPlease
13:  7b133af24b = 13:  07db9583bf config.mak.uname: set CSPRNG_METHOD to getrandom on Linux

-- 
2.49.0

----------------- >8 ------------------ [syms.pl]
#!/usr/bin/perl -w

open(I, "-|", qw(nm -g), $ARGV[0]) or die;
while (<I>) {
	unless (/^[0-9a-f ]+ ([A-Z]) (\S*)$/) {
		print STDERR "? $_";
		next;
	}
	next if ($2 =~ /^\.refptr\./);
	print "$1 -- $2\n";
}
close I;

[PATCH v3 01/13] meson.build: remove -DCURL_DISABLE_TYPECHECK

[Ramsay Jones]

Commit 9371322a60 ("sparse: suppress some \"using sizeof on a function\"
warnings", 2013-10-06) used target-specific variable assignments to add
-DCURL_DISABLE_TYPECHECK to SPARSE_FLAGS for each of the files affected
by the "typecheck-gcc.h" warnings. (http-push.c, http.c, http-walker.c
and remote-curl.c).

These warnings are only issued by sparse, and not by gcc, so we do not
want to disable the 'type checking' for non-sparse targets. The meson
build does not provide any sparse targets, so there is no need to use
the CURL_DISABLE_TYPECHECK preprocessor flag with the c compiler.

In order to re-enable the curl 'type checking' in the meson build, remove
the assignment of -DCURL_DISABLE_TYPECHECK to libgit_c_args.

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

diff --git a/meson.build b/meson.build
index efe2871c9d..88a29fd043 100644
--- a/meson.build
+++ b/meson.build
@@ -966,7 +966,6 @@ if curl.found()
   # Most executables don't have to link against libcurl, but we still need its
   # include directories so that we can resolve LIBCURL_VERSION in "help.c".
   libgit_dependencies += curl.partial_dependency(includes: true)
-  libgit_c_args += '-DCURL_DISABLE_TYPECHECK'
   build_options_config.set('NO_CURL', '')
 else
   libgit_c_args += '-DNO_CURL'
-- 
2.49.0

[PATCH v3 02/13] Makefile: only set some BASIC_CFLAGS when RUNTIME_PREFIX is set

[Ramsay Jones]

Several build variables only have any meaning when the RUNTIME_PREFIX
variable has been set. In particular, the following build variables are
otherwise ignored:

    HAVE_BSD_KERN_PROC_SYSCTL
    PROCFS_EXECUTABLE_PATH
    HAVE_NS_GET_EXECUTABLE_PATH
    HAVE_ZOS_GET_EXECUTABLE_PATH
    HAVE_WPGMPTR

Make setting BASIC_CFLAGS, for each of these variables, conditional on
the RUNTIME_PREFIX being defined.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 Makefile | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/Makefile b/Makefile
index 7315507381..818dde32a9 100644
--- a/Makefile
+++ b/Makefile
@@ -2179,10 +2179,6 @@ ifdef HAVE_BSD_SYSCTL
 	BASIC_CFLAGS += -DHAVE_BSD_SYSCTL
 endif
 
-ifdef HAVE_BSD_KERN_PROC_SYSCTL
-	BASIC_CFLAGS += -DHAVE_BSD_KERN_PROC_SYSCTL
-endif
-
 ifdef HAVE_GETDELIM
 	BASIC_CFLAGS += -DHAVE_GETDELIM
 endif
@@ -2213,25 +2209,33 @@ ifneq ($(findstring openssl,$(CSPRNG_METHOD)),)
 	EXTLIBS += -lcrypto -lssl
 endif
 
-ifneq ($(PROCFS_EXECUTABLE_PATH),)
-	procfs_executable_path_SQ = $(subst ','\'',$(PROCFS_EXECUTABLE_PATH))
-	BASIC_CFLAGS += '-DPROCFS_EXECUTABLE_PATH="$(procfs_executable_path_SQ)"'
-endif
-
 ifndef HAVE_PLATFORM_PROCINFO
 	COMPAT_OBJS += compat/stub/procinfo.o
 endif
 
-ifdef HAVE_NS_GET_EXECUTABLE_PATH
-	BASIC_CFLAGS += -DHAVE_NS_GET_EXECUTABLE_PATH
-endif
+ifdef RUNTIME_PREFIX
 
-ifdef HAVE_ZOS_GET_EXECUTABLE_PATH
-        BASIC_CFLAGS += -DHAVE_ZOS_GET_EXECUTABLE_PATH
-endif
+        ifdef HAVE_BSD_KERN_PROC_SYSCTL
+		BASIC_CFLAGS += -DHAVE_BSD_KERN_PROC_SYSCTL
+        endif
+
+        ifneq ($(PROCFS_EXECUTABLE_PATH),)
+		pep_SQ = $(subst ','\'',$(PROCFS_EXECUTABLE_PATH))
+		BASIC_CFLAGS += '-DPROCFS_EXECUTABLE_PATH="$(pep_SQ)"'
+        endif
+
+        ifdef HAVE_NS_GET_EXECUTABLE_PATH
+		BASIC_CFLAGS += -DHAVE_NS_GET_EXECUTABLE_PATH
+        endif
+
+        ifdef HAVE_ZOS_GET_EXECUTABLE_PATH
+		BASIC_CFLAGS += -DHAVE_ZOS_GET_EXECUTABLE_PATH
+        endif
+
+        ifdef HAVE_WPGMPTR
+		BASIC_CFLAGS += -DHAVE_WPGMPTR
+        endif
 
-ifdef HAVE_WPGMPTR
-	BASIC_CFLAGS += -DHAVE_WPGMPTR
 endif
 
 ifdef FILENO_IS_A_MACRO
-- 
2.49.0

[PATCH v3 03/13] meson.build: only set build variables for non-default values

[Ramsay Jones]

Some preprocessor -Defines have defaults set in the source code when
they have not been provided to the C compiler. In this case, there is
no need to pass them on the command-line, unless the build requires a
non-standard value.

The build variables for DEFAULT_EDITOR and DEFAULT_PAGER have appropriate
defaults ('vi' and 'less') set in the code. Add the preprocessor -Defines
to the 'libgit_c_args' only if the values set with the corresponding
'options' are different to these standard values.

Also, the 'git-var' documentation contains some conditional text which
documents the chosen compiled in value, which would not read well for
the standard values. Similar to the above, only add the corresponding
'-a' attribute arguments to the 'asciidoc_common_options' variable, if
the values set in the 'options' are different to these standard values.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 Documentation/meson.build | 20 ++++++++++++++++++++
 meson.build               | 13 +++++++++++--
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/Documentation/meson.build b/Documentation/meson.build
index 594546d68b..1642b6e2a3 100644
--- a/Documentation/meson.build
+++ b/Documentation/meson.build
@@ -242,6 +242,16 @@ if docs_backend == 'asciidoc'
     '--attribute=build_dir=' + meson.current_build_dir(),
   ]
 
+  pager_opt = get_option('default_pager')
+  if pager_opt != '' and pager_opt != 'less'
+    asciidoc_common_options += '-agit-default-pager=' + pager_opt
+  endif
+
+  editor_opt = get_option('default_editor')
+  if editor_opt != '' and editor_opt != 'vi'
+    asciidoc_common_options += '-agit-default-editor=' + editor_opt
+  endif
+
   documentation_deps = [
     asciidoc_conf,
   ]
@@ -279,6 +289,16 @@ elif docs_backend == 'asciidoctor'
     '--require', 'asciidoctor-extensions',
   ]
 
+  pager_opt = get_option('default_pager')
+  if pager_opt != '' and pager_opt != 'less'
+    asciidoc_common_options += '-agit-default-pager=' + pager_opt
+  endif
+
+  editor_opt = get_option('default_editor')
+  if editor_opt != '' and editor_opt != 'vi'
+    asciidoc_common_options += '-agit-default-editor=' + editor_opt
+  endif
+
   documentation_deps = [
     asciidoctor_extensions,
   ]
diff --git a/meson.build b/meson.build
index 88a29fd043..efd0bd3319 100644
--- a/meson.build
+++ b/meson.build
@@ -693,10 +693,8 @@ endif
 # These variables are used for building libgit.a.
 libgit_c_args = [
   '-DBINDIR="' + get_option('bindir') + '"',
-  '-DDEFAULT_EDITOR="' + get_option('default_editor') + '"',
   '-DDEFAULT_GIT_TEMPLATE_DIR="' + get_option('datadir') / 'git-core/templates' + '"',
   '-DDEFAULT_HELP_FORMAT="' + get_option('default_help_format') + '"',
-  '-DDEFAULT_PAGER="' + get_option('default_pager') + '"',
   '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
   '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
   '-DFALLBACK_RUNTIME_PREFIX="' + get_option('prefix') + '"',
@@ -708,6 +706,17 @@ libgit_c_args = [
   '-DPAGER_ENV="' + get_option('pager_environment') + '"',
   '-DSHELL_PATH="' + fs.as_posix(shell.full_path()) + '"',
 ]
+
+editor_opt = get_option('default_editor')
+if editor_opt != '' and editor_opt != 'vi'
+  libgit_c_args += '-DDEFAULT_EDITOR="' + editor_opt + '"'
+endif
+
+pager_opt = get_option('default_pager')
+if pager_opt != '' and pager_opt != 'less'
+  libgit_c_args += '-DDEFAULT_PAGER="' + pager_opt + '"'
+endif
+
 libgit_include_directories = [ '.' ]
 libgit_dependencies = [ ]
 
-- 
2.49.0

[PATCH v3 04/13] meson.build: set default help format to html on windows

[Ramsay Jones]

The build variable DEFAULT_HELP_FORMAT has an appropriate default
('man') set in the code, so there is no need to pass the -Define on
the compiler command-line, unless the build requires a non-standard
value.

In addition, on windows the make build overrides the default help
format to 'html', rather than 'man', in the 'config.mak.uname' file.

In order to suppress the -Define on the C compiler command-line, only
add the -Define to the 'libgit_c_args' variable when the requested
value is not the standard 'man'. In order to override the default value
on windows, add a 'platform' value to the 'default_help_format' combo
option and set it as the default choice. When this option is set to
'platform', use the 'host_machine.system()' method call to determine the
appropriate default value for the host system.

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

diff --git a/meson.build b/meson.build
index efd0bd3319..8f8a258064 100644
--- a/meson.build
+++ b/meson.build
@@ -694,7 +694,6 @@ endif
 libgit_c_args = [
   '-DBINDIR="' + get_option('bindir') + '"',
   '-DDEFAULT_GIT_TEMPLATE_DIR="' + get_option('datadir') / 'git-core/templates' + '"',
-  '-DDEFAULT_HELP_FORMAT="' + get_option('default_help_format') + '"',
   '-DETC_GITATTRIBUTES="' + get_option('gitattributes') + '"',
   '-DETC_GITCONFIG="' + get_option('gitconfig') + '"',
   '-DFALLBACK_RUNTIME_PREFIX="' + get_option('prefix') + '"',
@@ -717,6 +716,18 @@ if pager_opt != '' and pager_opt != 'less'
   libgit_c_args += '-DDEFAULT_PAGER="' + pager_opt + '"'
 endif
 
+help_format_opt = get_option('default_help_format')
+if help_format_opt == 'platform'
+  if host_machine.system() == 'windows'
+    help_format_opt = 'html'
+  else
+    help_format_opt = 'man'
+  endif
+endif
+if help_format_opt != 'man'
+    libgit_c_args += '-DDEFAULT_HELP_FORMAT="' + help_format_opt + '"'
+endif
+
 libgit_include_directories = [ '.' ]
 libgit_dependencies = [ ]
 
diff --git a/meson_options.txt b/meson_options.txt
index 78d172a740..8ac30a5223 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -95,7 +95,7 @@ option('highlight_bin', type: 'string', value: 'highlight')
 # Documentation.
 option('docs', type: 'array', choices: ['man', 'html'], value: [],
   description: 'Which documenattion formats to build and install.')
-option('default_help_format', type: 'combo', choices: ['man', 'html'], value: 'man',
+option('default_help_format', type: 'combo', choices: ['man', 'html', 'platform'], value: 'platform',
   description: 'Default format used when executing git-help(1).')
 option('docs_backend', type: 'combo', choices: ['asciidoc', 'asciidoctor', 'auto'], value: 'auto',
   description: 'Which backend to use to generate documentation.')
-- 
2.49.0

[PATCH v3 05/13] Makefile: remove NEEDS_LIBRT build variable

[Ramsay Jones]

Commit d19e3a5b21 ("Makefile: add NEEDS_LIBRT to optionally link with
librt", 2016-07-07) introduced the NEEDS_LIBRT build variable to
disassociate the HAVE_CLOCK_GETTIME variable with the unconditional
linking of the librt library. At one time, the clock_gettime() function
was not available as part of the libc library and (on some unix systems)
required linking with librt.

Commit 52fcec75ce ("config.mak.uname: define NEEDS_LIBRT under Linux, for
now", 2016-07-10) set the NEEDS_LIBRT variable in the Linux section of
the config.mak.uname file, since Debian 7 (wheezy) was one of the few
remaining distributions, with glibc 2.13, that required linking with
librt for clock_gettime(). Note that from glibc version 2.17, this is no
longer necessary.

Note that Debian 7.0 was released on May 4th, 2013 and benefited from
long term support until May 2018 when it went end-of-life. Since that
time, Linux distributions use a more up-to-date library, for example:

    Distribution   version  end of support

    Debian 8       2.19     30th June 2020
    RHEL   8       2.28     31st May  2024 *
    Ubuntu 16.04   2.23     30th Apr  2021

* paid 'Maintenance support' ends 31st May 2029

Since it is no longer required, remove NEEDS_LIBRT from the Makefile and
config.mak.uname.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 Makefile         | 7 -------
 config.mak.uname | 2 --
 2 files changed, 9 deletions(-)

diff --git a/Makefile b/Makefile
index 818dde32a9..8fa4d2664e 100644
--- a/Makefile
+++ b/Makefile
@@ -340,9 +340,6 @@ include shared.mak
 #
 # Define HAVE_SYNC_FILE_RANGE if your platform has sync_file_range.
 #
-# Define NEEDS_LIBRT if your platform requires linking with librt (glibc version
-# before 2.17) for clock_gettime and CLOCK_MONOTONIC.
-#
 # Define HAVE_BSD_SYSCTL if your platform has a BSD-compatible sysctl function.
 #
 # Define HAVE_GETDELIM if your system has the getdelim() function.
@@ -2171,10 +2168,6 @@ ifdef HAVE_SYNC_FILE_RANGE
 	BASIC_CFLAGS += -DHAVE_SYNC_FILE_RANGE
 endif
 
-ifdef NEEDS_LIBRT
-	EXTLIBS += -lrt
-endif
-
 ifdef HAVE_BSD_SYSCTL
 	BASIC_CFLAGS += -DHAVE_BSD_SYSCTL
 endif
diff --git a/config.mak.uname b/config.mak.uname
index b12d4e168a..88007c4f13 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -54,8 +54,6 @@ ifeq ($(uname_S),Linux)
 	HAVE_DEV_TTY = YesPlease
 	HAVE_CLOCK_GETTIME = YesPlease
 	HAVE_CLOCK_MONOTONIC = YesPlease
-	# -lrt is needed for clock_gettime on glibc <= 2.16
-	NEEDS_LIBRT = YesPlease
 	HAVE_SYNC_FILE_RANGE = YesPlease
 	HAVE_GETDELIM = YesPlease
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
-- 
2.49.0

[PATCH v3 06/13] config.mak.uname: add a note about NO_STRLCPY for Linux

[Ramsay Jones]

Commit 817151e61a ("Rename safe_strncpy() to strlcpy().", 2006-06-24)
added the NO_STRLCPY make variable to allow the conditional use of
the gitstrlcpy() compat function on those platforms which didn't
provide the 'standard' strlcpy() function.

Recently, in the summer of 2023, the strlcpy() and strlcat() functions
were added to the glibc library (v2.38), so some of the more up-to-date
Linux distributions no longer need to set NO_STRLCPY. For example, both
Ubuntu 24.04 LTS and RHEL 10 beta have glibc v2.39. However, several
distributions, which are still within their support window, have an
earlier version and must still use the 'compat' version of strlcpy().

If the meson or autoconf build systems are used on newer platforms, then
they will be configured to to use strlcpy() from glibc, whereas the make
build will always choose the 'compat' function instead. Add a note to
the config.mak.uname file, in the Linux section, to prompt make users to
override NO_STRLCPY in the config.mak file, if appropriate.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 config.mak.uname | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.mak.uname b/config.mak.uname
index 88007c4f13..ae6ba15586 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -48,6 +48,7 @@ ifeq ($(uname_S),OSF1)
 endif
 ifeq ($(uname_S),Linux)
 	HAVE_ALLOCA_H = YesPlease
+	# override in config.mak if you have glibc >= 2.38
 	NO_STRLCPY = YesPlease
 	HAVE_PATHS_H = YesPlease
 	LIBC_CONTAINS_LIBINTL = YesPlease
-- 
2.49.0

[PATCH v3 07/13] config.mak.uname: only set NO_REGEX on cygwin for v1.7

[Ramsay Jones]

Commit 92f63d2b05 ("Cygwin 1.7 needs compat/regex", 2013-07-19) set
the NO_REGEX build variable because the platform regex library failed
some of the tests (t4018 and t4034), which passed just fine with the
compat library.

After some time (maybe a year or two), the platform library had been
updated (with an import from FreeBSD, I believe) and now passed the full
test-suite. This would be about the time of the v1.7 -> v2.0 transition
in 2015. I had a patch ready to send, but just didn't get around to
submitting it to the list. At some point in the interim, the official
cygwin git package used the autoconf build system, which sets the
NO_REGEX variable to use the platform regex library functions. The new
meson build system does likewise.

The cygwin platform regex library, in addition to now passing the tests
which formerly failed, now passes an 'test_expect_failure' test in the
t7815-grep-binary test file. In particular, test #12 'git grep .fi a'
which determines that the regex pattern '.' matches a NUL character.
The commit f96e56733a ("grep: use REG_STARTEND for all matching if
available", 2010-05-22) added the test in question, but it does not
give any indication as to why the test was framed as an expected fail,
rather than a 'positive' test that the 'git grep' command fails to
match a NUL. Note that the previous test #11 was also originally
marked in that commit as a 'test_expect_failure', but was flipped to
an 'success' test in commit 7e36de5859 ("t/t7008-grep-binary.sh: un-TODO
a test that needs REG_STARTEND", 2010-08-17).

In order to produce the same NO_REGEX configuration from autoconf, meson
and make, modify config.mak.uname to only set NO_REGEX for cygwin v1.7.
In addition, skip test t7815.12 on cygwin, by adding the !CYGWIN pre-
requisite to the test header, which (among other things) removes an
'...; please update test(s)' comment.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 config.mak.uname       | 4 +++-
 t/t7815-grep-binary.sh | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/config.mak.uname b/config.mak.uname
index ae6ba15586..b6adce0bc4 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -245,7 +245,9 @@ ifeq ($(uname_O),Cygwin)
 		# Try commenting this out if you suspect MMAP is more efficient
 		NO_MMAP = YesPlease
         else
-		NO_REGEX = UnfortunatelyYes
+                ifeq ($(shell expr "$(uname_R)" : '1\.7\.'),4)
+		        NO_REGEX = UnfortunatelyYes
+                endif
         endif
 	HAVE_DEV_TTY = YesPlease
 	HAVE_ALLOCA_H = YesPlease
diff --git a/t/t7815-grep-binary.sh b/t/t7815-grep-binary.sh
index 90ebb64f46..b7000dfa32 100755
--- a/t/t7815-grep-binary.sh
+++ b/t/t7815-grep-binary.sh
@@ -63,7 +63,7 @@ test_expect_success 'git grep ile a' '
 	git grep ile a
 '
 
-test_expect_failure 'git grep .fi a' '
+test_expect_failure !CYGWIN 'git grep .fi a' '
 	git grep .fi a
 '
 
-- 
2.49.0

[PATCH v3 08/13] config.mak.uname: add HAVE_GETDELIM to the cygwin section

[Ramsay Jones]

Cygwin has provided the getdelim() function as far back as (at least)
2011. The autoconf and meson builds enable the use of this symbol.
In order to have the same configuration for autoconf, meson and make,
enable the HAVE_GETDELIM build variable in the cygwin section of the
config.mak.uname file.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 config.mak.uname | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.mak.uname b/config.mak.uname
index b6adce0bc4..1b3ba8a341 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -250,6 +250,7 @@ ifeq ($(uname_O),Cygwin)
                 endif
         endif
 	HAVE_DEV_TTY = YesPlease
+	HAVE_GETDELIM = YesPlease
 	HAVE_ALLOCA_H = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
-- 
2.49.0

[PATCH v3 09/13] config.mak.uname: add clock_gettime() to the cygwin build

[Ramsay Jones]

Cygwin supports the clock_gettime() function, along with the associated
CLOCK_MONOTONIC preprocessor symbol. The autoconf and meson builds both
enable the use of those symbols. In order to have the same configuration
for the make builds, add the HAVE_CLOCK_GETTIME and HAVE_CLOCK_MONOTONIC
build variables to the cygwin section of the config.mak.uname file.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 config.mak.uname | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/config.mak.uname b/config.mak.uname
index 1b3ba8a341..d058b34292 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -251,6 +251,8 @@ ifeq ($(uname_O),Cygwin)
         endif
 	HAVE_DEV_TTY = YesPlease
 	HAVE_GETDELIM = YesPlease
+	HAVE_CLOCK_GETTIME = YesPlease
+	HAVE_CLOCK_MONOTONIC = YesPlease
 	HAVE_ALLOCA_H = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
-- 
2.49.0

[PATCH v3 10/13] builtin/gc.c: correct RAM calculation when using sysinfo

[Ramsay Jones]

The man page for sysinfo(2) on Linux states that (from v2.3.48) the
sizes of the memory and swap fields, of the returned structure, are
given as multiples of 'mem_unit' bytes. In earlier versions (prior to
v2.3.23 on i386 in particular), the 'mem_unit' field was not part of
the structure, and all sizes were measured in bytes. The man page does
not discuss the motivation for this change, but it is possible that the
change was intended for the, relatively rare, 32-bit platform with more
than 4GB of memory.

The total_ram() function makes the assumption that the 'totalram' field
of the 'struct sysinfo' is measured in bytes, or alternatively that the
'mem_unit' field is always equal to one. Having writen a program to call
the sysinfo() function and print the structure fields, it seems that, on
Linux x84_64 and i686 anyway, the 'mem_unit' field is indeed set to one
(note that the 32-bit system had only 2GB ram). However, cygwin also has
an sysinfo() implementation, which gives the following values:

  $ ./sysinfo
  uptime:      21381
  loads:       0, 0, 0
  total ram:   2074637
  free ram:    843237
  shared ram:  0
  buffer ram:  0
  total swap:  327680
  free swap:   306932
  procs:       15
  total high:  0
  free high:   0
  mem_unit:    4096

  total ram: 8497713152
  $

[This laptop has 8GB ram, so a little bit seems to be missing. ;) ]

Modify the total_ram() function to allow for the possibility that the
memory size is not specified in bytes (ie 'mem_unit' is greater than
one).

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 builtin/gc.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 99431fd467..cdcf1dc6e7 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -373,8 +373,13 @@ static uint64_t total_ram(void)
 #if defined(HAVE_SYSINFO)
 	struct sysinfo si;
 
-	if (!sysinfo(&si))
-		return si.totalram;
+	if (!sysinfo(&si)) {
+		uint64_t total = si.totalram;
+
+		if (si.mem_unit > 1)
+			total *= (uint64_t)si.mem_unit;
+		return total;
+	}
 #elif defined(HAVE_BSD_SYSCTL) && (defined(HW_MEMSIZE) || defined(HW_PHYSMEM))
 	int64_t physical_memory;
 	int mib[2];
-- 
2.49.0

[PATCH v3 11/13] config.mak.uname: add sysinfo() configuration for cygwin

[Ramsay Jones]

Although sysinfo() is a 'Linux only' function, cygwin provides an
implementation which appears to be functional. The assumption that
this function is Linux only is reflected in the way the HAVE_SYSINFO
build variable is handled by the Makefile and config.mak.uname.

Rework the setting of HAVE_SYSINFO in the Linux section of the system
specific config file, along with the corresponding setting of the
BASIC_CFLAGS in the Makefile. Add the setting of HAVE_SYSINFO to the
cygwin section of 'config.mak.uname'. While here, add a test for the
sysinfo() function to the autoconf build system.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 Makefile         | 4 ++++
 config.mak.uname | 3 ++-
 configure.ac     | 8 ++++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 8fa4d2664e..ff89f2b81f 100644
--- a/Makefile
+++ b/Makefile
@@ -2168,6 +2168,10 @@ ifdef HAVE_SYNC_FILE_RANGE
 	BASIC_CFLAGS += -DHAVE_SYNC_FILE_RANGE
 endif
 
+ifdef HAVE_SYSINFO
+	BASIC_CFLAGS += -DHAVE_SYSINFO
+endif
+
 ifdef HAVE_BSD_SYSCTL
 	BASIC_CFLAGS += -DHAVE_BSD_SYSCTL
 endif
diff --git a/config.mak.uname b/config.mak.uname
index d058b34292..e7bd07cf6e 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -58,7 +58,7 @@ ifeq ($(uname_S),Linux)
 	HAVE_SYNC_FILE_RANGE = YesPlease
 	HAVE_GETDELIM = YesPlease
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
-	BASIC_CFLAGS += -DHAVE_SYSINFO
+	HAVE_SYSINFO = YesPlease
 	PROCFS_EXECUTABLE_PATH = /proc/self/exe
 	HAVE_PLATFORM_PROCINFO = YesPlease
 	COMPAT_OBJS += compat/linux/procinfo.o
@@ -253,6 +253,7 @@ ifeq ($(uname_O),Cygwin)
 	HAVE_GETDELIM = YesPlease
 	HAVE_CLOCK_GETTIME = YesPlease
 	HAVE_CLOCK_MONOTONIC = YesPlease
+	HAVE_SYSINFO = YesPlease
 	HAVE_ALLOCA_H = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
diff --git a/configure.ac b/configure.ac
index 5923edc44a..d7e0503f1e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1066,6 +1066,14 @@ AC_CHECK_LIB([iconv], [locale_charset],
        [AC_CHECK_LIB([charset], [locale_charset],
                      [CHARSET_LIB=-lcharset])])
 GIT_CONF_SUBST([CHARSET_LIB])
+
+#
+# Define HAVE_SYSINFO=YesPlease if sysinfo is available.
+GIT_CHECK_FUNC(sysinfo,
+	[HAVE_SYSINFO=YesPlease],
+	[HAVE_SYSINFO=])
+GIT_CONF_SUBST([HAVE_SYSINFO])
+
 #
 # Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
 GIT_CHECK_FUNC(clock_gettime,
-- 
2.49.0

[PATCH v3 12/13] config.mak.uname: add arc4random to the cygwin build

[Ramsay Jones]

The arc4random_buf() function has been available in cygwin since
about 2016 (somewhere in the v2.x branch). Set the CSPRNG_METHOD
build variable to 'arc4random', in the cygwin section, to enable
the use of this cryptographically-secure pseudorandom number
function. Note that the autoconf and new meson builds also enable
this function.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 config.mak.uname | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.mak.uname b/config.mak.uname
index e7bd07cf6e..330741eb5a 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -254,6 +254,7 @@ ifeq ($(uname_O),Cygwin)
 	HAVE_CLOCK_GETTIME = YesPlease
 	HAVE_CLOCK_MONOTONIC = YesPlease
 	HAVE_SYSINFO = YesPlease
+	CSPRNG_METHOD = arc4random
 	HAVE_ALLOCA_H = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
-- 
2.49.0

[PATCH v3 13/13] config.mak.uname: set CSPRNG_METHOD to getrandom on Linux

[Ramsay Jones]

Commit 05cd988dce ("wrapper: add a helper to generate numbers from a
CSPRNG", 2022-01-17) added a csprng_bytes() function which used one
of several interfaces to provide a source of cryptographically secure
pseudorandom numbers. The CSPRNG_METHOD make variable was provided to
determine the choice of available 'backends' for the source of random
bytes.

Commit 05cd988dce did not set CSPRNG_METHOD in the Linux section of
the config.mak.uname file, so it defaults to using '/dev/urandom' as
the source of random bytes. The 'backend' values which could be used
on Linux are 'arc4random', 'getrandom' or 'getentropy' ('openssl' is
an option, but seems to be discouraged).

The arc4random routines (ar4random_buf() is the one actually used) were
added to glibc in version 2.36, while both getrandom() and getentropy()
were included in 2.25. So, some of the more up-to-date distributions of
Linux (eg Debian 12, Ubuntu 24.04) would be able to use the 'arc4random'
setting. All currently supported distributions have glibc 2.25 or later
(RHEL 8 has v2.28) and, therefore, have support for the 'getrandom' and
'getentropy' settings.

The arc4random routines on the *BSDs (along with cygwin) implement the
ChaCha20 stream cipher algorithm (see RFC8439) in userspace, rather than
as a system call, and are thus somewhat faster (having avoided a context
switch to the kernel). In contrast, on Linux all three functions are
simple wrappers around the same kernel CSPRNG syscall.

If the meson build system is used on a newer platform, then they will be
configured to use 'arc4random', whereas the make build will currently
default to using '/dev/urandom' on Linux. Since there is no advantage,
in terms of performance, to the 'arc4random' setting, the 'getrandom'
setting should be preferred from an availability perspective. (Also, the
current uses of csprng_bytes() are not in any hot path).

In order to set an appropriate default, set the CSPRNG_METHOD build
variable to 'getrandom' in the Linux section of the 'config.mak.uname'
file.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
 config.mak.uname | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config.mak.uname b/config.mak.uname
index 330741eb5a..db22a8fb31 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -50,6 +50,7 @@ ifeq ($(uname_S),Linux)
 	HAVE_ALLOCA_H = YesPlease
 	# override in config.mak if you have glibc >= 2.38
 	NO_STRLCPY = YesPlease
+	CSPRNG_METHOD = getrandom
 	HAVE_PATHS_H = YesPlease
 	LIBC_CONTAINS_LIBINTL = YesPlease
 	HAVE_DEV_TTY = YesPlease
-- 
2.49.0

Re: [PATCH v3 00/13] miscellaneous build mods (part 1)

[Junio C Hamano]

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

> changes in v3:
> --------------
>
> - patch #9 adds some whitespace around the '=' sign in the
>   assignment to HAVE_CLOCK_GETTIME and HAVE_CLOCK_MONOTONIC
>   build variables
>
> The range-diff below was generated with a 'creation-factor'
> of 80%, otherwise patches after #9 are shown as complete
> re-writes.
>
> I have built and tested both the make and meson builds on Linux,
> along with the make build and meson build and test on cygwin.
> (A 'make test' on cygwin takes 6 hours, so I will test that
> tomorrow and let you know if it fails! ;) Highly unlikely to
> fail, since the v2 patches passed just fine).

Thanks, will queue.

Re: [PATCH v3 00/13] miscellaneous build mods (part 1)

[Patrick Steinhardt]

On Wed, Apr 16, 2025 at 08:45:34PM -0700, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
> > changes in v3:
> > --------------
> >
> > - patch #9 adds some whitespace around the '=' sign in the
> >   assignment to HAVE_CLOCK_GETTIME and HAVE_CLOCK_MONOTONIC
> >   build variables
> >
> > The range-diff below was generated with a 'creation-factor'
> > of 80%, otherwise patches after #9 are shown as complete
> > re-writes.
> >
> > I have built and tested both the make and meson builds on Linux,
> > along with the make build and meson build and test on cygwin.
> > (A 'make test' on cygwin takes 6 hours, so I will test that
> > tomorrow and let you know if it fails! ;) Highly unlikely to
> > fail, since the v2 patches passed just fine).
> 
> Thanks, will queue.

This version looks good to me based on reading the range-diff. Thanks!

Patrick

Re: [PATCH v3 13/13] config.mak.uname: set CSPRNG_METHOD to getrandom on Linux

[Junio C Hamano]

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

> Commit 05cd988dce ("wrapper: add a helper to generate numbers from a
> CSPRNG", 2022-01-17) added a csprng_bytes() function which used one
> of several interfaces to provide a source of cryptographically secure
> pseudorandom numbers. The CSPRNG_METHOD make variable was provided to
> determine the choice of available 'backends' for the source of random
> bytes.
>
> Commit 05cd988dce did not set CSPRNG_METHOD in the Linux section of
> the config.mak.uname file, so it defaults to using '/dev/urandom' as
> the source of random bytes. The 'backend' values which could be used
> on Linux are 'arc4random', 'getrandom' or 'getentropy' ('openssl' is
> an option, but seems to be discouraged).
>
> The arc4random routines (ar4random_buf() is the one actually used) were

arc4random_buf(), if I am not mistaken?

> added to glibc in version 2.36, while both getrandom() and getentropy()
> were included in 2.25. So, some of the more up-to-date distributions of
> Linux (eg Debian 12, Ubuntu 24.04) would be able to use the 'arc4random'
> setting. All currently supported distributions have glibc 2.25 or later
> (RHEL 8 has v2.28) and, therefore, have support for the 'getrandom' and
> 'getentropy' settings.

OK.  This explains that getrandom/getentropy have better
availability than arc4random.

> The arc4random routines on the *BSDs (along with cygwin) implement the
> ChaCha20 stream cipher algorithm (see RFC8439) in userspace, rather than
> as a system call, and are thus somewhat faster (having avoided a context
> switch to the kernel). In contrast, on Linux all three functions are
> simple wrappers around the same kernel CSPRNG syscall.

OK.  With this and the previous paragraph, we establish that there
is no reason to use arc4random on Linux, while on BSDs and Cygwin,
it is a natural choice.

Very clearly explained.  Thanks.

> If the meson build system is used on a newer platform, then they will be
> configured to use 'arc4random', whereas the make build will currently
> default to using '/dev/urandom' on Linux.



> Since there is no advantage,
> in terms of performance, to the 'arc4random' setting, the 'getrandom'
> setting should be preferred from an availability perspective. (Also, the
> current uses of csprng_bytes() are not in any hot path).
>
> In order to set an appropriate default, set the CSPRNG_METHOD build
> variable to 'getrandom' in the Linux section of the 'config.mak.uname'
> file.
>
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
>  config.mak.uname | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/config.mak.uname b/config.mak.uname
> index 330741eb5a..db22a8fb31 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -50,6 +50,7 @@ ifeq ($(uname_S),Linux)
>  	HAVE_ALLOCA_H = YesPlease
>  	# override in config.mak if you have glibc >= 2.38
>  	NO_STRLCPY = YesPlease
> +	CSPRNG_METHOD = getrandom
>  	HAVE_PATHS_H = YesPlease
>  	LIBC_CONTAINS_LIBINTL = YesPlease
>  	HAVE_DEV_TTY = YesPlease

Re: [PATCH v3 13/13] config.mak.uname: set CSPRNG_METHOD to getrandom on Linux

[Ramsay Jones]

On 17/04/2025 14:55, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
[snip]
>> The arc4random routines (ar4random_buf() is the one actually used) were
> 
> arc4random_buf(), if I am not mistaken?

Oops, yes ... an unfortunate tyop! ;)

Do you want a v4? (The cygwin v3 'make test' has been running for under
two hours, I could Ctrl-C it ...)

> 
>> added to glibc in version 2.36, while both getrandom() and getentropy()
>> were included in 2.25. So, some of the more up-to-date distributions of
>> Linux (eg Debian 12, Ubuntu 24.04) would be able to use the 'arc4random'
>> setting. All currently supported distributions have glibc 2.25 or later
>> (RHEL 8 has v2.28) and, therefore, have support for the 'getrandom' and
>> 'getentropy' settings.
> 
> OK.  This explains that getrandom/getentropy have better
> availability than arc4random.
> 
>> The arc4random routines on the *BSDs (along with cygwin) implement the
>> ChaCha20 stream cipher algorithm (see RFC8439) in userspace, rather than
>> as a system call, and are thus somewhat faster (having avoided a context
>> switch to the kernel). In contrast, on Linux all three functions are
>> simple wrappers around the same kernel CSPRNG syscall.
> 
> OK.  With this and the previous paragraph, we establish that there
> is no reason to use arc4random on Linux, while on BSDs and Cygwin,
> it is a natural choice.
> 
> Very clearly explained.  Thanks.
> 

Thanks!

ATB,
Ramsay Jones

Re: [PATCH v3 13/13] config.mak.uname: set CSPRNG_METHOD to getrandom on Linux

[Junio C Hamano]

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

> On 17/04/2025 14:55, Junio C Hamano wrote:
>> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> [snip]
>>> The arc4random routines (ar4random_buf() is the one actually used) were
>> 
>> arc4random_buf(), if I am not mistaken?
>
> Oops, yes ... an unfortunate tyop! ;)
>
> Do you want a v4? (The cygwin v3 'make test' has been running for under
> two hours, I could Ctrl-C it ...)

Nah, if you send one I will replace my copy with it, but in the
meantime I'll locally make a typofix myself to v3.

Thanks.

Re: [PATCH v3 13/13] config.mak.uname: set CSPRNG_METHOD to getrandom on Linux

[Ramsay Jones]

On 17/04/2025 21:13, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
>> On 17/04/2025 14:55, Junio C Hamano wrote:
>>> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>> [snip]
>>>> The arc4random routines (ar4random_buf() is the one actually used) were
>>>
>>> arc4random_buf(), if I am not mistaken?
>>
>> Oops, yes ... an unfortunate tyop! ;)
>>
>> Do you want a v4? (The cygwin v3 'make test' has been running for under
>> two hours, I could Ctrl-C it ...)
> 
> Nah, if you send one I will replace my copy with it, but in the
> meantime I'll locally make a typofix myself to v3.

OK, thanks. I have made the change locally just in case there is
a need to send another version. Hopefully, that won't be necessary!

(The cygwin 'make test' just finished and, as expected, passed
just fine).

Thanks.

ATB,
Ramsay Jones