macos: do let the build find the gettext headers/libraries/msgfmt

PATCH Superseded Johannes Schindelin via GitGitGadget 5 participants 12 messages Apr 23, 2020 – Apr 26, 2020 Kbuild

lore

Share & tools

lore

Johannes Schindelin via GitGitGadget

author

2020-04-23 07:52 UTC #0 lore raw

PATCH 1/1 macos: do let the build find the gettext headers/libraries/msgfmt

From: Johannes Schindelin <[email protected]>

Apparently a recent Homebrew update now installs gettext into a subdirectory under /usr/local/, requiring the CFLAGS/LDFLAGS to list explicit directories _even_ when asking to force-link the gettext package.

Likewise, the msgfmt tool is no longer in the PATH.

Let's work around this issue.

Signed-off-by Johannes Schindelin <[email protected]>

---
Prepare for Homebrew changing the gettext package

In an early Azure Pipelines preview of what is to come, I saw the osx-clang and osx-gcc jobs fail consistently.

This patch tries to prevent that from affecting our CI/PR builds.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-616%2Fdscho%2Fbrew-gettext-update-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-616/dscho/brew-gettext-update-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/616

 config.mak.uname | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

config.mak.uname +5 -2

index 0ab8e009383..540d124d2ef 100644
@@ -133,8 +133,11 @@ ifeq ($(uname_S),Darwin)
 	HAVE_BSD_SYSCTL = YesPlease
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
 	HAVE_NS_GET_EXECUTABLE_PATH = YesPlease
-	BASIC_CFLAGS += -I/usr/local/include
-	BASIC_LDFLAGS += -L/usr/local/lib
+	BASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include
+	BASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib
+	ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)
+		MSGFMT = /usr/local/opt/gettext/bin/msgfmt
+	endif
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease

base-commit: e870325ee8575d5c3d7afe0ba2c9be072c692b65

Signature · 1 line

gitgitgadget

Eric Sunshine

2020-04-23 16:17 UTC #1 lore raw

Quoted 6 lines · Johannes Schindelin via GitGitGadget

Apparently a recent Homebrew update now installs gettext into a subdirectory under /usr/local/, requiring the CFLAGS/LDFLAGS to list explicit directories _even_ when asking to force-link the gettext package.
Likewise, the msgfmt tool is no longer in the PATH.

Interesting. I wonder if this is indeed a recent Homebrew change or if something changed elsewhere in the environment. I ask because...

config.mak.uname ifeq ($(uname_S),Darwin) · line 137

Quoted 11 lines

Signed-off-by Johannes Schindelin <[email protected]>

---

config.mak.uname +5 -2

@@ -133,8 +133,11 @@ ifeq ($(uname_S),Darwin)
-       BASIC_CFLAGS += -I/usr/local/include
-       BASIC_LDFLAGS += -L/usr/local/lib
+       BASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include
+       BASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib
+       ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)
+               MSGFMT = /usr/local/opt/gettext/bin/msgfmt
+       endif

... I've needed these assignments in my local config.mak file ever since I switched over to Homebrew from Macports (which installed everything under top-level /opt) years ago.

I'm slightly leery of seeing these applied globally on Mac OS in config.mak.uname since various package managers on Mac OS install packages in wildly different locations, and these settings might cause the wrong version of a package to be picked up if a user has a preferred version installed elsewhere.

Would it be an alternative for the CI/PR build process to just create a local customized config.mak rather than patching the globally-applied config.mak.uname like this?

Eric Sunshine

2020-04-23 20:49 UTC #2 lore raw

Quoted 5 lines · Eric Sunshine

I'm slightly leery of seeing these applied globally on Mac OS in config.mak.uname since various package managers on Mac OS install packages in wildly different locations, and these settings might cause the wrong version of a package to be picked up if a user has a preferred version installed elsewhere.

As a follow up, although slightly leery of applying this change globally to config.mak.uname, I don't necessarily oppose it either. Considering how widely adopted Homebrew is on Mac OS, baking in a bit of Homebrew-specific knowledge would make it easier for a Git developer to get up and running by eliminating some of the manual fiddling and configuration currently necessary.

Johannes Schindelin

2020-04-25 12:54 UTC #3 lore raw

Hi Eric,

Quoted 7 lines · Eric Sunshine

Quoted 5 lines · Eric Sunshine
I'm slightly leery of seeing these applied globally on Mac OS in config.mak.uname since various package managers on Mac OS install packages in wildly different locations, and these settings might cause the wrong version of a package to be picked up if a user has a preferred version installed elsewhere.

Quoted 6 lines

As a follow up, although slightly leery of applying this change globally to config.mak.uname, I don't necessarily oppose it either. Considering how widely adopted Homebrew is on Mac OS, baking in a bit of Homebrew-specific knowledge would make it easier for a Git developer to get up and running by eliminating some of the manual fiddling and configuration currently necessary.

I share your concern. But in contrast to Fink and DarwinPorts, we have no Homebrew-specific knob in the Makefile (does this mean that we expect users to use Homebrew by default?).

With the update that I just sent out, which guards the added flags behind a check for that directory's existence, would you agree that the current state is "good enough"?

Thanks,
Dscho

Carlo Arenas

2020-04-26 15:55 UTC #4 lore raw

Quoted 17 lines · Johannes Schindelin

Quoted 7 lines · Eric Sunshine
Quoted 5 lines · Eric Sunshine
I'm slightly leery of seeing these applied globally on Mac OS in config.mak.uname since various package managers on Mac OS install packages in wildly different locations, and these settings might cause the wrong version of a package to be picked up if a user has a preferred version installed elsewhere.
Quoted 6 lines
As a follow up, although slightly leery of applying this change globally to config.mak.uname, I don't necessarily oppose it either. Considering how widely adopted Homebrew is on Mac OS, baking in a bit of Homebrew-specific knowledge would make it easier for a Git developer to get up and running by eliminating some of the manual fiddling and configuration currently necessary.

Quoted 3 lines

I share your concern. But in contrast to Fink and DarwinPorts, we have no Homebrew-specific knob in the Makefile (does this mean that we expect users to use Homebrew by default?).

IMHO the reason why there is no NO_BREW flag is because brew decided to use the "default" directory instead of one of their own (which is why it has so many issues with unlinked files that might or not conflict with the system, like gettext), this is also why a NO_BREW flag (similar to the other two introduced since 8eb38cad44 (Disable linking with Fink or DarwinPorts., 2006-07-24)) wouldn't make sense.

my assumption is also that most people in macOS use brew nowadays instead of fink, darwinports or any of the other alternatives, but there is also people that use none and it is that complication why my proposed patch was in what would seem like the wrong place to begin with. for more context in that last point see: 59f8674006 (Move Fink and Ports check to after config file, 2006-12-12).

on that last point, I am afraid there is still at least a problem that needs addressing, but will comment in patch instead for easy of reference.

Carlo

Johannes Schindelin

2020-04-26 16:59 UTC #5 lore raw

Hi Carlo,

Quoted 19 lines · Carlo Arenas

Quoted 17 lines · Johannes Schindelin
Quoted 13 lines · Eric Sunshine
Quoted 5 lines · Eric Sunshine
I'm slightly leery of seeing these applied globally on Mac OS in config.mak.uname since various package managers on Mac OS install packages in wildly different locations, and these settings might cause the wrong version of a package to be picked up if a user has a preferred version installed elsewhere.
As a follow up, although slightly leery of applying this change globally to config.mak.uname, I don't necessarily oppose it either. Considering how widely adopted Homebrew is on Mac OS, baking in a bit of Homebrew-specific knowledge would make it easier for a Git developer to get up and running by eliminating some of the manual fiddling and configuration currently necessary.

Quoted 5 lines

Quoted 3 lines
I share your concern. But in contrast to Fink and DarwinPorts, we have no Homebrew-specific knob in the Makefile (does this mean that we expect users to use Homebrew by default?).
IMHO the reason why there is no NO_BREW flag is because brew decided to

That is not an opinion, but an assumption.

Quoted 7 lines

use the "default" directory instead of one of their own (which is why it has so many issues with unlinked files that might or not conflict with the system, like gettext), this is also why a NO_BREW flag (similar to the other two introduced since 8eb38cad44 (Disable linking with Fink or DarwinPorts., 2006-07-24)) wouldn't make sense.
my assumption is also that most people in macOS use brew nowadays

Yes, that is an assumption. One I share, but I would say that it is more a suspicion because I do not really want to act on it.

Quoted 9 lines

instead of fink, darwinports or any of the other alternatives, but there is also people that use none and it is that complication why my proposed patch was in what would seem like the wrong place to begin with. for more context in that last point see: 59f8674006 (Move Fink and Ports check to after config file, 2006-12-12).
on that last point, I am afraid there is still at least a problem that needs addressing, but will comment in patch instead for easy of reference.

I would have preferred a straight answer right here than a reference to something I have not received (yet?).

Ciao,
Dscho

Carlo Marcelo Arenas Belón

2020-04-25 06:15 UTC #6 lore raw

PATCH 1/1 macos: do not assume brew and gettext are always available/wanted

In reply to Johannes Schindelin via GitGitGadget

since 27a7067308 (macos: do let the build find the gettext headers/libraries/msgfmt, 2020-04-23) a build with make NO_GETTEXT=1 will throw warnings like :

LINK git
ld: warning: directory not found for option '-L/usr/local/opt/gettext/lib'

localize the change together with all the other package specific tweaks and make sure it only applies when both gettext was needed and brew was the provider.

Signed-off-by Carlo Marcelo Arenas Belón <[email protected]>

---
 Makefile         | 9 +++++++++
 config.mak.uname | 7 ++-----
 2 files changed, 11 insertions(+), 5 deletions(-)

Makefile +9 -0

index 9804a0758b..031a231ad6 100644
@@ -1303,6 +1303,15 @@ ifeq ($(uname_S),Darwin)
 			BASIC_LDFLAGS += -L/opt/local/lib
 		endif
 	endif
+	ifndef NO_GETTEXT
+		ifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)
+			BASIC_CFLAGS += -I/usr/local/opt/gettext/include
+			BASIC_LDFLAGS += -L/usr/local/opt/gettext/lib
+			ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)
+				MSGFMT = /usr/local/opt/gettext/bin/msgfmt
+			endif
+		endif
+	endif
 	ifndef NO_APPLE_COMMON_CRYPTO
 		NO_OPENSSL = YesPlease
 		APPLE_COMMON_CRYPTO = YesPlease

config.mak.uname +2 -5

index 540d124d2e..0ab8e00938 100644
@@ -133,11 +133,8 @@ ifeq ($(uname_S),Darwin)
 	HAVE_BSD_SYSCTL = YesPlease
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
 	HAVE_NS_GET_EXECUTABLE_PATH = YesPlease
-	BASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include
-	BASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib
-	ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)
-		MSGFMT = /usr/local/opt/gettext/bin/msgfmt
-	endif
+	BASIC_CFLAGS += -I/usr/local/include
+	BASIC_LDFLAGS += -L/usr/local/lib
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease

base-commit: 27a706730868835ec02a21a9ac4c4fcb3e05d330

Signature · 1 line

2.26.2.569.g1d74ac4d14

Johannes Schindelin

2020-04-25 12:33 UTC #7 lore raw

Re: [PATCH] macos: do not assume brew and gettext are always available/wanted

Hi Carlo,

Quoted 10 lines · Carlo Marcelo Arenas Belón

since 27a7067308 (macos: do let the build find the gettext headers/libraries/msgfmt, 2020-04-23) a build with make NO_GETTEXT=1 will throw warnings like :
LINK git
ld: warning: directory not found for option '-L/usr/local/opt/gettext/lib'
localize the change together with all the other package specific tweaks and make sure it only applies when both gettext was needed and brew was the provider.

Okay, that makes sense, but...

Quoted 5 lines

Signed-off-by Carlo Marcelo Arenas Belón <[email protected]>
---
 Makefile         | 9 +++++++++
 config.mak.uname | 7 ++-----

... moving this into the Makefile, i.e. make this even more independent from Homebrew than before, is probably a bad idea.

I agree with Eric Sunshine who complained that my patch is not dependent on the use of Homebrew. I haven't found a way to make it more dependent (there is no NO_HOMEBREW knob, even though there are NO_FINK and NO_DARWIN_PORTS), but that does not mean that we should make it even worse.

Will update my patch to guard the options a bit better.

Ciao,
Dscho

Quoted 47 lines

 2 files changed, 11 insertions(+), 5 deletions(-)

Makefile +9 -0

index 9804a0758b..031a231ad6 100644
@@ -1303,6 +1303,15 @@ ifeq ($(uname_S),Darwin)
 			BASIC_LDFLAGS += -L/opt/local/lib
 		endif
 	endif
+	ifndef NO_GETTEXT
+		ifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)
+			BASIC_CFLAGS += -I/usr/local/opt/gettext/include
+			BASIC_LDFLAGS += -L/usr/local/opt/gettext/lib
+			ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)
+				MSGFMT = /usr/local/opt/gettext/bin/msgfmt
+			endif
+		endif
+	endif
 	ifndef NO_APPLE_COMMON_CRYPTO
 		NO_OPENSSL = YesPlease
 		APPLE_COMMON_CRYPTO = YesPlease

config.mak.uname +2 -5

index 540d124d2e..0ab8e00938 100644
@@ -133,11 +133,8 @@ ifeq ($(uname_S),Darwin)
 	HAVE_BSD_SYSCTL = YesPlease
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
 	HAVE_NS_GET_EXECUTABLE_PATH = YesPlease
-	BASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include
-	BASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib
-	ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)
-		MSGFMT = /usr/local/opt/gettext/bin/msgfmt
-	endif
+	BASIC_CFLAGS += -I/usr/local/include
+	BASIC_LDFLAGS += -L/usr/local/lib
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease

base-commit: 27a706730868835ec02a21a9ac4c4fcb3e05d330 --
2.26.2.569.g1d74ac4d14

Johannes Schindelin via GitGitGadget

author

2020-04-25 12:54 UTC #8 lore raw

PATCH v2 1/1 macos: do let the build find the gettext headers/libraries/msgfmt

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

Likewise, the msgfmt tool is no longer in the PATH.

While it is unclear which change is responsible for this breakage (that most notably only occurs on CI build agents that updated very recently),
https://github.com/Homebrew/homebrew-core/pull/53489 should fix it.

Nevertheless, let's work around this issue, as there are still quite a few build agents out there that need some help in this regard: we explicitly do not call brew update in our CI/PR builds anymore.

Helped-by Carlo Marcelo Arenas Belón <[email protected]>

Signed-off-by Johannes Schindelin <[email protected]>

---
Prepare for Homebrew changing the gettext package

In an early Azure Pipelines preview of what is to come, I saw the osx-clang and osx-gcc jobs fail consistently.

This patch tries to prevent that from affecting our CI/PR builds.

Changes since v1:

* Described a bit better what the issue is, and that there is a "de-keg" change that should fix this (but as we no longer call brew update, some build agents, that won't matter for slightly out of date agents).
* Guarded the added flags behind a check whether the directory exists in the first place.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-616%2Fdscho%2Fbrew-gettext-update-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-616/dscho/brew-gettext-update-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/616

Range-diff vs v1:

1: c56a2321f62 ! 1: 1aa1b049e5c macos: do let the build find the gettext headers/libraries/msgfmt

     @@ Commit message
      
          Likewise, the `msgfmt` tool is no longer in the `PATH`.
      
     -    Let's work around this issue.
     +    While it is unclear which change is responsible for this breakage (that
     +    most notably only occurs on CI build agents that updated very recently),
     +    https://github.com/Homebrew/homebrew-core/pull/53489 should fix it.
      
     +    Nevertheless, let's work around this issue, as there are still quite a
     +    few build agents out there that need some help in this regard: we
     +    explicitly do not call `brew update` in our CI/PR builds anymore.
     +
     +    Helped-by: Carlo Marcelo Arenas Belón <[email protected]>
          Signed-off-by: Johannes Schindelin <[email protected]>
      
       ## config.mak.uname ##
     @@ config.mak.uname: ifeq ($(uname_S),Darwin)
       	HAVE_NS_GET_EXECUTABLE_PATH = YesPlease
      -	BASIC_CFLAGS += -I/usr/local/include
      -	BASIC_LDFLAGS += -L/usr/local/lib
     -+	BASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include
     -+	BASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib
     -+	ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)
     -+		MSGFMT = /usr/local/opt/gettext/bin/msgfmt
     ++
     ++	# Workaround for `gettext` being keg-only and not even being linked via
     ++	# `brew link --force gettext`, should be obsolete as of
     ++	# https://github.com/Homebrew/homebrew-core/pull/53489
     ++	ifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)
     ++		BASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include
     ++		BASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib
     ++		ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)
     ++			MSGFMT = /usr/local/opt/gettext/bin/msgfmt
     ++		endif
      +	endif
       endif
       ifeq ($(uname_S),SunOS)

 config.mak.uname | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

config.mak.uname +11 -2

index 0ab8e009383..1ea16e89288 100644
@@ -133,8 +133,17 @@ ifeq ($(uname_S),Darwin)
 	HAVE_BSD_SYSCTL = YesPlease
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
 	HAVE_NS_GET_EXECUTABLE_PATH = YesPlease
-	BASIC_CFLAGS += -I/usr/local/include
-	BASIC_LDFLAGS += -L/usr/local/lib
+
+	# Workaround for `gettext` being keg-only and not even being linked via
+	# `brew link --force gettext`, should be obsolete as of
+	# https://github.com/Homebrew/homebrew-core/pull/53489
+	ifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)
+		BASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include
+		BASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib
+		ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)
+			MSGFMT = /usr/local/opt/gettext/bin/msgfmt
+		endif
+	endif
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease

base-commit: e870325ee8575d5c3d7afe0ba2c9be072c692b65

Signature · 1 line

gitgitgadget

Torsten Bögershausen

2020-04-26 17:05 UTC #9 lore raw

Quoted 13 lines · Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>
Apparently a recent Homebrew update now installs gettext into a subdirectory under /usr/local/, requiring the CFLAGS/LDFLAGS to list explicit directories _even_ when asking to force-link the gettext package.
Likewise, the msgfmt tool is no longer in the PATH.
While it is unclear which change is responsible for this breakage (that most notably only occurs on CI build agents that updated very recently),
https://github.com/Homebrew/homebrew-core/pull/53489 should fix it.

Quoted 6 lines

Nevertheless, let's work around this issue, as there are still quite a few build agents out there that need some help in this regard: we explicitly do not call brew update in our CI/PR builds anymore.
Helped-by Carlo Marcelo Arenas Belón <[email protected]>
Signed-off-by Johannes Schindelin <[email protected]>

It seems as if things are happening fast in the brew-business.
After debugging (on a local box) and travis-ing (remote) I may have a suggestion for a better commit-message - I can probably send that out as a patch later today. What do you think ?

=================================================================
MacOs/brew: Let the build find gettext headers/libraries/msgfmt

Apparently a recent Homebrew update now installs gettext into the subdirectory /usr/local/opt/gettext/[lib/include].

Sometimes the ci job succeeds:
brew link --force gettext
Linking /usr/local/Cellar/gettext/0.20.1... 179 symlinks created

And sometimes installing the package "gettext" with force-link fails:
brew link --force gettext
Warning: Refusing to link macOS provided/shadowed software: gettext If you need to have gettext first in your PATH run:
echo 'export PATH="/usr/local/opt/gettext/bin:$PATH"' >> ~/.bash_profile

(And the is not the final word either, since MacOs itself says:
The default interactive shell is now zsh.)

Anyway, The latter requires CFLAGS to include /usr/local/opt/gettext/include and LDFLAGS to include /usr/local/opt/gettext/lib.

Likewise, the msgfmt tool is no longer in the PATH.

Helped-by Carlo Marcelo Arenas Belón <[email protected]>

Signed-off-by Johannes Schindelin <[email protected]>

Carlo Marcelo Arenas Belón

2020-04-26 17:34 UTC #10 lore raw

In reply to Johannes Schindelin via GitGitGadget

config.mak.uname ifeq ($(uname_S),Darwin) · line 137

Quoted 10 lines · Johannes Schindelin via GitGitGadget

config.mak.uname +0 -2

index 0ab8e009383..1ea16e89288 100644
@@ -133,8 +133,17 @@ ifeq ($(uname_S),Darwin)
 	HAVE_BSD_SYSCTL = YesPlease
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
 	HAVE_NS_GET_EXECUTABLE_PATH = YesPlease
-	BASIC_CFLAGS += -I/usr/local/include
-	BASIC_LDFLAGS += -L/usr/local/lib

keeping these flags independenly allows people that doesn't have brew or that have brew but hadn't installed the gettext package, to still be able to use other libraries/packages that could be installed in those directories either through brew (ex: libgcrypt, openssl or pcre*) or manually while also using a compiler that doesn't use by default /usr/local/{include,lib}.

even if all this might sound like a stretch, notice that it happened before as shown by 2a1377a2a (macOS: make sure that gettext is found, 2019-04-14)

Quoted 4 lines

config.mak.uname +3 -0

+	# Workaround for `gettext` being keg-only and not even being linked via
+	# `brew link --force gettext`, should be obsolete as of
+	# https://github.com/Homebrew/homebrew-core/pull/53489

config.mak.uname ifeq ($(uname_S),Darwin) · line 149

Quoted 10 lines

config.mak.uname +7 -0

+	ifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)
+		BASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include
+		BASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib
+		ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)
+			MSGFMT = /usr/local/opt/gettext/bin/msgfmt
+		endif
+	endif
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease

since this doesn't depend on NO_GETTEXT and the gettext support doesn't get automatically configured and used if found (unlike most other) then having it here could work and is cleaner, but will still mean that MSGFMT will be called to compile the translation files even when git is built with NO_GETTEXT

just because of that oddity I think having it with the other package related entries in Makefile might still make sense (specially since we can't get rid of it unless we also deprecate the other package managers), but if cleaning it is what you had in mind would also appreciate in that line your review on [email protected][1] and [email protected][2] that do similar work and that in the later case improve performance and correctness of git grep.

Carlo

[1] https://lore.kernel.org/git/[email protected]/
[2] https://lore.kernel.org/git/[email protected]/

[email protected]

2020-04-26 20:09 UTC #11 lore raw

PATCH v3 1/1 MacOs/brew: Let the build find gettext headers/libraries/msgfmt

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

Apparently a recent Homebrew update now installs gettext into the subdirectory /usr/local/opt/gettext/[lib/include].

Sometimes the ci job succeeds:
brew link --force gettext
Linking /usr/local/Cellar/gettext/0.20.1... 179 symlinks created

(And the is not the final word either, since MacOs itself says:
The default interactive shell is now zsh.)

Anyway, The latter requires CFLAGS to include /usr/local/opt/gettext/include and LDFLAGS to include /usr/local/opt/gettext/lib.

Likewise, the msgfmt tool is no longer in the PATH.

Helped-by Carlo Marcelo Arenas Belón <[email protected]>

Signed-off-by Johannes Schindelin <[email protected]>

Signed-off-by Torsten Bögershausen <[email protected]>

---
Changes since V2:
Updated the commit message

config.mak.uname | 13 +++++++++++--

 1 file changed, 11 insertions(+), 2 deletions(-)

config.mak.uname +11 -2

index 0ab8e00938..1ea16e8928 100644
@@ -133,8 +133,17 @@ ifeq ($(uname_S),Darwin)
 	HAVE_BSD_SYSCTL = YesPlease
 	FREAD_READS_DIRECTORIES = UnfortunatelyYes
 	HAVE_NS_GET_EXECUTABLE_PATH = YesPlease
-	BASIC_CFLAGS += -I/usr/local/include
-	BASIC_LDFLAGS += -L/usr/local/lib
+
+	# Workaround for `gettext` being keg-only and not even being linked via
+	# `brew link --force gettext`, should be obsolete as of
+	# https://github.com/Homebrew/homebrew-core/pull/53489
+	ifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)
+		BASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include
+		BASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib
+		ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)
+			MSGFMT = /usr/local/opt/gettext/bin/msgfmt
+		endif
+	endif
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease

--
2.26.1.107.gefe3874640