Re: [[PATCH v2] ] INSTALL: add macOS gettext and sdk header explanation to INSTALL

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 16, 2018, 04:06 UTC
Message-ID: <xmqqftw1ll7n.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20181115120445.46277-1-yanke131415@gmail.com>

yanke131415@gmail.com writes:

> Subject: Re: [[PATCH v2] ] INSTALL: add macOS gettext and sdk header explanation to INSTALL

The above should look more like

	Subject: [PATCH v2] INSTALL: add macOS ...

> From: out0fmemory <jiu4majia2@163.com>

This line is to give information that records who the patch was writtten by, and later Signed-off-by: line certifies that the person (which typically is, and in this case also is, the person who wrote it) has the right to contribute it to the project under the project's licensing terms. We want to see the name and address on these two lines to match.

> * add macOS gettext explanation to get the i18n locale translation take effect in macOS, as the most polular way of gettext
>   install in macOS, the gettext is not linked by default, this commit give a tip on this thing.
>

Please wrap overlong lines to ~70 cols.

Also I am not quite sure what it wants to say. Perhaps you meant to say something like this?

	Explain how to make the gettext library usable on macOS, as
	with the most popular way to install it, it won't be linked
	to /usr/local.

I think the part that I had most trouble understanding was your use of the verb "link"; it was unclear (and I am guessing) that you meant there are missing links on the filesystem to make stuff from gettext package available to programs that want to build with it, and instead your original text (aside from grammatical issues) sounded as if you were reporting lack of linker flags when building binary or something.

> * add macOS Command Line Tool sdk header explanation to get correct build in macOS 10.14+, as the CLT not install
>   the header by default, we need install it by self, this commit give a way to install the loss headers.

Similarly, is

	Explain how to install the Command Line Tool SDK headers
	manually on macOS 10.14+ in order to correctly build Git, as
	they are not installed by default.

what you meant?

> Signed-off-by: yanke <jiu4majia2@163.com>

Show 16 quoted lines

> ---
>  INSTALL | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/INSTALL b/INSTALL
> index c39006e8e7..ed4bd29f8f 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -165,6 +165,13 @@ Issues of note:
>  	  use English. Under autoconf the configure script will do this
>  	  automatically if it can't find libintl on the system.
>  
> +    In macOS, can install gettext with brew as "brew install gettext"
> +    and "brew link --force gettext", the gettext is keg-only so brew not link
> +    it to /usr/local by default, so link operation is necessary, or you can
> +    follow the brew tips after install gettext.

Sorry, but I cannot quite understand this overlong and grammatically unparsable single sentence. There is no subject for verb phrase "can install" at the beginning of the sentence where I already got stuck X-<.

My best guess of what you wanted to say is

	On macOS, `gettext` can be installed with `brew install
	gettext`, but because the `gettext` package is keg-only and
	is not made available in `/usr/local` by default.  `brew
	link --force gettext` must be run after `brew install
	gettext` to correct this to use i18n features of Git.

but now the sentence structure is quite different and I no longer know if that is what you meant to say. And it does not help that I am not a Mac user.

> If not link gettext correctly,
> +    the git after build will not have correct locale translations, english is the
> +    default language.
> +

If my rephrasing above is correct, then these three lines become unnecessary, I think.

> +  - In macOs 10.14, the Command Line Tool not contains sdk headers as before, so
> +    need install Command Line Tool 10.14 and install the headers with command

	On macOS 10.14, the Command Line Tool no longer contains the
	SDK headers; you need to also install them with the command:

> +    "sudo installer -pkg /Library/Developer/CommandLineTools/Packages/macOS_SDK_headers_for_macOS_10.14.pkg -target".

> +    If not install the sdk headers correctly, git build will get errors blew, factly is
> +    is because of this problem.

I can guess this wants to say "without sdk headers your attempt to build Git will blow up in your face", but not quite.

	Unless you install the SDK headers, building git will fail
	with error messages like the following:

Perhaps.

Show 10 quoted lines

> +    ld: archive has no table of contents file 'xdiff/lib.a' for architecture x86_64
> +    clang: error: linker command failed with exit code 1 (use -v to see invocation)
> +    make: *** [Makefile:2369: git-fast-import] Error 1
> +    ld: archive has no table of contents file 'xdiff/lib.a' for architecture x86_64
> +    clang: error: linker command failed with exit code 1 (use -v to see invocation)
> +    make: *** [Makefile:2046: git] Error 1
> +
>   - To build and install documentation suite, you need to have
>     the asciidoc/xmlto toolchain.  Because not many people are
>     inclined to install the tools, the default build target