diff mbox series

[2/2] add liconv link for makefile

Message ID 20210908051340.13332-3-colinpcurtis826@ucla.edu (mailing list archive)
State New, archived
Headers show
Series Add cmd_gud and detect libiconv path for Mac OS | expand

Commit Message

Colin Curtis Sept. 8, 2021, 5:13 a.m. UTC
From: Colin Curtis <colinpcurtis@gmail.com>

---
 Makefile      | 8 ++++++--
 builtin/gud.c | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

Comments

Carlo Marcelo Arenas Belón Sept. 8, 2021, 5:52 a.m. UTC | #1
On Tue, Sep 7, 2021 at 10:18 PM Colin Curtis <colinpcurtis@gmail.com> wrote:
>
> diff --git a/Makefile b/Makefile
> index 379cd91a97..e1679cca47 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,6 +1,6 @@
>  # The default target of this Makefile is...
>  all::
> -
> +OS := $(shell uname)

There is no need for this, the section of code you modify below is
already macOS (indeed, even to the point that it won't trigger in a
Linux user using brew, or even a macOS user that has macports)
specific

>  # Define V=1 to have a more verbose compile.
>  #
>  # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
> @@ -1514,7 +1514,11 @@ ifndef NO_ICONV
>                 ifdef NEEDS_LIBINTL_BEFORE_LIBICONV
>                         ICONV_LINK += -lintl
>                 endif
> -               EXTLIBS += $(ICONV_LINK) /usr/local/Cellar/libiconv/1.16/lib/libiconv.dylib # -liconv
> +               ifeq ($(OS),Darwin)
> +                       EXTLIBS += $(ICONV_LINK) /usr/local/Cellar/libiconv/1.16/lib/libiconv.dylib
> +               else
> +                       EXTLIBS += $(ICONV_LINK) -liconv
> +               endif

Why is it not built with the libiconv library that is provided by the system?

$ otool -L git
git:
        /System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices
(compatibility version 1.0.0, current version 1122.33.0)
        /usr/lib/libz.1.dylib (compatibility version 1.0.0, current
version 1.2.11)
        /usr/lib/libiconv.2.dylib (compatibility version 7.0.0,
current version 7.0.0)
        /usr/local/opt/gettext/lib/libintl.8.dylib (compatibility
version 11.0.0, current version 11.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0,
current version 1292.100.5)
        /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
(compatibility version 150.0.0, current version 1775.118.101)

AFAIK there is a good reason why brew doesn't link that automatically,
and using the headers of one with the binary of the other is likely to
cause serious problems.

Carlo
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 379cd91a97..e1679cca47 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,6 @@ 
 # The default target of this Makefile is...
 all::
-
+OS := $(shell uname)
 # Define V=1 to have a more verbose compile.
 #
 # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
@@ -1514,7 +1514,11 @@  ifndef NO_ICONV
 		ifdef NEEDS_LIBINTL_BEFORE_LIBICONV
 			ICONV_LINK += -lintl
 		endif
-		EXTLIBS += $(ICONV_LINK) /usr/local/Cellar/libiconv/1.16/lib/libiconv.dylib # -liconv
+		ifeq ($(OS),Darwin)
+			EXTLIBS += $(ICONV_LINK) /usr/local/Cellar/libiconv/1.16/lib/libiconv.dylib
+		else
+			EXTLIBS += $(ICONV_LINK) -liconv
+		endif
 	endif
 endif
 ifdef ICONV_OMITS_BOM
diff --git a/builtin/gud.c b/builtin/gud.c
index 04808a08f5..9a5a1e71ac 100644
--- a/builtin/gud.c
+++ b/builtin/gud.c
@@ -24,4 +24,4 @@  int cmd_gud(int argc, const char **argv, const char *prefix)
     }
     
     return 0;
-}
\ No newline at end of file
+}