All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Ruderich <simon@ruderich.org>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: git@vger.kernel.org, Mahmoud Al-Qudsi <mqudsi@neosmart.net>
Subject: Re: [PATCH] Makefile: make NO_ICONV really mean "no iconv"
Date: Fri, 15 Jun 2018 08:58:05 +0200	[thread overview]
Message-ID: <20180615065805.GA15146@ruderich.org> (raw)
In-Reply-To: <20180615022503.34111-1-sunshine@sunshineco.com>

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.
>
> Reported by: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>
> 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.

Should we put the part about MacOS's make into the commit
message? Seems like relevant information for future readers.

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

  parent reply	other threads:[~2018-06-15  6:58 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-14 22:47 Is NO_ICONV misnamed or is it broken? Mahmoud Al-Qudsi
2018-06-15  2:25 ` [PATCH] Makefile: make NO_ICONV really mean "no iconv" Eric Sunshine
2018-06-15  4:20   ` Jeff King
2018-06-15  6:30     ` Eric Sunshine
2018-06-15  6:39       ` Jeff King
2018-06-15  6:58   ` Simon Ruderich [this message]
2018-06-15  7:43     ` Eric Sunshine
2018-06-17 17:32       ` Kaartic Sivaraam
2018-06-17 18:00         ` Eric Sunshine
2018-06-17 18:25           ` Doc/SubmittingPatches: re-phrashing a sentence about alternate solutions (was Re: [PATCH] Makefile: make NO_ICONV really mean "no iconv") Kaartic Sivaraam
2018-06-18  4:20             ` Jeff King
2018-06-18 16:09           ` [PATCH] Makefile: make NO_ICONV really mean "no iconv" Junio C Hamano
2018-06-15  8:15   ` Mahmoud Al-Qudsi
2018-06-17  2:57 ` Is NO_ICONV misnamed or is it broken? Christian Couder
2018-06-17  3:41   ` Eric Sunshine

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=20180615065805.GA15146@ruderich.org \
    --to=simon@ruderich.org \
    --cc=git@vger.kernel.org \
    --cc=mqudsi@neosmart.net \
    --cc=sunshine@sunshineco.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.