Re: [PATCH v2] config.mak.uname: update settings for FreeBSD

Re: [PATCH v2] config.mak.uname: update settings for FreeBSD

[Carlo Arenas]

On Thu, Jun 12, 2025 at 12:36:46AM -0800, Brad Smith wrote:
>
> FreeBSD 6.0 has memmem().

but AFAIK it was buggy, uncompatible with the "standard" and
didn't perform that well, at least until FreeBSD 12.

assuming that the system version is indeed faster than the
one provided with git (which should be true but worth testing)
then it might be better to only enable this for later versions?

> With making 6.0 the minimum version drop bits for supporting
> FreeBSD 4.x.

FreeBSD 4.x is no longer supported and wouldn't even build a
current git, since it predates C99 and is missing POSIX
compatibility with what we require (ex: no statvfs)

Carlo

Re: [PATCH v2] config.mak.uname: update settings for FreeBSD

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]

On 2025-06-12 at 13:52:03, Carlo Arenas wrote:
> On Thu, Jun 12, 2025 at 12:36:46AM -0800, Brad Smith wrote:
> >
> > FreeBSD 6.0 has memmem().
> 
> but AFAIK it was buggy, uncompatible with the "standard" and
> didn't perform that well, at least until FreeBSD 12.
> 
> assuming that the system version is indeed faster than the
> one provided with git (which should be true but worth testing)
> then it might be better to only enable this for later versions?

FreeBSD 11.4 (the last version of FreeBSD 11) went end of life in
September 2021, so nobody should be using it since it hasn't had
security support since then.  And it's even been functional (but slow)
since FreeBSD 11.0, and 10.4 went EOL in 2018.  So users shouldn't
actually be experiencing any actual functionality problems since then.

I don't think it's a big deal for people who want to use an obsolete OS
(which, to be clear, I'm not encouraging) to tweak the Makefile knobs a
bit.

> > With making 6.0 the minimum version drop bits for supporting
> > FreeBSD 4.x.
> 
> FreeBSD 4.x is no longer supported and wouldn't even build a
> current git, since it predates C99 and is missing POSIX
> compatibility with what we require (ex: no statvfs)

I definitely think getting rid of FreeBSD 4 support is fine.  It doesn't
even support AMD64, so as a practical matter it wouldn't be useful on
any sort of modern hardware.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH v2] config.mak.uname: update settings for FreeBSD

[Junio C Hamano]

Carlo Arenas <carenas@gmail.com> writes:

> On Thu, Jun 12, 2025 at 12:36:46AM -0800, Brad Smith wrote:
>>
>> FreeBSD 6.0 has memmem().
>
> but AFAIK it was buggy, uncompatible with the "standard" and
> didn't perform that well, at least until FreeBSD 12.

Declaring that we will not support anything older than 12, which was
from Dec 2018, feels a bit too harsh, so conditional to check if we
are at or above 12 is needed instead?

Documentation/technical/platform-support.adoc is probably a good
place to start a discussion.

 * It spells out Minimum Requirements which includes C99 at the
   minimum, which in turn disqualifies really ancient ones and ones
   perhaps before FreeBSD 7 (which had GCC 4)?

 * It also requires the platform has active security support.  If I
   trust https://www.freebsd.org/security/#sup page, it means
   anything older than 13.4-RELEASE are EoL already.

 * The document has a space at the end that is intended to list
   contacts for ports on platforms, but currently it is not very
   actively used.  Should we extend it to include various flavours
   of BSDs and other systems, and start listing the minimum
   supported versions as well?

Stepping back a bit, do we already have some mechanism to say "hey
you seem to be on FreeBSD but you are at release N that is way older
than the minimum version X we support" and stop the build?  If we
do, we should tell that mechanism about our decision in a patch like
this.

If we don't, I wonder if we want to have such a mechanism?  I am
personally undecided.  It would help those "casual" users and
builders who do not get their hands dirty at all (aka "I'll build
only from the official release tarballs") if we did so when they try
to build on something we know will not work well, especially if it
is kept up to date relative to what the platform-support document
lists.  But at the same time, those who do not mind fixing and
extending to make it work on out-of-support systems will be
inconvenienced with one more roadblock to dismantle before
proceeding.

Thoughts?

Re: [PATCH v2] config.mak.uname: update settings for FreeBSD

[Carlo Marcelo Arenas Belón]

On Thu, Jun 12, 2025 at 04:48:49PM -0800, brian m. carlson wrote:
> On 2025-06-12 at 13:52:03, Carlo Arenas wrote:
> > On Thu, Jun 12, 2025 at 12:36:46AM -0800, Brad Smith wrote:
> > >
> > > FreeBSD 6.0 has memmem().
> > 
> > but AFAIK it was buggy, uncompatible with the "standard" and
> > didn't perform that well, at least until FreeBSD 12.
> > 
> > assuming that the system version is indeed faster than the
> > one provided with git (which should be true but worth testing)
> > then it might be better to only enable this for later versions?
> 
> FreeBSD 11.4 (the last version of FreeBSD 11) went end of life in
> September 2021, so nobody should be using it since it hasn't had
> security support since then.  And it's even been functional (but slow)
> since FreeBSD 11.0, and 10.4 went EOL in 2018.  So users shouldn't
> actually be experiencing any actual functionality problems since then.
> 
> I don't think it's a big deal for people who want to use an obsolete OS
> (which, to be clear, I'm not encouraging) to tweak the Makefile knobs a
> bit.

Note that my concern wasn't about having to tweak the Makefile, but with
the fact that the system provided function would behave differently, and
there was no attempt to see if by no longer using the git provided compat
code, there was actually a performance improvement.

It is true that in our codebase there are no calls to memmem() where the
needlelen (the fourth parameter) could be zero, and that would result in
some of those old versions returning NULL, but it would seem to be safer
to only use the system provided function when those issues are no longer
a concern.

Carlo

[PATCH v3] config.mak.uname: update settings for FreeBSD

[Junio C Hamano]

Even though FreeBSD 6 introduced memmem(), the implementation in
that version was buggy and not performant until FreeBSD 12, FreeBSD
11.4 (the last version of FreeBSD 11) went end of life in September
2021, so nobody should be using it since it hasn't had security
support since then.  And memmem() has even been functional (but
slow) since FreeBSD 11.0, and 10.4 went EOL in 2018.  So users
shouldn't actually be experiencing any actual functionality problems
since then.

Let's draw the line to require FreeBSD 12 or newer (but we do not
officially document it or enforce it by breaking build when compiled
on older versions, at least not yet), which allows us to drop the
special casing of FreeBSD 4.x and rely on platform implementation of
memmem() unconditionally.

Signed-off-by: Brad Smith <brad@comstyle.com>
[jc: log message with help from info by brian carlson]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * So,... earlier I mentioned about officially documenting the EoL
   timeline for various platform support, but without any official
   one that documents the decision in one place, how about a commit
   with a detailed log like this one, which I stole from brian?

   No code changes since v2; only the log message talks more about
   where we draw the line and why.

 config.mak.uname | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/config.mak.uname b/config.mak.uname
index b12d4e168a..5d18d92cb1 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -273,16 +273,10 @@ ifeq ($(uname_S),FreeBSD)
         ifeq ($(firstword $(subst -, ,$(uname_R))),10.1)
 		OLD_ICONV = YesPlease
         endif
-	NO_MEMMEM = YesPlease
 	BASIC_CFLAGS += -I/usr/local/include
 	BASIC_LDFLAGS += -L/usr/local/lib
 	DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
 	USE_ST_TIMESPEC = YesPlease
-        ifeq ($(shell expr "$(uname_R)" : '4\.'),2)
-		PTHREAD_LIBS = -pthread
-		NO_UINTMAX_T = YesPlease
-		NO_STRTOUMAX = YesPlease
-        endif
 	PYTHON_PATH = /usr/local/bin/python
 	PERL_PATH = /usr/local/bin/perl
 	HAVE_PATHS_H = YesPlease
-- 
2.50.0-rc2-215-g56f75d5edf

Re: [PATCH v3] config.mak.uname: update settings for FreeBSD

[Carlo Marcelo Arenas Belón]

On Thu, Jun 12, 2025 at 02:51:02PM -0800, Junio C Hamano wrote:
> 
>  * So,... earlier I mentioned about officially documenting the EoL
>    timeline for various platform support, but without any official
>    one that documents the decision in one place, how about a commit
>    with a detailed log like this one, which I stole from brian?
> 
>    No code changes since v2; only the log message talks more about
>    where we draw the line and why.

I was hoping something more like with the following (untested) "fixup"
on toa, obviously the "unconditionally" in the commit message should
need adding "for the supported versions"

diff --git a/config.mak.uname b/config.mak.uname
index 9cac400d94..cbf1f4c0d2 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -280,6 +280,9 @@ ifeq ($(uname_S),FreeBSD)
         ifeq ($(firstword $(subst -, ,$(uname_R))),10.1)
 		OLD_ICONV = YesPlease
         endif
+	ifeq ($(shell test "`expr "$(uname_R)" : '\([1-9][0-9]*\)\.'`" -lt 12 && echo 1),1)
+		NO_MEMMEM = UnfortunatelyYes
+	endif
 	BASIC_CFLAGS += -I/usr/local/include
 	BASIC_LDFLAGS += -L/usr/local/lib
 	DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease

Note that either way the build won't be broken

Carlo

Re: [PATCH v3] config.mak.uname: update settings for FreeBSD

[Junio C Hamano]

Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:

> I was hoping something more like with the following (untested) "fixup"
> on toa, obviously the "unconditionally" in the commit message should
> need adding "for the supported versions"
>
> diff --git a/config.mak.uname b/config.mak.uname
> index 9cac400d94..cbf1f4c0d2 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -280,6 +280,9 @@ ifeq ($(uname_S),FreeBSD)
>          ifeq ($(firstword $(subst -, ,$(uname_R))),10.1)
>  		OLD_ICONV = YesPlease
>          endif
> +	ifeq ($(shell test "`expr "$(uname_R)" : '\([1-9][0-9]*\)\.'`" -lt 12 && echo 1),1)
> +		NO_MEMMEM = UnfortunatelyYes
> +	endif
>  	BASIC_CFLAGS += -I/usr/local/include
>  	BASIC_LDFLAGS += -L/usr/local/lib
>  	DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
>
> Note that either way the build won't be broken

As I do not think we quite care about anything older than 12, I am
perfectly fine being a bit extra defensive like your version.

Care to assemble the final version with both code and log message
updates?  We are not in a hurry, as we are talking about a rather
ancient issue and this will not come close to 'master' before the
final release next week anyway.

Thanks.

[PATCH v4 0/2] config.mak.uname: update settings for FreeBSD

[Carlo Marcelo Arenas Belón]

Modernize config.mak.uname defaults for FreeBSD to prioritize using the
platform implementation of memmem().

Carlo Marcelo Arenas Belón (2):
  config.mak.uname: set NO_MEMMEM only for functional version
  build: retire NO_UINTMAX_T

 Makefile         |  5 -----
 config.mak.uname |  9 +++------
 configure.ac     |  8 --------
 meson.build      | 11 -----------
 4 files changed, 3 insertions(+), 30 deletions(-)

-- 
2.50.0.147.gafe0d4ec5b

[PATCH v4 1/2] config.mak.uname: set NO_MEMMEM only for functional version

[Carlo Marcelo Arenas Belón]

FreeBSD 6 introduced memmem(), but the implementation diverged
from what was standard everywhere else (including our "compat"
fallback).

FreeBSD 10.4 (went EOL in 2018) corrected the functionality bugs
but kept a suboptimal implementation until FreeBSD 11.4 (the last
version of FreeBSD 11, that went EOL in September 2021).

Let's draw the line to require FreeBSD 12 or newer, which allows us
to drop the special casing of FreeBSD 4.x and rely on the platform
implementation of memmem() unconditionally for all versions that are
still being supported.

Suggested-by: Brad Smith <brad@comstyle.com>
Helped-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 config.mak.uname | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/config.mak.uname b/config.mak.uname
index b12d4e168a..2b434df9e5 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -273,16 +273,13 @@ ifeq ($(uname_S),FreeBSD)
         ifeq ($(firstword $(subst -, ,$(uname_R))),10.1)
 		OLD_ICONV = YesPlease
         endif
-	NO_MEMMEM = YesPlease
+        ifeq ($(shell v=$(uname_R) && test $${v%%.*} -lt 12 && echo 1),1)
+		NO_MEMMEM = UnfortunatelyYes
+        endif
 	BASIC_CFLAGS += -I/usr/local/include
 	BASIC_LDFLAGS += -L/usr/local/lib
 	DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
 	USE_ST_TIMESPEC = YesPlease
-        ifeq ($(shell expr "$(uname_R)" : '4\.'),2)
-		PTHREAD_LIBS = -pthread
-		NO_UINTMAX_T = YesPlease
-		NO_STRTOUMAX = YesPlease
-        endif
 	PYTHON_PATH = /usr/local/bin/python
 	PERL_PATH = /usr/local/bin/perl
 	HAVE_PATHS_H = YesPlease
-- 
2.50.0.147.gafe0d4ec5b

[PATCH v4 2/2] build: retire NO_UINTMAX_T

[Carlo Marcelo Arenas Belón]

A previous commit removed the last user of it, and it is no
longer useful with the codebase moving towards C99, which
specifies its definition.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 Makefile     |  5 -----
 configure.ac |  8 --------
 meson.build  | 11 -----------
 3 files changed, 24 deletions(-)

diff --git a/Makefile b/Makefile
index 3868edd349..ba111f191f 100644
--- a/Makefile
+++ b/Makefile
@@ -114,8 +114,6 @@ include shared.mak
 #
 # Define NO_INTPTR_T if you don't have intptr_t or uintptr_t.
 #
-# Define NO_UINTMAX_T if you don't have uintmax_t.
-#
 # Define NEEDS_SOCKET if linking with libc is not enough (SunOS,
 # Patrick Mauritz).
 #
@@ -1915,9 +1913,6 @@ endif
 ifdef NO_INTPTR_T
 	COMPAT_CFLAGS += -DNO_INTPTR_T
 endif
-ifdef NO_UINTMAX_T
-	BASIC_CFLAGS += -Duintmax_t=uint32_t
-endif
 ifdef NO_SOCKADDR_STORAGE
 ifdef NO_IPV6
 	BASIC_CFLAGS += -Dsockaddr_storage=sockaddr_in
diff --git a/configure.ac b/configure.ac
index 5923edc44a..d8c3af161b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1121,14 +1121,6 @@ GIT_CHECK_FUNC(strlcpy,
 [NO_STRLCPY=YesPlease])
 GIT_CONF_SUBST([NO_STRLCPY])
 #
-# Define NO_UINTMAX_T if your platform does not have uintmax_t
-AC_CHECK_TYPE(uintmax_t,
-[NO_UINTMAX_T=],
-[NO_UINTMAX_T=YesPlease],[
-#include <inttypes.h>
-])
-GIT_CONF_SUBST([NO_UINTMAX_T])
-#
 # Define NO_STRTOUMAX if you don't have strtoumax in the C library.
 GIT_CHECK_FUNC(strtoumax,
 [NO_STRTOUMAX=],
diff --git a/meson.build b/meson.build
index efe2871c9d..27d5f40741 100644
--- a/meson.build
+++ b/meson.build
@@ -1331,17 +1331,6 @@ if compiler.compiles('''
   libgit_c_args += '-DHAVE_CLOCK_MONOTONIC'
 endif
 
-if not compiler.compiles('''
-  #include <inttypes.h>
-
-  void func(void)
-  {
-    uintmax_t x = 0;
-  }
-''', name: 'uintmax_t')
-  libgit_c_args += '-DNO_UINTMAX_T'
-endif
-
 has_bsd_sysctl = false
 if compiler.has_header('sys/sysctl.h')
   if compiler.compiles('''
-- 
2.50.0.147.gafe0d4ec5b

Re: [PATCH v4 0/2] config.mak.uname: update settings for FreeBSD

[Junio C Hamano]

Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:

> Modernize config.mak.uname defaults for FreeBSD to prioritize using the
> platform implementation of memmem().
>
> Carlo Marcelo Arenas Belón (2):
>   config.mak.uname: set NO_MEMMEM only for functional version
>   build: retire NO_UINTMAX_T
>
>  Makefile         |  5 -----
>  config.mak.uname |  9 +++------
>  configure.ac     |  8 --------
>  meson.build      | 11 -----------
>  4 files changed, 3 insertions(+), 30 deletions(-)

Thanks.  As far as I (without access to FreeBSD boxes) can see, the
patches look good---the changes are reasonable and well described.

Will replace the single patch from Brad.

Thanks.