Is NO_ICONV misnamed or is it broken?

Is NO_ICONV misnamed or is it broken?

[Mahmoud Al-Qudsi]

Hello list,

With regards to the Makefile define/variable `NO_ICONV` - the Makefile
comments imply that it should be used if "your libc doesn't properly support
iconv," which could mean anything from "a patch will be applied" to "iconv
won't be used."

Based off the name of the varibale, the assumption is that iconv is an
optional dependency that can be omitted if compiled with NO_ICONV. However, in
practice attempting to compile git with `make ... NO_ICONV=1` and libiconv not
installed results in linker errors as follows:

```
~> make clean
# omitted
~> make NO_ICONV=1
# ommitted
    LINK git-credential-store
/usr/bin/ld: cannot find -liconv
cc: error: linker command failed with exit code 1 (use -v to see invocation)
gmake: *** [Makefile:2327: git-credential-store] Error 1
```

Am I misunderstanding the intended behavior when NO_ICONV is defined (i.e. it
does not remove the dependency on libiconv) or is this a bug and iconv should
not, in fact, be required?

Many thanks,

Mahmoud Al-Qudsi
NeoSmart Technologies

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

[Eric Sunshine]

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.

Reported here: https://public-inbox.org/git/CACcTrKePbgyCbXneN5NZ+cS-tiDyYe_dkdwttXpP0CUeEicvHw@mail.gmail.com/T/#u

 Makefile | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index 1d27f36365..e4b503d259 100644
--- a/Makefile
+++ b/Makefile
@@ -1351,17 +1351,19 @@ ifdef APPLE_COMMON_CRYPTO
 	LIB_4_CRYPTO += -framework Security -framework CoreFoundation
 endif
 endif
-ifdef NEEDS_LIBICONV
-	ifdef ICONVDIR
-		BASIC_CFLAGS += -I$(ICONVDIR)/include
-		ICONV_LINK = -L$(ICONVDIR)/$(lib) $(CC_LD_DYNPATH)$(ICONVDIR)/$(lib)
-	else
-		ICONV_LINK =
-	endif
-	ifdef NEEDS_LIBINTL_BEFORE_LIBICONV
-		ICONV_LINK += -lintl
+ifndef NO_ICONV
+	ifdef NEEDS_LIBICONV
+		ifdef ICONVDIR
+			BASIC_CFLAGS += -I$(ICONVDIR)/include
+			ICONV_LINK = -L$(ICONVDIR)/$(lib) $(CC_LD_DYNPATH)$(ICONVDIR)/$(lib)
+		else
+			ICONV_LINK =
+		endif
+		ifdef NEEDS_LIBINTL_BEFORE_LIBICONV
+			ICONV_LINK += -lintl
+		endif
+		EXTLIBS += $(ICONV_LINK) -liconv
 	endif
-	EXTLIBS += $(ICONV_LINK) -liconv
 endif
 ifdef NEEDS_LIBGEN
 	EXTLIBS += -lgen
-- 
2.18.0.rc1.256.g331a1db143

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

[Jeff King]

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

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

[Eric Sunshine]

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.

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

[Jeff King]

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

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

[Simon Ruderich]

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

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

[Eric Sunshine]

On Fri, Jun 15, 2018 at 2:58 AM Simon Ruderich <simon@ruderich.org> wrote:
> On Thu, Jun 14, 2018 at 10:25:03PM -0400, Eric Sunshine wrote:
> > 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.

No. The bit of commentary mentioning MacOS's very old 'make' was just
talking about a possible alternate way of implementing the change.
That alternative was not chosen, so talking about old 'make' in the
commit message would be confusing for readers. More importantly,
although that alternative would have made a less noisy patch, the
actual result would have made the Makefile itself noisier and uglier,
particularly for people just reading the Makefile in the future,
people who did not read the patch. Specifically, these alternatives
were considered:

    ifdef NO_ICONV
        undefine NEEDS_LIBICONV
    endif
    ifdef NEEDS_LIBICONV
        ...as before...
    endif

and:

    ifdef NO_ICONV
        NEEDS_LIBICONV=
    endif
    ifeq ($(NEEDS_LIBICONV),)
        ...as before...
    endif

Both of which are uglier for a future reader of Makefile than the
end-result actually implemented by this patch:

    ifndef NO_ICONV
        ifdef NEEDS_LIBICONV
            ...as before...
        endif
    endif

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

[Kaartic Sivaraam]

[-- Attachment #1.1: Type: text/plain, Size: 2296 bytes --]

On Friday 15 June 2018 01:13 PM, Eric Sunshine wrote:
> On Fri, Jun 15, 2018 at 2:58 AM Simon Ruderich <simon@ruderich.org> wrote:
>> On Thu, Jun 14, 2018 at 10:25:03PM -0400, Eric Sunshine wrote:
>>> 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.
> 
> No. The bit of commentary mentioning MacOS's very old 'make' was just
> talking about a possible alternate way of implementing the change.
> That alternative was not chosen, so talking about old 'make' in the
> commit message would be confusing for readers.

Interesting. Documentation/SubmittinPatches reads:

    The body should provide a meaningful commit message, which:
    ...
    <snip>
    ...
    . alternate solutions considered but discarded, if any.

The consensus has changed, maybe? In which case, should we remove that
statement from there?


-- 
Sivaraam

QUOTE:

“The three principal virtues of a programmer are Laziness, Impatience,
and Hubris.”

	- Camel book

Sivaraam?

You possibly might have noticed that my signature recently changed from
'Kaartic' to 'Sivaraam' both of which are parts of my name. I find the
new signature to be better for several reasons one of which is that the
former signature has a lot of ambiguities in the place I live as it is a
common name (NOTE: it's not a common spelling, just a common name). So,
I switched signatures before it's too late.

That said, I won't mind you calling me 'Kaartic' if you like it [of
course ;-)]. You can always call me using either of the names.


KIND NOTE TO THE NATIVE ENGLISH SPEAKER:

As I'm not a native English speaker myself, there might be mistaeks in
my usage of English. I apologise for any mistakes that I make.

It would be "helpful" if you take the time to point out the mistakes.

It would be "super helpful" if you could provide suggestions about how
to correct those mistakes.

Thanks in advance!


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Eric Sunshine]

On Sun, Jun 17, 2018 at 1:32 PM Kaartic Sivaraam
<kaartic.sivaraam@gmail.com> wrote:
> On Friday 15 June 2018 01:13 PM, Eric Sunshine wrote:
> > On Fri, Jun 15, 2018 at 2:58 AM Simon Ruderich <simon@ruderich.org> wrote:
> >> Should we put the part about MacOS's make into the commit
> >> message? Seems like relevant information for future readers.
> >
> > No. The bit of commentary mentioning MacOS's very old 'make' was just
> > talking about a possible alternate way of implementing the change.
> > That alternative was not chosen, so talking about old 'make' in the
> > commit message would be confusing for readers.
>
> Interesting. Documentation/SubmittinPatches reads:
>
>     The body should provide a meaningful commit message, which:
>     <snip>
>     . alternate solutions considered but discarded, if any.
>
> The consensus has changed, maybe? In which case, should we remove that
> statement from there?

Whether or not to talk about alternate solutions in the commit message
is a judgment call. Same for deciding what belongs in the commit
message proper and what belongs in the "commentary" section of a
patch. A patch author should strive to convey the problem succinctly
in the commit message, to not overload the reader with unnecessary (or
confusing) information, while, at the same time, not be sparing with
information which is genuinely needed to understand the problem and
solution.

Often, this can be done without talking about alternatives; often even
without spelling out the solution in detail or at all since the
solution may be "obvious", given a well-written problem description.
Complex cases, or cases in which multiple solutions may be or seem
valid, on the other hand, might warrant talking about those alternate
solutions, so we probably don't want to drop that bullet point.
Perhaps, instead, it can be re-worded a bit to make it sound something
other than mandatory (but I can't think of a good way to phrase it;
maybe you can?).

Doc/SubmittingPatches: re-phrashing a sentence about alternate solutions (was Re: [PATCH] Makefile: make NO_ICONV really mean "no iconv")

[Kaartic Sivaraam]

On Sun, 2018-06-17 at 14:00 -0400, Eric Sunshine wrote:
> Whether or not to talk about alternate solutions in the commit message
> is a judgment call. Same for deciding what belongs in the commit
> message proper and what belongs in the "commentary" section of a
> patch. A patch author should strive to convey the problem succinctly
> in the commit message, to not overload the reader with unnecessary (or
> confusing) information, while, at the same time, not be sparing with
> information which is genuinely needed to understand the problem and
> solution.
> 
> Often, this can be done without talking about alternatives; often even
> without spelling out the solution in detail or at all since the
> solution may be "obvious", given a well-written problem description.
> Complex cases, or cases in which multiple solutions may be or seem
> valid, on the other hand, might warrant talking about those alternate
> solutions, so we probably don't want to drop that bullet point.

Well explained, thanks. (Thinking out loud, it might be even nice to
including the above paragraphs into Documentation/SubmittingPatches as
I find it to be more "humane" than the terse bullets. But I refrained
from doing so as the document is already a bit too-long ;-)

> Perhaps, instead, it can be re-worded a bit to make it sound something
> other than mandatory (but I can't think of a good way to phrase it;
> maybe you can?).

How about the following patch? (warning: patch only for discussion
purposes, might be white-space broken). It might be superfluous,
though.

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index a1d0feca3..565bc4397 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -128,7 +128,7 @@ The body should provide a meaningful commit message, which:
 . justifies the way the change solves the problem, i.e. why the
   result with the change is better.
 
-. alternate solutions considered but discarded, if any.
+. alternate solutions considered but discarded, where necessary.
 
 [[imperative-mood]]
 Describe your changes in imperative mood, e.g. "make xyzzy do frotz"


Regards,
Sivaraam

Re: Doc/SubmittingPatches: re-phrashing a sentence about alternate solutions (was Re: [PATCH] Makefile: make NO_ICONV really mean "no iconv")

[Jeff King]

On Sun, Jun 17, 2018 at 11:55:33PM +0530, Kaartic Sivaraam wrote:

> On Sun, 2018-06-17 at 14:00 -0400, Eric Sunshine wrote:
> > Whether or not to talk about alternate solutions in the commit message
> > is a judgment call. Same for deciding what belongs in the commit
> > message proper and what belongs in the "commentary" section of a
> > patch. A patch author should strive to convey the problem succinctly
> > in the commit message, to not overload the reader with unnecessary (or
> > confusing) information, while, at the same time, not be sparing with
> > information which is genuinely needed to understand the problem and
> > solution.
> > 
> > Often, this can be done without talking about alternatives; often even
> > without spelling out the solution in detail or at all since the
> > solution may be "obvious", given a well-written problem description.
> > Complex cases, or cases in which multiple solutions may be or seem
> > valid, on the other hand, might warrant talking about those alternate
> > solutions, so we probably don't want to drop that bullet point.
> 
> Well explained, thanks. (Thinking out loud, it might be even nice to
> including the above paragraphs into Documentation/SubmittingPatches as
> I find it to be more "humane" than the terse bullets. But I refrained
> from doing so as the document is already a bit too-long ;-)

Yes, the first paragraph especially. The _most_ important thing is
writing well with consideration for your readers. All the other rules
are really guidelines to help you remember how to do that. When in
doubt follow the guidelines, but it's OK to break them if it serves the
ultimate purpose.

All IMHO, of course. :)

-Peff

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

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Sun, Jun 17, 2018 at 1:32 PM Kaartic Sivaraam
> <kaartic.sivaraam@gmail.com> wrote:
>> On Friday 15 June 2018 01:13 PM, Eric Sunshine wrote:
>> > On Fri, Jun 15, 2018 at 2:58 AM Simon Ruderich <simon@ruderich.org> wrote:
>> >> Should we put the part about MacOS's make into the commit
>> >> message? Seems like relevant information for future readers.
>> >
>> > No. The bit of commentary mentioning MacOS's very old 'make' was just
>> > talking about a possible alternate way of implementing the change.
>> > That alternative was not chosen, so talking about old 'make' in the
>> > commit message would be confusing for readers.
>>
>> Interesting. Documentation/SubmittinPatches reads:
>>
>>     The body should provide a meaningful commit message, which:
>>     <snip>
>>     . alternate solutions considered but discarded, if any.
>>
>> The consensus has changed, maybe? In which case, should we remove that
>> statement from there?
>
> Whether or not to talk about alternate solutions in the commit message
> is a judgment call. Same for deciding what belongs in the commit
> message proper and what belongs in the "commentary" section of a
> patch. A patch author should strive to convey the problem succinctly
> in the commit message, to not overload the reader with unnecessary (or
> confusing) information, while, at the same time, not be sparing with
> information which is genuinely needed to understand the problem and
> solution.
>
> Often, this can be done without talking about alternatives; often even
> without spelling out the solution in detail or at all since the
> solution may be "obvious", given a well-written problem description.
> Complex cases, or cases in which multiple solutions may be or seem
> valid, on the other hand, might warrant talking about those alternate
> solutions, so we probably don't want to drop that bullet point.
> Perhaps, instead, it can be re-worded a bit to make it sound something
> other than mandatory (but I can't think of a good way to phrase it;
> maybe you can?).

Yup, "if any" is a bad thing to say, as it does not set the bar for
that "any" random garbage idea.  A phrase like "when appropriate" is
a relatively safe but mostly useless cop-out, as these guidelines
are written primarily for those who don't yet have proper yardsticks
to gauge what is appropriate and what isn't.

I think it maybe better to either drop it or make it a sample way to
do the second point, i.e. if there are seemingly valid alternative
which may entice readers, explaining why the alternative does not
work well and the solution you chose works better *is* a good way to
justify the way you chose in your change.  Off the top of my head,
something like this?  I am not very happy with the text, though.


 Documentation/SubmittingPatches | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 2488544407..4294d0f068 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -125,10 +125,12 @@ The body should provide a meaningful commit message, which:
 . explains the problem the change tries to solve, i.e. what is wrong
   with the current code without the change.
 
-. justifies the way the change solves the problem, i.e. why the
-  result with the change is better.
+. justifies the way the change solves the problem, i.e. why the result
+  with the change is better (e.g. explaining the reason why an
+  seemingly obvious alternative does not work but the solution in the
+  patch does may be a good way to illustrate the nature of the problem
+  and how your approach fits it better).
 
-. alternate solutions considered but discarded, if any.
 
 [[imperative-mood]]
 Describe your changes in imperative mood, e.g. "make xyzzy do frotz"

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

[Mahmoud Al-Qudsi]

On Thu, Jun 14, 2018 at 9:25 PM, Eric Sunshine <sunshine@sunshineco.com> 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>
> ---
> ...

Thanks, Eric. I can confirm that on a clean FreeBSD 12 installation with
libiconv and with this patch applied, git builds and installs from source
(though other dependencies are obviously needed).

Mahmoud Al-Qudsi
NeoSmart Technologies

Re: Is NO_ICONV misnamed or is it broken?

[Christian Couder]

Hi,

On Fri, Jun 15, 2018 at 12:47 AM, Mahmoud Al-Qudsi <mqudsi@neosmart.net> wrote:
> Hello list,
>
> With regards to the Makefile define/variable `NO_ICONV` - the Makefile
> comments imply that it should be used if "your libc doesn't properly support
> iconv," which could mean anything from "a patch will be applied" to "iconv
> won't be used."

b6e56eca8a (Allow building Git in systems without iconv, 2006-02-16)
which added NO_ICONV says:

    Systems using some uClibc versions do not properly support
    iconv stuff. This patch allows Git to be built on those
    systems by passing NO_ICONV=YesPlease to make. The only
    drawback is mailinfo won't do charset conversion in those
    systems.

> Based off the name of the varibale, the assumption is that iconv is an
> optional dependency that can be omitted if compiled with NO_ICONV. However, in
> practice attempting to compile git with `make ... NO_ICONV=1` and libiconv not
> installed results in linker errors as follows:
>
> ```
> ~> make clean
> # omitted
> ~> make NO_ICONV=1
> # ommitted
>     LINK git-credential-store
> /usr/bin/ld: cannot find -liconv
> cc: error: linker command failed with exit code 1 (use -v to see invocation)
> gmake: *** [Makefile:2327: git-credential-store] Error 1
> ```

Yeah, this might be an issue with the Makefile options.

> Am I misunderstanding the intended behavior when NO_ICONV is defined (i.e. it
> does not remove the dependency on libiconv) or is this a bug and iconv should
> not, in fact, be required?

It's difficult to tell from reading the comments and commit messages.

I think 597c9cc540 (Flatten tools/ directory to make build procedure
simpler., 2005-09-07) which introduces NEEDS_LIBICONV is even older
than the commit that introduced NO_ICONV (see above), so you might
want to play with NEEDS_LIBICONV too and see if it works better for
you.

(I understand that "you might want to play with such and such other
options" is perhaps not as helpful as what you expected, but I
previously tried to tighten the way we handle dependencies in the
Makefile and it was considered "too heavy handed". So yeah, we
consider it ok if people have to tinker a bit when they want to build
Git.)

I CC'ed the people involved in related commits. Maybe they can give
you a better answer. It might also help if you could tell us on which
OS/Platform and perhaps for which purpose you want to compile Git.

Best,
Christian.

Re: Is NO_ICONV misnamed or is it broken?

[Eric Sunshine]

On Sat, Jun 16, 2018 at 10:57 PM Christian Couder
<christian.couder@gmail.com> wrote:
> On Fri, Jun 15, 2018 at 12:47 AM, Mahmoud Al-Qudsi <mqudsi@neosmart.net> wrote:
> > ~> make NO_ICONV=1
> > # ommitted
> >     LINK git-credential-store
> > /usr/bin/ld: cannot find -liconv
> I think 597c9cc540 (Flatten tools/ directory to make build procedure
> simpler., 2005-09-07) which introduces NEEDS_LIBICONV is even older
> than the commit that introduced NO_ICONV (see above), so you might
> want to play with NEEDS_LIBICONV too and see if it works better for
> you.
>
> I CC'ed the people involved in related commits. Maybe they can give
> you a better answer. It might also help if you could tell us on which
> OS/Platform and perhaps for which purpose you want to compile Git.

For completeness, for those reading this thread, a patch fixing this
issue has already been sent[1] to the list.

[1]: https://public-inbox.org/git/20180615022503.34111-1-sunshine@sunshineco.com/