git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Vadym Kochan <vadim4j@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/1] configure.ac: Properly check for libintl
Date: Fri, 19 Apr 2019 13:56:48 +0900	[thread overview]
Message-ID: <xmqqmukmegof.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20190418082844.GA10068@lwo1-lhp-f71841> (Vadym Kochan's message of "Thu, 18 Apr 2019 11:28:44 +0300")

Vadym Kochan <vadim4j@gmail.com> writes:

>>     Some libc implementations have function called gettext() that
>>     can be linked via -lc without -lintl, but these are mere stubs
>>     and do not do useful i18n.  On these systems, if a program that
>>     calls gettext() is built _with_ "#include <libintl.h>", the
>>     linker calls for the real version (e.g. libintl_gettext()) and
>>     that can be satisfied only by linking with -lintl.
>> 
>>     The current check to see if -lc provides with gettext() is
>>     sufficient for libc implementations like GNU libc that actually
>>     has full fledged gettext(); to detect libc with stub gettext()
>>     and libintl with real gettext(), aliased via <libintl.h>, the
>>     check to see if -lintl is necessary must be done with a sample
>>     source that #include's the header file.
>> 
>> Is that what is going on and why this patch is needed?
>> 
> Yes you are correct. 'gettext' even might be defined as libintl_gettext.

With this exchange, I was aiming for extracting a more useful title
for this patch out of you ;-), and I think I accomplished my goal.

"Properly" is fairly a useless adverb in the context of a patch
title, as it does not tell us why we thought the way in which the
updated code works is more "proper".  In addition, because no code
is perfect, future developers are bound to find something inproperly
done in checking for libintl after this patch gets applied.  It is
better to say the most important thing the change does concisely and
concretely.

I think

	autoconf: #include <libintl.h> when checking for gettext()

is probably a better title.  

Together with your originally proposed log message, which we now
know explains why this inclusion makes a difference sufficiently to
be understandable by an average Git developer, the resulting commit
will communicate to our future developers the reason why we thought
this was a good change clearly.

Thanks.

  reply	other threads:[~2019-04-19 18:49 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-18  5:04 [PATCH 1/1] configure.ac: Properly check for libintl Vadim Kochan
2019-04-18  5:45 ` Junio C Hamano
2019-04-18  8:28   ` Vadym Kochan
2019-04-19  4:56     ` Junio C Hamano [this message]
2019-04-19  5:12       ` Vadim Kochan
2019-04-19  6:37         ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqmukmegof.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=vadim4j@gmail.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.
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).