* [PATCH] configure.ac: link with -liconv for locale_charset()
@ 2014-03-11 18:35 Dmitry Marakasov
2014-03-11 20:35 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Marakasov @ 2014-03-11 18:35 UTC (permalink / raw)
To: git; +Cc: wxs
On e.g. FreeBSD 10.x, the following situation is common:
- there's iconv implementation in libc, which has no locale_charset()
function
- there's GNU libiconv installed from Ports Collection
Git build process
- detects that iconv is in libc and thus -liconv is not needed for it
- detects locale_charset in -liconv, but for some reason doesn't add it
to CHARSET_LIB (as it would do with -lcharset if locale_charset() was
found there instead of -liconv)
- git doesn't build due to unresolved external locale_charset()
Fix this by adding -liconv to CHARSET_LIB if locale_charset() is
detected in this library.
Signed-off-by: Dmitry Marakasov <amdmi3@amdmi3.ru>
---
configure.ac | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git configure.ac configure.ac
index 2f43393..3f5c644 100644
--- configure.ac
+++ configure.ac
@@ -890,7 +890,7 @@ GIT_CONF_SUBST([HAVE_STRINGS_H])
# and libcharset does
CHARSET_LIB=
AC_CHECK_LIB([iconv], [locale_charset],
- [],
+ [CHARSET_LIB=-liconv],
[AC_CHECK_LIB([charset], [locale_charset],
[CHARSET_LIB=-lcharset])])
GIT_CONF_SUBST([CHARSET_LIB])
--
1.9.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] configure.ac: link with -liconv for locale_charset()
2014-03-11 18:35 [PATCH] configure.ac: link with -liconv for locale_charset() Dmitry Marakasov
@ 2014-03-11 20:35 ` Junio C Hamano
2014-03-11 22:37 ` Дилян Палаузов
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Junio C Hamano @ 2014-03-11 20:35 UTC (permalink / raw)
To: Дилян Палаузов
Cc: git, wxs, Dmitry Marakasov
Dmitry Marakasov <amdmi3@amdmi3.ru> writes:
> On e.g. FreeBSD 10.x, the following situation is common:
> - there's iconv implementation in libc, which has no locale_charset()
> function
> - there's GNU libiconv installed from Ports Collection
>
> Git build process
> - detects that iconv is in libc and thus -liconv is not needed for it
> - detects locale_charset in -liconv, but for some reason doesn't add it
> to CHARSET_LIB (as it would do with -lcharset if locale_charset() was
> found there instead of -liconv)
> - git doesn't build due to unresolved external locale_charset()
>
> Fix this by adding -liconv to CHARSET_LIB if locale_charset() is
> detected in this library.
>
> Signed-off-by: Dmitry Marakasov <amdmi3@amdmi3.ru>
> ---
Looks sensible; Dilyan, any comments?
> configure.ac | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git configure.ac configure.ac
> index 2f43393..3f5c644 100644
> --- configure.ac
> +++ configure.ac
> @@ -890,7 +890,7 @@ GIT_CONF_SUBST([HAVE_STRINGS_H])
> # and libcharset does
> CHARSET_LIB=
> AC_CHECK_LIB([iconv], [locale_charset],
> - [],
> + [CHARSET_LIB=-liconv],
> [AC_CHECK_LIB([charset], [locale_charset],
> [CHARSET_LIB=-lcharset])])
> GIT_CONF_SUBST([CHARSET_LIB])
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] configure.ac: link with -liconv for locale_charset()
2014-03-11 20:35 ` Junio C Hamano
@ 2014-03-11 22:37 ` Дилян Палаузов
2014-03-20 21:12 ` Junio C Hamano
2014-03-11 22:39 ` Dmitry Marakasov
2014-03-12 0:57 ` Dmitry Marakasov
2 siblings, 1 reply; 6+ messages in thread
From: Дилян Палаузов @ 2014-03-11 22:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, wxs, Dmitry Marakasov
Hello,
this changes effectively the meaning of CHARSET_LIB to
always/unconditionally contain the library with the charset_locale ()
function. The snippet at the end of the email updates the description
in /Makefile .
However, I checked now how gnulib deals with locale_charset (). Contary
to my expectation, where gnulib builds only functions if they are not
found in libraries on the target system, gnulib module localcharset
builds unconditionally from source the function locale_charset (). I
guess they do this in a portable way, but still.
My preference is to agree on universal approach for finding this
function, something like:
OK, if found in libiconv, else
OK, if found in libcharset, else
OK, here are the sources, build the function from them, and don't
look for it in (shared) libaries
I asked at bug-gnulib@ why they build locale_charset() before checking
for it in libiconv / libcharset. My posting shall appear at
http://lists.gnu.org/archive/html/bug-gnulib/2014-03/index.html . Let's
see what the answer will be.
I don't know if in the git codebase generally is refused to use gnulib
code, but if you agree on the above procedure with 3xOK, then
locale_charset() will be available even on systems, that don't have this
function in libcharset or libiconv.... Maybe /compat/poll/poll.[ch] in
git-core from gnulib had similar history.
Kind regards
Дилян
On 11.03.2014 21:35, Junio C Hamano wrote:
> Dmitry Marakasov <amdmi3@amdmi3.ru> writes:
>
>> On e.g. FreeBSD 10.x, the following situation is common:
>> - there's iconv implementation in libc, which has no locale_charset()
>> function
>> - there's GNU libiconv installed from Ports Collection
>>
>> Git build process
>> - detects that iconv is in libc and thus -liconv is not needed for it
>> - detects locale_charset in -liconv, but for some reason doesn't add it
>> to CHARSET_LIB (as it would do with -lcharset if locale_charset() was
>> found there instead of -liconv)
>> - git doesn't build due to unresolved external locale_charset()
>>
>> Fix this by adding -liconv to CHARSET_LIB if locale_charset() is
>> detected in this library.
>>
>> Signed-off-by: Dmitry Marakasov <amdmi3@amdmi3.ru>
>> ---
>
> Looks sensible; Dilyan, any comments?
>
>> configure.ac | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git configure.ac configure.ac
>> index 2f43393..3f5c644 100644
>> --- configure.ac
>> +++ configure.ac
>> @@ -890,7 +890,7 @@ GIT_CONF_SUBST([HAVE_STRINGS_H])
>> # and libcharset does
>> CHARSET_LIB=
>> AC_CHECK_LIB([iconv], [locale_charset],
>> - [],
>> + [CHARSET_LIB=-liconv],
>> [AC_CHECK_LIB([charset], [locale_charset],
>> [CHARSET_LIB=-lcharset])])
>> GIT_CONF_SUBST([CHARSET_LIB])
----------
diff --git a/Makefile b/Makefile
index dddaf4f..dce4694 100644
--- a/Makefile
+++ b/Makefile
@@ -59,9 +59,9 @@ all::
# FreeBSD can use either, but MinGW and some others need to use
# libcharset.h's locale_charset() instead.
#
-# Define CHARSET_LIB to you need to link with library other than -liconv to
+# Define CHARSET_LIB to the library you need to link with in order to
# use locale_charset() function. On some platforms this needs to set to
-# -lcharset
+# -lcharset, on others to -liconv .
#
# Define LIBC_CONTAINS_LIBINTL if your gettext implementation doesn't
# need -lintl when linking.
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] configure.ac: link with -liconv for locale_charset()
2014-03-11 20:35 ` Junio C Hamano
2014-03-11 22:37 ` Дилян Палаузов
@ 2014-03-11 22:39 ` Dmitry Marakasov
2014-03-12 0:57 ` Dmitry Marakasov
2 siblings, 0 replies; 6+ messages in thread
From: Dmitry Marakasov @ 2014-03-11 22:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: Дилян Палаузов,
git, wxs
* Junio C Hamano (gitster@pobox.com) wrote:
> > On e.g. FreeBSD 10.x, the following situation is common:
> > - there's iconv implementation in libc, which has no locale_charset()
> > function
> > - there's GNU libiconv installed from Ports Collection
> >
> > Git build process
> > - detects that iconv is in libc and thus -liconv is not needed for it
> > - detects locale_charset in -liconv, but for some reason doesn't add it
> > to CHARSET_LIB (as it would do with -lcharset if locale_charset() was
> > found there instead of -liconv)
> > - git doesn't build due to unresolved external locale_charset()
> >
> > Fix this by adding -liconv to CHARSET_LIB if locale_charset() is
> > detected in this library.
> >
> > Signed-off-by: Dmitry Marakasov <amdmi3@amdmi3.ru>
> > ---
>
> Looks sensible; Dilyan, any comments?
Addendum: build logs before and after the fix:
http://people.freebsd.org/~amdmi3/git-iconv-fail.log
http://people.freebsd.org/~amdmi3/git-iconv-fixed.log
--
Dmitry Marakasov . 55B5 0596 FF1E 8D84 5F56 9510 D35A 80DD F9D2 F77D
amdmi3@amdmi3.ru ..: jabber: amdmi3@jabber.ru http://www.amdmi3.ru
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] configure.ac: link with -liconv for locale_charset()
2014-03-11 20:35 ` Junio C Hamano
2014-03-11 22:37 ` Дилян Палаузов
2014-03-11 22:39 ` Dmitry Marakasov
@ 2014-03-12 0:57 ` Dmitry Marakasov
2 siblings, 0 replies; 6+ messages in thread
From: Dmitry Marakasov @ 2014-03-12 0:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: Дилян Палаузов,
git, wxs
* Junio C Hamano (gitster@pobox.com) wrote:
> Looks sensible; Dilyan, any comments?
Another addendum, comment from Tijl Coosemans <tijl@FreeBSD.org> who
just fixed this problem in FreeBSD ports (differently):
---
Please let upstream know they should either use iconv from libc +
nl_langinfo from libc, or iconv from libiconv + locale_charset from
libiconv, but not mix the two. Actually they could just always use
nl_langinfo when it's available because locale_charset is not much
more than an alias for it.
---
The fix used in ports was to just disable check for libcharset.h:
http://www.freebsd.org/cgi/query-pr.cgi?pr=187326#reply3
--
Dmitry Marakasov . 55B5 0596 FF1E 8D84 5F56 9510 D35A 80DD F9D2 F77D
amdmi3@amdmi3.ru ..: jabber: amdmi3@jabber.ru http://www.amdmi3.ru
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] configure.ac: link with -liconv for locale_charset()
2014-03-11 22:37 ` Дилян Палаузов
@ 2014-03-20 21:12 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2014-03-20 21:12 UTC (permalink / raw)
To: Дилян Палаузов
Cc: git, wxs, Dmitry Marakasov
Дилян Палаузов <dilyan.palauzov@aegee.org> writes:
> diff --git a/Makefile b/Makefile
> index dddaf4f..dce4694 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -59,9 +59,9 @@ all::
> # FreeBSD can use either, but MinGW and some others need to use
> # libcharset.h's locale_charset() instead.
> #
> -# Define CHARSET_LIB to you need to link with library other than -liconv to
> +# Define CHARSET_LIB to the library you need to link with in order to
> # use locale_charset() function. On some platforms this needs to set to
> -# -lcharset
> +# -lcharset, on others to -liconv .
> #
> # Define LIBC_CONTAINS_LIBINTL if your gettext implementation doesn't
> # need -lintl when linking.
This was unfortunately lost in the noise and I missed it. I think
the updated text is much more clear than the original.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-20 21:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-11 18:35 [PATCH] configure.ac: link with -liconv for locale_charset() Dmitry Marakasov
2014-03-11 20:35 ` Junio C Hamano
2014-03-11 22:37 ` Дилян Палаузов
2014-03-20 21:12 ` Junio C Hamano
2014-03-11 22:39 ` Dmitry Marakasov
2014-03-12 0:57 ` Dmitry Marakasov
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).