All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Git List <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 02:39:38 -0400	[thread overview]
Message-ID: <20180615063937.GA10802@sigill.intra.peff.net> (raw)
In-Reply-To: <CAPig+cTSirqhayBDJFZhn2HL4EMr36GHuCOYAMaCeptqFtoR=w@mail.gmail.com>

On Fri, Jun 15, 2018 at 02:30:43AM -0400, Eric Sunshine wrote:

> On Fri, Jun 15, 2018 at 12:20 AM Jeff King <peff@peff.net> wrote:
> > 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.
> 
> Right, that was my conclusion, as well. Since it works as is, I'm not
> sure suppressing -DOLD_ICONV in Makefile is worth the extra patch
> noise. I can re-roll with that change too, if someone thinks it's
> worthwhile, though.

Nah, I was just thinking out loud. I don't think it's worth changing the
Makefile (and I wouldn't be surprised if there are other "dependent"
cases like this that work just fine because of the #ifdefs in the code).

-Peff

  reply	other threads:[~2018-06-15  6:39 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 [this message]
2018-06-15  6:58   ` Simon Ruderich
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=20180615063937.GA10802@sigill.intra.peff.net \
    --to=peff@peff.net \
    --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.