Re: Stable ab/i18n branch

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Sun, Oct 17, 2010 at 04:44, Junio C Hamano <gitster@pobox.com> wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Ævar Arnfjörð Bjarmason wrote:
>>
>>> Could you please pick up the 160 commit version of this at:
>>>
>>>     git://github.com/avar/git.git ab/i18n
>>
>> This is a "give an inch and they'll ask for a mile" sort of thing, but
>> would it be possible to maintain a stable branch with the i18n
>> infrastructure that only gets rebased when there is reorganization
>> going on?
>
> People might have noticed that I've refrained to take other topics that
> may add new messages to 'next'.  I would wanted to merge ab/i18n early in
> the cycle soon after dust has settled after 1.7.3 release.  And I still
> do.

Good to hear. I didn't know that before, and that's really all I
wanted to know.

> Having said that, there are different classes of risks associated with
> i18n effort.
>
> (1) Regressions that even hit a NO_I18N build.
> (2) Regressions that hit LC_ALL=C execution in a !NO_I18N build.
> (3) Regressions that hit plumbing run in a non-C locale.
>
>  . i18n needs not just marking strings with _("string") but also needs to
>   fix code that manually formulates messages by series of strcat().  It
>   may need to start using allocations on the heap, with potential risk of
>   usual bugs (leaks, use-after-free, etc.) and performance degradation.
>
>  . Messages left unmarked with _("string"), or messages that are marked
>   with _("string") that shouldn't have, won't be serious issues for the
>   first two classes.  The latter is a serious regression for the
>   plumbing.
>
> We are all human, and misconversion during this process is possible, even
> though the above classes of regressions are unacceptable.  On the other
> hand, as long as the above three classes of regressions are minimum and
> quickly fixed/fixable, issues in non-C locale Porcelains are tolerable
> during the initial cut.
>
> I've looked at the patches in the series, and plan to take another look.
> I'm sure others on the list have checked the series, some with fine combs,
> too, and hopefully Ævar has fixed any such regression that has been
> reported and plans to do so for the ones discovered in the future.  As
> long as we are sure that we have done a reasonable effort to eyeball the
> patches, the logical next step would be to merge the series to 'next' for
> further testing.

Right. I'll have time to deal with any bugs that crop up, and I'm
reasonably sure it's OK as-is.

> (4) Incomplete *.po file, and languages without *.po file.
>
> Once we are sure that the series does not have the first two classes of
> issues, we can ask everybody to mark new strings in their series, iow,
> merge the i18n part to 'master'.  If we can do that sooner, it would be
> better, and we do not need specific l10n part from the series during that
> stage.
>
> A language that already has *.po file may lack necessary translation;
> there may be languages that do not have *.po file.  They can be added with
> a lot smaller risk later without unstabilizing the codebase.

Do you mean to re-arrange it so that there's a patch at the front of
the series that introduces gettext.h with only the fallbacks:

    #define _(s) (s)
    #define N_(s) (s)

And then merge the ~120 gettextize patches first and do the
infrastructure later?

That could be done, but just merging the whole 160 patch series and
turning it off by default would have pretty much the same effect. And
since I thought this was going to get merged soon-ish anyway I didn't
spend time on something like that.

> So where are we now?  I think a constantly rebased 160-patch series that
> has infrastructure bits and l10n bits mixed together is not very friendly
> to review for the first three classes of regressions (which are all I care
> about at this point) to help the series hit 'master' sooner.

I think we're basically at a point where merging it down to next ->
master is the logical next step.

> In any case, the branch merged to 'pu' has been replaced with the tip of
> the said branch from Ævar's repository now.

Thanks.