Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare for Homebrew changing the gettext package #616

Closed
wants to merge 1 commit into from

Conversation

dscho
Copy link
Member

@dscho dscho commented Apr 22, 2020

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.

Cc: SZEDER Gábor szeder.dev@gmail.com, Eric Sunshine sunshine@sunshineco.com, Carlo Marcelo Arenas Belón carenas@gmail.com

@dscho
Copy link
Member Author

dscho commented Apr 23, 2020

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 23, 2020

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 23, 2020

On the Git mailing list, Eric Sunshine wrote (reply to this):

On Thu, Apr 23, 2020 at 3:54 AM Johannes Schindelin via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> 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...

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> diff --git a/config.mak.uname b/config.mak.uname
> @@ -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?

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 23, 2020

On the Git mailing list, Eric Sunshine wrote (reply to this):

On Thu, Apr 23, 2020 at 12:17 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> 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.

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 24, 2020

This branch is now known as ds/build-homebrew-gettext-fix.

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 24, 2020

This patch series was integrated into pu via git@5bee21c.

@gitgitgadget gitgitgadget bot added the pu label Apr 24, 2020
@gitgitgadget
Copy link

gitgitgadget bot commented Apr 25, 2020

On the Git mailing list, Carlo Marcelo Arenas Belón wrote (reply to this):

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 <carenas@gmail.com>
---
 Makefile         | 9 +++++++++
 config.mak.uname | 7 ++-----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 9804a0758b..031a231ad6 100644
--- a/Makefile
+++ b/Makefile
@@ -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
diff --git a/config.mak.uname b/config.mak.uname
index 540d124d2e..0ab8e00938 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -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

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 25, 2020

On the Git mailing list, Johannes Schindelin wrote (reply to this):

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-1561261481-1587818028=:18039
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Hi Carlo,

On Fri, 24 Apr 2020, Carlo Marcelo Arenas Bel=C3=B3n wrote:

> since 27a7067308 (macos: do let the build find the gettext
> headers/libraries/msgfmt, 2020-04-23) a build with `make NO_GETTEXT=3D1`
> will throw warnings like :
>
>     LINK git
> ld: warning: directory not found for option '-L/usr/local/opt/gettext/li=
b'
>
> 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...

>
> Signed-off-by: Carlo Marcelo Arenas Bel=C3=B3n <carenas@gmail.com>
> ---
>  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

>  2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 9804a0758b..031a231ad6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1303,6 +1303,15 @@ ifeq ($(uname_S),Darwin)
>  			BASIC_LDFLAGS +=3D -L/opt/local/lib
>  		endif
>  	endif
> +	ifndef NO_GETTEXT
> +		ifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)
> +			BASIC_CFLAGS +=3D -I/usr/local/opt/gettext/include
> +			BASIC_LDFLAGS +=3D -L/usr/local/opt/gettext/lib
> +			ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y=
)
> +				MSGFMT =3D /usr/local/opt/gettext/bin/msgfmt
> +			endif
> +		endif
> +	endif
>  	ifndef NO_APPLE_COMMON_CRYPTO
>  		NO_OPENSSL =3D YesPlease
>  		APPLE_COMMON_CRYPTO =3D YesPlease
> diff --git a/config.mak.uname b/config.mak.uname
> index 540d124d2e..0ab8e00938 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -133,11 +133,8 @@ ifeq ($(uname_S),Darwin)
>  	HAVE_BSD_SYSCTL =3D YesPlease
>  	FREAD_READS_DIRECTORIES =3D UnfortunatelyYes
>  	HAVE_NS_GET_EXECUTABLE_PATH =3D YesPlease
> -	BASIC_CFLAGS +=3D -I/usr/local/include -I/usr/local/opt/gettext/includ=
e
> -	BASIC_LDFLAGS +=3D -L/usr/local/lib -L/usr/local/opt/gettext/lib
> -	ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)
> -		MSGFMT =3D /usr/local/opt/gettext/bin/msgfmt
> -	endif
> +	BASIC_CFLAGS +=3D -I/usr/local/include
> +	BASIC_LDFLAGS +=3D -L/usr/local/lib
>  endif
>  ifeq ($(uname_S),SunOS)
>  	NEEDS_SOCKET =3D YesPlease
>
> base-commit: 27a706730868835ec02a21a9ac4c4fcb3e05d330
> --
> 2.26.2.569.g1d74ac4d14
>
>
>

--8323328-1561261481-1587818028=:18039--

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),
Homebrew/homebrew-core#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 <carenas@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Member Author

dscho commented Apr 25, 2020

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 25, 2020

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 25, 2020

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Eric,

On Thu, 23 Apr 2020, Eric Sunshine wrote:

> On Thu, Apr 23, 2020 at 12:17 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > 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.

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

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 26, 2020

On the Git mailing list, Carlo Arenas wrote (reply to this):

On Sat, Apr 25, 2020 at 5:59 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> On Thu, 23 Apr 2020, Eric Sunshine wrote:
>
> > On Thu, Apr 23, 2020 at 12:17 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > > 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.
>
> 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

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 26, 2020

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Carlo,

On Sun, 26 Apr 2020, Carlo Arenas wrote:

> On Sat, Apr 25, 2020 at 5:59 AM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Thu, 23 Apr 2020, Eric Sunshine wrote:
> >
> > > On Thu, Apr 23, 2020 at 12:17 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > > > 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.
> >
> > 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.

> 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.

> 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

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 26, 2020

On the Git mailing list, Torsten Bögershausen wrote (reply to this):

On Sat, Apr 25, 2020 at 12:54:26PM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> 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.
>
> 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 <carenas@gmail.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

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`.

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 has fixed 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 <carenas@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 26, 2020

On the Git mailing list, Carlo Marcelo Arenas Belón wrote (reply to this):

On Sat, Apr 25, 2020 at 12:54:26PM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/config.mak.uname b/config.mak.uname
> index 0ab8e009383..1ea16e89288 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -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)

> +	# 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
> 

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 20200425102651.51961-1-carenas@gmail.com[1] and
20200425091549.42293-1-carenas@gmail.com[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/20200425102651.51961-1-carenas@gmail.com/
[2] https://lore.kernel.org/git/20200425091549.42293-1-carenas@gmail.com/

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 26, 2020

On the Git mailing list, tboegi@web.de wrote (reply to this):

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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`.

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 has fixed 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 <carenas@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 Changes since V2:
   Updated the commit message

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

diff --git a/config.mak.uname b/config.mak.uname
index 0ab8e00938..1ea16e8928 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -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

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 28, 2020

This patch series was integrated into pu via git@af78f83.

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 28, 2020

This patch series was integrated into pu via git@e01b23a.

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 29, 2020

This patch series was integrated into pu via git@7e63811.

@gitgitgadget
Copy link

gitgitgadget bot commented Apr 29, 2020

This patch series was integrated into next via git@70c6eca.

@gitgitgadget gitgitgadget bot added the next label Apr 29, 2020
@gitgitgadget
Copy link

gitgitgadget bot commented Apr 30, 2020

This patch series was integrated into pu via git@4b3afcb.

@gitgitgadget
Copy link

gitgitgadget bot commented May 1, 2020

This patch series was integrated into pu via git@aabf3ea.

@gitgitgadget
Copy link

gitgitgadget bot commented May 1, 2020

This patch series was integrated into next via git@aabf3ea.

@gitgitgadget
Copy link

gitgitgadget bot commented May 1, 2020

This patch series was integrated into master via git@aabf3ea.

@gitgitgadget gitgitgadget bot added the master label May 1, 2020
@gitgitgadget gitgitgadget bot closed this May 1, 2020
@gitgitgadget
Copy link

gitgitgadget bot commented May 1, 2020

Closed via aabf3ea.

@dscho dscho deleted the brew-gettext-update branch May 2, 2020 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant