RFC patch, 17 partsgettext: make the simple parts of git-init localizable

Now that Git has the infrastructure for translation in next I'm going to start submitting patches to make the main porcelain translatable.

This series starts that work, and fixes and also fixes up some of the infrastructure (like the bug discussed in "Odd encoding issue with UTF-8 + gettext yields ? on non-ASCII"), and adds tests to make sure it's all working.

With it applied git-init is the one and only utility of the porcelain that's translatable. The series includes a translation of it into Icelandic and Polish.

I think it's ready to be applied. I tested it on Solaris, FreeBSD and Debian. But there's almost definitely something I'm missing in a series this big, so it's an RFC.

Marcin Cieślak (1):
  po/pl.po: add Polish translation

Ævar Arnfjörð Bjarmason (16):
  Makefile: A variable for options used by xgettext(1) calls
  Makefile: provide a --msgid-bugs-address to xgettext(1)
  Makefile: tell xgettext(1) that our source is in UTF-8
  builtin.h: Include gettext.h
  gettext: make the simple parts of git-init localizable
  gettext: localize the main git-init message
  gettext.c: work around us not using setlocale(LC_CTYPE, "")
  gettext tests: test if $VERSION exists before using it
  gettext tests: update test/is.po to match t/t0200/test.c
  gettext tests: add detection for is_IS.ISO-8859-1 locale
  gettext tests: test message re-encoding under Shell
  gettext tests: test re-encoding with a UTF-8 msgid under Shell
  gettext tests: mark a test message as not needing translation
  po/is.po: msgmerge and add Language: header
  po/is.po: add Icelandic translation
  gettext tests: test message re-encoding under C

 Makefile                           |    7 +-
 builtin.h                          |    1 +
 builtin/init-db.c                  |   56 ++++++-----
 gettext.c                          |    6 +
 po/is.po                           |  153 ++++++++++++++++++++++++++++--
 po/pl.po                           |  187 ++++++++++++++++++++++++++++++++++++
 t/lib-gettext.sh                   |   26 +++++-
 t/t0200-gettext-basic.sh           |    4 +-
 t/t0200/test.c                     |   10 ++
 t/t0202/test.pl                    |    4 +-
 t/t0204-gettext-reencode-sanity.sh |   78 +++++++++++++++
 11 files changed, 490 insertions(+), 42 deletions(-)
 create mode 100644 po/pl.po
 create mode 100755 t/t0204-gettext-reencode-sanity.sh

Change the Makefile code to use a variable for the options that all the xgettext(1) invocations are using. This makes it more readable, and makes it easier to add more standard options.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

Ævar Arnfjörð Bjarmason wrote:

The long lines are a bit scary. :)

Maybe more of it could be pulled out into variables. As a side-effect, users could override some settings from the command line. Maybe something like this?

 LOCALIZED_C = $(C_OBJ:o=c) t/t0200/test.c
 LOCALIZED_SH = $(SCRIPT_SH) t/t0200/test.sh
 LOCALIZED_PERL = $(SCRIPT_PERL) t/t0200/test.perl

 XGETTEXT_OPTIONS = --add-comments
 XGETTEXT_OPTIONS_C = $(XGETTEXT_OPTIONS) -k_ -kN_ -LC
 XGETTEXT_OPTIONS_SH = $(XGETTEXT_OPTIONS) -LShell
 XGETTEXT_OPTIONS_PERL = $(XGETTEXT_OPTIONS) -k__ -LPerl

 po/git.pot:
	rm -f $@+
	$(XGETTEXT) -o$@+ $(XGETTEXT_OPTIONS_C) $(LOCALIZED_C)
	$(XGETTEXT) -j -o$@+ $(XGETTEXT_OPTIONS_SH) $(LOCALIZED_SH)
	$(XGETTEXT) -j -o$@+ $(XGETTEXT_OPTIONS_PERL) $(LOCALIZED_PERL)
	mv $@+ $@

On Tue, Aug 31, 2010 at 14:51, Jonathan Nieder <jrnieder@gmail.com> wrote:

Thanks. I worked that into v2.

Change the invocations of xgettext to use the --msgid-bugs-address option. This has the effect of adding a Report-Msgid-Bugs-To header to the git.pot and the derived *.po files. Doing so is recommended by the gettext manual.

If this isn't added the Report-Msgid-Bugs-To header already in po/is.po and other PO files will be removed by msgmerge(1).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

By default xgettext(1) assumes that source code is in US-ASCII, change that to UTF-8 for our case.

I'm not planning to include non-ASCII in any of the main Git interface strings. But this'll be used for a gettext regression test to make sure this works if we ever want to go this route, and to check that the gettext implementation is sane in this regard.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Change builtin.h to include gettext.h. This is needed in order to translate Git's builtin commands (like git-init), which don't include gettext.h otherwise.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Change the user visible strings in init-db.c to use gettext localizations. This only converts messages which needed to be changed from "foo" to _("foo"), and didn't need any TRANSLATORS comments.

I haven't marked the messages in init_db_usage or init_db_options for translation, since that would require additional changes in parse-options.c. Those can be done later.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/init-db.c |   46 +++++++++++++++++++++++-----------------------
 1 files changed, 23 insertions(+), 23 deletions(-)

Ævar Arnfjörð Bjarmason wrote:

Sensible.

I wonder if die() should not just be taught to automatically look up translations for its format string (could gettext handle that?).

Although we try not to change plumbing error messages without good reason, details of error messages change often enough that imvho scripts should not be relying on them.

Will strerror() cope correctly without LC_CTYPE set up? (Not part of this series, just something I was reminded of.)

On Tue, Aug 31, 2010 at 15:03, Jonathan Nieder <jrnieder@gmail.com> wrote:

It's always a two step process. First you have to mark the messages for translation, then you have to call gettext() (or _())on them to make a lookup in the message catalog.

The only way that could work is if I taught xgettext to extract strings passed to die(), but then managing the false positives would probably be more effort than just marking them manually, and it would be a big load on the translators:

    $ ack 'die\("(.*?)"' --output '$1' *[ch] builtin/*[ch] | sort -u | wc -l
    1153

My GNU/Linux strerror(3) claims to use LC_MESSAGES, but I didn't test it.

Ævar Arnfjörð Bjarmason wrote:

To pursue this a little further: would there be any false positives?

We could avoid overwhelming translators by waiting until a file has been fulling gettextized before allowing xgettext to scavenge it (i.e., temporarily using a hard-coded list of files in the xgettext invocation).

Sounds like no, then.

$ cat foo.c #include <stdio.h> #include <locale.h> #include <errno.h>

int main(void)
{
        setlocale(LC_ALL, "");
        setlocale(LC_CTYPE, "C");
        errno = ENODEV;
        perror("test");
        return 0;
}
$ make foo
cc     foo.c   -o foo
$ ./foo
test: No such device
$ LANG=de_DE.UTF-8 ./foo 
test: Kein passendes Ger?t gefunden

On Tue, Aug 31, 2010 at 15:44, Jonathan Nieder <jrnieder@gmail.com> wrote:

Maybe I can see a few probable plumbing messages here, but probably nothing that's unmanagable: http://gist.github.com/559255

Yeah we could go this route. But I'm a bit uneasy about it. The way I'm doing it now I have to manually look at every message to determine if it's a porcelain error or not.

But it's easier to accidentally mark something with a filelist like that, or to mark a future plumbing die message that gets introduced after a file has made the OK list.

Maybe we should have die_plumbing() and die_porcelain() functions to indicate the nature of the message, although we could get the same thing with die() and die(_()) once I'm done.

What about with MESSAGES instead of ALL, like we're doing?

    setlocale(LC_MESSAGES, "");
    setlocale(LC_CTYPE, "C");

I don't have a box here with a localized C library to test it on.

Ævar Arnfjörð Bjarmason wrote:

Same result, alas.

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

To me, die() vs die(_()) feels like the lessor of two evils ;-).

Thanks.

Change the git-init "Initialized empty Git repository" message and its variants to use gettext.

This is one of the messages that could do with splitting up, I had a WIP patch to do that that began like this:

    const char *reinit_shared   = _("Reinitialized existing shared Git repository in %s\n");
    const char *init_shared	    = _("Initialized empty shared Git repository in %s\n");
    const char *reinit_noshared = _("Reinitialized existing Git repository in %s\n");
    const char *init_noshared   = _("Initialized empty Git repository in %s\n");

But in the first round of gettextization I'm aiming to keep code changes to a minimum for ease of review.

We can solicit input from translators about which messages that use too much sprintf-ing are troublesome, and change those later.

Note that the TRANSLATORS comment doesn't use the usual Git style. This is because everything from "/* TRANSLATORS: " to "*/" will extracted as-is xgettext(1) and presented to translators, including newlines and leading "*"'s. There seems to be no way to change that, short of patching xgettext itself.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/init-db.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

Ævar Arnfjörð Bjarmason wrote:

How would it cope with the following?

	/* TRANSLATORS:
	 * The first '%s' is either "Reinitialized existing" or
	 * "Initialized empty", the second " shared" or "", and
	 * the last '%s%s' is the verbatim directory name.
	 */

The leading column of stars makes it easier to distinguish code from comments. (Plus I am not too happy to read code with two inconsistent comment styles used.)

On Tue, Aug 31, 2010 at 15:10, Jonathan Nieder <jrnieder@gmail.com> wrote:

As I was (trying) to get across in the the commit message it'll make the *'s part of the message. I.e.:

    TRANSLATORS: * The first '%s' is either "Reinitialized existing"
    or * "Initialized empty", the second " shared" or "", and * the
    last '%s%s' is the verbatim directory name.

Ævar Arnfjörð Bjarmason wrote:

[...]

Ah, so you meant "not including newlines but including leading '*''s".

Hmm, this is annoying. Searching existing projects, it seems that a lot of people are just not using the TRANSLATORS: feature except for one-line comments.

Thanks for the explanation.

Change the gettext setup code to arrange for messages to be emitted in the user's requested encoding, but avoid setting LC_CTYPE from the environment for the whole program.

In 107880a I removed our use of setlocale(LC_CTYPE, "") because of a bug in the GNU C Library [1]. Even if it wasn't for that bug we wouldn't want to use LC_CTYPE at this point, because it'd require auditing all the code that uses C functions whose semantics are modified by LC_CTYPE.

But only setting LC_MESSAGES as we do creates a problem, since we declare the encoding of our PO files[2] the gettext implementation will try to recode it to the user's locale, but without LC_CTYPE it'll emit something like this on 'git init'

    Bj? til t?ma Git lind ? /hl/agh/.git/

Gettext knows about the encoding of our PO file, but we haven't told it about the user's encoding, so all the non-US-ASCII characters get encoded to question marks.

But we're in luck! We can set LC_CTYPE from the environment only while we call nl_langinfo and bind_textdomain_codeset. That suffices to tell gettext what encoding it should emit in, so it'll now say:

    Bjó til tóma Git lind í /hl/agh/.git/

And the equivalent ISO-8859-1 string will be emitted under a ISO-8859-1 locale.

With this change way we get the advantages of setting LC_CTYPE (talk to the user in his language/encoding), without the drawbacks (changed semantics for C functions we rely on).

In the long term we should probably see about getting that bug in glibc fixed, and audit our code so it won't fall apart under a non-C locale.

1. http://sourceware.org/bugzilla/show_bug.cgi?id=6530
2. E.g. "Content-Type: text/plain; charset=UTF-8\n" in po/is.po

Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 gettext.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

Ævar Arnfjörð Bjarmason wrote:

Future readers might benefit from a reminder that it is vsnprintf that is broken.

Aside, not about this patch: glibc printf can be very convenient for translators, because of format strings like "%4$s". Do other common platforms like FreeBSD and Mingw have something similar?

For the curious: we cannot use

	setlocale(LC_CTYPE, "");
	charset = nl_langinfo(CODESET);
	setlocale(LC_CTYPE, "C");
	bind_textdomain_codeset("git", charset);

because nl_langinfo returns a pointer to a static buffer that might be wiped out by setlocale() iirc.

Thanks.

On Tue, 31 Aug 2010, Jonathan Nieder wrote:

Speaking for FreeBSD:

% svn log -r21674 printf.3 ------------------------------------------------------------------------ r21674 | jkh | 1997-01-14 08:31:39 +0100 (wto) | 8 linii

The following patch to lib/libc/stdio implements positional arguments in a manner consistent with other implementations. Its done in a way that adds only a tiny amount of overhead when positional arguments are not used. I also have a test program to go with this, but don't know where it belongs in the tree.

Submitted-By: Bill Fenner <fenner@FreeBSD.ORG>

------------------------------------------------------------------------

Solaris 9 has it, too.

--Marcin

Marcin Cieslak wrote:

[...]

Thanks for checking.  That's good to hear.

On Tue, Aug 31, 2010 at 15:18, Jonathan Nieder <jrnieder@gmail.com> wrote:

I'll work something in about that.

I certainly hope so. I was planning on documenting its usage, Johannes?

But I'm hopefully sidestepping that issue by passing it to bind_textdomain_codeset() right away.

Ævar Arnfjörð Bjarmason wrote:

Sorry, my knowledge was just outdated. It's in posix[1] now, which I think means we can expect it to be in any recent libc.

Unfortunately msys uses newlib 1.9.0 afaict, from 2001. The %4$s support was introduced[2] to newlib by Eric Blake on 2007-04-25.

[1] http://www.opengroup.org/onlinepubs/9699919799/functions/printf.html#tag_16_159_03 [2] http://sourceware.org/cgi-bin/cvsweb.cgi/src/newlib/libc/stdio/vfprintf.c?cvsroot=src

On Wed, Sep 1, 2010 at 12:45 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Git for Windows doesn't use the MSYS-runtime for Git itself, it only use the MSYS-environment to build etc. It's using MSVCRT.DLL as the CRT, which is even worse feature-wise.

http://msdn.microsoft.com/en-us/library/56e442dc.aspx describes the format specification fields supported, and it does not look promising.

Versions of Locale::Messages before 1.17 didn't have a $VERSION variable. This caused test failures on boxes that had this old version installed, since the warnings pragma emits warnings on STDERR, which fails the test.

Change the test to work around this by first checking if the $VERSION variable is defined before using it.

Reported-by: Jens Lehmann <Jens.Lehmann@web.de>
Tested-by: Jens Lehmann <Jens.Lehmann@web.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0202/test.pl |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

Change test.c to use '' quotes around "git help COMMAND" as git.c does. An earlier version of the gettext series didn't use '' quotes, but I hadn't run msgmerge since then so I didn't spot it.

For reference, the msgmerge command:

    msgmerge --backup=off -U is.po git.pot

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 po/is.po                 |    4 ++--
 t/t0200-gettext-basic.sh |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Add a GETTEXT_ISO_LOCALE prerequisite to lib-gettext.sh, it'll be set if we have an is_IS.ISO-8859-1 locale.

This is needed for an upcoming test that checks if our gettext library can recode our UTF-8 po/is.po to is_IS.ISO-8859-1 on request.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/lib-gettext.sh |   26 +++++++++++++++++++++++---
 1 files changed, 23 insertions(+), 3 deletions(-)

Our PO files are written in UTF-8, but We're not using setlocale(LC_CTYPE, "") so it's not a given that someone with e.g. a ISO-8859-1 locale will get messages in ISO-8859-1, and not UTF-8.

Introduce a new test to test for this, it uses the recently added GETTEXT_ISO_LOCALE prerequisite.

This patch only tests the shellscript portion of our gettext interface. I can't get any of these tests to fail on any of the gettext implementations I have around, even without the previous patch to gettext.c. But having exhaustive tests in this area is good regardless.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 po/is.po                           |   12 ++++++++++
 t/t0200/test.c                     |    6 +++++
 t/t0204-gettext-reencode-sanity.sh |   43 ++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 0 deletions(-)
 create mode 100755 t/t0204-gettext-reencode-sanity.sh

A test that tests that calling gettext on a UTF-8 msgid works, and that recoding the resulting string works too.

This test uses the --from-code=UTF-8 xgettext(1) argument introduced in an earlier patch.

This patch only tests the shellscript portion of our gettext interface. I can't get any of these tests to fail on any of the gettext implementations I have around, even without the previous patch to gettext.c. But having exhaustive tests in this area is good regardless.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 po/is.po                           |    6 ++++++
 t/t0200/test.c                     |    3 +++
 t/t0204-gettext-reencode-sanity.sh |   21 +++++++++++++++++++++
 3 files changed, 30 insertions(+), 0 deletions(-)

This was the only message in t/t0200/* that didn't have a TRANSLATORS comment, without it translators will waste time translating this needlessly.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0200/test.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Change is.po to use the header order added by msgmerge(1), this'll make subsequent diffs to it smaller and easier to manage. While I'm at it add a Language header indicating that the file is in Icelandic.

When we add more *.po files later we should add a Language header to those as well.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 po/is.po |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

Translate the non-TEST messages added in recent patches against init-db.c. This brings Icelandic translation coverage up to 100%.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 po/is.po |  135 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 127 insertions(+), 8 deletions(-)

Ævar Arnfjörð Bjarmason wrote:

Is there a diff driver that will ignore these --add-location lines?

Alternatively, would it be possible to get msgmerge and xgettext to provide the filenames without the line numbers? My experience is that most translation diffs are very hard to read because about 80% noise. :(

Aside from that, this looks good and sane (well, the English part I can read does).

On Tue, Aug 31, 2010 at 15:29, Jonathan Nieder <jrnieder@gmail.com> wrote:

Some context, an earlier discussion on this: http://kerneltrap.org/mailarchive/git/2010/5/30/31415/thread#mid-31415

Removing them will have some negative effects. Gettext uses them for its message fuzzying logic, and they enable you to jump directly to a source definition.

I've found that it's easier to just go with the flow as far as the gettext tools are concerned, and they seem to really like to have these line numbers in :)

On Tue, Aug 31, 2010 at 7:01 PM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

msgmerge and xgettext does seem to have the --no-location flag to avoid these annotations from being generated. The documentation does say "Note that using this option makes it harder for technically skilled translators to understand each message's context. ", though. But perhaps the annotated versions could be generated when needed and never checked-in (similar to what you suggested in that e-mail)? It sounds to me like that would give us the best of all worlds. If it's possible, that is.

On Tue, Aug 31, 2010 at 19:14, Erik Faye-Lund <kusmabite@gmail.com> wrote:

It's certainly possible. But each time you worked with these files you'd have add the line numbers yourself to generate the context, then translate, then remove them again, then submit your patch.

I just think it's overly tedious work for getting smaller diffs.

On Tue, Aug 31, 2010 at 9:32 PM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

You know, computers are pretty good at automating tedious jobs ;)

I was thinking something along the lines of having po/is.po etc, and generating po/is.ann.po (or something) from these (+source) so translators can see the text in the context of the code. That is, they edit is.po, and generate is.ann.po when they want to see the context. Then we can put *.ann.po in .gitignore, and it's really just a matter of editing, doing "make po/is.ann.po", and voila.

But I dunno, I haven't been using these tools much myself.

From: Marcin Cieślak <saper@saper.info>

Translate all the translatable messages currently in Git, except for the 10 TEST messages that shouldn't be translated.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Marcin Cieślak <saper@saper.info>
---
 po/pl.po |  187 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 187 insertions(+), 0 deletions(-)
 create mode 100644 po/pl.po

Add tests for message re-encoding under C. Unlike the Shell tests these tests will break under GNU libintl if the recent patch to gettext.c is reverted. So this serves as a regression test for that issue.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0204-gettext-reencode-sanity.sh |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Thanks; will queue them.

I however strongly suspect that we would be better off first kicking the earlier parts of i18n topic out of 'next' back to 'pu', as I am hoping that we can declare feature freeze for 1.7.3 by the end of this week at the latest, and you can never tell if we got the "infrastructure" right without playing with a real "user of the infrastructure" like this 17 patch series, which means that the part of i18n topic that is already in 'next' cannot be part of 1.7.3 --- it is way premature.

It was somewhat unfortunate and sad that your "test" series depended on a few patches from the i18n series, which means it is now taken hostage to the latter. I'd rather want to have the "test" series in the 1.7.3, and we need to think about a way to untangle the two topics.

Change the Git build process so that gettext is no longer built by default as part of Git releases, and include a loud warning to downstream distributors in INSTALL saying that they shouldn't enable it.

We're still working out the details of the gettext infrastructure and currently only a small portion of Git can be translated, so it's premature to enable it by default.

However it's painful for everyone if the source changes needed for gettext can't be made in pu without conflicting with other series that apply to maint/master/next.

When gettext is disabled the whole functionality basically boils down to this macro definition (or Shell and Perl equivalents) and a few skipping tests:

    #define _(s) (s)

So it's a lot less to get right than if we were enabling it by default.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

On Mon, Aug 30, 2010 at 21:42, Junio C Hamano <gitster@pobox.com> wrote:

Thanks!

I definitely agree that it's premature at this point, if for no other reason than that I'll never be able to make most of the main-porcelain translatable by the end of the week.

However, there's another way to do this. Simply disable it by default in releases. Which this patch implements.

I considered the possibility that we might want to disable it in select branches when I wrote it, so it's easy to do so. And IMO a better option due to the reasons cited in the commit message attached to the PATCH.

Yeah I wasn't very careful about keeping the bits already in pu separate. Untangling them shouldn't be that hard though, but hopefully with this patch we won't have to go that route.

 INSTALL      |   17 +++++++++++++++--
 Makefile     |    4 ++++
 configure.ac |    2 +-
 3 files changed, 20 insertions(+), 3 deletions(-)

On Mon, Aug 30, 2010 at 21:42, Junio C Hamano <gitster@pobox.com> wrote:

There's a v2 available here: http://github.com/avar/git/tree/gettextize-git-mainporcelain-v2

Or: git://github.com/avar/git.git gettextize-git-mainporcelain-v2

It:

 * contains the "gettext: Make NO_GETTEXT=YesPlease the default in
   releases" patch. Please only apply that to next, not pu.

 * A "Makefile: use variables and shorter lines for xgettext" patch,
   which implements Jonathan's suggestion of using more variables for
   the xgettext invocation.

 * Elaborates on the issues facing us in the "gettext.c: work around
   us not using setlocale(LC_CTYPE, "")" commit message, and mentions
   the perror(3) issue.

I didn't send it to the list because it's a huge series and this is a little fixup, and I rebased it to inject the Makefile patch early in the series, which isn't easy to express in a v2 PATCH.

Thanks.

I can also send it to list if you want, but I suspect this is better.

Ævar Arnfjörð Bjarmason wrote:

Is that against "next"? For an RFC that's okay, but for a series that will be part of a release it can be tedious to separate out (not all topics in "next" necessarily are released, and "next" traditionally gets rewound after each release).

Comments on the patches:

 Makefile: use variables and shorter lines for xgettext

The -o<whatever> passed to cc isn't usually included in CFLAGS, and a part of me is similarly uncomfortable with including it in XGETTEXT_OPTIONS. Isn't that parameter something that should be possible to change in the build system independently from the user's XGETTEXT_OPTIONS preferences?

 gettext.c: work around us not using setlocale(LC_CTYPE, "")

The perror() problem shows up with strerror(), too, of course. (perror just made for an easier demo.)

 gettext: Make NO_GETTEXT=YesPlease the default in releases

Copy-edits for the notes in INSTALL:

s/EXPERIMENTAL/experimental/?  No need to shout.

I'd also s/currently // since this will not be current after a while.

Similarly I'd s/really //. If we want to dissuade people from trying it out, we should probably do that with more explicit statements.

Not sure what this means.  Maybe:

 The infrastructure is only included in this release to avoid
 complications in building other work on top of it.  If you turn
 it on, expect breakage.

On Tue, Aug 31, 2010 at 18:08, Jonathan Nieder <jrnieder@gmail.com> wrote:

Yes it's against next since it had the gettext series merged and next is about to be released isn't it? I could base it on something else if that's appropriate.

Maybe, but it'll always be --output=po/git.pot so I saw no reason to seperate it. Should it be? The -o for the C compiler changes, but the "make pot" target will always write to po/git.pot.

Yeah, and everything external like that, unfortunately.

OKEY THEN!

ok.

Thanks.

Yes, that's better. I've worked all these into a gettextize-git-mainporcelain-v3 branch at the same repository:

    http://github.com/avar/git/compare/gettextize-git-mainporcelain-v2...gettextize-git-mainporcelain-v3

Or: git://github.com/avar/git.git gettextize-git-mainporcelain-v3

Ævar Arnfjörð Bjarmason wrote:

No big deal. I am vaguely worried about interrupted "make" invocations. Don't gettextized packages typically do something like this?

	remove_creation_date() {
		sed '#!/bin/sed -f
			/^"POT-Creation-Date: .*"$/!b
			x
			# Test if the hold space is empty.
			s/P/P/
			ta
			# Yes it was empty. First occurrence. Remove the line.
			g
			d
			bb
			:a
			# The hold space was nonempty. Following occurrences. Do nothing.
			x
			:b'
	}
	(cd po && xgettext --default-domain=git ...)
	set -e; \
	if test -f po/git.pot &&
		remove_creation_date <po/git.pot >po/git.1po+ &&
		remove_creation_date <po/git.po >po/git.2po+ &&
		cmp po/git.1po+ po/git.2po+; \
	then \
		rm -f po/git.1po+ po/git.2po+ po/git.po; \
	else \
		rm -f po/git.1po+ po/git.2po+ po/git.pot; \
		mv po/git.po po/git.pot; \
	fi

In particular, to avoid using a partial .pot file, one might want to write to po/git.pot+, po/git.po, or similar and then rename it.

Looks good.

On Tue, Aug 31, 2010 at 19:22, Jonathan Nieder <jrnieder@gmail.com> wrote:

I don't know what they usually do. But that looks like a lot of work to work around a very rare potential edge case. "make pot" is only ever run manually by a translator right before msgmerge.

In the very rare case where make is interrupted and git.pot is left in a partial state you'll notice because your msgmerge fails.

Thanks for checking it out.

Ævar Arnfjörð Bjarmason wrote:

[...]

Ah, to work around the interrupted build case is simpler. grepping for "mv" in Documentation/Makefile would show some examples.

The above incantation is to avoid changing the timestamp on the .pot file when it does not change.

I don't know enough about translation workflows. Don't people sometimes automatically run msgmerge at build time to get some okay fuzzy messages when the translation team is off on vacation?

Thanks for doing the hard work.

Ævar Arnfjörð Bjarmason:

Very interesting. I would like to contribute a Swedish translation as time permits.

Any chance the translations could be co-ordinated through Translation Project <URL:http://translationproject.org/>? I know I suggested this, and was turned down, for gitk and git-gui, but this translation is potentially larger and could benefit from the co-ordinated effort.

On Tue, Aug 31, 2010 at 11:08, Peter Krefting <peter@softwolves.pp.se> wrote:

Great!

Something like that would be welcome. Personally I'm happy with editing *.po files locally with Emacs's po-mode, but to get more translators we probably want a friendly web interface like that at some point. Preferably with an active translation community.

I meant to look at this myself at some point, but if you could help that'd be great!

The only reference I could find to a previous git + translationproject.org discussion was this: http://kerneltrap.org/mailarchive/git/2008/3/14/1163164 Is that the one you're talking about?

I've used Launchpad somewhat for translating and it's friendly to contributors & has an active community, but it seems to require that we BSD-license our translations[1], which would be a showstopper since we'd have to contact everyone who's been submitting GPL-2-only strings to Git for the last 5 years.

Translationproject seems to have a similar requirement[2], but they seem require you to send a letter to the FSF through snail mail before you can contribute (maybe not, I didn't read all their docs carefully). That would be a major hurdle to casual contributors.

1. https://help.launchpad.net/Translations/LicensingFAQ
2. http://translationproject.org/disclaim.txt

Ævar Arnfjörð Bjarmason:

Well, it's not a web interface for translating, currently, just for keeping track on what needs to be translated by whom (and for language teams to find what to translate). You still edit the PO files in whatever you prefer.

Translation Project is indeed very active.

Yes.

Only when you translate FSF software for which you need to send one of the letters that claim that you are handing over copyright to the FSF. That is not needed in this case, and translators working on such projects do not need to send one.

For the project side, you bascially send a POT file every now and then, between string-freeze and release, and update the PO files that come back from the project every now and then. I've been using the system from both sides and it's lightweight enough to be workable.

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

I don't think so:

,----[ https://help.launchpad.net/Translations/LicensingFAQ ]
| I have no problem with BSD myself, but I also uploaded translations
| from upstream. What do I do?
| 
| As long as the uploads were marked as translations that were published
| elsewhere, they fall under a separate copyright regime: those imports
| will retain their original copyright license. The BSD licence only
| applies to translations that are (as far as the system knows) original
| to Launchpad.
`----

So, my understanding is: Git's code, and strings, would remain what they are, but things contributed _through launchpad_ would be BSD.

On Tue, Aug 31, 2010 at 12:02, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:

Ah yes. It seems as if Git's strings and derived works would be marked GPL-2. That sounds good. Like I said I hadn't looked in any of this in detail, or the legal issues involved.

Anyway, the main issue with Launchpad that I've had as a user/translator is getting translations back out of the system. To download a *.po file you have to log in and submit a request for a *.po file download. Sometimes it can take hours for you to get a mail back.

Downloading a large amount of translations through this system in Launchpad would be very painful. But maybe they have a better API now that we could use.

Ævar Arnfjörð Bjarmason wrote:

I've written some comments on specific patches; the rest looks good to me.

Thanks for moving this forward.

On Tue, Aug 31, 2010 at 15:32, Jonathan Nieder <jrnieder@gmail.com> wrote:

I'll try to reply to them all.

And thanks for your continuing efforts in reviewing all of this.