threads / patch / 63129

patch, 12 partsmiscellaneous build mods (part 1)

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

## tl;dr

51 messages between Mar 15, 2025 and Apr 17, 2025. Diffs are folded; open one to read it.

replies: 50people: 3as markdown or json

Ramsay Jones· Mar 15, 2025, 02:46 UTC · lore

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;
Ramsay Jones· Apr 6, 2025, 19:38 UTC · re: Ramsay Jones · lore

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

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;
Ramsay Jones· Apr 6, 2025, 19:38 UTC · re: Ramsay Jones · lore

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

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(+)
Show changes to config.mak.uname +1 −0
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
Ramsay Jones· Apr 6, 2025, 19:38 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to 2 files +4 −2

config.mak.uname, t/t7815-grep-binary.sh

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
Patrick Steinhardt· Apr 14, 2025, 07:55 UTC · re: Ramsay Jones · lore

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

On Sun, Apr 06, 2025 at 08:38:33PM +0100, Ramsay Jones wrote:
Show 32 quoted lines
> 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
Ramsay Jones· Apr 14, 2025, 20:03 UTC · re: Patrick Steinhardt · lore

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

On 14/04/2025 08:55, Patrick Steinhardt wrote:
Show 39 quoted lines
> 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!

Patrick Steinhardt· Apr 15, 2025, 05:59 UTC · re: Ramsay Jones · lore

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

On Mon, Apr 14, 2025 at 09:03:30PM +0100, Ramsay Jones wrote:
Show 50 quoted lines
> 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
Junio C Hamano· Apr 15, 2025, 15:05 UTC · re: Patrick Steinhardt · lore

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

Patrick Steinhardt <ps@pks.im> writes:
Show 15 quoted lines
> 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.
Ramsay Jones· Apr 6, 2025, 19:38 UTC · re: Ramsay Jones · lore

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

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(+)
Show changes to config.mak.uname +1 −0
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
Ramsay Jones· Apr 6, 2025, 19:38 UTC · re: Ramsay Jones · lore

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

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(+)
Show changes to config.mak.uname +2 −0
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
Patrick Steinhardt· Apr 14, 2025, 07:55 UTC · re: Ramsay Jones · lore

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

On Sun, Apr 06, 2025 at 08:38:35PM +0100, Ramsay Jones wrote:
Show 10 quoted lines
> 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
Ramsay Jones· Apr 14, 2025, 20:05 UTC · re: Patrick Steinhardt · lore

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

On 14/04/2025 08:55, Patrick Steinhardt wrote:
Show 13 quoted lines
> 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

Show 7 quoted lines
> 
>>  	HAVE_ALLOCA_H = YesPlease
>>  	NEEDS_LIBICONV = YesPlease
>>  	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
> 
> Patrick
> 
Ramsay Jones· Apr 6, 2025, 19:38 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to builtin/gc.c +7 −2
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
Patrick Steinhardt· Apr 14, 2025, 07:55 UTC · re: Ramsay Jones · lore

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

On Sun, Apr 06, 2025 at 08:38:36PM +0100, Ramsay Jones wrote:
Show 35 quoted lines
> 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.

Show 26 quoted lines
> 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
Ramsay Jones· Apr 14, 2025, 20:11 UTC · re: Patrick Steinhardt · lore

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

On 14/04/2025 08:55, Patrick Steinhardt wrote:
Show 71 quoted lines
> 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

Ramsay Jones· Apr 6, 2025, 19:38 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to 3 files +14 −1

Makefile, config.mak.uname, configure.ac

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
Patrick Steinhardt· Apr 14, 2025, 07:55 UTC · re: Ramsay Jones · lore

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

On Sun, Apr 06, 2025 at 08:38:37PM +0100, Ramsay Jones wrote:
Show 10 quoted lines
> 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
Ramsay Jones· Apr 6, 2025, 19:38 UTC · re: Ramsay Jones · lore

[PATCH v2 12/13] 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 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(+)
Show changes to config.mak.uname +1 −0
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
Ramsay Jones· Apr 6, 2025, 19:38 UTC · re: Ramsay Jones · lore

[PATCH v2 13/13] 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 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(+)
Show changes to config.mak.uname +1 −0
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
Ramsay Jones· Apr 6, 2025, 19:38 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to meson.build +0 −1
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
Ramsay Jones· Apr 6, 2025, 19:38 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to Makefile +21 −17
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
Patrick Steinhardt· Apr 14, 2025, 07:54 UTC · re: Ramsay Jones · lore

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

On Sun, Apr 06, 2025 at 08:38:28PM +0100, Ramsay Jones wrote:
Show 12 quoted lines
> 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
Ramsay Jones· Apr 6, 2025, 19:38 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to 2 files +31 −2

Documentation/meson.build, meson.build

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
Ramsay Jones· Apr 6, 2025, 19:49 UTC · re: Ramsay Jones · lore

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

On 06/04/2025 20:38, Ramsay Jones wrote: [snip]

Show 33 quoted lines
> 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

Patrick Steinhardt· Apr 14, 2025, 07:54 UTC · re: Ramsay Jones · lore

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

On Sun, Apr 06, 2025 at 08:49:54PM +0100, Ramsay Jones wrote:
Show 48 quoted lines
> 
> 
> 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
Ramsay Jones· Apr 14, 2025, 19:19 UTC · re: Patrick Steinhardt · lore

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

On 14/04/2025 08:54, Patrick Steinhardt wrote:
Show 52 quoted lines
> 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
Patrick Steinhardt· Apr 15, 2025, 05:59 UTC · re: Ramsay Jones · lore

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

On Mon, Apr 14, 2025 at 08:19:15PM +0100, Ramsay Jones wrote:
Show 110 quoted lines
> 
> 
> 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
Ramsay Jones· Apr 6, 2025, 19:38 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to 2 files +13 −2

meson.build, meson_options.txt

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
Ramsay Jones· Apr 6, 2025, 20:16 UTC · re: Ramsay Jones · lore

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

On 06/04/2025 20:38, Ramsay Jones wrote:
Show 50 quoted lines
> 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

Patrick Steinhardt· Apr 14, 2025, 07:54 UTC · re: Ramsay Jones · lore

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

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
Ramsay Jones· Apr 6, 2025, 19:38 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to 2 files +0 −9

Makefile, config.mak.uname

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
Ramsay Jones· Apr 16, 2025, 23:18 UTC · re: Ramsay Jones · lore

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

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;
Ramsay Jones· Apr 16, 2025, 23:18 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to meson.build +0 −1
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
Ramsay Jones· Apr 16, 2025, 23:18 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to Makefile +21 −17
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
Ramsay Jones· Apr 16, 2025, 23:18 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to 2 files +31 −2

Documentation/meson.build, meson.build

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
Ramsay Jones· Apr 16, 2025, 23:18 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to 2 files +13 −2

meson.build, meson_options.txt

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
Ramsay Jones· Apr 16, 2025, 23:18 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to 2 files +0 −9

Makefile, config.mak.uname

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
Ramsay Jones· Apr 16, 2025, 23:18 UTC · re: Ramsay Jones · lore

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

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(+)
Show changes to config.mak.uname +1 −0
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
Ramsay Jones· Apr 16, 2025, 23:18 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to 2 files +4 −2

config.mak.uname, t/t7815-grep-binary.sh

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
Ramsay Jones· Apr 16, 2025, 23:18 UTC · re: Ramsay Jones · lore

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

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(+)
Show changes to config.mak.uname +1 −0
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
Ramsay Jones· Apr 16, 2025, 23:18 UTC · re: Ramsay Jones · lore

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

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(+)
Show changes to config.mak.uname +2 −0
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
Ramsay Jones· Apr 16, 2025, 23:18 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to builtin/gc.c +7 −2
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
Ramsay Jones· Apr 16, 2025, 23:18 UTC · re: Ramsay Jones · lore

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

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(-)
Show changes to 3 files +14 −1

Makefile, config.mak.uname, configure.ac

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
Ramsay Jones· Apr 16, 2025, 23:18 UTC · re: Ramsay Jones · lore

[PATCH v3 12/13] 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 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(+)
Show changes to config.mak.uname +1 −0
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
Ramsay Jones· Apr 16, 2025, 23:18 UTC · re: Ramsay Jones · lore

[PATCH v3 13/13] 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 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(+)
Show changes to config.mak.uname +1 −0
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
Junio C Hamano· Apr 17, 2025, 13:55 UTC · re: Ramsay Jones · lore

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

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
Show 14 quoted lines
> 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?
Show 6 quoted lines
> 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.

Show 5 quoted lines
> 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.
Show 26 quoted lines
> 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
Ramsay Jones· Apr 17, 2025, 18:27 UTC · re: Junio C Hamano · lore

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

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

Show 23 quoted lines
> 
>> 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

Junio C Hamano· Apr 17, 2025, 20:13 UTC · re: Ramsay Jones · lore

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

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
Show 11 quoted lines
> 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.
Ramsay Jones· Apr 17, 2025, 22:06 UTC · re: Junio C Hamano · lore

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

On 17/04/2025 21:13, Junio C Hamano wrote:
Show 16 quoted lines
> 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

Junio C Hamano· Apr 17, 2025, 03:45 UTC · re: Ramsay Jones · lore

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

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
Show 16 quoted lines
> 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.
Patrick Steinhardt· Apr 17, 2025, 08:36 UTC · re: Junio C Hamano · lore

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

On Wed, Apr 16, 2025 at 08:45:34PM -0700, Junio C Hamano wrote:
Show 20 quoted lines
> 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

← back to recent threads