git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] meson: disable PCRE2 dependency by default
@ 2025-07-12 17:26 Carlo Marcelo Arenas Belón
  2025-07-12 17:51 ` brian m. carlson
  2025-07-13 12:23 ` [PATCH v2] meson: disable PCRE2 dependency by default in macOS Carlo Marcelo Arenas Belón
  0 siblings, 2 replies; 28+ messages in thread
From: Carlo Marcelo Arenas Belón @ 2025-07-12 17:26 UTC (permalink / raw)
  To: git; +Cc: Carlo Marcelo Arenas Belón

Makefile requires the user to provide the USE_LIBPCRE2 flag to
enable this dependency, but meson has it enabled by default,
which can be problematic, at least in macOS.

While a popular option and matching what was done by cmake
(which itself reflects what is preferred by Git for Windows)
could result in a broken build or linking with the wrong PCRE2
library.

While not git's fault, macOS provides a PCRE2 library in base
that is not usable (even if it would pass the test) and not
configured properly, as it installs a pkgconf module that
points to a non existent pcre2.h header in /usr/local/include.

Change the default to off, and let the user enable it once a
proper dependency is installed or meson instructed to fallback to
the wrap.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 meson_options.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson_options.txt b/meson_options.txt
index e7f768df24..0a0cac6f99 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -45,7 +45,7 @@ option('gitweb', type: 'feature', value: 'auto',
   description: 'Build Git web interface. Requires Perl.')
 option('iconv', type: 'feature', value: 'auto',
   description: 'Support reencoding strings with different encodings.')
-option('pcre2', type: 'feature', value: 'enabled',
+option('pcre2', type: 'feature', value: 'disabled',
   description: 'Support Perl-compatible regular expressions in e.g. git-grep(1).')
 option('perl', type: 'feature', value: 'auto',
   description: 'Build tools written in Perl.')
-- 
2.39.5 (Apple Git-154)


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] meson: disable PCRE2 dependency by default
  2025-07-12 17:26 [PATCH] meson: disable PCRE2 dependency by default Carlo Marcelo Arenas Belón
@ 2025-07-12 17:51 ` brian m. carlson
  2025-07-14 14:00   ` Carlo Arenas
  2025-07-13 12:23 ` [PATCH v2] meson: disable PCRE2 dependency by default in macOS Carlo Marcelo Arenas Belón
  1 sibling, 1 reply; 28+ messages in thread
From: brian m. carlson @ 2025-07-12 17:51 UTC (permalink / raw)
  To: Carlo Marcelo Arenas Belón; +Cc: git

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

On 2025-07-12 at 17:26:15, Carlo Marcelo Arenas Belón wrote:
> Makefile requires the user to provide the USE_LIBPCRE2 flag to
> enable this dependency, but meson has it enabled by default,
> which can be problematic, at least in macOS.
> 
> While a popular option and matching what was done by cmake
> (which itself reflects what is preferred by Git for Windows)
> could result in a broken build or linking with the wrong PCRE2
> library.
> 
> While not git's fault, macOS provides a PCRE2 library in base
> that is not usable (even if it would pass the test) and not
> configured properly, as it installs a pkgconf module that
> points to a non existent pcre2.h header in /usr/local/include.
> 
> Change the default to off, and let the user enable it once a
> proper dependency is installed or meson instructed to fallback to
> the wrap.

Can we disable it by default on macOS instead of everywhere?  For most
builds on Linux, the system libpcre2 is the right one and users will
expect to find PCRE support by default.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2] meson: disable PCRE2 dependency by default in macOS
  2025-07-12 17:26 [PATCH] meson: disable PCRE2 dependency by default Carlo Marcelo Arenas Belón
  2025-07-12 17:51 ` brian m. carlson
@ 2025-07-13 12:23 ` Carlo Marcelo Arenas Belón
  2025-07-13 15:42   ` Junio C Hamano
  2025-07-13 17:48   ` [PATCH v3] " Carlo Marcelo Arenas Belón
  1 sibling, 2 replies; 28+ messages in thread
From: Carlo Marcelo Arenas Belón @ 2025-07-13 12:23 UTC (permalink / raw)
  To: git; +Cc: sandals, Carlo Marcelo Arenas Belón

Makefile requires the user to provide the USE_LIBPCRE2 flag to
enable this dependency, but meson has it enabled by default,
which can be problematic, at least in macOS.

macOS provides a PCRE2 library in base that is not usable and not
configured properly, as it installs a pkgconf module that
points to a non existent pcre2.h header in /usr/local/include.

Add an option that will need to be turned to true once an
alternative PCRE2 library is installed (which hopefully provides
its own pkgconf module earlier in PKG_CONFIG_PATH) or meson has
been instructed to use the wrap by `--force-fallback-for=pcre2`

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 meson.build       | 3 ++-
 meson_options.txt | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 7fea4a34d6..e1475be6c8 100644
--- a/meson.build
+++ b/meson.build
@@ -1055,7 +1055,8 @@ else
   build_options_config.set('NO_ICONV', '1')
 endif
 
-pcre2 = dependency('libpcre2-8', required: get_option('pcre2'), default_options: ['default_library=static', 'test=false'])
+pcre2_feature = get_option('pcre2').disable_auto_if(host_machine.system() == 'darwin' and not get_option('macos_workaround_system_pcre2'))
+pcre2 = dependency('libpcre2-8', required: pcre2_feature, default_options: ['default_library=static', 'test=false'])
 if pcre2.found()
   libgit_dependencies += pcre2
   libgit_c_args += '-DUSE_LIBPCRE2'
diff --git a/meson_options.txt b/meson_options.txt
index e7f768df24..9c0cb6bbfa 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -45,7 +45,7 @@ option('gitweb', type: 'feature', value: 'auto',
   description: 'Build Git web interface. Requires Perl.')
 option('iconv', type: 'feature', value: 'auto',
   description: 'Support reencoding strings with different encodings.')
-option('pcre2', type: 'feature', value: 'enabled',
+option('pcre2', type: 'feature', value: 'auto',
   description: 'Support Perl-compatible regular expressions in e.g. git-grep(1).')
 option('perl', type: 'feature', value: 'auto',
   description: 'Build tools written in Perl.')
@@ -73,6 +73,8 @@ option('breaking_changes', type: 'boolean', value: false,
   description: 'Enable upcoming breaking changes.')
 option('macos_use_homebrew_gettext', type: 'boolean', value: true,
   description: 'Use gettext from Homebrew instead of the slightly-broken system-provided one.')
+option('macos_workaround_system_pcre2', type: 'boolean', value: false,
+  description: 'A working PCRE2 was provided or the fallback to the wrap is being forced.')
 
 # gitweb configuration.
 option('gitweb_config', type: 'string', value: 'gitweb_config.perl')
-- 
2.39.5 (Apple Git-154)


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v2] meson: disable PCRE2 dependency by default in macOS
  2025-07-13 12:23 ` [PATCH v2] meson: disable PCRE2 dependency by default in macOS Carlo Marcelo Arenas Belón
@ 2025-07-13 15:42   ` Junio C Hamano
  2025-07-13 17:48   ` [PATCH v3] " Carlo Marcelo Arenas Belón
  1 sibling, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2025-07-13 15:42 UTC (permalink / raw)
  To: Carlo Marcelo Arenas Belón; +Cc: git, sandals

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

> Makefile requires the user to provide the USE_LIBPCRE2 flag to
> enable this dependency, but meson has it enabled by default,
> which can be problematic, at least in macOS.
>
> macOS provides a PCRE2 library in base that is not usable and not
> configured properly, as it installs a pkgconf module that
> points to a non existent pcre2.h header in /usr/local/include.
>
> Add an option that will need to be turned to true once an
> alternative PCRE2 library is installed (which hopefully provides
> its own pkgconf module earlier in PKG_CONFIG_PATH) or meson has
> been instructed to use the wrap by `--force-fallback-for=pcre2`

Here, and ...

> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  meson.build       | 3 ++-
>  meson_options.txt | 4 +++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 7fea4a34d6..e1475be6c8 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1055,7 +1055,8 @@ else
>    build_options_config.set('NO_ICONV', '1')
>  endif
>  
> -pcre2 = dependency('libpcre2-8', required: get_option('pcre2'), default_options: ['default_library=static', 'test=false'])
> +pcre2_feature = get_option('pcre2').disable_auto_if(host_machine.system() == 'darwin' and not get_option('macos_workaround_system_pcre2'))
> +pcre2 = dependency('libpcre2-8', required: pcre2_feature, default_options: ['default_library=static', 'test=false'])
>  if pcre2.found()
>    libgit_dependencies += pcre2
>    libgit_c_args += '-DUSE_LIBPCRE2'
> diff --git a/meson_options.txt b/meson_options.txt
> index e7f768df24..9c0cb6bbfa 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -45,7 +45,7 @@ option('gitweb', type: 'feature', value: 'auto',
>    description: 'Build Git web interface. Requires Perl.')
>  option('iconv', type: 'feature', value: 'auto',
>    description: 'Support reencoding strings with different encodings.')
> -option('pcre2', type: 'feature', value: 'enabled',
> +option('pcre2', type: 'feature', value: 'auto',
>    description: 'Support Perl-compatible regular expressions in e.g. git-grep(1).')
>  option('perl', type: 'feature', value: 'auto',
>    description: 'Build tools written in Perl.')
> @@ -73,6 +73,8 @@ option('breaking_changes', type: 'boolean', value: false,
>    description: 'Enable upcoming breaking changes.')
>  option('macos_use_homebrew_gettext', type: 'boolean', value: true,
>    description: 'Use gettext from Homebrew instead of the slightly-broken system-provided one.')
> +option('macos_workaround_system_pcre2', type: 'boolean', value: false,
> +  description: 'A working PCRE2 was provided or the fallback to the wrap is being forced.')

... here, I do not understand what you wanted to refer to with the
noun "wrap".  Can you please rephrase them to clarify?

Thanks.

>  
>  # gitweb configuration.
>  option('gitweb_config', type: 'string', value: 'gitweb_config.perl')

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3] meson: disable PCRE2 dependency by default in macOS
  2025-07-13 12:23 ` [PATCH v2] meson: disable PCRE2 dependency by default in macOS Carlo Marcelo Arenas Belón
  2025-07-13 15:42   ` Junio C Hamano
@ 2025-07-13 17:48   ` Carlo Marcelo Arenas Belón
  2025-07-15  1:55     ` Eli Schwartz
  2025-07-15 11:44     ` [PATCH v4] meson: woraround broken system PCRE2 dependency " Carlo Marcelo Arenas Belón
  1 sibling, 2 replies; 28+ messages in thread
From: Carlo Marcelo Arenas Belón @ 2025-07-13 17:48 UTC (permalink / raw)
  To: git; +Cc: gitster, carenas, sandals

Makefile requires the user to provide the USE_LIBPCRE2 flag to
enable this dependency, but meson has it enabled by default,
which can be problematic, at least in macOS.

macOS provides a PCRE2 library in base that is not usable and not
configured properly, as it installs a pkgconf module that
points to a non existent pcre2.h header in /usr/local/include.

Add an option that will need to be turned to true once an
alternative PCRE2 library is installed (which hopefully provides
its own pkgconf module earlier in PKG_CONFIG_PATH) or meson has
been instructed to use its subproject as a suitable dependency
by `--force-fallback-for=pcre2`.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 meson.build       | 3 ++-
 meson_options.txt | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 7fea4a34d6..e1475be6c8 100644
--- a/meson.build
+++ b/meson.build
@@ -1055,7 +1055,8 @@ else
   build_options_config.set('NO_ICONV', '1')
 endif
 
-pcre2 = dependency('libpcre2-8', required: get_option('pcre2'), default_options: ['default_library=static', 'test=false'])
+pcre2_feature = get_option('pcre2').disable_auto_if(host_machine.system() == 'darwin' and not get_option('macos_workaround_system_pcre2'))
+pcre2 = dependency('libpcre2-8', required: pcre2_feature, default_options: ['default_library=static', 'test=false'])
 if pcre2.found()
   libgit_dependencies += pcre2
   libgit_c_args += '-DUSE_LIBPCRE2'
diff --git a/meson_options.txt b/meson_options.txt
index e7f768df24..f63ff32556 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -45,7 +45,7 @@ option('gitweb', type: 'feature', value: 'auto',
   description: 'Build Git web interface. Requires Perl.')
 option('iconv', type: 'feature', value: 'auto',
   description: 'Support reencoding strings with different encodings.')
-option('pcre2', type: 'feature', value: 'enabled',
+option('pcre2', type: 'feature', value: 'auto',
   description: 'Support Perl-compatible regular expressions in e.g. git-grep(1).')
 option('perl', type: 'feature', value: 'auto',
   description: 'Build tools written in Perl.')
@@ -73,6 +73,8 @@ option('breaking_changes', type: 'boolean', value: false,
   description: 'Enable upcoming breaking changes.')
 option('macos_use_homebrew_gettext', type: 'boolean', value: true,
   description: 'Use gettext from Homebrew instead of the slightly-broken system-provided one.')
+option('macos_workaround_system_pcre2', type: 'boolean', value: false,
+  description: 'A working PCRE2 library is available or will be provided by a subproject.')
 
 # gitweb configuration.
 option('gitweb_config', type: 'string', value: 'gitweb_config.perl')
-- 
2.39.5 (Apple Git-154)


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] meson: disable PCRE2 dependency by default
  2025-07-12 17:51 ` brian m. carlson
@ 2025-07-14 14:00   ` Carlo Arenas
  2025-07-14 15:20     ` Junio C Hamano
  0 siblings, 1 reply; 28+ messages in thread
From: Carlo Arenas @ 2025-07-14 14:00 UTC (permalink / raw)
  To: brian m. carlson, Carlo Marcelo Arenas Belón, git

On Sat, Jul 12, 2025 at 10:51 AM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> On 2025-07-12 at 17:26:15, Carlo Marcelo Arenas Belón wrote:
> > Makefile requires the user to provide the USE_LIBPCRE2 flag to
> > enable this dependency, but meson has it enabled by default,
> > which can be problematic, at least in macOS.
> >
> > While a popular option and matching what was done by cmake
> > (which itself reflects what is preferred by Git for Windows)
> > could result in a broken build or linking with the wrong PCRE2
> > library.
> >
> > While not git's fault, macOS provides a PCRE2 library in base
> > that is not usable (even if it would pass the test) and not
> > configured properly, as it installs a pkgconf module that
> > points to a non existent pcre2.h header in /usr/local/include.
> >
> > Change the default to off, and let the user enable it once a
> > proper dependency is installed or meson instructed to fallback to
> > the wrap.
>
> Can we disable it by default on macOS instead of everywhere?

Yes, but I don't like it much, as it is:

* inconsistent with artifacts created with other build systems
* really an issue that should be fixed by Apple or at least handled
better in meson itself.

> For most
> builds on Linux, the system libpcre2 is the right one and users will
> expect to find PCRE support by default.

Agree with you on that, and indeed I think every packager
of git (except for NonStop) does enable it at packaging time.

Maybe this is an argument to enable it by default?, one thing
that I wonder though, is if we should first isolate the code on
its own and link it only with `git grep`.

I think the rationale behind the current setup was that we will
eventually replace all other regex engines with PCRE2, having
better performance and retiring compat/regex, but code to do
that failed to materialise, and I am not sure how realistic that
was.

Carlo
> --
> brian m. carlson (they/them)
> Toronto, Ontario, CA

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] meson: disable PCRE2 dependency by default
  2025-07-14 14:00   ` Carlo Arenas
@ 2025-07-14 15:20     ` Junio C Hamano
  2025-07-14 16:46       ` Carlo Marcelo Arenas Belón
  0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2025-07-14 15:20 UTC (permalink / raw)
  To: Carlo Arenas; +Cc: brian m. carlson, git

Carlo Arenas <carenas@gmail.com> writes:

>> For most
>> builds on Linux, the system libpcre2 is the right one and users will
>> expect to find PCRE support by default.
>
> Agree with you on that, and indeed I think every packager
> of git (except for NonStop) does enable it at packaging time.
>
> Maybe this is an argument to enable it by default?, one thing
> that I wonder though, is if we should first isolate the code on
> its own and link it only with `git grep`.

If we decide that PCRE is good enough to do BRE and ERE emulation in
compatible enough way more performantly everywhere, it certainly is
an option that wrap our calls to platform native regcomp/regexec
that we use for our use of BRE/ERE and internally use PCRE for them.

Before that happens, "struct grep_pat" that encapsulates the
distinction between using BRE/ERE and PCRE and compile_regexp() that
compiles an end-user supplied regular expression string into members
of "struct grep_pat" so that it can be used to match against in-core
buffer we have, would need to be exposed outside the "grep.[ch]"
machinery, and direct uses of regcomp() and regexec() in the rest of
the codebase has to be rewritten to work with "struct grep_pat".

And after that happens, teaching "git log --grep=<foo>" and "git
blame -L'/<foo>/,/<bar>/'" an equivalent to "git grep -P" option
that tells the command that the pattern given is PCRE would come
almost for free.

Is that the kind of isolation you are referring to?

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] meson: disable PCRE2 dependency by default
  2025-07-14 15:20     ` Junio C Hamano
@ 2025-07-14 16:46       ` Carlo Marcelo Arenas Belón
  2025-07-14 16:58         ` Junio C Hamano
  0 siblings, 1 reply; 28+ messages in thread
From: Carlo Marcelo Arenas Belón @ 2025-07-14 16:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: brian m. carlson, git

On Mon, Jul 14, 2025 at 08:20:31AM -0800, Junio C Hamano wrote:
> Carlo Arenas <carenas@gmail.com> writes:
> 
> >> For most
> >> builds on Linux, the system libpcre2 is the right one and users will
> >> expect to find PCRE support by default.
> >
> > Agree with you on that, and indeed I think every packager
> > of git (except for NonStop) does enable it at packaging time.
> >
> > Maybe this is an argument to enable it by default?, one thing
> > that I wonder though, is if we should first isolate the code on
> > its own and link it only with `git grep`.
> 
> If we decide that PCRE is good enough to do BRE and ERE emulation in
> compatible enough way more performantly everywhere, it certainly is
> an option that wrap our calls to platform native regcomp/regexec
> that we use for our use of BRE/ERE and internally use PCRE for them.
> 
> Before that happens, "struct grep_pat" that encapsulates the
> distinction between using BRE/ERE and PCRE and compile_regexp() that
> compiles an end-user supplied regular expression string into members
> of "struct grep_pat" so that it can be used to match against in-core
> buffer we have, would need to be exposed outside the "grep.[ch]"
> machinery, and direct uses of regcomp() and regexec() in the rest of
> the codebase has to be rewritten to work with "struct grep_pat".
> 
> And after that happens, teaching "git log --grep=<foo>" and "git
> blame -L'/<foo>/,/<bar>/'" an equivalent to "git grep -P" option
> that tells the command that the pattern given is PCRE would come
> almost for free.
> 
> Is that the kind of isolation you are referring to?

Thah describes perfectly the probably unrealistic plan I mentioned
in the other part of my response that is not quoted above.

This part was more of a: let's assume that we enable PCRE2 by default
in the Makefile as well, what is the impact to the libification
efforts now that there is a chance that libgit will be linked (probably
statically if using meson) with libpcre2?

Sinvce the plan you mentioned above is still dreamware, wouldn't it be
better to move all the pcre2 functions out of grep.c, export them back
to it through a semi private header and convert `git-grep` into a
standalone binary that might link with pcre2 as needed?

Carlo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] meson: disable PCRE2 dependency by default
  2025-07-14 16:46       ` Carlo Marcelo Arenas Belón
@ 2025-07-14 16:58         ` Junio C Hamano
  0 siblings, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2025-07-14 16:58 UTC (permalink / raw)
  To: Carlo Marcelo Arenas Belón; +Cc: brian m. carlson, git

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

> This part was more of a: let's assume that we enable PCRE2 by default
> in the Makefile as well, what is the impact to the libification
> efforts now that there is a chance that libgit will be linked (probably
> statically if using meson) with libpcre2?
>
> Since the plan you mentioned above is still dreamware, wouldn't it be
> better to move all the pcre2 functions out of grep.c, export them back
> to it through a semi private header and convert `git-grep` into a
> standalone binary that might link with pcre2 as needed?

The engineering effort that such a move (and encapsulation of
"grep_pat" that may or may not have pcre enabled) would go quite a
long way and brings us quite a lot close to that "dreamware", I
suspect.  So it may not be a bad thing.

But even if you move code out of grep.c to a new "abstracted regcomp
and regexec that may or may not use pcre" source file, "git grep"
would need to run with the code you move to the latter anyway, so
unless your plan for libified "git grep" engine is to make the
library user responsible for supplying their own "abstracted regcomp
and regexec that may or may not use pcre" (and "git grep" brings in
its own in that new file), I do not know it changes the picture all
that much.

Thanks.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3] meson: disable PCRE2 dependency by default in macOS
  2025-07-13 17:48   ` [PATCH v3] " Carlo Marcelo Arenas Belón
@ 2025-07-15  1:55     ` Eli Schwartz
  2025-07-15  8:46       ` Patrick Steinhardt
  2025-07-15 12:01       ` Carlo Arenas
  2025-07-15 11:44     ` [PATCH v4] meson: woraround broken system PCRE2 dependency " Carlo Marcelo Arenas Belón
  1 sibling, 2 replies; 28+ messages in thread
From: Eli Schwartz @ 2025-07-15  1:55 UTC (permalink / raw)
  To: Carlo Marcelo Arenas Belón, git; +Cc: gitster, sandals


[-- Attachment #1.1: Type: text/plain, Size: 5154 bytes --]

On 7/13/25 1:48 PM, Carlo Marcelo Arenas Belón wrote:
> Makefile requires the user to provide the USE_LIBPCRE2 flag to
> enable this dependency, but meson has it enabled by default,
> which can be problematic, at least in macOS.
> 
> macOS provides a PCRE2 library in base that is not usable and not
> configured properly, as it installs a pkgconf module that
> points to a non existent pcre2.h header in /usr/local/include.
> 
> Add an option that will need to be turned to true once an
> alternative PCRE2 library is installed (which hopefully provides
> its own pkgconf module earlier in PKG_CONFIG_PATH) or meson has
> been instructed to use its subproject as a suitable dependency
> by `--force-fallback-for=pcre2`.


I cannot possibly agree with any part of this. Right bug report, wrong
patch.

Problem:

Meson supports three modes:

- require pcre2 and fail if missing

- automatically, optimistically use it if possible

- disable and reject it even if available


macOS has a problem:

- pcre2 (in modes 1 and 2) is detected as available via a system
  package, but upon successfully configuring a ninja file, the compile
  fails


Solution proposed here:

- v1: switch default mode from "enabled" to "disabled", leaving feature
  broken if anyone tries it out

- v2 / v3: switch default mode from "enabled" to "auto", then ignoring
  the config setting by default, and permitting users to use an ugly
  "are you sure" option to get... a failing build.



Nowhere are we checking what we got. Trying to build with pcre2 will
fail in confusingly awkward ways at the worst time: compile time. We
don't need a build option here to ask for this bad experience -- we
should just get it correct. :)

The goal should be, if the pcre2 feature is:

- enabled, meson should *fail early* if the demand cannot be met,
  providing a clear explanation of what is wrong

- auto, meson should *detect* it cannot work and build without it


> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  meson.build       | 3 ++-
>  meson_options.txt | 4 +++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 7fea4a34d6..e1475be6c8 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1055,7 +1055,8 @@ else
>    build_options_config.set('NO_ICONV', '1')
>  endif
>  
> -pcre2 = dependency('libpcre2-8', required: get_option('pcre2'), default_options: ['default_library=static', 'test=false'])
> +pcre2_feature = get_option('pcre2').disable_auto_if(host_machine.system() == 'darwin' and not get_option('macos_workaround_system_pcre2'))
> +pcre2 = dependency('libpcre2-8', required: pcre2_feature, default_options: ['default_library=static', 'test=false'])
>  if pcre2.found()
>    libgit_dependencies += pcre2
>    libgit_c_args += '-DUSE_LIBPCRE2'


Instead of disable_auto_if, we should simply verify a working install.

if pcre2.found() and pcre2.type_name() != 'internal' and
host_machine.system() == 'darwin'
    # macOS installs a broken system package, double check
    if not compiler.has_header('pcre2.h', dependencies: pcre2)
        if get_option('pcre2').enabled()
            error('broken pcre2 install found but pcre2 is required')
        endif
        # Replace with not-found-dependency
        pcre2 = dependency('', required: false)
        warning('broken pcre2 install found, disabling pcre2 feature')
    endif
endif

if pcre2.found()
    libgit_dependencies += pcre2

[...]


Please double-check my work, that this compiler.has_header() is
sufficient on your reproducer system to detect and disable the
non-working feature.


> diff --git a/meson_options.txt b/meson_options.txt
> index e7f768df24..f63ff32556 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -45,7 +45,7 @@ option('gitweb', type: 'feature', value: 'auto',
>    description: 'Build Git web interface. Requires Perl.')
>  option('iconv', type: 'feature', value: 'auto',
>    description: 'Support reencoding strings with different encodings.')
> -option('pcre2', type: 'feature', value: 'enabled',
> +option('pcre2', type: 'feature', value: 'auto',


This part is fine. We shouldn't default-fail if it isn't found, when we
can't expect it to be universally available.


>    description: 'Support Perl-compatible regular expressions in e.g. git-grep(1).')
>  option('perl', type: 'feature', value: 'auto',
>    description: 'Build tools written in Perl.')
> @@ -73,6 +73,8 @@ option('breaking_changes', type: 'boolean', value: false,
>    description: 'Enable upcoming breaking changes.')
>  option('macos_use_homebrew_gettext', type: 'boolean', value: true,
>    description: 'Use gettext from Homebrew instead of the slightly-broken system-provided one.')
> +option('macos_workaround_system_pcre2', type: 'boolean', value: false,
> +  description: 'A working PCRE2 library is available or will be provided by a subproject.')
>  
>  # gitweb configuration.
>  option('gitweb_config', type: 'string', value: 'gitweb_config.perl')


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3] meson: disable PCRE2 dependency by default in macOS
  2025-07-15  1:55     ` Eli Schwartz
@ 2025-07-15  8:46       ` Patrick Steinhardt
  2025-07-15  8:56         ` Carlo Arenas
  2025-07-15 12:01       ` Carlo Arenas
  1 sibling, 1 reply; 28+ messages in thread
From: Patrick Steinhardt @ 2025-07-15  8:46 UTC (permalink / raw)
  To: Eli Schwartz; +Cc: Carlo Marcelo Arenas Belón, git, gitster, sandals

On Mon, Jul 14, 2025 at 09:55:27PM -0400, Eli Schwartz wrote:
> On 7/13/25 1:48 PM, Carlo Marcelo Arenas Belón wrote:
> > diff --git a/meson.build b/meson.build
> > index 7fea4a34d6..e1475be6c8 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -1055,7 +1055,8 @@ else
> >    build_options_config.set('NO_ICONV', '1')
> >  endif
> >  
> > -pcre2 = dependency('libpcre2-8', required: get_option('pcre2'), default_options: ['default_library=static', 'test=false'])
> > +pcre2_feature = get_option('pcre2').disable_auto_if(host_machine.system() == 'darwin' and not get_option('macos_workaround_system_pcre2'))
> > +pcre2 = dependency('libpcre2-8', required: pcre2_feature, default_options: ['default_library=static', 'test=false'])
> >  if pcre2.found()
> >    libgit_dependencies += pcre2
> >    libgit_c_args += '-DUSE_LIBPCRE2'
> 
> 
> Instead of disable_auto_if, we should simply verify a working install.
> 
> if pcre2.found() and pcre2.type_name() != 'internal' and
> host_machine.system() == 'darwin'
>     # macOS installs a broken system package, double check
>     if not compiler.has_header('pcre2.h', dependencies: pcre2)
>         if get_option('pcre2').enabled()
>             error('broken pcre2 install found but pcre2 is required')
>         endif
>         # Replace with not-found-dependency
>         pcre2 = dependency('', required: false)
>         warning('broken pcre2 install found, disabling pcre2 feature')

Okay. So if the `pcre2` feature was explicitly enabled we error out and
abort, otherwise we print a warning and disable it. This makes a lot of
sense to me.

>     endif
> endif
> 
> if pcre2.found()
>     libgit_dependencies += pcre2
> 
> [...]
> 
> 
> Please double-check my work, that this compiler.has_header() is
> sufficient on your reproducer system to detect and disable the
> non-working feature.

I think auto-detecting such broken PCRE2 libraries is indeed the best
way forward, thanks!

> > diff --git a/meson_options.txt b/meson_options.txt
> > index e7f768df24..f63ff32556 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -45,7 +45,7 @@ option('gitweb', type: 'feature', value: 'auto',
> >    description: 'Build Git web interface. Requires Perl.')
> >  option('iconv', type: 'feature', value: 'auto',
> >    description: 'Support reencoding strings with different encodings.')
> > -option('pcre2', type: 'feature', value: 'enabled',
> > +option('pcre2', type: 'feature', value: 'auto',
> 
> 
> This part is fine. We shouldn't default-fail if it isn't found, when we
> can't expect it to be universally available.

Agreed. I guess tha only reason why I picked "enabled" here is because
we also got a wrapper in "subprojects/". But with this new workaround in
place I agree that it is sensible to switch to "auto".

Patrick

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3] meson: disable PCRE2 dependency by default in macOS
  2025-07-15  8:46       ` Patrick Steinhardt
@ 2025-07-15  8:56         ` Carlo Arenas
  2025-07-15 10:32           ` Patrick Steinhardt
  0 siblings, 1 reply; 28+ messages in thread
From: Carlo Arenas @ 2025-07-15  8:56 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Eli Schwartz, git, gitster, sandals

On Tue, Jul 15, 2025 at 1:46 AM Patrick Steinhardt <ps@pks.im> wrote:
> On Mon, Jul 14, 2025 at 09:55:27PM -0400, Eli Schwartz wrote:
> > On 7/13/25 1:48 PM, Carlo Marcelo Arenas B
> > > diff --git a/meson_options.txt b/meson_options.txt
> > > index e7f768df24..f63ff32556 100644
> > > --- a/meson_options.txt
> > > +++ b/meson_options.txt
> > > @@ -45,7 +45,7 @@ option('gitweb', type: 'feature', value: 'auto',
> > >    description: 'Build Git web interface. Requires Perl.')
> > >  option('iconv', type: 'feature', value: 'auto',
> > >    description: 'Support reencoding strings with different encodings.')
> > > -option('pcre2', type: 'feature', value: 'enabled',
> > > +option('pcre2', type: 'feature', value: 'auto',
> >
> > This part is fine. We shouldn't default-fail if it isn't found, when we
> > can't expect it to be universally available.
>
> Agreed. I guess tha only reason why I picked "enabled" here is because
> we also got a wrapper in "subprojects/". But with this new workaround in
> place I agree that it is sensible to switch to "auto".

AFAIK the "wrapper" fallback still kicks in when the feature is "auto"

Carlo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3] meson: disable PCRE2 dependency by default in macOS
  2025-07-15  8:56         ` Carlo Arenas
@ 2025-07-15 10:32           ` Patrick Steinhardt
  2025-07-15 12:08             ` Carlo Arenas
  0 siblings, 1 reply; 28+ messages in thread
From: Patrick Steinhardt @ 2025-07-15 10:32 UTC (permalink / raw)
  To: Carlo Arenas; +Cc: Eli Schwartz, git, gitster, sandals

On Tue, Jul 15, 2025 at 01:56:44AM -0700, Carlo Arenas wrote:
> On Tue, Jul 15, 2025 at 1:46 AM Patrick Steinhardt <ps@pks.im> wrote:
> > On Mon, Jul 14, 2025 at 09:55:27PM -0400, Eli Schwartz wrote:
> > > On 7/13/25 1:48 PM, Carlo Marcelo Arenas B
> > > > diff --git a/meson_options.txt b/meson_options.txt
> > > > index e7f768df24..f63ff32556 100644
> > > > --- a/meson_options.txt
> > > > +++ b/meson_options.txt
> > > > @@ -45,7 +45,7 @@ option('gitweb', type: 'feature', value: 'auto',
> > > >    description: 'Build Git web interface. Requires Perl.')
> > > >  option('iconv', type: 'feature', value: 'auto',
> > > >    description: 'Support reencoding strings with different encodings.')
> > > > -option('pcre2', type: 'feature', value: 'enabled',
> > > > +option('pcre2', type: 'feature', value: 'auto',
> > >
> > > This part is fine. We shouldn't default-fail if it isn't found, when we
> > > can't expect it to be universally available.
> >
> > Agreed. I guess tha only reason why I picked "enabled" here is because
> > we also got a wrapper in "subprojects/". But with this new workaround in
> > place I agree that it is sensible to switch to "auto".
> 
> AFAIK the "wrapper" fallback still kicks in when the feature is "auto"

It does, yes. But with 'auto' as default it means that we're free to
disable PCRE2 if we have detected a broken PCRE2 dependency.

Patrick

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v4] meson: woraround broken system PCRE2 dependency in macOS
  2025-07-13 17:48   ` [PATCH v3] " Carlo Marcelo Arenas Belón
  2025-07-15  1:55     ` Eli Schwartz
@ 2025-07-15 11:44     ` Carlo Marcelo Arenas Belón
  2025-07-15 16:48       ` Junio C Hamano
                         ` (2 more replies)
  1 sibling, 3 replies; 28+ messages in thread
From: Carlo Marcelo Arenas Belón @ 2025-07-15 11:44 UTC (permalink / raw)
  To: git; +Cc: gitster, sandals, ps, eschwartz, Carlo Marcelo Arenas Belón

macOS provides a PCRE2 library in base that is not usable and not
configured properly, as it installs a pkgconf module that
points to a non existent pcre2.h header in /usr/local/include.

Detect that case and allow a fallback to a wrapped submodule
if the feature is enabled and that is possible, or print a
warning and disable the feature if the feature was set to "auto".
which is the new default.

Suggested-by: Eli Schwartz <eschwartz@gentoo.org>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 meson.build       | 20 +++++++++++++++++++-
 meson_options.txt |  2 +-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 596f5ac711..0e480e65cf 100644
--- a/meson.build
+++ b/meson.build
@@ -1055,7 +1055,25 @@ else
   build_options_config.set('NO_ICONV', '1')
 endif
 
-pcre2 = dependency('libpcre2-8', required: get_option('pcre2'), default_options: ['default_library=static', 'test=false'])
+pcre2_feature = get_option('pcre2')
+pcre2 = dependency('libpcre2-8', required: pcre2_feature, default_options: ['default_library=static', 'test=false'])
+if pcre2.found() and pcre2.type_name() != 'internal' and host_machine.system() == 'darwin'
+  # macOS installs a broken system package, double check
+  if not compiler.has_header('pcre2.h', dependencies: pcre2)
+    if pcre2_feature.enabled()
+      # Attempt to fallback
+      pcre2 = dependency('libpcre2-8', required: true, method: 'builtin', default_options: ['default_library=static', 'test=false'])
+      if not pcre2.found()
+        error('only a broken pcre2 install found and pcre2 is required')
+      endif
+    elif pcre2_feature.auto()
+      # Replace with not-found-dependency
+      pcre2 = dependency('', required: false)
+      warning('broken pcre2 install found, disabling pcre2 feature')
+    endif
+  endif
+endif
+
 if pcre2.found()
   libgit_dependencies += pcre2
   libgit_c_args += '-DUSE_LIBPCRE2'
diff --git a/meson_options.txt b/meson_options.txt
index e7f768df24..1668f260a1 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -45,7 +45,7 @@ option('gitweb', type: 'feature', value: 'auto',
   description: 'Build Git web interface. Requires Perl.')
 option('iconv', type: 'feature', value: 'auto',
   description: 'Support reencoding strings with different encodings.')
-option('pcre2', type: 'feature', value: 'enabled',
+option('pcre2', type: 'feature', value: 'auto',
   description: 'Support Perl-compatible regular expressions in e.g. git-grep(1).')
 option('perl', type: 'feature', value: 'auto',
   description: 'Build tools written in Perl.')
-- 
2.39.5 (Apple Git-154)


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v3] meson: disable PCRE2 dependency by default in macOS
  2025-07-15  1:55     ` Eli Schwartz
  2025-07-15  8:46       ` Patrick Steinhardt
@ 2025-07-15 12:01       ` Carlo Arenas
  2025-07-15 14:22         ` Eli Schwartz
  1 sibling, 1 reply; 28+ messages in thread
From: Carlo Arenas @ 2025-07-15 12:01 UTC (permalink / raw)
  To: Eli Schwartz; +Cc: git, gitster, sandals

On Mon, Jul 14, 2025 at 6:55 PM Eli Schwartz <eschwartz@gentoo.org> wrote:
>
> Please double-check my work, that this compiler.has_header() is
> sufficient on your reproducer system to detect and disable the
> non-working feature.

it is indeed all that was needed, abd makes me wonder
if a future version of dependency() shouldn't have a "has_header"
parameter like the one used in find_libray() to allow for this
validation to happen internally and fallback as needed.

Posted a v4 with your code and modifications to still allow a
fallback to the wrap.

Carlo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3] meson: disable PCRE2 dependency by default in macOS
  2025-07-15 10:32           ` Patrick Steinhardt
@ 2025-07-15 12:08             ` Carlo Arenas
  2025-07-15 14:14               ` Eli Schwartz
  0 siblings, 1 reply; 28+ messages in thread
From: Carlo Arenas @ 2025-07-15 12:08 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Eli Schwartz, git, gitster, sandals

On Tue, Jul 15, 2025 at 3:32 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Tue, Jul 15, 2025 at 01:56:44AM -0700, Carlo Arenas wrote:
> > On Tue, Jul 15, 2025 at 1:46 AM Patrick Steinhardt <ps@pks.im> wrote:
> > > On Mon, Jul 14, 2025 at 09:55:27PM -0400, Eli Schwartz wrote:
> > > > On 7/13/25 1:48 PM, Carlo Marcelo Arenas B
> > > > > diff --git a/meson_options.txt b/meson_options.txt
> > > > > index e7f768df24..f63ff32556 100644
> > > > > --- a/meson_options.txt
> > > > > +++ b/meson_options.txt
> > > > > @@ -45,7 +45,7 @@ option('gitweb', type: 'feature', value: 'auto',
> > > > >    description: 'Build Git web interface. Requires Perl.')
> > > > >  option('iconv', type: 'feature', value: 'auto',
> > > > >    description: 'Support reencoding strings with different encodings.')
> > > > > -option('pcre2', type: 'feature', value: 'enabled',
> > > > > +option('pcre2', type: 'feature', value: 'auto',
> > > >
> > > > This part is fine. We shouldn't default-fail if it isn't found, when we
> > > > can't expect it to be universally available.
> > >
> > > Agreed. I guess tha only reason why I picked "enabled" here is because
> > > we also got a wrapper in "subprojects/". But with this new workaround in
> > > place I agree that it is sensible to switch to "auto".
> >
> > AFAIK the "wrapper" fallback still kicks in when the feature is "auto"
>
> It does, yes. But with 'auto' as default it means that we're free to
> disable PCRE2 if we have detected a broken PCRE2 dependency.

My bad, I was mistaken and indeed auto doesn't fallback to the wrap, so
this will likely regress in windows if it is not invoked with `-Dpcre2=enabled`

Carlo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3] meson: disable PCRE2 dependency by default in macOS
  2025-07-15 12:08             ` Carlo Arenas
@ 2025-07-15 14:14               ` Eli Schwartz
  0 siblings, 0 replies; 28+ messages in thread
From: Eli Schwartz @ 2025-07-15 14:14 UTC (permalink / raw)
  To: Carlo Arenas, Patrick Steinhardt; +Cc: git, gitster, sandals


[-- Attachment #1.1: Type: text/plain, Size: 1329 bytes --]

On 7/15/25 8:08 AM, Carlo Arenas wrote:
> On Tue, Jul 15, 2025 at 3:32 AM Patrick Steinhardt <ps@pks.im> wrote:
>>
>> On Tue, Jul 15, 2025 at 01:56:44AM -0700, Carlo Arenas wrote:
>>> On Tue, Jul 15, 2025 at 1:46 AM Patrick Steinhardt <ps@pks.im> wrote:

>>>> Agreed. I guess tha only reason why I picked "enabled" here is because
>>>> we also got a wrapper in "subprojects/". But with this new workaround in
>>>> place I agree that it is sensible to switch to "auto".
>>>
>>> AFAIK the "wrapper" fallback still kicks in when the feature is "auto"
>>
>> It does, yes. But with 'auto' as default it means that we're free to
>> disable PCRE2 if we have detected a broken PCRE2 dependency.
> 
> My bad, I was mistaken and indeed auto doesn't fallback to the wrap, so
> this will likely regress in windows if it is not invoked with `-Dpcre2=enabled`


See
https://mesonbuild.com/Reference-manual_functions.html#dependency_allow_fallback

The default is to fallback to the wrap for *required* dependencies
(rather than abort the build). If it's desirable to have "auto" attempt
to automatically enable the feature if

system_found || \
(!wrap_mode_nofallback && (have_internet_for_wrap || \
    have_locally_downloaded_wrap ))


then you can use that "allow_fallback" kwarg.


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3] meson: disable PCRE2 dependency by default in macOS
  2025-07-15 12:01       ` Carlo Arenas
@ 2025-07-15 14:22         ` Eli Schwartz
  0 siblings, 0 replies; 28+ messages in thread
From: Eli Schwartz @ 2025-07-15 14:22 UTC (permalink / raw)
  To: Carlo Arenas; +Cc: git, gitster, sandals


[-- Attachment #1.1: Type: text/plain, Size: 1244 bytes --]

On 7/15/25 8:01 AM, Carlo Arenas wrote:
> On Mon, Jul 14, 2025 at 6:55 PM Eli Schwartz <eschwartz@gentoo.org> wrote:
>>
>> Please double-check my work, that this compiler.has_header() is
>> sufficient on your reproducer system to detect and disable the
>> non-working feature.
> 
> it is indeed all that was needed, abd makes me wonder
> if a future version of dependency() shouldn't have a "has_header"
> parameter like the one used in find_libray() to allow for this
> validation to happen internally and fallback as needed.


It's... possible, I suppose, but usually not needed unless a distributor
doesn't respond to reports that their OS is broken. Which well, fair
enough, happens. :)

Another possibility is to add a quirk to meson's underlying python code.
If pcre2 is searched, and the platform is darwin, also check the header.
You'll need a small override in mesonbuild/dependencies/*.py, like:


class PCRE2PkgConfigDependency(PkgConfigDependency):
    def __init__(...):
        super().__init__(...)
        if is_darwin:
            self.is_found = check_that_header()


> Posted a v4 with your code and modifications to still allow a
> fallback to the wrap.


Thanks. :)

-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4] meson: woraround broken system PCRE2 dependency in macOS
  2025-07-15 11:44     ` [PATCH v4] meson: woraround broken system PCRE2 dependency " Carlo Marcelo Arenas Belón
@ 2025-07-15 16:48       ` Junio C Hamano
  2025-07-15 16:50       ` Eric Sunshine
  2025-07-16 19:30       ` [PATCH v5] meson: work around " Carlo Marcelo Arenas Belón
  2 siblings, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2025-07-15 16:48 UTC (permalink / raw)
  To: Carlo Marcelo Arenas Belón; +Cc: git, sandals, ps, eschwartz

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

> macOS provides a PCRE2 library in base that is not usable and not
> configured properly, as it installs a pkgconf module that
> points to a non existent pcre2.h header in /usr/local/include.
>
> Detect that case and allow a fallback to a wrapped submodule
> if the feature is enabled and that is possible, or print a
> warning and disable the feature if the feature was set to "auto".
> which is the new default.
>
> Suggested-by: Eli Schwartz <eschwartz@gentoo.org>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---

Thanks.  Will queue.




>  meson.build       | 20 +++++++++++++++++++-
>  meson_options.txt |  2 +-
>  2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 596f5ac711..0e480e65cf 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1055,7 +1055,25 @@ else
>    build_options_config.set('NO_ICONV', '1')
>  endif
>  
> -pcre2 = dependency('libpcre2-8', required: get_option('pcre2'), default_options: ['default_library=static', 'test=false'])
> +pcre2_feature = get_option('pcre2')
> +pcre2 = dependency('libpcre2-8', required: pcre2_feature, default_options: ['default_library=static', 'test=false'])
> +if pcre2.found() and pcre2.type_name() != 'internal' and host_machine.system() == 'darwin'
> +  # macOS installs a broken system package, double check
> +  if not compiler.has_header('pcre2.h', dependencies: pcre2)
> +    if pcre2_feature.enabled()
> +      # Attempt to fallback
> +      pcre2 = dependency('libpcre2-8', required: true, method: 'builtin', default_options: ['default_library=static', 'test=false'])
> +      if not pcre2.found()
> +        error('only a broken pcre2 install found and pcre2 is required')
> +      endif
> +    elif pcre2_feature.auto()
> +      # Replace with not-found-dependency
> +      pcre2 = dependency('', required: false)
> +      warning('broken pcre2 install found, disabling pcre2 feature')
> +    endif
> +  endif
> +endif
> +
>  if pcre2.found()
>    libgit_dependencies += pcre2
>    libgit_c_args += '-DUSE_LIBPCRE2'
> diff --git a/meson_options.txt b/meson_options.txt
> index e7f768df24..1668f260a1 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -45,7 +45,7 @@ option('gitweb', type: 'feature', value: 'auto',
>    description: 'Build Git web interface. Requires Perl.')
>  option('iconv', type: 'feature', value: 'auto',
>    description: 'Support reencoding strings with different encodings.')
> -option('pcre2', type: 'feature', value: 'enabled',
> +option('pcre2', type: 'feature', value: 'auto',
>    description: 'Support Perl-compatible regular expressions in e.g. git-grep(1).')
>  option('perl', type: 'feature', value: 'auto',
>    description: 'Build tools written in Perl.')

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4] meson: woraround broken system PCRE2 dependency in macOS
  2025-07-15 11:44     ` [PATCH v4] meson: woraround broken system PCRE2 dependency " Carlo Marcelo Arenas Belón
  2025-07-15 16:48       ` Junio C Hamano
@ 2025-07-15 16:50       ` Eric Sunshine
  2025-07-16 19:30       ` [PATCH v5] meson: work around " Carlo Marcelo Arenas Belón
  2 siblings, 0 replies; 28+ messages in thread
From: Eric Sunshine @ 2025-07-15 16:50 UTC (permalink / raw)
  To: Carlo Marcelo Arenas Belón; +Cc: git, gitster, sandals, ps, eschwartz

On Tue, Jul 15, 2025 at 7:44 AM Carlo Marcelo Arenas Belón
<carenas@gmail.com> wrote:
> meson: woraround broken system PCRE2 dependency in macOS

s/woraround/work around/

> macOS provides a PCRE2 library in base that is not usable and not
> configured properly, as it installs a pkgconf module that
> points to a non existent pcre2.h header in /usr/local/include.

s/non existent/non-existent/

> Detect that case and allow a fallback to a wrapped submodule
> if the feature is enabled and that is possible, or print a
> warning and disable the feature if the feature was set to "auto".
> which is the new default.
>
> Suggested-by: Eli Schwartz <eschwartz@gentoo.org>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v5] meson: work around broken system PCRE2 dependency in macOS
  2025-07-15 11:44     ` [PATCH v4] meson: woraround broken system PCRE2 dependency " Carlo Marcelo Arenas Belón
  2025-07-15 16:48       ` Junio C Hamano
  2025-07-15 16:50       ` Eric Sunshine
@ 2025-07-16 19:30       ` Carlo Marcelo Arenas Belón
  2025-07-16 21:13         ` Junio C Hamano
                           ` (2 more replies)
  2 siblings, 3 replies; 28+ messages in thread
From: Carlo Marcelo Arenas Belón @ 2025-07-16 19:30 UTC (permalink / raw)
  To: git
  Cc: gitster, ps, sandals, Carlo Marcelo Arenas Belón,
	Eric Sunshine, Eli Schwartz

macOS provides a PCRE2 library in base that is not usable and not
configured properly, as it installs a pkgconf module that
points to a non-existent pcre2.h header in /usr/local/include.

Detect that case and allow a fallback to a wrapped submodule
if the feature is enabled and that is possible, or print a
warning and disable the feature if the feature was set to "auto",
which is the new default.

To keep consistency with the cmake build system used on Windows,
add a special rule to re-enable the pcre2 feature there.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Suggested-by: Eli Schwartz <eschwartz@gentoo.org>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 meson.build       | 25 ++++++++++++++++++++++++-
 meson_options.txt |  2 +-
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 596f5ac711..c77ce82bf2 100644
--- a/meson.build
+++ b/meson.build
@@ -1055,7 +1055,30 @@ else
   build_options_config.set('NO_ICONV', '1')
 endif
 
-pcre2 = dependency('libpcre2-8', required: get_option('pcre2'), default_options: ['default_library=static', 'test=false'])
+# can't use enable_auto_if() because it is only available in meson 1.1
+if host_machine.system() == 'windows' and get_option('pcre2').allowed()
+  pcre2_feature = true
+else
+  pcre2_feature = get_option('pcre2')
+endif
+pcre2 = dependency('libpcre2-8', required: pcre2_feature, default_options: ['default_library=static', 'test=false'])
+if pcre2.found() and pcre2.type_name() != 'internal' and host_machine.system() == 'darwin'
+  # macOS installs a broken system package, double check
+  if not compiler.has_header('pcre2.h', dependencies: pcre2)
+    if pcre2_feature.enabled()
+      # Attempt to fallback, method can't be pkg-config 
+      pcre2 = dependency('libpcre2-8', method: 'builtin', default_options: ['default_library=static', 'test=false'])
+      if not pcre2.found()
+        error('only a broken pcre2 install found and pcre2 is required')
+      endif
+    else
+      # Replace with not-found-dependency
+      pcre2 = dependency('', required: false)
+      warning('broken pcre2 install found, disabling pcre2 feature')
+    endif
+  endif
+endif
+
 if pcre2.found()
   libgit_dependencies += pcre2
   libgit_c_args += '-DUSE_LIBPCRE2'
diff --git a/meson_options.txt b/meson_options.txt
index e7f768df24..1668f260a1 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -45,7 +45,7 @@ option('gitweb', type: 'feature', value: 'auto',
   description: 'Build Git web interface. Requires Perl.')
 option('iconv', type: 'feature', value: 'auto',
   description: 'Support reencoding strings with different encodings.')
-option('pcre2', type: 'feature', value: 'enabled',
+option('pcre2', type: 'feature', value: 'auto',
   description: 'Support Perl-compatible regular expressions in e.g. git-grep(1).')
 option('perl', type: 'feature', value: 'auto',
   description: 'Build tools written in Perl.')
-- 
2.39.5 (Apple Git-154)


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v5] meson: work around broken system PCRE2 dependency in macOS
  2025-07-16 19:30       ` [PATCH v5] meson: work around " Carlo Marcelo Arenas Belón
@ 2025-07-16 21:13         ` Junio C Hamano
  2025-07-16 21:17           ` Junio C Hamano
  2025-07-16 22:10         ` Eli Schwartz
  2025-07-18 17:02         ` [PATCH v6] " Carlo Marcelo Arenas Belón
  2 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2025-07-16 21:13 UTC (permalink / raw)
  To: Carlo Marcelo Arenas Belón
  Cc: git, ps, sandals, Eric Sunshine, Eli Schwartz

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

> macOS provides a PCRE2 library in base that is not usable and not
> configured properly, as it installs a pkgconf module that
> points to a non-existent pcre2.h header in /usr/local/include.

Thanks but unfortunately this came a bit too late after the previous
round was merged to 'next' already.  If needed, could you make it
incremental update on top?

Thanks.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5] meson: work around broken system PCRE2 dependency in macOS
  2025-07-16 21:13         ` Junio C Hamano
@ 2025-07-16 21:17           ` Junio C Hamano
  0 siblings, 0 replies; 28+ messages in thread
From: Junio C Hamano @ 2025-07-16 21:17 UTC (permalink / raw)
  To: Carlo Marcelo Arenas Belón
  Cc: git, ps, sandals, Eric Sunshine, Eli Schwartz

Junio C Hamano <gitster@pobox.com> writes:

> Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:
>
>> macOS provides a PCRE2 library in base that is not usable and not
>> configured properly, as it installs a pkgconf module that
>> points to a non-existent pcre2.h header in /usr/local/include.
>
> Thanks but unfortunately this came a bit too late after the previous
> round was merged to 'next' already.  If needed, could you make it
> incremental update on top?

Oops, sorry, please disregard.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5] meson: work around broken system PCRE2 dependency in macOS
  2025-07-16 19:30       ` [PATCH v5] meson: work around " Carlo Marcelo Arenas Belón
  2025-07-16 21:13         ` Junio C Hamano
@ 2025-07-16 22:10         ` Eli Schwartz
  2025-07-16 22:17           ` Carlo Arenas
  2025-07-18 17:02         ` [PATCH v6] " Carlo Marcelo Arenas Belón
  2 siblings, 1 reply; 28+ messages in thread
From: Eli Schwartz @ 2025-07-16 22:10 UTC (permalink / raw)
  To: Carlo Marcelo Arenas Belón, git; +Cc: gitster, ps, sandals, Eric Sunshine


[-- Attachment #1.1: Type: text/plain, Size: 1133 bytes --]

On 7/16/25 3:30 PM, Carlo Marcelo Arenas Belón wrote:

> +if pcre2.found() and pcre2.type_name() != 'internal' and host_machine.system() == 'darwin'
> +  # macOS installs a broken system package, double check
> +  if not compiler.has_header('pcre2.h', dependencies: pcre2)
> +    if pcre2_feature.enabled()
> +      # Attempt to fallback, method can't be pkg-config 
> +      pcre2 = dependency('libpcre2-8', method: 'builtin', default_options: ['default_library=static', 'test=false'])
> +      if not pcre2.found()
> +        error('only a broken pcre2 install found and pcre2 is required')


If you want to override the message from a specific dependency() call,
"required" defaults to true and aborts on the line before
"if not pcre2.found()"


You could do:
https://mesonbuild.com/Reference-manual_functions.html#dependency_not_found_message


not_found_message: 'only a broken pcre2 install found and pcre2 is required'

Alternatively, required: false followed by if not pcre2.found() -->
error(). But you will need "allow_fallback: true" as I responded in a
previous message.


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5] meson: work around broken system PCRE2 dependency in macOS
  2025-07-16 22:10         ` Eli Schwartz
@ 2025-07-16 22:17           ` Carlo Arenas
  0 siblings, 0 replies; 28+ messages in thread
From: Carlo Arenas @ 2025-07-16 22:17 UTC (permalink / raw)
  To: Eli Schwartz; +Cc: git, gitster, ps, sandals, Eric Sunshine

On Wed, Jul 16, 2025 at 3:10 PM Eli Schwartz <eschwartz@gentoo.org> wrote:
>
> On 7/16/25 3:30 PM, Carlo Marcelo Arenas Belón wrote:
>
> > +if pcre2.found() and pcre2.type_name() != 'internal' and host_machine.system() == 'darwin'
> > +  # macOS installs a broken system package, double check
> > +  if not compiler.has_header('pcre2.h', dependencies: pcre2)
> > +    if pcre2_feature.enabled()
> > +      # Attempt to fallback, method can't be pkg-config
> > +      pcre2 = dependency('libpcre2-8', method: 'builtin', default_options: ['default_library=static', 'test=false'])
> > +      if not pcre2.found()
> > +        error('only a broken pcre2 install found and pcre2 is required')
>
> If you want to override the message from a specific dependency() call,
> "required" defaults to true and aborts on the line before
> "if not pcre2.found()"

It is a little hacky, but because we are not using the method: auto, then won't
try pkg-config again and fallback to the wrap because as you said, required
defaults to true, and allow_fallback is true if required is true.

The only way this dependency() will fail is if --wrap-mode=nofallback and that
won't cause it to abort.

Carlo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v6] meson: work around broken system PCRE2 dependency in macOS
  2025-07-16 19:30       ` [PATCH v5] meson: work around " Carlo Marcelo Arenas Belón
  2025-07-16 21:13         ` Junio C Hamano
  2025-07-16 22:10         ` Eli Schwartz
@ 2025-07-18 17:02         ` Carlo Marcelo Arenas Belón
  2025-07-23 22:17           ` Junio C Hamano
  2 siblings, 1 reply; 28+ messages in thread
From: Carlo Marcelo Arenas Belón @ 2025-07-18 17:02 UTC (permalink / raw)
  To: git
  Cc: gitster, ps, sandals, Carlo Marcelo Arenas Belón,
	Eric Sunshine, Eli Schwartz

macOS provides a PCRE2 library in base that is not usable and not
configured properly, as it installs a pkgconf module that
points to a non-existent pcre2.h header in /usr/local/include.

Detect that case and if the feature is enabled, try to fallback
to a wrapped subproject through an anonymous dependency, aborting
with an error if that is not possible.

Change the feature to "auto" and print a warning and disable it
if a broken dependency was detected, but to keep consistency
with the cmake build system used on Windows, add a special rule
to re-enable the pcre2 feature by default there.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Suggested-by: Eli Schwartz <eschwartz@gentoo.org>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 meson.build       | 28 +++++++++++++++++++++++++++-
 meson_options.txt |  2 +-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 596f5ac711..5225efb4a6 100644
--- a/meson.build
+++ b/meson.build
@@ -1055,7 +1055,33 @@ else
   build_options_config.set('NO_ICONV', '1')
 endif
 
-pcre2 = dependency('libpcre2-8', required: get_option('pcre2'), default_options: ['default_library=static', 'test=false'])
+# can't use enable_auto_if() because it is only available in meson 1.1
+if host_machine.system() == 'windows' and get_option('pcre2').allowed()
+  pcre2_feature = true
+else
+  pcre2_feature = get_option('pcre2')
+endif
+pcre2 = dependency('libpcre2-8', required: pcre2_feature, default_options: ['default_library=static', 'test=false'])
+if pcre2.found() and pcre2.type_name() != 'internal' and host_machine.system() == 'darwin'
+  # macOS installs a broken system package, double check
+  if not compiler.has_header('pcre2.h', dependencies: pcre2)
+    if pcre2_feature.enabled()
+      pcre2_fallback = ['pcre2', 'libpcre2_8']
+    else
+      pcre2_fallback = []
+    endif
+    # Attempt to fallback or replace with not-found-dependency
+    pcre2 = dependency('', required: false, fallback: pcre2_fallback, default_options: ['default_library=static', 'test=false'])
+    if not pcre2.found()
+      if pcre2_feature.enabled()
+        error('only a broken pcre2 install found and pcre2 is required')
+      else
+        warning('broken pcre2 install found, disabling pcre2 feature')
+      endif
+    endif
+  endif
+endif
+
 if pcre2.found()
   libgit_dependencies += pcre2
   libgit_c_args += '-DUSE_LIBPCRE2'
diff --git a/meson_options.txt b/meson_options.txt
index e7f768df24..1668f260a1 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -45,7 +45,7 @@ option('gitweb', type: 'feature', value: 'auto',
   description: 'Build Git web interface. Requires Perl.')
 option('iconv', type: 'feature', value: 'auto',
   description: 'Support reencoding strings with different encodings.')
-option('pcre2', type: 'feature', value: 'enabled',
+option('pcre2', type: 'feature', value: 'auto',
   description: 'Support Perl-compatible regular expressions in e.g. git-grep(1).')
 option('perl', type: 'feature', value: 'auto',
   description: 'Build tools written in Perl.')
-- 
2.39.5 (Apple Git-154)


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v6] meson: work around broken system PCRE2 dependency in macOS
  2025-07-18 17:02         ` [PATCH v6] " Carlo Marcelo Arenas Belón
@ 2025-07-23 22:17           ` Junio C Hamano
  2025-07-24  5:28             ` Patrick Steinhardt
  0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2025-07-23 22:17 UTC (permalink / raw)
  To: Carlo Marcelo Arenas Belón
  Cc: git, ps, sandals, Eric Sunshine, Eli Schwartz

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

> macOS provides a PCRE2 library in base that is not usable and not
> configured properly, as it installs a pkgconf module that
> points to a non-existent pcre2.h header in /usr/local/include.
>
> Detect that case and if the feature is enabled, try to fallback
> to a wrapped subproject through an anonymous dependency, aborting
> with an error if that is not possible.
>
> Change the feature to "auto" and print a warning and disable it
> if a broken dependency was detected, but to keep consistency
> with the cmake build system used on Windows, add a special rule
> to re-enable the pcre2 feature by default there.
>
> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> Suggested-by: Eli Schwartz <eschwartz@gentoo.org>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  meson.build       | 28 +++++++++++++++++++++++++++-
>  meson_options.txt |  2 +-
>  2 files changed, 28 insertions(+), 2 deletions(-)

The thread went silent after this iteration.

I _think_ it incorporates all the good suggestions offered during
the discussion on the previous iterations, but is everybody happy
with this version?  If so, let me mark the topic for 'next'.

Thanks, all.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v6] meson: work around broken system PCRE2 dependency in macOS
  2025-07-23 22:17           ` Junio C Hamano
@ 2025-07-24  5:28             ` Patrick Steinhardt
  0 siblings, 0 replies; 28+ messages in thread
From: Patrick Steinhardt @ 2025-07-24  5:28 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Carlo Marcelo Arenas Belón, git, sandals, Eric Sunshine,
	Eli Schwartz

On Wed, Jul 23, 2025 at 03:17:27PM -0700, Junio C Hamano wrote:
> Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:
> 
> > macOS provides a PCRE2 library in base that is not usable and not
> > configured properly, as it installs a pkgconf module that
> > points to a non-existent pcre2.h header in /usr/local/include.
> >
> > Detect that case and if the feature is enabled, try to fallback
> > to a wrapped subproject through an anonymous dependency, aborting
> > with an error if that is not possible.
> >
> > Change the feature to "auto" and print a warning and disable it
> > if a broken dependency was detected, but to keep consistency
> > with the cmake build system used on Windows, add a special rule
> > to re-enable the pcre2 feature by default there.
> >
> > Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> > Suggested-by: Eli Schwartz <eschwartz@gentoo.org>
> > Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> > ---
> >  meson.build       | 28 +++++++++++++++++++++++++++-
> >  meson_options.txt |  2 +-
> >  2 files changed, 28 insertions(+), 2 deletions(-)
> 
> The thread went silent after this iteration.
> 
> I _think_ it incorporates all the good suggestions offered during
> the discussion on the previous iterations, but is everybody happy
> with this version?  If so, let me mark the topic for 'next'.
> 
> Thanks, all.

I think this version is fine. Thanks!

Patrick

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2025-07-24  5:28 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-12 17:26 [PATCH] meson: disable PCRE2 dependency by default Carlo Marcelo Arenas Belón
2025-07-12 17:51 ` brian m. carlson
2025-07-14 14:00   ` Carlo Arenas
2025-07-14 15:20     ` Junio C Hamano
2025-07-14 16:46       ` Carlo Marcelo Arenas Belón
2025-07-14 16:58         ` Junio C Hamano
2025-07-13 12:23 ` [PATCH v2] meson: disable PCRE2 dependency by default in macOS Carlo Marcelo Arenas Belón
2025-07-13 15:42   ` Junio C Hamano
2025-07-13 17:48   ` [PATCH v3] " Carlo Marcelo Arenas Belón
2025-07-15  1:55     ` Eli Schwartz
2025-07-15  8:46       ` Patrick Steinhardt
2025-07-15  8:56         ` Carlo Arenas
2025-07-15 10:32           ` Patrick Steinhardt
2025-07-15 12:08             ` Carlo Arenas
2025-07-15 14:14               ` Eli Schwartz
2025-07-15 12:01       ` Carlo Arenas
2025-07-15 14:22         ` Eli Schwartz
2025-07-15 11:44     ` [PATCH v4] meson: woraround broken system PCRE2 dependency " Carlo Marcelo Arenas Belón
2025-07-15 16:48       ` Junio C Hamano
2025-07-15 16:50       ` Eric Sunshine
2025-07-16 19:30       ` [PATCH v5] meson: work around " Carlo Marcelo Arenas Belón
2025-07-16 21:13         ` Junio C Hamano
2025-07-16 21:17           ` Junio C Hamano
2025-07-16 22:10         ` Eli Schwartz
2025-07-16 22:17           ` Carlo Arenas
2025-07-18 17:02         ` [PATCH v6] " Carlo Marcelo Arenas Belón
2025-07-23 22:17           ` Junio C Hamano
2025-07-24  5:28             ` Patrick Steinhardt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).