{"thread":{"id":"63322","subject":"[PATCH 1/6] meson: simplify and parameterize various standard function checks","startedAt":"2025-04-21T17:53:02Z","lastAt":"2025-04-25T09:53:24Z","messageCount":40,"participants":["Eli Schwartz","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"516434","messageId":"20250421175247.240971-1-eschwartz@gentoo.org","threadId":"63322","inReplyTo":null,"subject":"[PATCH 1/6] meson: simplify and parameterize various standard function checks","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-21T17:51:45Z","receivedAt":"2025-04-21T17:53:02Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"This is repetitive logic. We either want to use some -lc function, or if\nit is not available we define it as -DNO_XXX and usually (but not\nalways) provide some custom compatibility impl instead.\n\nChecking the intent of each block when reading through the file is slow\nand not very DRY. Switch to taking an array of checkable functions\ninstead.\n\nNot all functions are straightforward to move, since different macro\nprefixes are used.\n\nSigned-off-by: Eli Schwartz \n---\n meson.build | 73 ++++++++++++++++++++++-------------------------------\n 1 file changed, 30 insertions(+), 43 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex c47cb79af0..6c147c22a4 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1290,23 +1290,40 @@ if not compiler.has_member('struct passwd', 'pw_gecos', prefix: '#include \n---\n meson.build | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/meson.build b/meson.build\nindex 6c147c22a4..f5d9ffcd7f 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1300,6 +1300,8 @@ checkfuncs = [\n 'mkdtemp',\n # no compat\n 'initgroups',\n+ # no compat\n+ 'getpagesize',\n ]\n \n if host_machine.system() == 'windows'\n-- \n2.49.0\n\n"},{"id":"516436","messageId":"20250421175247.240971-3-eschwartz@gentoo.org","threadId":"63322","inReplyTo":"20250421175247.240971-1-eschwartz@gentoo.org","subject":"[PATCH 3/6] meson: do a full usage-based compile check for sysinfo","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-21T17:51:47Z","receivedAt":"2025-04-21T17:53:07Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"On Solaris, sys/sysinfo.h is a completely different file and doesn't\nresemble the linux file at all. There is also a sysinfo() function, but\nit takes a totally different call signature, which asks for:\n\n- the field you wish to receive\n- a `char *buf` to copy the data to\n\nand is very useful IFF you want to know, say, the hardware provider. Or,\nget *specific* fields from uname(2).\n\nhttps://docs.oracle.com/cd/E86824_01/html/E54765/sysinfo-2.html\n\nIt is surely possible to do this manually via `sysconf(3)` without the\nnice API. I can't find anything more direct. Either way, I'm not very\nattached to Solaris, so someone who cares can add it. Either way, it's\nwrong to assume that sysinfo.h contains what we are looking for.\n\nCheck that sysinfo.h defines the struct we actually utilize in\nbuiltins/gc.c, which will correctly fail on systems that don't have it.\n\nSigned-off-by: Eli Schwartz \n---\n meson.build | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex f5d9ffcd7f..8037e536dd 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1058,10 +1058,6 @@ if compiler.has_header('alloca.h')\n libgit_c_args += '-DHAVE_ALLOCA_H'\n endif\n \n-if compiler.has_header('sys/sysinfo.h')\n- libgit_c_args += '-DHAVE_SYSINFO'\n-endif\n-\n # Windows has libgen.h and a basename implementation, but we still need our own\n # implementation to threat things like drive prefixes specially.\n if host_machine.system() == 'windows' or not compiler.has_header('libgen.h')\n@@ -1272,6 +1268,10 @@ if host_machine.system() != 'windows'\n endif\n endif\n \n+if compiler.has_member('struct sysinfo', 'totalram', prefix: '#include ')\n+ libgit_c_args += '-DHAVE_SYSINFO'\n+endif\n+\n if compiler.has_member('struct stat', 'st_mtimespec.tv_nsec', prefix: '#include ')\n libgit_c_args += '-DUSE_ST_TIMESPEC'\n elif not compiler.has_member('struct stat', 'st_mtim.tv_nsec', prefix: '#include ')\n-- \n2.49.0\n\n"},{"id":"516437","messageId":"20250421175247.240971-4-eschwartz@gentoo.org","threadId":"63322","inReplyTo":"20250421175247.240971-1-eschwartz@gentoo.org","subject":"[PATCH 4/6] meson: add a couple missing networking dependencies","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-21T17:51:48Z","receivedAt":"2025-04-21T17:53:07Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"As evidenced in config.mak.uname and configure.ac, there are various\npossible scenarios where these libraries are default-enabled in the\nbuild, which mainly boils down to: SunOS. -lresolv is simply not the\nonly library that, when it exists, probably needs to be linked to for\nnetworking.\n\nCheck for and add -lnsl -lsocket as well.\n\nSigned-off-by: Eli Schwartz \n---\n meson.build | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex 8037e536dd..8fad10379a 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1080,10 +1080,11 @@ if host_machine.system() == 'windows'\n networking_dependencies += winsock\n endif\n else\n- libresolv = compiler.find_library('resolv', required: false)\n- if libresolv.found()\n- networking_dependencies += libresolv\n- endif\n+ networking_dependencies += [\n+ compiler.find_library('nsl', required: false),\n+ compiler.find_library('resolv', required: false),\n+ compiler.find_library('socket', required: false),\n+ ]\n endif\n libgit_dependencies += networking_dependencies\n \n-- \n2.49.0\n\n"},{"id":"516438","messageId":"20250421175247.240971-5-eschwartz@gentoo.org","threadId":"63322","inReplyTo":"20250421175247.240971-1-eschwartz@gentoo.org","subject":"[PATCH 5/6] meson: fix typo in function check that prevented checking for hstrerror","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-21T17:51:49Z","receivedAt":"2025-04-21T17:53:08Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"Nowhere in the codebase do we otherwise check for strerror. Nowhere in\nthe codebase do we make use of -DNO_STRERROR. `strerror` is not a\nnetworking function at all.\n\nWe do utilize `hstrerror` though, which is a networking function we\nshould have been checking here.\n\nSigned-off-by: Eli Schwartz \n---\n meson.build | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/meson.build b/meson.build\nindex 8fad10379a..1b7e55756b 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1088,7 +1088,7 @@ else\n endif\n libgit_dependencies += networking_dependencies\n \n-foreach symbol : ['inet_ntop', 'inet_pton', 'strerror']\n+foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']\n if not compiler.has_function(symbol, dependencies: networking_dependencies)\n libgit_c_args += '-DNO_' + symbol.to_upper()\n endif\n-- \n2.49.0\n\n"},{"id":"516439","messageId":"20250421175247.240971-6-eschwartz@gentoo.org","threadId":"63322","inReplyTo":"20250421175247.240971-1-eschwartz@gentoo.org","subject":"[PATCH 6/6] meson: only check for missing networking syms on non-Windows; add compat impls","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-21T17:51:50Z","receivedAt":"2025-04-21T17:53:09Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"These are added in the Makefile, but not in meson. They probably won't\nwork well on systems without them.\n\nCMake adds them, but only on non-Windows. Actually, it only performs\ncompiler checks for hstrerror, but excludes that check on Windows with\nthe note that it is \"incompatible with the Windows build\". This seems to\nbe misleading -- it is not incompatible, it simply doesn't exist. Still,\nthe compat version should not be used.\n\nI interpret this cmake logic to mean we shouldn't even be checking for\nsymbol availability on Windows. In addition to making it simple to add\ncompat definitions, this also probably shaves off a second or two of\nconfigure time on Windows as no compiler check needs to be performed.\n\nSigned-off-by: Eli Schwartz \n---\n meson.build | 13 ++++++++-----\n 1 file changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex 1b7e55756b..24b304fb57 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1088,11 +1088,14 @@ else\n endif\n libgit_dependencies += networking_dependencies\n \n-foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']\n- if not compiler.has_function(symbol, dependencies: networking_dependencies)\n- libgit_c_args += '-DNO_' + symbol.to_upper()\n- endif\n-endforeach\n+if host_machine.system() != 'windows'\n+ foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']\n+ if not compiler.has_function(symbol, dependencies: networking_dependencies)\n+ libgit_c_args += '-DNO_' + symbol.to_upper()\n+ libgit_sources += 'compat/' + symbol + '.c'\n+ endif\n+ endforeach\n+endif\n \n has_ipv6 = compiler.has_function('getaddrinfo', dependencies: networking_dependencies)\n if not has_ipv6\n-- \n2.49.0\n\n"},{"id":"516441","messageId":"83d9fda5-8399-47fb-87b2-a8b376cf1625@gentoo.org","threadId":"63322","inReplyTo":"20250421175247.240971-1-eschwartz@gentoo.org","subject":"Re: [PATCH 1/6] meson: simplify and parameterize various standard function checks","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-21T20:04:30Z","receivedAt":"2025-04-21T20:04:34Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"On 4/21/25 1:51 PM, Eli Schwartz wrote:\n> This is repetitive logic. We either want to use some -lc function, or if\n> it is not available we define it as -DNO_XXX and usually (but not\n> always) provide some custom compatibility impl instead.\n> \n> Checking the intent of each block when reading through the file is slow\n> and not very DRY. Switch to taking an array of checkable functions\n> instead.\n> \n> Not all functions are straightforward to move, since different macro\n> prefixes are used.\n\n\nBy the way, when reviewing this I was having a slightly hard time\nfiguring out which stuff belonged here... specifically, because of the\ndifferences in macro prefixes lead me to believe it's not always so\nsimple as \"does it exist\".\n\n\n\n> Signed-off-by: Eli Schwartz \n> ---\n> meson.build | 73 ++++++++++++++++++++++-------------------------------\n> 1 file changed, 30 insertions(+), 43 deletions(-)\n> \n> diff --git a/meson.build b/meson.build\n> index c47cb79af0..6c147c22a4 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -1290,23 +1290,40 @@ if not compiler.has_member('struct passwd', 'pw_gecos', prefix: '#include libgit_c_args += '-DNO_GECOS_IN_PWENT'\n> endif\n> \n> -if compiler.has_function('sync_file_range')\n> - libgit_c_args += '-DHAVE_SYNC_FILE_RANGE'\n> -endif\n> +checkfuncs = [\n> + 'strcasestr',\n> + 'memmem',\n> + 'strlcpy',\n> + # no compat\n> + 'strtoull',\n> + 'setenv',\n> + 'mkdtemp',\n> + # no compat\n> + 'initgroups',\n> +]\n> \n> -if not compiler.has_function('strcasestr')\n> - libgit_c_args += '-DNO_STRCASESTR'\n> - libgit_sources += 'compat/strcasestr.c'\n> +if host_machine.system() == 'windows'\n> + libgit_c_args += '-DUSE_WIN32_MMAP'\n> +else\n> + checkfuncs += [\n> + 'mmap',\n> + # unsetenv is provided by compat/mingw.c.\n> + 'unsetenv',\n> + ]\n> endif\n> \n> -if not compiler.has_function('memmem')\n> - libgit_c_args += '-DNO_MEMMEM'\n> - libgit_sources += 'compat/memmem.c'\n> -endif\n> +foreach func: checkfuncs\n> + if not compiler.has_function(func)\n> + libgit_c_args += '-DNO_' + func.to_upper()\n> + impl = 'compat/' + func + '.c'\n> + if fs.exists(impl)\n> + libgit_sources += impl\n> + endif\n> + endif\n> +endforeach\n> \n> -if not compiler.has_function('strlcpy')\n> - libgit_c_args += '-DNO_STRLCPY'\n> - libgit_sources += 'compat/strlcpy.c'\n> +if compiler.has_function('sync_file_range')\n> + libgit_c_args += '-DHAVE_SYNC_FILE_RANGE'\n> endif\n> \n> if not compiler.has_function('strdup')\n> @@ -1322,45 +1339,15 @@ if not compiler.has_function('strtoumax')\n> ]\n> endif\n> \n> -if not compiler.has_function('strtoull')\n> - libgit_c_args += '-DNO_STRTOULL'\n> -endif\n> -\n> -if not compiler.has_function('setenv')\n> - libgit_c_args += '-DNO_SETENV'\n> - libgit_sources += 'compat/setenv.c'\n> -endif\n> -\n> if not compiler.has_function('qsort')\n> libgit_c_args += '-DINTERNAL_QSORT'\n> endif\n> libgit_sources += 'compat/qsort_s.c'\n\n\n... for example, the Makefile says here:\n\n\n# Define INTERNAL_QSORT to use Git's implementation of qsort(), which\n# is a simplified version of the merge sort used in glibc. This is\n# recommended if Git triggers O(n^2) behavior in your platform's\n# qsort().\n\ncmake unconditionally defines it (???)\n\nconfig.mak.uname says:\n\n- AIX:\n INTERNAL_QSORT = UnfortunatelyYes\n\n Seems to date back to commit 377d9c409ffe0f0d994b929aeb94716139207b9d.\n \"Unfortunate\" indeed.\n\n\n- MinGW:\n INTERNAL_QSORT = YesPlease\n\n Windows claims to have a qsort but perhaps it is very slow and bes\n avoided?\n\nWe should probably stop *checking* for qsort and simply encode the\nplatforms we know are slow and automatically skip it there. Can I get\nconfirmation regarding Windows? :)\n\n\n> -# unsetenv is provided by compat/mingw.c.\n> -if host_machine.system() != 'windows' and not compiler.has_function('unsetenv')\n> - libgit_c_args += '-DNO_UNSETENV'\n> - libgit_sources += 'compat/unsetenv.c'\n> -endif\n> -\n> -if not compiler.has_function('mkdtemp')\n> - libgit_c_args += '-DNO_MKDTEMP'\n> - libgit_sources += 'compat/mkdtemp.c'\n> -endif\n> -\n> -if not compiler.has_function('initgroups')\n> - libgit_c_args += '-DNO_INITGROUPS'\n> -endif\n> -\n> if compiler.has_function('getdelim')\n> libgit_c_args += '-DHAVE_GETDELIM'\n> endif\n\n\nBut stuff like this, why isn't it consistent with the other functions?\nWhat's the difference between HAVE_XXX and NO_XXX?\n\n\n> -if host_machine.system() == 'windows'\n> - libgit_c_args += '-DUSE_WIN32_MMAP'\n> -elif not compiler.has_function('mmap')\n> - libgit_c_args += '-DNO_MMAP'\n> - libgit_sources += 'compat/mmap.c'\n> -endif\n> \n> if compiler.has_function('clock_gettime')\n> libgit_c_args += '-DHAVE_CLOCK_GETTIME'\n\n\n\n-- \nEli Schwartz\n"},{"id":"516447","messageId":"xmqqh62h591v.fsf@gitster.g","threadId":"63322","inReplyTo":"83d9fda5-8399-47fb-87b2-a8b376cf1625@gentoo.org","subject":"Re: [PATCH 1/6] meson: simplify and parameterize various standard function checks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-04-22T00:33:48Z","receivedAt":"2025-04-22T00:33:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eli Schwartz writes:\n\n> On 4/21/25 1:51 PM, Eli Schwartz wrote:\n>> This is repetitive logic. We either want to use some -lc function, or if\n>> it is not available we define it as -DNO_XXX and usually (but not\n>> always) provide some custom compatibility impl instead.\n>> \n>> Checking the intent of each block when reading through the file is slow\n>> and not very DRY. Switch to taking an array of checkable functions\n>> instead.\n>> \n>> Not all functions are straightforward to move, since different macro\n>> prefixes are used.\n>\n>\n> By the way, when reviewing this I was having a slightly hard time\n> figuring out which stuff belonged here... specifically, because of the\n> differences in macro prefixes lead me to believe it's not always so\n> simple as \"does it exist\".\n\n\nAs there are non-zero number of meson related topics in flight, I'd\nlike to know where this new series is meant to apply, if you need\nsome of them before we can apply it, and what is the overall goal\nthis series has (\"there is no theme, they are just random set of\nchanges to do such and such things\" is perfectly acceptable answer).\n\nAnd the best place to describe these things is in the cover letter\n[PATCH 0/6] of the series.\n\nThanks.\n"},{"id":"516448","messageId":"f68460ea-2754-466c-8078-ca51bcb291ac@gentoo.org","threadId":"63322","inReplyTo":"xmqqh62h591v.fsf@gitster.g","subject":"Re: [PATCH 1/6] meson: simplify and parameterize various standard function checks","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-22T00:58:47Z","receivedAt":"2025-04-22T00:58:51Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"On 4/21/25 8:33 PM, Junio C Hamano wrote:\n> Eli Schwartz writes:\n> \n>> On 4/21/25 1:51 PM, Eli Schwartz wrote:\n>>> This is repetitive logic. We either want to use some -lc function, or if\n>>> it is not available we define it as -DNO_XXX and usually (but not\n>>> always) provide some custom compatibility impl instead.\n>>>\n>>> Checking the intent of each block when reading through the file is slow\n>>> and not very DRY. Switch to taking an array of checkable functions\n>>> instead.\n>>>\n>>> Not all functions are straightforward to move, since different macro\n>>> prefixes are used.\n>>\n>>\n>> By the way, when reviewing this I was having a slightly hard time\n>> figuring out which stuff belonged here... specifically, because of the\n>> differences in macro prefixes lead me to believe it's not always so\n>> simple as \"does it exist\".\n> \n> \n> As there are non-zero number of meson related topics in flight, I'd\n> like to know where this new series is meant to apply, if you need\n> some of them before we can apply it, and what is the overall goal\n> this series has (\"there is no theme, they are just random set of\n> changes to do such and such things\" is perfectly acceptable answer).\n> \n> And the best place to describe these things is in the cover letter\n> [PATCH 0/6] of the series.\n\n\nMy apologies. There was no big theme other than that they were things I\ndetermined were relevant to more closely match the Makefile\nexpectations, while investigating a badly worded report (in fact, a\nwholly uncommunicated :( local patch ) of git not building on Gentoo's\nSolaris environment ( https://wiki.gentoo.org/wiki/Project:Prefix )\n\ne.g. I simplified the repetitive lists because it made it easier to do\nthe followup patch adding a new check for getpagesize (which I needed,\nbecause it needed to be checked on Solaris).\n\nI think that I sort of subconsciously assumed that \"if in doubt, assume\nit's independently developed against the current state of the master\nbranch\".\n\nI do not need any other series merged, I think it should apply to\n`master` independently of all of them. I can't see anything available in\norigin/master..origin/seen that would clash, at least.\n\n\n-- \nEli Schwartz\n"},{"id":"516469","messageId":"aAdFysi-n_5Aa4Au@pks.im","threadId":"63322","inReplyTo":"20250421175247.240971-1-eschwartz@gentoo.org","subject":"Re: [PATCH 1/6] meson: simplify and parameterize various standard function checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-22T07:31:22Z","receivedAt":"2025-04-22T07:31:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Apr 21, 2025 at 01:51:45PM -0400, Eli Schwartz wrote:\n> This is repetitive logic. We either want to use some -lc function, or if\n> it is not available we define it as -DNO_XXX and usually (but not\n> always) provide some custom compatibility impl instead.\n> \n> Checking the intent of each block when reading through the file is slow\n> and not very DRY. Switch to taking an array of checkable functions\n> instead.\n> \n> Not all functions are straightforward to move, since different macro\n> prefixes are used.\n\nYeah, this is somewhat unfortunate indeed. I think in the long term we\nmight want to unify our approach so that we consistently use e.g.\n`HAVE_SOME_FUNCTION` or `NO_SOME_FUNCTION`.\n\n> Signed-off-by: Eli Schwartz \n> ---\n> meson.build | 73 ++++++++++++++++++++++-------------------------------\n> 1 file changed, 30 insertions(+), 43 deletions(-)\n> \n> diff --git a/meson.build b/meson.build\n> index c47cb79af0..6c147c22a4 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -1290,23 +1290,40 @@ if not compiler.has_member('struct passwd', 'pw_gecos', prefix: '#include libgit_c_args += '-DNO_GECOS_IN_PWENT'\n> endif\n> \n> -if compiler.has_function('sync_file_range')\n> - libgit_c_args += '-DHAVE_SYNC_FILE_RANGE'\n> -endif\n> +checkfuncs = [\n> + 'strcasestr',\n> + 'memmem',\n> + 'strlcpy',\n> + # no compat\n> + 'strtoull',\n> + 'setenv',\n> + 'mkdtemp',\n> + # no compat\n> + 'initgroups',\n> +]\n> \n> -if not compiler.has_function('strcasestr')\n> - libgit_c_args += '-DNO_STRCASESTR'\n> - libgit_sources += 'compat/strcasestr.c'\n> +if host_machine.system() == 'windows'\n> + libgit_c_args += '-DUSE_WIN32_MMAP'\n> +else\n> + checkfuncs += [\n> + 'mmap',\n> + # unsetenv is provided by compat/mingw.c.\n> + 'unsetenv',\n> + ]\n> endif\n> \n> -if not compiler.has_function('memmem')\n> - libgit_c_args += '-DNO_MEMMEM'\n> - libgit_sources += 'compat/memmem.c'\n> -endif\n> +foreach func: checkfuncs\n\nOur current code style puts a space both before and after the colon.\n\n> + if not compiler.has_function(func)\n> + libgit_c_args += '-DNO_' + func.to_upper()\n> + impl = 'compat/' + func + '.c'\n> + if fs.exists(impl)\n> + libgit_sources += impl\n> + endif\n\nI think this is a bit too magic-y. An alternative might be to have\n`checkfuncs` be a dictionary where the value of each function is the\ncompat sources that we fall back to.\n\nOther than that I really like the simplifications introduced by this\npatch, thanks!\n\nPatrick\n"},{"id":"516470","messageId":"aAdF0XH2evrJ4ZQL@pks.im","threadId":"63322","inReplyTo":"20250421175247.240971-2-eschwartz@gentoo.org","subject":"Re: [PATCH 2/6] meson: check for getpagesize before using it","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-22T07:31:29Z","receivedAt":"2025-04-22T07:31:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Apr 21, 2025 at 01:51:46PM -0400, Eli Schwartz wrote:\n> It is deprecated and removed in SUS v3 / POSIX 2001, so various systems\n> may not include it. Solaris, in particular, carefully refrains from\n> defining it except inside of a maze of `#ifdef` to make sure you have\n> kept your nose clean and only used it in code that *targets* SUS v2 or\n> earlier.\n> \n> config.mak.uname defines this automatically, though only for QNX.\n\nAh, interesting. I mostly went by our autoconf infrastructure when\nconverting the checks, which didn't have a check for `getpagesize()`\neither. We might want to teach autoconf to check for this function while\nat it.\n\nIn all honesty though, I rather hope that we're soon in a state where we\ncan just drop autoconf altogether in favor of Meson. The only two\nblockers I'm aware of are wiring up git-gui and gitk. The former project\nhas already been adapted upstream, the latter is still in review. But\nonce those have landed, we should be ready to mark Meson as stable and\nthen we can start deprecating autoconf unless there are good reasons not\nto do so.\n\nPatrick\n"},{"id":"516471","messageId":"aAdF1MvGJcqfpI4p@pks.im","threadId":"63322","inReplyTo":"20250421175247.240971-3-eschwartz@gentoo.org","subject":"Re: [PATCH 3/6] meson: do a full usage-based compile check for sysinfo","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-22T07:31:32Z","receivedAt":"2025-04-22T07:31:35Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Apr 21, 2025 at 01:51:47PM -0400, Eli Schwartz wrote:\n> diff --git a/meson.build b/meson.build\n> index f5d9ffcd7f..8037e536dd 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -1058,10 +1058,6 @@ if compiler.has_header('alloca.h')\n> libgit_c_args += '-DHAVE_ALLOCA_H'\n> endif\n> \n> -if compiler.has_header('sys/sysinfo.h')\n> - libgit_c_args += '-DHAVE_SYSINFO'\n> -endif\n> -\n> # Windows has libgen.h and a basename implementation, but we still need our own\n> # implementation to threat things like drive prefixes specially.\n> if host_machine.system() == 'windows' or not compiler.has_header('libgen.h')\n> @@ -1272,6 +1268,10 @@ if host_machine.system() != 'windows'\n> endif\n> endif\n> \n> +if compiler.has_member('struct sysinfo', 'totalram', prefix: '#include ')\n> + libgit_c_args += '-DHAVE_SYSINFO'\n> +endif\n\nMakes sense. We do have c9a51775a36 (builtin/gc.c: correct RAM\ncalculation when using sysinfo, 2025-04-17) in flight which also causes\nus to use `struct sysinfo::mem_unit`. But I think it's fine to check for\nonly one of the members here.\n\nPatrick\n"},{"id":"516472","messageId":"aAdF138y9wPD-G_t@pks.im","threadId":"63322","inReplyTo":"20250421175247.240971-4-eschwartz@gentoo.org","subject":"Re: [PATCH 4/6] meson: add a couple missing networking dependencies","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-22T07:31:35Z","receivedAt":"2025-04-22T07:31:38Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Apr 21, 2025 at 01:51:48PM -0400, Eli Schwartz wrote:\n> As evidenced in config.mak.uname and configure.ac, there are various\n> possible scenarios where these libraries are default-enabled in the\n> build, which mainly boils down to: SunOS. -lresolv is simply not the\n> only library that, when it exists, probably needs to be linked to for\n> networking.\n> \n> Check for and add -lnsl -lsocket as well.\n> \n> Signed-off-by: Eli Schwartz \n> ---\n> meson.build | 9 +++++----\n> 1 file changed, 5 insertions(+), 4 deletions(-)\n> \n> diff --git a/meson.build b/meson.build\n> index 8037e536dd..8fad10379a 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -1080,10 +1080,11 @@ if host_machine.system() == 'windows'\n> networking_dependencies += winsock\n> endif\n> else\n> - libresolv = compiler.find_library('resolv', required: false)\n> - if libresolv.found()\n> - networking_dependencies += libresolv\n> - endif\n> + networking_dependencies += [\n> + compiler.find_library('nsl', required: false),\n> + compiler.find_library('resolv', required: false),\n> + compiler.find_library('socket', required: false),\n> + ]\n> endif\n> libgit_dependencies += networking_dependencies\n\nFair. We could extend this check to verify which combination of\nlibraries we actually require to make desired functions available. But\nI'm not sure whether that would really be worth the effort.\n\nPatrick\n"},{"id":"516473","messageId":"aAdF2tBnLihgx8zX@pks.im","threadId":"63322","inReplyTo":"20250421175247.240971-5-eschwartz@gentoo.org","subject":"Re: [PATCH 5/6] meson: fix typo in function check that prevented checking for hstrerror","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-22T07:31:38Z","receivedAt":"2025-04-22T07:31:41Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Apr 21, 2025 at 01:51:49PM -0400, Eli Schwartz wrote:\n> Nowhere in the codebase do we otherwise check for strerror. Nowhere in\n> the codebase do we make use of -DNO_STRERROR. `strerror` is not a\n> networking function at all.\n> \n> We do utilize `hstrerror` though, which is a networking function we\n> should have been checking here.\n> \n> Signed-off-by: Eli Schwartz \n> ---\n> meson.build | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/meson.build b/meson.build\n> index 8fad10379a..1b7e55756b 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -1088,7 +1088,7 @@ else\n> endif\n> libgit_dependencies += networking_dependencies\n> \n> -foreach symbol : ['inet_ntop', 'inet_pton', 'strerror']\n> +foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']\n> if not compiler.has_function(symbol, dependencies: networking_dependencies)\n> libgit_c_args += '-DNO_' + symbol.to_upper()\n> endif\n\nGood catch and obviously correct, thanks!\n\nPatrick\n"},{"id":"516474","messageId":"aAdF3eu1heCycaLJ@pks.im","threadId":"63322","inReplyTo":"20250421175247.240971-6-eschwartz@gentoo.org","subject":"Re: [PATCH 6/6] meson: only check for missing networking syms on non-Windows; add compat impls","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-22T07:31:41Z","receivedAt":"2025-04-22T07:31:45Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Apr 21, 2025 at 01:51:50PM -0400, Eli Schwartz wrote:\n> These are added in the Makefile, but not in meson. They probably won't\n> work well on systems without them.\n> \n> CMake adds them, but only on non-Windows. Actually, it only performs\n> compiler checks for hstrerror, but excludes that check on Windows with\n> the note that it is \"incompatible with the Windows build\". This seems to\n> be misleading -- it is not incompatible, it simply doesn't exist. Still,\n> the compat version should not be used.\n\nCMake only checks for `hstrerror()` though -- it doesn't check for the\nother functions at all.\n\n> I interpret this cmake logic to mean we shouldn't even be checking for\n> symbol availability on Windows. In addition to making it simple to add\n> compat definitions, this also probably shaves off a second or two of\n> configure time on Windows as no compiler check needs to be performed.\n\nI dunno. In this case I'd lean towards just using the check on Windows,\ntoo. The less platform-specific configuration we do the easier the build\nsystem is to reason about.\n\n> Signed-off-by: Eli Schwartz \n> ---\n> meson.build | 13 ++++++++-----\n> 1 file changed, 8 insertions(+), 5 deletions(-)\n> \n> diff --git a/meson.build b/meson.build\n> index 1b7e55756b..24b304fb57 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -1088,11 +1088,14 @@ else\n> endif\n> libgit_dependencies += networking_dependencies\n> \n> -foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']\n> - if not compiler.has_function(symbol, dependencies: networking_dependencies)\n> - libgit_c_args += '-DNO_' + symbol.to_upper()\n> - endif\n> -endforeach\n> +if host_machine.system() != 'windows'\n> + foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']\n> + if not compiler.has_function(symbol, dependencies: networking_dependencies)\n> + libgit_c_args += '-DNO_' + symbol.to_upper()\n> + libgit_sources += 'compat/' + symbol + '.c'\n> + endif\n> + endforeach\n> +endif\n\nWe do have compat sources for `inet_ntop()` and `inet_pton()` indeed, so\nadding those makes sense. But we don't have a replacement for\n`hstrerror()`, so if that function wasn't found we would error out\nbecause \"compat/hstrerror.c\" wasn't found.\n\nPatrick\n"},{"id":"516475","messageId":"aAdF4DzFCZ3uOJCx@pks.im","threadId":"63322","inReplyTo":"83d9fda5-8399-47fb-87b2-a8b376cf1625@gentoo.org","subject":"Re: [PATCH 1/6] meson: simplify and parameterize various standard function checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-22T07:31:44Z","receivedAt":"2025-04-22T07:31:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Apr 21, 2025 at 04:04:30PM -0400, Eli Schwartz wrote:\n> On 4/21/25 1:51 PM, Eli Schwartz wrote:\n> > diff --git a/meson.build b/meson.build\n> > index c47cb79af0..6c147c22a4 100644\n> > --- a/meson.build\n> > +++ b/meson.build\n> > @@ -1322,45 +1339,15 @@ if not compiler.has_function('strtoumax')\n> > ]\n> > endif\n> > \n> > -if not compiler.has_function('strtoull')\n> > - libgit_c_args += '-DNO_STRTOULL'\n> > -endif\n> > -\n> > -if not compiler.has_function('setenv')\n> > - libgit_c_args += '-DNO_SETENV'\n> > - libgit_sources += 'compat/setenv.c'\n> > -endif\n> > -\n> > if not compiler.has_function('qsort')\n> > libgit_c_args += '-DINTERNAL_QSORT'\n> > endif\n> > libgit_sources += 'compat/qsort_s.c'\n> \n> \n> ... for example, the Makefile says here:\n> \n> \n> # Define INTERNAL_QSORT to use Git's implementation of qsort(), which\n> # is a simplified version of the merge sort used in glibc. This is\n> # recommended if Git triggers O(n^2) behavior in your platform's\n> # qsort().\n> \n> cmake unconditionally defines it (???)\n\nOur CMake build instructions shouldn't be treated as canonical source of\ntruth. They're good enough for some usecases, but they are not as\nfeature complete as any of Makefile/autoconf/Meson.\n\n> config.mak.uname says:\n> \n> - AIX:\n> INTERNAL_QSORT = UnfortunatelyYes\n> \n> Seems to date back to commit 377d9c409ffe0f0d994b929aeb94716139207b9d.\n> \"Unfortunate\" indeed.\n> \n> \n> - MinGW:\n> INTERNAL_QSORT = YesPlease\n> \n> Windows claims to have a qsort but perhaps it is very slow and bes\n> avoided?\n> \n> We should probably stop *checking* for qsort and simply encode the\n> platforms we know are slow and automatically skip it there. Can I get\n> confirmation regarding Windows? :)\n\nI'd rather prefer to try and detect this generically instead of adding\nmore platform-specific configuration. It is way simpler to maintain, and\nif we ever see that things don't work well on a specific platform we may\nstill reconsider at that point in time.\n\n> > -# unsetenv is provided by compat/mingw.c.\n> > -if host_machine.system() != 'windows' and not compiler.has_function('unsetenv')\n> > - libgit_c_args += '-DNO_UNSETENV'\n> > - libgit_sources += 'compat/unsetenv.c'\n> > -endif\n> > -\n> > -if not compiler.has_function('mkdtemp')\n> > - libgit_c_args += '-DNO_MKDTEMP'\n> > - libgit_sources += 'compat/mkdtemp.c'\n> > -endif\n> > -\n> > -if not compiler.has_function('initgroups')\n> > - libgit_c_args += '-DNO_INITGROUPS'\n> > -endif\n> > -\n> > if compiler.has_function('getdelim')\n> > libgit_c_args += '-DHAVE_GETDELIM'\n> > endif\n> \n> \n> But stuff like this, why isn't it consistent with the other functions?\n> What's the difference between HAVE_XXX and NO_XXX?\n\nInconsistencies like this are what you get in a codebase that is 20\nyears old. Many things have grown organically, and one hand doesn't\nalways know what the other hand is doing.\n\nI agree that in the best case we'd unify these. But unfortunately, this\nis not trivial because those are part of the build interface for our\nMakefiles. People may have `HAVE_GETDELIM = YesPlease` in their\n`config.mak` file, and we don't want to break this usecase. So if we\nwere to change this in the future, we'd have to also introduce shims for\nbackwards compatibility.\n\nPatrick\n"},{"id":"516504","messageId":"xmqq4iyg5ip2.fsf@gitster.g","threadId":"63322","inReplyTo":"aAdFysi-n_5Aa4Au@pks.im","subject":"Re: [PATCH 1/6] meson: simplify and parameterize various standard function checks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-04-22T15:17:45Z","receivedAt":"2025-04-22T15:17:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> Yeah, this is somewhat unfortunate indeed. I think in the long term we\n> might want to unify our approach so that we consistently use e.g.\n> `HAVE_SOME_FUNCTION` or `NO_SOME_FUNCTION`.\n\nYes and no, because the intention, at least in the original form,\nwas that HAVE_FOO is not necessarily !NO_FOO. You cannot claim to\nHAVE_FOO on a system that does not have FOO, and expect the result\nto build and/or function, but you should be able to build with\nNO_FOO on a system that supports FOO and use an alternative\nimplementation that does not rely on system-supplied FOO. NO_MMAP\nand NO_REGEX comes to mind (I often have to build NO_REGEX locally\nwhen doing \"make sparse\", for example, to avoid warnings triggering\non system headers, for example).\n\nI suspect that majority of these feature symbols do not fall into\nthe same category as NO_REGEX, i.e. the user/builder may choose to\ndecline using the system-supplied one. So for them this is a total\noverkill, but conceptually, building with feature FOO enabled should\nbe done iff (HAVE_FOO && !NO_FOO).\n\nI do not mind changing the stance Makefile takes on HAVE/!NO\ndivision, and see us declare that from now on, even when\nauto-detection flips HAVE_FOO on, the way for the builder to decline\nuse of FOO is to flip HAVE_FOO off manually. I and others may need\nto tweak some scripts and figure out to pass !HAVE_REGEX instead of\nNO_REGEX when that happens, but that is a one-time cost to make\nthings more consistent.\n"},{"id":"516509","messageId":"aadfbd6b-ea1c-475e-b6a9-f1552afa06c8@gentoo.org","threadId":"63322","inReplyTo":"aAdF3eu1heCycaLJ@pks.im","subject":"Re: [PATCH 6/6] meson: only check for missing networking syms on non-Windows; add compat impls","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-22T15:27:03Z","receivedAt":"2025-04-22T15:27:07Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"On 4/22/25 3:31 AM, Patrick Steinhardt wrote:\n> On Mon, Apr 21, 2025 at 01:51:50PM -0400, Eli Schwartz wrote:\n>> These are added in the Makefile, but not in meson. They probably won't\n>> work well on systems without them.\n>>\n>> CMake adds them, but only on non-Windows. Actually, it only performs\n>> compiler checks for hstrerror, but excludes that check on Windows with\n>> the note that it is \"incompatible with the Windows build\". This seems to\n>> be misleading -- it is not incompatible, it simply doesn't exist. Still,\n>> the compat version should not be used.\n> \n> CMake only checks for `hstrerror()` though -- it doesn't check for the\n> other functions at all.\n\n\nRight, that's what I meant.\n\n\"cmake, like this patch, adds the compat/*.c when checking function\navailability. Actually, it only performs compiler checks for hstrerror,\nbut does add the compat impl in that case\".\n\n\n>> I interpret this cmake logic to mean we shouldn't even be checking for\n>> symbol availability on Windows. In addition to making it simple to add\n>> compat definitions, this also probably shaves off a second or two of\n>> configure time on Windows as no compiler check needs to be performed.\n> \n> I dunno. In this case I'd lean towards just using the check on Windows,\n> too. The less platform-specific configuration we do the easier the build\n> system is to reason about.\n\n\nMaybe, but the issue is that it appears on Windows it is not correct to\nadd the compat impl for hstrerror, which would still be a\nplatform-specific configuration. Do I check for hstrerror everywhere but\nconfigure Windows to not add the compat impl, but do add it for\ninet_ntop and inet_pton? That is more configuration than skipping the\nchecks on Windows...\n\n\n\n>> Signed-off-by: Eli Schwartz \n>> ---\n>> meson.build | 13 ++++++++-----\n>> 1 file changed, 8 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/meson.build b/meson.build\n>> index 1b7e55756b..24b304fb57 100644\n>> --- a/meson.build\n>> +++ b/meson.build\n>> @@ -1088,11 +1088,14 @@ else\n>> endif\n>> libgit_dependencies += networking_dependencies\n>> \n>> -foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']\n>> - if not compiler.has_function(symbol, dependencies: networking_dependencies)\n>> - libgit_c_args += '-DNO_' + symbol.to_upper()\n>> - endif\n>> -endforeach\n>> +if host_machine.system() != 'windows'\n>> + foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']\n>> + if not compiler.has_function(symbol, dependencies: networking_dependencies)\n>> + libgit_c_args += '-DNO_' + symbol.to_upper()\n>> + libgit_sources += 'compat/' + symbol + '.c'\n>> + endif\n>> + endforeach\n>> +endif\n> \n> We do have compat sources for `inet_ntop()` and `inet_pton()` indeed, so\n> adding those makes sense. But we don't have a replacement for\n> `hstrerror()`, so if that function wasn't found we would error out\n> because \"compat/hstrerror.c\" wasn't found.\n\n\nI don't really understand what you mean by this. Of course the file will\nbe found.\n\n$ grep hstrerror compat/hstrerror.c\nconst char *githstrerror(int err)\n\n\nFile is there. (The function name is then #defined by compat/posix.h, no\ncomment.)\n\n\n\n-- \nEli Schwartz\n"},{"id":"516510","messageId":"590d0daa-59ac-44f3-b277-e3e83ad1add5@gentoo.org","threadId":"63322","inReplyTo":"aAdF4DzFCZ3uOJCx@pks.im","subject":"Re: [PATCH 1/6] meson: simplify and parameterize various standard function checks","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-22T15:36:06Z","receivedAt":"2025-04-22T15:36:09Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"On 4/22/25 3:31 AM, Patrick Steinhardt wrote:\n> On Mon, Apr 21, 2025 at 04:04:30PM -0400, Eli Schwartz wrote:\n>> On 4/21/25 1:51 PM, Eli Schwartz wrote:\n>>> diff --git a/meson.build b/meson.build\n>>> index c47cb79af0..6c147c22a4 100644\n>>> --- a/meson.build\n>>> +++ b/meson.build\n>>> @@ -1322,45 +1339,15 @@ if not compiler.has_function('strtoumax')\n>>> ]\n>>> endif\n>>> \n>>> -if not compiler.has_function('strtoull')\n>>> - libgit_c_args += '-DNO_STRTOULL'\n>>> -endif\n>>> -\n>>> -if not compiler.has_function('setenv')\n>>> - libgit_c_args += '-DNO_SETENV'\n>>> - libgit_sources += 'compat/setenv.c'\n>>> -endif\n>>> -\n>>> if not compiler.has_function('qsort')\n>>> libgit_c_args += '-DINTERNAL_QSORT'\n>>> endif\n>>> libgit_sources += 'compat/qsort_s.c'\n>>\n>>\n>> ... for example, the Makefile says here:\n>>\n>>\n>> # Define INTERNAL_QSORT to use Git's implementation of qsort(), which\n>> # is a simplified version of the merge sort used in glibc. This is\n>> # recommended if Git triggers O(n^2) behavior in your platform's\n>> # qsort().\n>>\n>> cmake unconditionally defines it (???)\n> \n> Our CMake build instructions shouldn't be treated as canonical source of\n> truth. They're good enough for some usecases, but they are not as\n> feature complete as any of Makefile/autoconf/Meson.\n\n\n... yes, which is why I'm using it as a springboard to ask questions? :)\n\nMy working theory is it unconditionally defines it because this is the\ncorrect behavior on Windows, and the cmake files were primarily written\nto be used on Windows, which leads us to...\n\n\n>> config.mak.uname says:\n>>\n>> - AIX:\n>> INTERNAL_QSORT = UnfortunatelyYes\n>>\n>> Seems to date back to commit 377d9c409ffe0f0d994b929aeb94716139207b9d.\n>> \"Unfortunate\" indeed.\n>>\n>>\n>> - MinGW:\n>> INTERNAL_QSORT = YesPlease\n>>\n>> Windows claims to have a qsort but perhaps it is very slow and bes\n>> avoided?\n>>\n>> We should probably stop *checking* for qsort and simply encode the\n>> platforms we know are slow and automatically skip it there. Can I get\n>> confirmation regarding Windows? :)\n\n\n... this. config.mak.uname's mingw case appears to agree with my theory\nabout the motivations for the cmake file.\n\n\n> I'd rather prefer to try and detect this generically instead of adding\n> more platform-specific configuration. It is way simpler to maintain, and\n> if we ever see that things don't work well on a specific platform we may\n> still reconsider at that point in time.\n\n\nOkay but, how do we generically detect that a platform triggers the\nMakefile advice \"recommended if Git triggers O(n^2) behavior in your\nplatform's qsort()\"? I'm not sure how to write a compile-time check for\nthis.\n\nIt's easy to write a compile-time check for whether a function exists,\nbut it seems to have been an error that meson assumes some platforms\nwill not provide the function, as that was never the intent of Git's\nsupport for internal qsort.\n\n\n-- \nEli Schwartz\n"},{"id":"516593","messageId":"aAjOIEs5Lwb9CvkU@pks.im","threadId":"63322","inReplyTo":"590d0daa-59ac-44f3-b277-e3e83ad1add5@gentoo.org","subject":"Re: [PATCH 1/6] meson: simplify and parameterize various standard function checks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-23T11:25:20Z","receivedAt":"2025-04-23T11:25:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Apr 22, 2025 at 11:36:06AM -0400, Eli Schwartz wrote:\n> On 4/22/25 3:31 AM, Patrick Steinhardt wrote:\n> > On Mon, Apr 21, 2025 at 04:04:30PM -0400, Eli Schwartz wrote:\n> >> On 4/21/25 1:51 PM, Eli Schwartz wrote:\n> > I'd rather prefer to try and detect this generically instead of adding\n> > more platform-specific configuration. It is way simpler to maintain, and\n> > if we ever see that things don't work well on a specific platform we may\n> > still reconsider at that point in time.\n> \n> \n> Okay but, how do we generically detect that a platform triggers the\n> Makefile advice \"recommended if Git triggers O(n^2) behavior in your\n> platform's qsort()\"? I'm not sure how to write a compile-time check for\n> this.\n> \n> It's easy to write a compile-time check for whether a function exists,\n> but it seems to have been an error that meson assumes some platforms\n> will not provide the function, as that was never the intent of Git's\n> support for internal qsort.\n\nThe question to me is whether this is still an issue that we need to\ncare about nowadays. If we _know_ that it is still an issue that we need\nto address then I'm okay with adapting as required. But if we think that\nit's probably not an issue anymore then I'd rather wait and see whether\nanybody complains. Because if nobody does, then we can eventually just\nthrow out this logic altogether.\n\nI treat this as a bit of a canary. Git is quite old by now, so we need\nto question existing infrastructure every now and then so that we can in\nthe best case throw out unneeded bits and pieces every once in a while\nas the surrounding ecosystem matures.\n\nPatrick\n"},{"id":"516594","messageId":"aAjOKBIJiNarzkYa@pks.im","threadId":"63322","inReplyTo":"aadfbd6b-ea1c-475e-b6a9-f1552afa06c8@gentoo.org","subject":"Re: [PATCH 6/6] meson: only check for missing networking syms on non-Windows; add compat impls","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-23T11:25:28Z","receivedAt":"2025-04-23T11:25:31Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Apr 22, 2025 at 11:27:03AM -0400, Eli Schwartz wrote:\n> On 4/22/25 3:31 AM, Patrick Steinhardt wrote:\n> > On Mon, Apr 21, 2025 at 01:51:50PM -0400, Eli Schwartz wrote:\n> > We do have compat sources for `inet_ntop()` and `inet_pton()` indeed, so\n> > adding those makes sense. But we don't have a replacement for\n> > `hstrerror()`, so if that function wasn't found we would error out\n> > because \"compat/hstrerror.c\" wasn't found.\n> \n> \n> I don't really understand what you mean by this. Of course the file will\n> be found.\n> \n> $ grep hstrerror compat/hstrerror.c\n> const char *githstrerror(int err)\n> \n> \n> File is there. (The function name is then #defined by compat/posix.h, no\n> comment.)\n\nOops, I somehow missed this file altogether. Please ignore this.\n\nPatrick\n"},{"id":"516703","messageId":"xmqqecxhruin.fsf@gitster.g","threadId":"63322","inReplyTo":"20250421175247.240971-2-eschwartz@gentoo.org","subject":"Re: [PATCH 2/6] meson: check for getpagesize before using it","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-04-24T23:48:16Z","receivedAt":"2025-04-24T23:48:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eli Schwartz writes:\n\n> It is deprecated and removed in SUS v3 / POSIX 2001, so various systems\n> may not include it. Solaris, in particular, carefully refrains from\n> defining it except inside of a maze of `#ifdef` to make sure you have\n> kept your nose clean and only used it in code that *targets* SUS v2 or\n> earlier.\n>\n> config.mak.uname defines this automatically, though only for QNX.\n>\n> Signed-off-by: Eli Schwartz \n> ---\n> meson.build | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/meson.build b/meson.build\n> index 6c147c22a4..f5d9ffcd7f 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -1300,6 +1300,8 @@ checkfuncs = [\n> 'mkdtemp',\n> # no compat\n> 'initgroups',\n> + # no compat\n> + 'getpagesize',\n> ]\n> \n> if host_machine.system() == 'windows'\n\nIs this related to this breakge we started seeing for 'seen'\nrecently?\n\nhttps://github.com/git/git/actions/runs/14653573748/job/41124519642\n\n"},{"id":"516704","messageId":"83897914-0b97-4a1d-b2aa-714d42043e63@gentoo.org","threadId":"63322","inReplyTo":"xmqqecxhruin.fsf@gitster.g","subject":"Re: [PATCH 2/6] meson: check for getpagesize before using it","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-25T00:06:34Z","receivedAt":"2025-04-25T00:06:39Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"On 4/24/25 7:48 PM, Junio C Hamano wrote:\n> Eli Schwartz writes:\n> \n>> It is deprecated and removed in SUS v3 / POSIX 2001, so various systems\n>> may not include it. Solaris, in particular, carefully refrains from\n>> defining it except inside of a maze of `#ifdef` to make sure you have\n>> kept your nose clean and only used it in code that *targets* SUS v2 or\n>> earlier.\n>>\n>> config.mak.uname defines this automatically, though only for QNX.\n>>\n>> Signed-off-by: Eli Schwartz \n>> ---\n>> meson.build | 2 ++\n>> 1 file changed, 2 insertions(+)\n>>\n>> diff --git a/meson.build b/meson.build\n>> index 6c147c22a4..f5d9ffcd7f 100644\n>> --- a/meson.build\n>> +++ b/meson.build\n>> @@ -1300,6 +1300,8 @@ checkfuncs = [\n>> 'mkdtemp',\n>> # no compat\n>> 'initgroups',\n>> + # no compat\n>> + 'getpagesize',\n>> ]\n>> \n>> if host_machine.system() == 'windows'\n> \n> Is this related to this breakge we started seeing for 'seen'\n> recently?\n> \n> https://github.com/git/git/actions/runs/14653573748/job/41124519642\n\n\nYes.\n\ncompat/mingw.c defines mingw_getpagesize, and posix.h defines -- when\nthe function is detected as missing:\n\n#define getpagesize() sysconf(_SC_PAGESIZE)\n\n\nThat means we can't check for it on mingw or we end up with two\ndefinitions. I will move it to the else block in:\n\nif host_machine.system() == 'windows'\n libgit_c_args += '-DUSE_WIN32_MMAP'\nelse\n\n\n\n-- \nEli Schwartz\n"},{"id":"516705","messageId":"20250425002017.246985-1-eschwartz@gentoo.org","threadId":"63322","inReplyTo":"20250421175247.240971-1-eschwartz@gentoo.org","subject":"[PATCH v2 0/6] meson: miscellaneous system detection fixes","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-25T00:13:29Z","receivedAt":"2025-04-25T00:20:50Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"Principally motivated to handle an issue where these were failing to\ndetect the system properly, on Solaris.\n\nEli Schwartz (6):\n meson: simplify and parameterize various standard function checks\n meson: check for getpagesize before using it\n meson: do a full usage-based compile check for sysinfo\n meson: add a couple missing networking dependencies\n meson: fix typo in function check that prevented checking for\n hstrerror\n meson: only check for missing networking syms on non-Windows; add\n compat impls\n\n meson.build | 105 ++++++++++++++++++++++++----------------------------\n 1 file changed, 49 insertions(+), 56 deletions(-)\n\nRange-diff against v1:\n1: e137afaff2 = 1: e137afaff2 meson: simplify and parameterize various standard function checks\n2: df82ee7872 ! 2: 3c4918a7b3 meson: check for getpagesize before using it\n @@ Commit message\n config.mak.uname defines this automatically, though only for QNX.\n \n ## meson.build ##\n -@@ meson.build: checkfuncs = [\n - 'mkdtemp',\n - # no compat\n - 'initgroups',\n -+ # no compat\n -+ 'getpagesize',\n - ]\n +@@ meson.build: else\n + 'mmap',\n + # unsetenv is provided by compat/mingw.c.\n + 'unsetenv',\n ++ # no compat, is provided by compat/mingw.c\n ++ 'getpagesize',\n + ]\n + endif\n \n - if host_machine.system() == 'windows'\n3: 2ec759d7be = 3: 2c19f04f3e meson: do a full usage-based compile check for sysinfo\n4: 61f2addd47 = 4: e62057094f meson: add a couple missing networking dependencies\n5: c67d10a337 = 5: a6a7bdacd1 meson: fix typo in function check that prevented checking for hstrerror\n6: c5b8b89dd4 = 6: 15f48b8991 meson: only check for missing networking syms on non-Windows; add compat impls\n-- \n2.49.0\n\n"},{"id":"516706","messageId":"20250425002017.246985-2-eschwartz@gentoo.org","threadId":"63322","inReplyTo":"20250425002017.246985-1-eschwartz@gentoo.org","subject":"[PATCH v2 1/6] meson: simplify and parameterize various standard function checks","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-25T00:13:30Z","receivedAt":"2025-04-25T00:20:51Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"This is repetitive logic. We either want to use some -lc function, or if\nit is not available we define it as -DNO_XXX and usually (but not\nalways) provide some custom compatibility impl instead.\n\nChecking the intent of each block when reading through the file is slow\nand not very DRY. Switch to taking an array of checkable functions\ninstead.\n\nNot all functions are straightforward to move, since different macro\nprefixes are used.\n\nSigned-off-by: Eli Schwartz \n---\n meson.build | 73 ++++++++++++++++++++++-------------------------------\n 1 file changed, 30 insertions(+), 43 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex c47cb79af0..6c147c22a4 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1290,23 +1290,40 @@ if not compiler.has_member('struct passwd', 'pw_gecos', prefix: '#include \n---\n\nv2: add this only for !windows\n\n meson.build | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/meson.build b/meson.build\nindex 6c147c22a4..29c188af99 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1309,6 +1309,8 @@ else\n 'mmap',\n # unsetenv is provided by compat/mingw.c.\n 'unsetenv',\n+ # no compat, is provided by compat/mingw.c\n+ 'getpagesize',\n ]\n endif\n \n-- \n2.49.0\n\n"},{"id":"516708","messageId":"20250425002017.246985-4-eschwartz@gentoo.org","threadId":"63322","inReplyTo":"20250425002017.246985-1-eschwartz@gentoo.org","subject":"[PATCH v2 3/6] meson: do a full usage-based compile check for sysinfo","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-25T00:13:32Z","receivedAt":"2025-04-25T00:20:53Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"On Solaris, sys/sysinfo.h is a completely different file and doesn't\nresemble the linux file at all. There is also a sysinfo() function, but\nit takes a totally different call signature, which asks for:\n\n- the field you wish to receive\n- a `char *buf` to copy the data to\n\nand is very useful IFF you want to know, say, the hardware provider. Or,\nget *specific* fields from uname(2).\n\nhttps://docs.oracle.com/cd/E86824_01/html/E54765/sysinfo-2.html\n\nIt is surely possible to do this manually via `sysconf(3)` without the\nnice API. I can't find anything more direct. Either way, I'm not very\nattached to Solaris, so someone who cares can add it. Either way, it's\nwrong to assume that sysinfo.h contains what we are looking for.\n\nCheck that sysinfo.h defines the struct we actually utilize in\nbuiltins/gc.c, which will correctly fail on systems that don't have it.\n\nSigned-off-by: Eli Schwartz \n---\n meson.build | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex 29c188af99..ea0722a216 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1058,10 +1058,6 @@ if compiler.has_header('alloca.h')\n libgit_c_args += '-DHAVE_ALLOCA_H'\n endif\n \n-if compiler.has_header('sys/sysinfo.h')\n- libgit_c_args += '-DHAVE_SYSINFO'\n-endif\n-\n # Windows has libgen.h and a basename implementation, but we still need our own\n # implementation to threat things like drive prefixes specially.\n if host_machine.system() == 'windows' or not compiler.has_header('libgen.h')\n@@ -1272,6 +1268,10 @@ if host_machine.system() != 'windows'\n endif\n endif\n \n+if compiler.has_member('struct sysinfo', 'totalram', prefix: '#include ')\n+ libgit_c_args += '-DHAVE_SYSINFO'\n+endif\n+\n if compiler.has_member('struct stat', 'st_mtimespec.tv_nsec', prefix: '#include ')\n libgit_c_args += '-DUSE_ST_TIMESPEC'\n elif not compiler.has_member('struct stat', 'st_mtim.tv_nsec', prefix: '#include ')\n-- \n2.49.0\n\n"},{"id":"516709","messageId":"20250425002017.246985-6-eschwartz@gentoo.org","threadId":"63322","inReplyTo":"20250425002017.246985-1-eschwartz@gentoo.org","subject":"[PATCH v2 5/6] meson: fix typo in function check that prevented checking for hstrerror","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-25T00:13:34Z","receivedAt":"2025-04-25T00:20:54Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"Nowhere in the codebase do we otherwise check for strerror. Nowhere in\nthe codebase do we make use of -DNO_STRERROR. `strerror` is not a\nnetworking function at all.\n\nWe do utilize `hstrerror` though, which is a networking function we\nshould have been checking here.\n\nSigned-off-by: Eli Schwartz \n---\n meson.build | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/meson.build b/meson.build\nindex ff65d36cac..7927c54dc3 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1088,7 +1088,7 @@ else\n endif\n libgit_dependencies += networking_dependencies\n \n-foreach symbol : ['inet_ntop', 'inet_pton', 'strerror']\n+foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']\n if not compiler.has_function(symbol, dependencies: networking_dependencies)\n libgit_c_args += '-DNO_' + symbol.to_upper()\n endif\n-- \n2.49.0\n\n"},{"id":"516710","messageId":"20250425002017.246985-5-eschwartz@gentoo.org","threadId":"63322","inReplyTo":"20250425002017.246985-1-eschwartz@gentoo.org","subject":"[PATCH v2 4/6] meson: add a couple missing networking dependencies","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-25T00:13:33Z","receivedAt":"2025-04-25T00:20:54Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"As evidenced in config.mak.uname and configure.ac, there are various\npossible scenarios where these libraries are default-enabled in the\nbuild, which mainly boils down to: SunOS. -lresolv is simply not the\nonly library that, when it exists, probably needs to be linked to for\nnetworking.\n\nCheck for and add -lnsl -lsocket as well.\n\nSigned-off-by: Eli Schwartz \n---\n meson.build | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex ea0722a216..ff65d36cac 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1080,10 +1080,11 @@ if host_machine.system() == 'windows'\n networking_dependencies += winsock\n endif\n else\n- libresolv = compiler.find_library('resolv', required: false)\n- if libresolv.found()\n- networking_dependencies += libresolv\n- endif\n+ networking_dependencies += [\n+ compiler.find_library('nsl', required: false),\n+ compiler.find_library('resolv', required: false),\n+ compiler.find_library('socket', required: false),\n+ ]\n endif\n libgit_dependencies += networking_dependencies\n \n-- \n2.49.0\n\n"},{"id":"516711","messageId":"20250425002017.246985-7-eschwartz@gentoo.org","threadId":"63322","inReplyTo":"20250425002017.246985-1-eschwartz@gentoo.org","subject":"[PATCH v2 6/6] meson: only check for missing networking syms on non-Windows; add compat impls","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-25T00:13:35Z","receivedAt":"2025-04-25T00:20:55Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"These are added in the Makefile, but not in meson. They probably won't\nwork well on systems without them.\n\nCMake adds them, but only on non-Windows. Actually, it only performs\ncompiler checks for hstrerror, but excludes that check on Windows with\nthe note that it is \"incompatible with the Windows build\". This seems to\nbe misleading -- it is not incompatible, it simply doesn't exist. Still,\nthe compat version should not be used.\n\nI interpret this cmake logic to mean we shouldn't even be checking for\nsymbol availability on Windows. In addition to making it simple to add\ncompat definitions, this also probably shaves off a second or two of\nconfigure time on Windows as no compiler check needs to be performed.\n\nSigned-off-by: Eli Schwartz \n---\n meson.build | 13 ++++++++-----\n 1 file changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex 7927c54dc3..59a7782da0 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1088,11 +1088,14 @@ else\n endif\n libgit_dependencies += networking_dependencies\n \n-foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']\n- if not compiler.has_function(symbol, dependencies: networking_dependencies)\n- libgit_c_args += '-DNO_' + symbol.to_upper()\n- endif\n-endforeach\n+if host_machine.system() != 'windows'\n+ foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']\n+ if not compiler.has_function(symbol, dependencies: networking_dependencies)\n+ libgit_c_args += '-DNO_' + symbol.to_upper()\n+ libgit_sources += 'compat/' + symbol + '.c'\n+ endif\n+ endforeach\n+endif\n \n has_ipv6 = compiler.has_function('getaddrinfo', dependencies: networking_dependencies)\n if not has_ipv6\n-- \n2.49.0\n\n"},{"id":"516716","messageId":"aAsSDA5y_Dur9PGj@pks.im","threadId":"63322","inReplyTo":"20250425002017.246985-1-eschwartz@gentoo.org","subject":"Re: [PATCH v2 0/6] meson: miscellaneous system detection fixes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-25T04:39:40Z","receivedAt":"2025-04-25T04:39:44Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Apr 24, 2025 at 08:13:29PM -0400, Eli Schwartz wrote:\n> Principally motivated to handle an issue where these were failing to\n> detect the system properly, on Solaris.\n\nI was expecting to also see my comments addressed around the one style\nissue as well as the comment regarding `fs.exists()` being a bit too\nmagical (both mentioned in [1]). This is the only remaining item that\nI'd like to see addressed, and other than that this series looks good to\nme.\n\nPatrick\n\n[1]: \n"},{"id":"516720","messageId":"7d7c22c9-953f-43b4-8976-9303effd63ea@gentoo.org","threadId":"63322","inReplyTo":"aAsSDA5y_Dur9PGj@pks.im","subject":"Re: [PATCH v2 0/6] meson: miscellaneous system detection fixes","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-25T05:27:21Z","receivedAt":"2025-04-25T05:27:24Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"On 4/25/25 12:39 AM, Patrick Steinhardt wrote:\n> On Thu, Apr 24, 2025 at 08:13:29PM -0400, Eli Schwartz wrote:\n>> Principally motivated to handle an issue where these were failing to\n>> detect the system properly, on Solaris.\n> \n> I was expecting to also see my comments addressed around the one style\n> issue as well as the comment regarding `fs.exists()` being a bit too\n> magical (both mentioned in [1]). This is the only remaining item that\n> I'd like to see addressed, and other than that this series looks good to\n> me.\n\n\nI overlooked the style comment, will fix.\n\nRegarding the fs.exists I was worried that it would be repetitive and\nunwieldy but I think I found a good solution, as a bonus I can now\nhandle strtoumax.\n\n\n-- \nEli Schwartz\n"},{"id":"516721","messageId":"20250425053345.17288-1-eschwartz@gentoo.org","threadId":"63322","inReplyTo":"20250421175247.240971-1-eschwartz@gentoo.org","subject":"[PATCH v3 0/6] meson: miscellaneous system detection fixes","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-25T05:25:39Z","receivedAt":"2025-04-25T05:35:02Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"Principally motivated to handle an issue where these were failing to\ndetect the system properly, on Solaris.\n\nChanges:\n\nv2: getpagesize check moved to !windows\nv3: style fixes, pass source files in\n\nEli Schwartz (6):\n meson: simplify and parameterize various standard function checks\n meson: check for getpagesize before using it\n meson: do a full usage-based compile check for sysinfo\n meson: add a couple missing networking dependencies\n meson: fix typo in function check that prevented checking for\n hstrerror\n meson: only check for missing networking syms on non-Windows; add\n compat impls\n\n meson.build | 117 +++++++++++++++++++++-------------------------------\n 1 file changed, 48 insertions(+), 69 deletions(-)\n\nRange-diff against v2:\n1: e137afaff2 ! 1: 037d2f8610 meson: simplify and parameterize various standard function checks\n @@ Commit message\n prefixes are used.\n \n ## meson.build ##\n +@@ meson.build: else\n + build_options_config.set('NO_UNIX_SOCKETS', '1')\n + endif\n + \n +-if not compiler.has_function('pread')\n +- libgit_c_args += '-DNO_PREAD'\n +- libgit_sources += 'compat/pread.c'\n +-endif\n +-\n + if host_machine.system() == 'darwin'\n + libgit_sources += 'compat/precompose_utf8.c'\n + libgit_c_args += '-DPRECOMPOSE_UNICODE'\n @@ meson.build: if not compiler.has_member('struct passwd', 'pw_gecos', prefix: '#include \n---\n\nv3:\n- use a dictionary for checkfuncs, and list the filenames\n- now we can handle strtoumax too, so, do that\n- pread was overlooked previously; move it down and handle it with the\n other function checks\n\n meson.build | 85 ++++++++++++++++++-----------------------------------\n 1 file changed, 29 insertions(+), 56 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex c47cb79af0..ed0359b9c9 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1133,11 +1133,6 @@ else\n build_options_config.set('NO_UNIX_SOCKETS', '1')\n endif\n \n-if not compiler.has_function('pread')\n- libgit_c_args += '-DNO_PREAD'\n- libgit_sources += 'compat/pread.c'\n-endif\n-\n if host_machine.system() == 'darwin'\n libgit_sources += 'compat/precompose_utf8.c'\n libgit_c_args += '-DPRECOMPOSE_UNICODE'\n@@ -1290,23 +1285,39 @@ if not compiler.has_member('struct passwd', 'pw_gecos', prefix: '#include \n---\n\nv2: add this only for !windows\n\nv3: rebase now that it is a dict\n\n meson.build | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/meson.build b/meson.build\nindex ed0359b9c9..e575231129 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1304,6 +1304,8 @@ else\n 'mmap' : ['mmap.c'],\n # provided by compat/mingw.c.\n 'unsetenv' : ['unsetenv.c'],\n+ # provided by compat/mingw.c.\n+ 'getpagesize' : [],\n }\n endif\n \n-- \n2.49.0\n\n"},{"id":"516724","messageId":"20250425053345.17288-4-eschwartz@gentoo.org","threadId":"63322","inReplyTo":"20250425053345.17288-1-eschwartz@gentoo.org","subject":"[PATCH v3 3/6] meson: do a full usage-based compile check for sysinfo","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-25T05:25:42Z","receivedAt":"2025-04-25T05:35:05Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"On Solaris, sys/sysinfo.h is a completely different file and doesn't\nresemble the linux file at all. There is also a sysinfo() function, but\nit takes a totally different call signature, which asks for:\n\n- the field you wish to receive\n- a `char *buf` to copy the data to\n\nand is very useful IFF you want to know, say, the hardware provider. Or,\nget *specific* fields from uname(2).\n\nhttps://docs.oracle.com/cd/E86824_01/html/E54765/sysinfo-2.html\n\nIt is surely possible to do this manually via `sysconf(3)` without the\nnice API. I can't find anything more direct. Either way, I'm not very\nattached to Solaris, so someone who cares can add it. Either way, it's\nwrong to assume that sysinfo.h contains what we are looking for.\n\nCheck that sysinfo.h defines the struct we actually utilize in\nbuiltins/gc.c, which will correctly fail on systems that don't have it.\n\nSigned-off-by: Eli Schwartz \n---\n meson.build | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex e575231129..b21b191d25 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1058,10 +1058,6 @@ if compiler.has_header('alloca.h')\n libgit_c_args += '-DHAVE_ALLOCA_H'\n endif\n \n-if compiler.has_header('sys/sysinfo.h')\n- libgit_c_args += '-DHAVE_SYSINFO'\n-endif\n-\n # Windows has libgen.h and a basename implementation, but we still need our own\n # implementation to threat things like drive prefixes specially.\n if host_machine.system() == 'windows' or not compiler.has_header('libgen.h')\n@@ -1267,6 +1263,10 @@ if host_machine.system() != 'windows'\n endif\n endif\n \n+if compiler.has_member('struct sysinfo', 'totalram', prefix: '#include ')\n+ libgit_c_args += '-DHAVE_SYSINFO'\n+endif\n+\n if compiler.has_member('struct stat', 'st_mtimespec.tv_nsec', prefix: '#include ')\n libgit_c_args += '-DUSE_ST_TIMESPEC'\n elif not compiler.has_member('struct stat', 'st_mtim.tv_nsec', prefix: '#include ')\n-- \n2.49.0\n\n"},{"id":"516725","messageId":"20250425053345.17288-5-eschwartz@gentoo.org","threadId":"63322","inReplyTo":"20250425053345.17288-1-eschwartz@gentoo.org","subject":"[PATCH v3 4/6] meson: add a couple missing networking dependencies","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-25T05:25:43Z","receivedAt":"2025-04-25T05:35:06Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"As evidenced in config.mak.uname and configure.ac, there are various\npossible scenarios where these libraries are default-enabled in the\nbuild, which mainly boils down to: SunOS. -lresolv is simply not the\nonly library that, when it exists, probably needs to be linked to for\nnetworking.\n\nCheck for and add -lnsl -lsocket as well.\n\nSigned-off-by: Eli Schwartz \n---\n meson.build | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex b21b191d25..66b69f2471 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1080,10 +1080,11 @@ if host_machine.system() == 'windows'\n networking_dependencies += winsock\n endif\n else\n- libresolv = compiler.find_library('resolv', required: false)\n- if libresolv.found()\n- networking_dependencies += libresolv\n- endif\n+ networking_dependencies += [\n+ compiler.find_library('nsl', required: false),\n+ compiler.find_library('resolv', required: false),\n+ compiler.find_library('socket', required: false),\n+ ]\n endif\n libgit_dependencies += networking_dependencies\n \n-- \n2.49.0\n\n"},{"id":"516726","messageId":"20250425053345.17288-6-eschwartz@gentoo.org","threadId":"63322","inReplyTo":"20250425053345.17288-1-eschwartz@gentoo.org","subject":"[PATCH v3 5/6] meson: fix typo in function check that prevented checking for hstrerror","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-25T05:25:44Z","receivedAt":"2025-04-25T05:35:06Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"Nowhere in the codebase do we otherwise check for strerror. Nowhere in\nthe codebase do we make use of -DNO_STRERROR. `strerror` is not a\nnetworking function at all.\n\nWe do utilize `hstrerror` though, which is a networking function we\nshould have been checking here.\n\nSigned-off-by: Eli Schwartz \n---\n meson.build | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/meson.build b/meson.build\nindex 66b69f2471..25bac8d89f 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1088,7 +1088,7 @@ else\n endif\n libgit_dependencies += networking_dependencies\n \n-foreach symbol : ['inet_ntop', 'inet_pton', 'strerror']\n+foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']\n if not compiler.has_function(symbol, dependencies: networking_dependencies)\n libgit_c_args += '-DNO_' + symbol.to_upper()\n endif\n-- \n2.49.0\n\n"},{"id":"516727","messageId":"20250425053345.17288-7-eschwartz@gentoo.org","threadId":"63322","inReplyTo":"20250425053345.17288-1-eschwartz@gentoo.org","subject":"[PATCH v3 6/6] meson: only check for missing networking syms on non-Windows; add compat impls","fromName":"Eli Schwartz","fromEmail":"eschwartz@gentoo.org","sentAt":"2025-04-25T05:25:45Z","receivedAt":"2025-04-25T05:35:07Z","isPatch":true,"sender":{"key":"eschwartz@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"These are added in the Makefile, but not in meson. They probably won't\nwork well on systems without them.\n\nCMake adds them, but only on non-Windows. Actually, it only performs\ncompiler checks for hstrerror, but excludes that check on Windows with\nthe note that it is \"incompatible with the Windows build\". This seems to\nbe misleading -- it is not incompatible, it simply doesn't exist. Still,\nthe compat version should not be used.\n\nI interpret this cmake logic to mean we shouldn't even be checking for\nsymbol availability on Windows. In addition to making it simple to add\ncompat definitions, this also probably shaves off a second or two of\nconfigure time on Windows as no compiler check needs to be performed.\n\nSigned-off-by: Eli Schwartz \n---\n meson.build | 13 ++++++++-----\n 1 file changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex 25bac8d89f..fbe43be949 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -1088,11 +1088,14 @@ else\n endif\n libgit_dependencies += networking_dependencies\n \n-foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']\n- if not compiler.has_function(symbol, dependencies: networking_dependencies)\n- libgit_c_args += '-DNO_' + symbol.to_upper()\n- endif\n-endforeach\n+if host_machine.system() != 'windows'\n+ foreach symbol : ['inet_ntop', 'inet_pton', 'hstrerror']\n+ if not compiler.has_function(symbol, dependencies: networking_dependencies)\n+ libgit_c_args += '-DNO_' + symbol.to_upper()\n+ libgit_sources += 'compat/' + symbol + '.c'\n+ endif\n+ endforeach\n+endif\n \n has_ipv6 = compiler.has_function('getaddrinfo', dependencies: networking_dependencies)\n if not has_ipv6\n-- \n2.49.0\n\n"},{"id":"516781","messageId":"aAtbkJiL9Xp_1Dpr@pks.im","threadId":"63322","inReplyTo":"20250425053345.17288-1-eschwartz@gentoo.org","subject":"Re: [PATCH v3 0/6] meson: miscellaneous system detection fixes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-25T09:53:20Z","receivedAt":"2025-04-25T09:53:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Apr 25, 2025 at 01:25:39AM -0400, Eli Schwartz wrote:\n> Principally motivated to handle an issue where these were failing to\n> detect the system properly, on Solaris.\n> \n> Changes:\n> \n> v2: getpagesize check moved to !windows\n> v3: style fixes, pass source files in\n\nThis looks as expected to me now. Thanks for working on it!\n\nPatrick\n"}]}