git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Jun 15, 2018, 02:25 UTC
Message-ID
<20180615022503.34111-1-sunshine@sunshineco.com>
In-Reply-To
<CACcTrKePbgyCbXneN5NZ+cS-tiDyYe_dkdwttXpP0CUeEicvHw@mail.gmail.com>

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
Previous: Mahmoud Al-QudsiNext: Jeff King
Message 2 of 15 in “Is NO_ICONV misnamed or is it broken?”
  1. Mahmoud Al-QudsiJun 14, 2018
  2. Makefile: make NO_ICONV really mean "no iconv"Eric Sunshine, Jun 15, 2018
  3. Jeff KingJun 15, 2018
  4. Eric SunshineJun 15, 2018
  5. Jeff KingJun 15, 2018
  6. Simon RuderichJun 15, 2018
  7. Eric SunshineJun 15, 2018
  8. Kaartic SivaraamJun 17, 2018
  9. Eric SunshineJun 17, 2018
  10. Doc/SubmittingPatches: re-phrashing a sentence about alternate solutions (was Re: [PATCH] Makefile: make NO_ICONV really mean "no iconv")Kaartic Sivaraam, Jun 17, 2018
  11. Jeff KingJun 18, 2018
  12. Junio C HamanoJun 18, 2018
  13. Mahmoud Al-QudsiJun 15, 2018
  14. Christian CouderJun 17, 2018
  15. Eric SunshineJun 17, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.