{"thread":{"id":"53294","subject":"[PATCH] macos: do let the build find the gettext headers/libraries/msgfmt","startedAt":"2020-04-23T07:52:52Z","lastAt":"2020-04-26T20:09:41Z","messageCount":12,"participants":["Johannes Schindelin via GitGitGadget","Eric Sunshine","Carlo Marcelo Arenas Belón","Johannes Schindelin","Carlo Arenas","Torsten Bögershausen","tboegi@web.de"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"395996","messageId":"pull.616.git.1587628367528.gitgitgadget@gmail.com","threadId":"53294","inReplyTo":null,"subject":"[PATCH] macos: do let the build find the gettext headers/libraries/msgfmt","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-23T07:52:47Z","receivedAt":"2020-04-23T07:52:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nApparently a recent Homebrew update now installs `gettext` into a\nsubdirectory under /usr/local/, requiring the CFLAGS/LDFLAGS to list\nexplicit directories _even_ when asking to force-link the `gettext`\npackage.\n\nLikewise, the `msgfmt` tool is no longer in the `PATH`.\n\nLet's work around this issue.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    Prepare for Homebrew changing the gettext package\n    \n    In an early Azure Pipelines preview of what is to come, I saw the \n    osx-clang and osx-gcc jobs fail consistently.\n    \n    This patch tries to prevent that from affecting our CI/PR builds.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-616%2Fdscho%2Fbrew-gettext-update-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-616/dscho/brew-gettext-update-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/616\n\n config.mak.uname | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 0ab8e009383..540d124d2ef 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -133,8 +133,11 @@ ifeq ($(uname_S),Darwin)\n \tHAVE_BSD_SYSCTL = YesPlease\n \tFREAD_READS_DIRECTORIES = UnfortunatelyYes\n \tHAVE_NS_GET_EXECUTABLE_PATH = YesPlease\n-\tBASIC_CFLAGS += -I/usr/local/include\n-\tBASIC_LDFLAGS += -L/usr/local/lib\n+\tBASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include\n+\tBASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib\n+\tifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)\n+\t\tMSGFMT = /usr/local/opt/gettext/bin/msgfmt\n+\tendif\n endif\n ifeq ($(uname_S),SunOS)\n \tNEEDS_SOCKET = YesPlease\n\nbase-commit: e870325ee8575d5c3d7afe0ba2c9be072c692b65\n-- \ngitgitgadget\n"},{"id":"396015","messageId":"CAPig+cSDQe7YDke=fyfdeSZOimcR5jj3FFk=Q4iOk6uiHsT-Zw@mail.gmail.com","threadId":"53294","inReplyTo":"pull.616.git.1587628367528.gitgitgadget@gmail.com","subject":"Re: [PATCH] macos: do let the build find the gettext headers/libraries/msgfmt","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-04-23T16:17:14Z","receivedAt":"2020-04-23T16:17:29Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 23, 2020 at 3:54 AM Johannes Schindelin via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> Apparently a recent Homebrew update now installs `gettext` into a\n> subdirectory under /usr/local/, requiring the CFLAGS/LDFLAGS to list\n> explicit directories _even_ when asking to force-link the `gettext`\n> package.\n>\n> Likewise, the `msgfmt` tool is no longer in the `PATH`.\n\nInteresting. I wonder if this is indeed a recent Homebrew change or if\nsomething changed elsewhere in the environment. I ask because...\n\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n> diff --git a/config.mak.uname b/config.mak.uname\n> @@ -133,8 +133,11 @@ ifeq ($(uname_S),Darwin)\n> -       BASIC_CFLAGS += -I/usr/local/include\n> -       BASIC_LDFLAGS += -L/usr/local/lib\n> +       BASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include\n> +       BASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib\n> +       ifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)\n> +               MSGFMT = /usr/local/opt/gettext/bin/msgfmt\n> +       endif\n\n... I've needed these assignments in my local config.mak file ever\nsince I switched over to Homebrew from Macports (which installed\neverything under top-level /opt) years ago.\n\nI'm slightly leery of seeing these applied globally on Mac OS in\nconfig.mak.uname since various package managers on Mac OS install\npackages in wildly different locations, and these settings might cause\nthe wrong version of a package to be picked up if a user has a\npreferred version installed elsewhere.\n\nWould it be an alternative for the CI/PR build process to just create\na local customized config.mak rather than patching the\nglobally-applied config.mak.uname like this?\n"},{"id":"396033","messageId":"CAPig+cSkr+2dExzETScru0N5_=JhjVR=QZKuY5CbhstwrUUk5w@mail.gmail.com","threadId":"53294","inReplyTo":"CAPig+cSDQe7YDke=fyfdeSZOimcR5jj3FFk=Q4iOk6uiHsT-Zw@mail.gmail.com","subject":"Re: [PATCH] macos: do let the build find the gettext headers/libraries/msgfmt","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-04-23T20:49:30Z","receivedAt":"2020-04-23T20:49:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 23, 2020 at 12:17 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> I'm slightly leery of seeing these applied globally on Mac OS in\n> config.mak.uname since various package managers on Mac OS install\n> packages in wildly different locations, and these settings might cause\n> the wrong version of a package to be picked up if a user has a\n> preferred version installed elsewhere.\n\nAs a follow up, although slightly leery of applying this change\nglobally to config.mak.uname, I don't necessarily oppose it either.\nConsidering how widely adopted Homebrew is on Mac OS, baking in a bit\nof Homebrew-specific knowledge would make it easier for a Git\ndeveloper to get up and running by eliminating some of the manual\nfiddling and configuration currently necessary.\n"},{"id":"396235","messageId":"20200425061531.12845-1-carenas@gmail.com","threadId":"53294","inReplyTo":"pull.616.git.1587628367528.gitgitgadget@gmail.com","subject":"[PATCH] macos: do not assume brew and gettext are always available/wanted","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2020-04-25T06:15:31Z","receivedAt":"2020-04-25T06:15:38Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"since 27a7067308 (macos: do let the build find the gettext\nheaders/libraries/msgfmt, 2020-04-23) a build with `make NO_GETTEXT=1`\nwill throw warnings like :\n\n    LINK git\nld: warning: directory not found for option '-L/usr/local/opt/gettext/lib'\n\nlocalize the change together with all the other package specific tweaks\nand make sure it only applies when both gettext was needed and brew was\nthe provider.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n Makefile         | 9 +++++++++\n config.mak.uname | 7 ++-----\n 2 files changed, 11 insertions(+), 5 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 9804a0758b..031a231ad6 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1303,6 +1303,15 @@ ifeq ($(uname_S),Darwin)\n \t\t\tBASIC_LDFLAGS += -L/opt/local/lib\n \t\tendif\n \tendif\n+\tifndef NO_GETTEXT\n+\t\tifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)\n+\t\t\tBASIC_CFLAGS += -I/usr/local/opt/gettext/include\n+\t\t\tBASIC_LDFLAGS += -L/usr/local/opt/gettext/lib\n+\t\t\tifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)\n+\t\t\t\tMSGFMT = /usr/local/opt/gettext/bin/msgfmt\n+\t\t\tendif\n+\t\tendif\n+\tendif\n \tifndef NO_APPLE_COMMON_CRYPTO\n \t\tNO_OPENSSL = YesPlease\n \t\tAPPLE_COMMON_CRYPTO = YesPlease\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 540d124d2e..0ab8e00938 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -133,11 +133,8 @@ ifeq ($(uname_S),Darwin)\n \tHAVE_BSD_SYSCTL = YesPlease\n \tFREAD_READS_DIRECTORIES = UnfortunatelyYes\n \tHAVE_NS_GET_EXECUTABLE_PATH = YesPlease\n-\tBASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include\n-\tBASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib\n-\tifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)\n-\t\tMSGFMT = /usr/local/opt/gettext/bin/msgfmt\n-\tendif\n+\tBASIC_CFLAGS += -I/usr/local/include\n+\tBASIC_LDFLAGS += -L/usr/local/lib\n endif\n ifeq ($(uname_S),SunOS)\n \tNEEDS_SOCKET = YesPlease\n\nbase-commit: 27a706730868835ec02a21a9ac4c4fcb3e05d330\n-- \n2.26.2.569.g1d74ac4d14\n\n"},{"id":"396246","messageId":"nycvar.QRO.7.76.6.2004251430380.18039@tvgsbejvaqbjf.bet","threadId":"53294","inReplyTo":"20200425061531.12845-1-carenas@gmail.com","subject":"Re: [PATCH] macos: do not assume brew and gettext are always available/wanted","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-04-25T12:33:47Z","receivedAt":"2020-04-25T12:33:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Carlo,\n\nOn Fri, 24 Apr 2020, Carlo Marcelo Arenas Belón wrote:\n\n> since 27a7067308 (macos: do let the build find the gettext\n> headers/libraries/msgfmt, 2020-04-23) a build with `make NO_GETTEXT=1`\n> will throw warnings like :\n>\n>     LINK git\n> ld: warning: directory not found for option '-L/usr/local/opt/gettext/lib'\n>\n> localize the change together with all the other package specific tweaks\n> and make sure it only applies when both gettext was needed and brew was\n> the provider.\n\nOkay, that makes sense, but...\n\n>\n> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n> ---\n>  Makefile         | 9 +++++++++\n>  config.mak.uname | 7 ++-----\n\n... moving this into the `Makefile`, i.e. make this even more independent\nfrom Homebrew than before, is probably a bad idea.\n\nI agree with Eric Sunshine who complained that my patch is not dependent\non the use of Homebrew. I haven't found a way to make it more dependent\n(there is no `NO_HOMEBREW` knob, even though there are `NO_FINK` and\n`NO_DARWIN_PORTS`), but that does not mean that we should make it even\nworse.\n\nWill update my patch to guard the options a bit better.\n\nCiao,\nDscho\n\n>  2 files changed, 11 insertions(+), 5 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index 9804a0758b..031a231ad6 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1303,6 +1303,15 @@ ifeq ($(uname_S),Darwin)\n>  \t\t\tBASIC_LDFLAGS += -L/opt/local/lib\n>  \t\tendif\n>  \tendif\n> +\tifndef NO_GETTEXT\n> +\t\tifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)\n> +\t\t\tBASIC_CFLAGS += -I/usr/local/opt/gettext/include\n> +\t\t\tBASIC_LDFLAGS += -L/usr/local/opt/gettext/lib\n> +\t\t\tifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)\n> +\t\t\t\tMSGFMT = /usr/local/opt/gettext/bin/msgfmt\n> +\t\t\tendif\n> +\t\tendif\n> +\tendif\n>  \tifndef NO_APPLE_COMMON_CRYPTO\n>  \t\tNO_OPENSSL = YesPlease\n>  \t\tAPPLE_COMMON_CRYPTO = YesPlease\n> diff --git a/config.mak.uname b/config.mak.uname\n> index 540d124d2e..0ab8e00938 100644\n> --- a/config.mak.uname\n> +++ b/config.mak.uname\n> @@ -133,11 +133,8 @@ ifeq ($(uname_S),Darwin)\n>  \tHAVE_BSD_SYSCTL = YesPlease\n>  \tFREAD_READS_DIRECTORIES = UnfortunatelyYes\n>  \tHAVE_NS_GET_EXECUTABLE_PATH = YesPlease\n> -\tBASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include\n> -\tBASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib\n> -\tifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)\n> -\t\tMSGFMT = /usr/local/opt/gettext/bin/msgfmt\n> -\tendif\n> +\tBASIC_CFLAGS += -I/usr/local/include\n> +\tBASIC_LDFLAGS += -L/usr/local/lib\n>  endif\n>  ifeq ($(uname_S),SunOS)\n>  \tNEEDS_SOCKET = YesPlease\n>\n> base-commit: 27a706730868835ec02a21a9ac4c4fcb3e05d330\n> --\n> 2.26.2.569.g1d74ac4d14\n>\n>\n>\n"},{"id":"396247","messageId":"nycvar.QRO.7.76.6.2004251452200.18039@tvgsbejvaqbjf.bet","threadId":"53294","inReplyTo":"CAPig+cSkr+2dExzETScru0N5_=JhjVR=QZKuY5CbhstwrUUk5w@mail.gmail.com","subject":"Re: [PATCH] macos: do let the build find the gettext headers/libraries/msgfmt","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-04-25T12:54:13Z","receivedAt":"2020-04-25T12:54:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric,\n\nOn Thu, 23 Apr 2020, Eric Sunshine wrote:\n\n> On Thu, Apr 23, 2020 at 12:17 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> > I'm slightly leery of seeing these applied globally on Mac OS in\n> > config.mak.uname since various package managers on Mac OS install\n> > packages in wildly different locations, and these settings might cause\n> > the wrong version of a package to be picked up if a user has a\n> > preferred version installed elsewhere.\n>\n> As a follow up, although slightly leery of applying this change\n> globally to config.mak.uname, I don't necessarily oppose it either.\n> Considering how widely adopted Homebrew is on Mac OS, baking in a bit\n> of Homebrew-specific knowledge would make it easier for a Git\n> developer to get up and running by eliminating some of the manual\n> fiddling and configuration currently necessary.\n\nI share your concern. But in contrast to Fink and DarwinPorts, we have no\nHomebrew-specific knob in the Makefile (does this mean that we expect\nusers to use Homebrew by default?).\n\nWith the update that I just sent out, which guards the added flags behind\na check for that directory's existence, would you agree that the current\nstate is \"good enough\"?\n\nThanks,\nDscho\n"},{"id":"396248","messageId":"pull.616.v2.git.1587819266388.gitgitgadget@gmail.com","threadId":"53294","inReplyTo":"pull.616.git.1587628367528.gitgitgadget@gmail.com","subject":"[PATCH v2] macos: do let the build find the gettext headers/libraries/msgfmt","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-25T12:54:26Z","receivedAt":"2020-04-25T12:54:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nApparently a recent Homebrew update now installs `gettext` into a\nsubdirectory under /usr/local/, requiring the CFLAGS/LDFLAGS to list\nexplicit directories _even_ when asking to force-link the `gettext`\npackage.\n\nLikewise, the `msgfmt` tool is no longer in the `PATH`.\n\nWhile it is unclear which change is responsible for this breakage (that\nmost notably only occurs on CI build agents that updated very recently),\nhttps://github.com/Homebrew/homebrew-core/pull/53489 should fix it.\n\nNevertheless, let's work around this issue, as there are still quite a\nfew build agents out there that need some help in this regard: we\nexplicitly do not call `brew update` in our CI/PR builds anymore.\n\nHelped-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    Prepare for Homebrew changing the gettext package\n    \n    In an early Azure Pipelines preview of what is to come, I saw the \n    osx-clang and osx-gcc jobs fail consistently.\n    \n    This patch tries to prevent that from affecting our CI/PR builds.\n    \n    Changes since v1:\n    \n     * Described a bit better what the issue is, and that there is a\n       \"de-keg\" change that should fix this (but as we no longer call brew\n       update, some build agents, that won't matter for slightly out of date\n       agents).\n     * Guarded the added flags behind a check whether the directory exists\n       in the first place.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-616%2Fdscho%2Fbrew-gettext-update-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-616/dscho/brew-gettext-update-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/616\n\nRange-diff vs v1:\n\n 1:  c56a2321f62 ! 1:  1aa1b049e5c macos: do let the build find the gettext headers/libraries/msgfmt\n     @@ Commit message\n      \n          Likewise, the `msgfmt` tool is no longer in the `PATH`.\n      \n     -    Let's work around this issue.\n     +    While it is unclear which change is responsible for this breakage (that\n     +    most notably only occurs on CI build agents that updated very recently),\n     +    https://github.com/Homebrew/homebrew-core/pull/53489 should fix it.\n      \n     +    Nevertheless, let's work around this issue, as there are still quite a\n     +    few build agents out there that need some help in this regard: we\n     +    explicitly do not call `brew update` in our CI/PR builds anymore.\n     +\n     +    Helped-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n      \n       ## config.mak.uname ##\n     @@ config.mak.uname: ifeq ($(uname_S),Darwin)\n       \tHAVE_NS_GET_EXECUTABLE_PATH = YesPlease\n      -\tBASIC_CFLAGS += -I/usr/local/include\n      -\tBASIC_LDFLAGS += -L/usr/local/lib\n     -+\tBASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include\n     -+\tBASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib\n     -+\tifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)\n     -+\t\tMSGFMT = /usr/local/opt/gettext/bin/msgfmt\n     ++\n     ++\t# Workaround for `gettext` being keg-only and not even being linked via\n     ++\t# `brew link --force gettext`, should be obsolete as of\n     ++\t# https://github.com/Homebrew/homebrew-core/pull/53489\n     ++\tifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)\n     ++\t\tBASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include\n     ++\t\tBASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib\n     ++\t\tifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)\n     ++\t\t\tMSGFMT = /usr/local/opt/gettext/bin/msgfmt\n     ++\t\tendif\n      +\tendif\n       endif\n       ifeq ($(uname_S),SunOS)\n\n\n config.mak.uname | 13 +++++++++++--\n 1 file changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 0ab8e009383..1ea16e89288 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -133,8 +133,17 @@ ifeq ($(uname_S),Darwin)\n \tHAVE_BSD_SYSCTL = YesPlease\n \tFREAD_READS_DIRECTORIES = UnfortunatelyYes\n \tHAVE_NS_GET_EXECUTABLE_PATH = YesPlease\n-\tBASIC_CFLAGS += -I/usr/local/include\n-\tBASIC_LDFLAGS += -L/usr/local/lib\n+\n+\t# Workaround for `gettext` being keg-only and not even being linked via\n+\t# `brew link --force gettext`, should be obsolete as of\n+\t# https://github.com/Homebrew/homebrew-core/pull/53489\n+\tifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)\n+\t\tBASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include\n+\t\tBASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib\n+\t\tifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)\n+\t\t\tMSGFMT = /usr/local/opt/gettext/bin/msgfmt\n+\t\tendif\n+\tendif\n endif\n ifeq ($(uname_S),SunOS)\n \tNEEDS_SOCKET = YesPlease\n\nbase-commit: e870325ee8575d5c3d7afe0ba2c9be072c692b65\n-- \ngitgitgadget\n"},{"id":"396288","messageId":"CAPUEspiici3-tTTe=AWcNWgS4CN8OiqtaE46dQGcNW5weFC8Aw@mail.gmail.com","threadId":"53294","inReplyTo":"nycvar.QRO.7.76.6.2004251452200.18039@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] macos: do let the build find the gettext headers/libraries/msgfmt","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2020-04-26T15:54:50Z","receivedAt":"2020-04-26T15:55:05Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Sat, Apr 25, 2020 at 5:59 AM Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n>\n> On Thu, 23 Apr 2020, Eric Sunshine wrote:\n>\n> > On Thu, Apr 23, 2020 at 12:17 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> > > I'm slightly leery of seeing these applied globally on Mac OS in\n> > > config.mak.uname since various package managers on Mac OS install\n> > > packages in wildly different locations, and these settings might cause\n> > > the wrong version of a package to be picked up if a user has a\n> > > preferred version installed elsewhere.\n> >\n> > As a follow up, although slightly leery of applying this change\n> > globally to config.mak.uname, I don't necessarily oppose it either.\n> > Considering how widely adopted Homebrew is on Mac OS, baking in a bit\n> > of Homebrew-specific knowledge would make it easier for a Git\n> > developer to get up and running by eliminating some of the manual\n> > fiddling and configuration currently necessary.\n>\n> I share your concern. But in contrast to Fink and DarwinPorts, we have no\n> Homebrew-specific knob in the Makefile (does this mean that we expect\n> users to use Homebrew by default?).\n\nIMHO the reason why there is no NO_BREW flag is because brew decided\nto use the \"default\" directory instead of one of their own (which is\nwhy it has so many issues with unlinked files that might or not\nconflict with the system, like gettext), this is also why a NO_BREW\nflag (similar to the other two introduced since 8eb38cad44 (Disable\nlinking with Fink or DarwinPorts., 2006-07-24)) wouldn't make sense.\n\nmy assumption is also that most people in macOS use brew nowadays\ninstead of fink, darwinports or any of the other alternatives, but\nthere is also people that use none and it is that complication why my\nproposed patch was in what would seem like the wrong place to begin\nwith.  for more context in that last point see: 59f8674006 (Move Fink\nand Ports check to after config file, 2006-12-12).\n\non that last point, I am afraid there is still at least a problem that\nneeds addressing, but will comment in patch instead for easy of\nreference.\n\nCarlo\n"},{"id":"396291","messageId":"nycvar.QRO.7.76.6.2004261856500.18039@tvgsbejvaqbjf.bet","threadId":"53294","inReplyTo":"CAPUEspiici3-tTTe=AWcNWgS4CN8OiqtaE46dQGcNW5weFC8Aw@mail.gmail.com","subject":"Re: [PATCH] macos: do let the build find the gettext headers/libraries/msgfmt","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-04-26T16:59:04Z","receivedAt":"2020-04-26T16:59:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Carlo,\n\nOn Sun, 26 Apr 2020, Carlo Arenas wrote:\n\n> On Sat, Apr 25, 2020 at 5:59 AM Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> >\n> > On Thu, 23 Apr 2020, Eric Sunshine wrote:\n> >\n> > > On Thu, Apr 23, 2020 at 12:17 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> > > > I'm slightly leery of seeing these applied globally on Mac OS in\n> > > > config.mak.uname since various package managers on Mac OS install\n> > > > packages in wildly different locations, and these settings might\n> > > > cause the wrong version of a package to be picked up if a user has\n> > > > a preferred version installed elsewhere.\n> > >\n> > > As a follow up, although slightly leery of applying this change\n> > > globally to config.mak.uname, I don't necessarily oppose it either.\n> > > Considering how widely adopted Homebrew is on Mac OS, baking in a\n> > > bit of Homebrew-specific knowledge would make it easier for a Git\n> > > developer to get up and running by eliminating some of the manual\n> > > fiddling and configuration currently necessary.\n> >\n> > I share your concern. But in contrast to Fink and DarwinPorts, we have\n> > no Homebrew-specific knob in the Makefile (does this mean that we\n> > expect users to use Homebrew by default?).\n>\n> IMHO the reason why there is no NO_BREW flag is because brew decided to\n\nThat is not an opinion, but an assumption.\n\n> use the \"default\" directory instead of one of their own (which is why it\n> has so many issues with unlinked files that might or not conflict with\n> the system, like gettext), this is also why a NO_BREW flag (similar to\n> the other two introduced since 8eb38cad44 (Disable linking with Fink or\n> DarwinPorts., 2006-07-24)) wouldn't make sense.\n>\n> my assumption is also that most people in macOS use brew nowadays\n\nYes, that is an assumption. One I share, but I would say that it is more a\nsuspicion because I do not really want to act on it.\n\n> instead of fink, darwinports or any of the other alternatives, but there\n> is also people that use none and it is that complication why my proposed\n> patch was in what would seem like the wrong place to begin with.  for\n> more context in that last point see: 59f8674006 (Move Fink and Ports\n> check to after config file, 2006-12-12).\n>\n> on that last point, I am afraid there is still at least a problem that\n> needs addressing, but will comment in patch instead for easy of\n> reference.\n\nI would have preferred a straight answer right here than a reference to\nsomething I have not received (yet?).\n\nCiao,\nDscho\n"},{"id":"396292","messageId":"20200426170508.6uaexvcmorj2ntv3@tb-raspi4","threadId":"53294","inReplyTo":"pull.616.v2.git.1587819266388.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] macos: do let the build find the gettext headers/libraries/msgfmt","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2020-04-26T17:05:08Z","receivedAt":"2020-04-26T17:05:12Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Sat, Apr 25, 2020 at 12:54:26PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> Apparently a recent Homebrew update now installs `gettext` into a\n> subdirectory under /usr/local/, requiring the CFLAGS/LDFLAGS to list\n> explicit directories _even_ when asking to force-link the `gettext`\n> package.\n>\n> Likewise, the `msgfmt` tool is no longer in the `PATH`.\n>\n> While it is unclear which change is responsible for this breakage (that\n> most notably only occurs on CI build agents that updated very recently),\n> https://github.com/Homebrew/homebrew-core/pull/53489 should fix it.\n>\n> Nevertheless, let's work around this issue, as there are still quite a\n> few build agents out there that need some help in this regard: we\n> explicitly do not call `brew update` in our CI/PR builds anymore.\n>\n> Helped-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIt seems as if things are happening fast in the brew-business.\nAfter debugging (on a local box) and travis-ing (remote) I may have a suggestion\nfor a better commit-message - I can probably send that out as a patch later today.\nWhat do you think ?\n\n=================================================================\nMacOs/brew: Let the build find gettext headers/libraries/msgfmt\n\nApparently a recent Homebrew update now installs `gettext` into the\nsubdirectory /usr/local/opt/gettext/[lib/include].\n\nSometimes the ci job succeeds:\n brew link --force gettext\n Linking /usr/local/Cellar/gettext/0.20.1... 179 symlinks created\n\nAnd sometimes installing the package \"gettext\" with force-link fails:\n brew link --force gettext\n Warning: Refusing to link macOS provided/shadowed software: gettext\n If you need to have gettext first in your PATH run:\n  echo 'export PATH=\"/usr/local/opt/gettext/bin:$PATH\"' >> ~/.bash_profile\n\n(And the is not the final word either, since MacOs itself says:\n The default interactive shell is now zsh.)\n\nAnyway, The latter requires CFLAGS to include /usr/local/opt/gettext/include\nand LDFLAGS to include /usr/local/opt/gettext/lib.\n\nLikewise, the `msgfmt` tool is no longer in the `PATH`.\n\nWhile it is unclear which change is responsible for this breakage (that\nmost notably only occurs on CI build agents that updated very recently),\nhttps://github.com/Homebrew/homebrew-core/pull/53489 has fixed it.\n\nNevertheless, let's work around this issue, as there are still quite a\nfew build agents out there that need some help in this regard: we\nexplicitly do not call `brew update` in our CI/PR builds anymore.\n\nHelped-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n"},{"id":"396294","messageId":"20200426173419.GA95483@Carlos-MBP","threadId":"53294","inReplyTo":"pull.616.v2.git.1587819266388.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] macos: do let the build find the gettext headers/libraries/msgfmt","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2020-04-26T17:34:19Z","receivedAt":"2020-04-26T17:34:23Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Sat, Apr 25, 2020 at 12:54:26PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> diff --git a/config.mak.uname b/config.mak.uname\n> index 0ab8e009383..1ea16e89288 100644\n> --- a/config.mak.uname\n> +++ b/config.mak.uname\n> @@ -133,8 +133,17 @@ ifeq ($(uname_S),Darwin)\n>  \tHAVE_BSD_SYSCTL = YesPlease\n>  \tFREAD_READS_DIRECTORIES = UnfortunatelyYes\n>  \tHAVE_NS_GET_EXECUTABLE_PATH = YesPlease\n> -\tBASIC_CFLAGS += -I/usr/local/include\n> -\tBASIC_LDFLAGS += -L/usr/local/lib\n\nkeeping these flags independenly allows people that doesn't have brew or that\nhave brew but hadn't installed the gettext package, to still be able to use\nother libraries/packages that could be installed in those directories\neither through brew (ex: libgcrypt, openssl or pcre*) or manually while\nalso using a compiler that doesn't use by default /usr/local/{include,lib}.\n\neven if all this might sound like a stretch, notice that it happened before\nas shown by 2a1377a2a (macOS: make sure that gettext is found, 2019-04-14)\n\n> +\t# Workaround for `gettext` being keg-only and not even being linked via\n> +\t# `brew link --force gettext`, should be obsolete as of\n> +\t# https://github.com/Homebrew/homebrew-core/pull/53489\n> +\tifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)\n> +\t\tBASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include\n> +\t\tBASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib\n> +\t\tifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)\n> +\t\t\tMSGFMT = /usr/local/opt/gettext/bin/msgfmt\n> +\t\tendif\n> +\tendif\n>  endif\n>  ifeq ($(uname_S),SunOS)\n>  \tNEEDS_SOCKET = YesPlease\n> \n\nsince this doesn't depend on NO_GETTEXT and the gettext support doesn't get\nautomatically configured and used if found (unlike most other) then having\nit here could work and is cleaner, but will still mean that MSGFMT will be\ncalled to compile the translation files even when git is built with NO_GETTEXT\n\njust because of that oddity I think having it with the other package related\nentries in Makefile might still make sense (specially since we can't get\nrid of it unless we also deprecate the other package managers), but if\ncleaning it is what you had in mind  would also appreciate in that line your\nreview on 20200425102651.51961-1-carenas@gmail.com[1] and\n20200425091549.42293-1-carenas@gmail.com[2] that do similar work and that\nin the later case improve performance and correctness of git grep.\n\nCarlo\n\n[1] https://lore.kernel.org/git/20200425102651.51961-1-carenas@gmail.com/\n[2] https://lore.kernel.org/git/20200425091549.42293-1-carenas@gmail.com/\n"},{"id":"396302","messageId":"20200426200932.3769-1-tboegi@web.de","threadId":"53294","inReplyTo":"pull.616.v2.git.1587819266388.gitgitgadget@gmail.com","subject":"[PATCH v3 1/1] MacOs/brew: Let the build find gettext headers/libraries/msgfmt","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2020-04-26T20:09:32Z","receivedAt":"2020-04-26T20:09:41Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nApparently a recent Homebrew update now installs `gettext` into the\nsubdirectory /usr/local/opt/gettext/[lib/include].\n\nSometimes the ci job succeeds:\n brew link --force gettext\n Linking /usr/local/Cellar/gettext/0.20.1... 179 symlinks created\n\nAnd sometimes installing the package \"gettext\" with force-link fails:\n brew link --force gettext\n Warning: Refusing to link macOS provided/shadowed software: gettext\n If you need to have gettext first in your PATH run:\n  echo 'export PATH=\"/usr/local/opt/gettext/bin:$PATH\"' >> ~/.bash_profile\n\n(And the is not the final word either, since MacOs itself says:\n The default interactive shell is now zsh.)\n\nAnyway, The latter requires CFLAGS to include /usr/local/opt/gettext/include\nand LDFLAGS to include /usr/local/opt/gettext/lib.\n\nLikewise, the `msgfmt` tool is no longer in the `PATH`.\n\nWhile it is unclear which change is responsible for this breakage (that\nmost notably only occurs on CI build agents that updated very recently),\nhttps://github.com/Homebrew/homebrew-core/pull/53489 has fixed it.\n\nNevertheless, let's work around this issue, as there are still quite a\nfew build agents out there that need some help in this regard: we\nexplicitly do not call `brew update` in our CI/PR builds anymore.\n\nHelped-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n Changes since V2:\n   Updated the commit message\n\nconfig.mak.uname | 13 +++++++++++--\n 1 file changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 0ab8e00938..1ea16e8928 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -133,8 +133,17 @@ ifeq ($(uname_S),Darwin)\n \tHAVE_BSD_SYSCTL = YesPlease\n \tFREAD_READS_DIRECTORIES = UnfortunatelyYes\n \tHAVE_NS_GET_EXECUTABLE_PATH = YesPlease\n-\tBASIC_CFLAGS += -I/usr/local/include\n-\tBASIC_LDFLAGS += -L/usr/local/lib\n+\n+\t# Workaround for `gettext` being keg-only and not even being linked via\n+\t# `brew link --force gettext`, should be obsolete as of\n+\t# https://github.com/Homebrew/homebrew-core/pull/53489\n+\tifeq ($(shell test -d /usr/local/opt/gettext/ && echo y),y)\n+\t\tBASIC_CFLAGS += -I/usr/local/include -I/usr/local/opt/gettext/include\n+\t\tBASIC_LDFLAGS += -L/usr/local/lib -L/usr/local/opt/gettext/lib\n+\t\tifeq ($(shell test -x /usr/local/opt/gettext/bin/msgfmt && echo y),y)\n+\t\t\tMSGFMT = /usr/local/opt/gettext/bin/msgfmt\n+\t\tendif\n+\tendif\n endif\n ifeq ($(uname_S),SunOS)\n \tNEEDS_SOCKET = YesPlease\n--\n2.26.1.107.gefe3874640\n\n"}]}