Re: [PATCH] Makefile: make NO_ICONV really mean "no iconv"

[Jeff King <peff@peff.net>]

On Thu, Jun 14, 2018 at 10:25:03PM -0400, Eric Sunshine wrote:

> The Makefile tweak NO_ICONV is meant to allow Git to be built without
> iconv in case iconv is not installed or is otherwise dysfunctional.
> However, NO_ICONV's disabling of iconv is incomplete and can incorrectly
> allow "-liconv" to slip into the linker flags when NEEDS_LIBICONV is
> defined, which breaks the build when iconv is not installed.
> 
> On some platforms, iconv lives directly in libc, whereas, on others it
> resides in libiconv. For the latter case, NEEDS_LIBICONV instructs the
> Makefile to add "-liconv" to the linker flags. config.mak.uname
> automatically defines NEEDS_LIBICONV for platforms which require it.
> The adding of "-liconv" is done unconditionally, despite NO_ICONV.
> 
> Work around this problem by making NO_ICONV take precedence over
> NEEDS_LIBICONV.

Nicely explained.

We have OLD_ICONV, too, which should probably do nothing if NO_ICONV is
set. I think that works OK. We end up setting -DOLD_ICONV on the command
line, but that's only consider inside "#ifndef NO_ICONV" within the
code.

> This patch is extra noisy due to the indentation change. Viewing it with
> "git diff -w" helps. An alternative to re-indenting would have been to
> "undefine NEEDS_LIBICONV", however, 'undefine' was added to GNU make in
> 3.82 but MacOS is stuck on 3.81 (from 2006) so 'undefine' was avoided.

Yeah, with "-w" it looks pretty obviously correct.

-Peff