threads / patch / 63633

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

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

## tl;dr

11 messages between Jun 12, 2025 and Jul 2, 2025. Diffs are folded; open one to read it.

replies: 10people: 3as markdown or json

Carlo Arenas· Jun 12, 2025, 13:52 UTC · lore
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
brian m. carlson· Jun 12, 2025, 16:48 UTC · re: Carlo Arenas · lore
On 2025-06-12 at 13:52:03, Carlo Arenas wrote:
Show 10 quoted lines
> 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.

Show 6 quoted lines
> > 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
Carlo Marcelo Arenas Belón· Jun 12, 2025, 21:31 UTC · re: brian m. carlson · lore
On Thu, Jun 12, 2025 at 04:48:49PM -0800, brian m. carlson wrote:
Show 21 quoted lines
> 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
Junio C Hamano· Jun 12, 2025, 21:51 UTC · re: brian m. carlson · lore

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

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(-)
Show changes to config.mak.uname +0 −6
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
Carlo Marcelo Arenas Belón· Jun 12, 2025, 22:30 UTC · re: Junio C Hamano · lore

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

On Thu, Jun 12, 2025 at 02:51:02PM -0800, Junio C Hamano wrote:
Show 8 quoted lines
> 
>  * 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"

Show changes to config.mak.uname +3 −0
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
Junio C Hamano· Jun 12, 2025, 22:37 UTC · re: Carlo Marcelo Arenas Belón · lore

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

Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:
Show 20 quoted lines
> 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.
Carlo Marcelo Arenas Belón· Jul 2, 2025, 09:37 UTC · re: Junio C Hamano · lore

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

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
Carlo Marcelo Arenas Belón· Jul 2, 2025, 09:37 UTC · re: Carlo Marcelo Arenas Belón · lore

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

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(-)
Show changes to config.mak.uname +3 −6
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
Carlo Marcelo Arenas Belón· Jul 2, 2025, 09:37 UTC · re: Carlo Marcelo Arenas Belón · lore

[PATCH v4 2/2] build: retire NO_UINTMAX_T

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

Makefile, configure.ac, meson.build

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
Junio C Hamano· Jul 2, 2025, 16:21 UTC · re: Carlo Marcelo Arenas Belón · lore

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

Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:
Show 12 quoted lines
> 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.
Junio C Hamano· Jun 12, 2025, 16:52 UTC · re: Carlo Arenas · lore
Carlo Arenas <carenas@gmail.com> writes:
Show 6 quoted lines
> 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?

← back to recent threads