git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Carlo Marcelo Arenas Belón" <carenas@gmail.com>
To: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, "SZEDER Gábor" <szeder.dev@gmail.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>
Subject: Re: [PATCH v2] macos: do let the build find the gettext headers/libraries/msgfmt
Date: Sun, 26 Apr 2020 10:34:19 -0700	[thread overview]
Message-ID: <20200426173419.GA95483@Carlos-MBP> (raw)
In-Reply-To: <pull.616.v2.git.1587819266388.gitgitgadget@gmail.com>

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/

  parent reply	other threads:[~2020-04-26 17:34 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-23  7:52 [PATCH] macos: do let the build find the gettext headers/libraries/msgfmt Johannes Schindelin via GitGitGadget
2020-04-23 16:17 ` Eric Sunshine
2020-04-23 20:49   ` Eric Sunshine
2020-04-25 12:54     ` Johannes Schindelin
2020-04-26 15:54       ` Carlo Arenas
2020-04-26 16:59         ` Johannes Schindelin
2020-04-25  6:15 ` [PATCH] macos: do not assume brew and gettext are always available/wanted Carlo Marcelo Arenas Belón
2020-04-25 12:33   ` Johannes Schindelin
2020-04-25 12:54 ` [PATCH v2] macos: do let the build find the gettext headers/libraries/msgfmt Johannes Schindelin via GitGitGadget
2020-04-26 17:05   ` Torsten Bögershausen
2020-04-26 17:34   ` Carlo Marcelo Arenas Belón [this message]
2020-04-26 20:09   ` [PATCH v3 1/1] MacOs/brew: Let the build find " tboegi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200426173419.GA95483@Carlos-MBP \
    --to=carenas@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=sunshine@sunshineco.com \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).