1445001 - Migration: strategy for strings removal

Status: RESOLVED WONTFIX

(Localization Infrastructure and Tools :: Fluent Migration, enhancement)

Description

[Francesco Lodolo [:flod]]

I've already thought about this problem, but it came up again while looking into bug 1444969.

If a bug removes a Fluent string from en-US (mozilla-central), and we run migration, we remove the string from all locales. But that's still needed for x-channel, e.g. in Beta.

A possible alternative would be to provide a path to gecko-strings-quarantine while invoking the migration script, and use that as reference?

Comment 1

[Francesco Lodolo [:flod]]

(In reply to Francesco Lodolo [:flod] from comment #0)
> A possible alternative would be to provide a path to
> gecko-strings-quarantine while invoking the migration script, and use that
> as reference?

On second thoughts, the migration should simply never remove strings. It's not its role to clean up files from obsolete strings, that's up to localizers or Pontoon.

Comment 2

[Francesco Lodolo [:flod]]

(In reply to Francesco Lodolo [:flod] from comment #1)
> On second thoughts, the migration should simply never remove strings. It's
> not its role to clean up files from obsolete strings, that's up to
> localizers or Pontoon.

To make sure we're on the same page: currently, the migration code removes a string if it's not available in en-US. See for example (we renamed a string in the 60 Nightly train to fix a migration error): https://hg.mozilla.org/l10n-central/pl/rev/a748e57d7be0

Comment 3

[Staś Małolepszy :stas]

> On second thoughts, the migration should simply never remove strings. It's not its role to clean up files from obsolete strings, that's up to localizers or Pontoon.

The migration code may remove strings because it uses the current en-US file as the reference. This is currently required to keep the order of messages as well as in order to populate comments in the resulting file.

If we were to solve this bug via a process change rather than a code change (which I think would be a good idea to at least try), the most natural approach would be to run the migrations against the xchannel's gecko-strings repository with en-US translations. Are there plans to have the xchannel code run on each checkin to mozilla-central?

Otherwise, we could try to integrate xchannel's merge into the migration code. It would help replace this part of `merge_resource`: https://github.com/projectfluent/python-fluent/blob/90984a0bf5e499a022da6a9bab311d4b187ceb1b/fluent/migrate/merge.py#L38-L43.

To test the waters, I gave it a try and ran into two (solvable) challenges with this approach:

  - xchannel uses compare_locales.Parser while fluent.migrate uses fluent.syntax.FluentParser,
  - xchannel's merge_channels operates on byte strings while fluent.migrate's merge_resource operates on Fluent ASTs.

With the WIP patch I'm also seeing 11 test failures, majority of them related to different ordering of messages in the final result and to duplicated comments (which might be a bug?).

Yet another approach might be to migrate locales into an interim directory and then xchannel-merge this directory with the existing files in l10n-central.


PS I'd like to rename merge_resource to tame the proliferation of the word "merge" in our code bases. We have l10n merge, xchannel merge, migration merge... How about `migrate_resource`?

Comment 4

[Staś Małolepszy :stas]

Attachment: WIP: use merge_channels in fluent.migrate.merge.merge_resource

Comment 5

[Francesco Lodolo [:flod]]

(In reply to Staś Małolepszy :stas from comment #3)
> If we were to solve this bug via a process change rather than a code change
> (which I think would be a good idea to at least try), the most natural
> approach would be to run the migrations against the xchannel's gecko-strings
> repository with en-US translations. 

It should run against the quarantine repository. gecko-strings doesn't have new strings until we actually run the migration (see FTL Migration Steps).

To run against gecko-strings-quarantine, would we only need to update all migrations? E.g. from the current code

    ctx.add_transforms(
        'browser/browser/preferences/preferences.ftl',
        'browser/locales/en-US/browser/preferences/preferences.ftl',

to this?

    ctx.add_transforms(
        'browser/browser/preferences/preferences.ftl',
        'browser/browser/preferences/preferences.ftl',

> Are there plans to have the xchannel code run on each checkin to mozilla-central?

Yes (bug 1353673), but there's still a lot of work to do before that happens.

Comment 6

[Staś Małolepszy :stas]

(In reply to Staś Małolepszy :stas from comment #3)

> To test the waters, I gave it a try and ran into two (solvable) challenges
> with this approach:

One more note to self: xchannel merge is not a silver bullet solution. We may need to keep using the en-US file as the template for cases when the target localized FTL file doesn't exist yet in l10n-central.

(In reply to Francesco Lodolo [:flod] from comment #5)

> It should run against the quarantine repository. gecko-strings doesn't have
> new strings until we actually run the migration (see FTL Migration Steps).

Right, thanks for the correction.

> To run against gecko-strings-quarantine, would we only need to update all
> migrations? E.g. from the current code
> 
>     ctx.add_transforms(
>         'browser/browser/preferences/preferences.ftl',
>         'browser/locales/en-US/browser/preferences/preferences.ftl',
> 
> to this?
> 
>     ctx.add_transforms(
>         'browser/browser/preferences/preferences.ftl',
>         'browser/browser/preferences/preferences.ftl',

I think so, yes.

Comment 7

[Francesco Lodolo [:flod]]

We're running migrations against gecko-strings-quarantine these days.