1315005 - New command: migrate-l10n

Status: NEW

(Firefox Build System :: Mach Core, enhancement)

Description

[Staś Małolepszy :stas]

In bug 1280685 we want to create a workflow for migrating legacy translations (DTD, properties) to the new FTL format used by L20n.

We'd like to have a new mach command, migrate-l10n, which is able to run migrations "spec" files written by developers and apply the results to l10n repos in form of commits, preserving the attribution of localizers.

The migration specification was discussed in https://github.com/mozilla-l10n/roadmap/issues/13#event-821050149. The attribution requirement was discussed in bug 1288141 and bug 1288146.

The migration tool will be implements as part of the python-l20n package. It requires an update to the in-tree version of compare-locale.  The mach command will also require python-hglib.

Comment 1

[Staś Małolepszy :stas]

Attachment: Bug 1315005 - Part 1. Python dependencies.

compare-locales:
    https://hg.mozilla.org/l10n/compare-locales/rev/e67f1d96992a

python-hglib:
    https://selenic.com/repo/python-hglib/rev/2.2

python-l20n:
    https://github.com/l20n/python-l20n/pull/5/commits/eb38d0f3cd6de9d6fca951caad0ac65e69542cbc

Review commit: https://reviewboard.mozilla.org/r/90510/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/90510/

Comment 2

[Staś Małolepszy :stas]

Attachment: Bug 1315005 - Part 2. The migrate-l10n command for mach.

Review commit: https://reviewboard.mozilla.org/r/90512/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/90512/

Comment 3

[Staś Małolepszy :stas]

Comment on attachment 8807316 [details]
Bug 1315005 - Part 2. The migrate-l10n command for mach.

Pike, can you take a look and let me know if this is going into the right direction?  Thanks!

Comment 4

[Mike Hommey [:glandium]]

I don't think this needs to be a mach command. Especially considering the limited scope and the fact that it's a one shot thing for every directory it will be run for.

Comment 5

[Staś Małolepszy :stas]

Axel, can you provide some more rationale behind the idea for this being a mach command?

Comment 6

[Axel Hecht [:Pike]]

Some expectations:

We'll incrementally migrate existing Firefox UI to l20n, and for each patch, there will be a migration. So we're likely looking at double-digit number of those migrations.

Given they're associated with the code landing on mozilla-central, I think it makes sense for those migrations to be in m-c, too. And by that, the code to run them.

We'll have the dependency on python-l20n as part of our build automation anyway, it's basically a dependency of compare-locales at the point where we land l20n.

Wether this is a mach command or not is really just icing on the cake, in terms of that command being able to run. I like the idea of having python module discovery done, some formalized out, etc.

I expect us to need this command for localizers on central on a more or less daily basis, and the whole migration should take at least two cycles. So I don't think of this as a one-time thing.

Comment 7

[Staś Małolepszy :stas]

Comment on attachment 8807315 [details]
Bug 1315005 - Part 1. Python dependencies.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/90510/diff/1-2/

Comment 8

[Staś Małolepszy :stas]

Comment on attachment 8807316 [details]
Bug 1315005 - Part 2. The migrate-l10n command for mach.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/90512/diff/1-2/

Comment 9

[Staś Małolepszy :stas]

Comment on attachment 8807315 [details]
Bug 1315005 - Part 1. Python dependencies.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/90510/diff/2-3/

Comment 10

[Staś Małolepszy :stas]

Comment on attachment 8807316 [details]
Bug 1315005 - Part 2. The migrate-l10n command for mach.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/90512/diff/2-3/

Comment 11

[Axel Hecht [:Pike]]

Comment on attachment 8807316 [details]
Bug 1315005 - Part 2. The migrate-l10n command for mach.

https://reviewboard.mozilla.org/r/90512/#review95254

I'm a bit wonky about our encoding story here at large, didn't yet give this a real spin.

Do we care about python3 in this code at all? 'cause hglib.util.b() is a no-op on python 2, too.

I think we should take out the argument parsing stuff out of l10n_migrate.py completely. Also, that probably doesn't need to be called l10n_migrate.py. Once we strip the args handling, is there enough code in there to warrant it still being a dedicated file?

For file structure, it's quite common to have python/foo/foo/__init__.py. Maybe python/mozl10n/mozl10n ? To avoid the obvious conflicts at the toplevel.

::: python/l10n/l10n_migrate.py:62
(Diff revision 3)
> +            )
> +
> +            print('    Committing changeset: {}'.format(message))
> +            if not dry_run:
> +                client.commit(
> +                    b(message), user=b(author), addremove=True

I don't think we should use b() here explicitly.

::: python/l10n/l10n_migrate.py:86
(Diff revision 3)
> +    parser.add_argument(
> +        '--localization-dir', type=str,
> +        help='directory for localization files'
> +    )
> +    parser.add_argument(
> +        '--blame', type=argparse.FileType(), default=None,

We should have this option on the mach command, too.

::: python/l10n/mach_commands.py:62
(Diff revision 3)
> +        from l10n_migrate import main
> +        from l10n_blame import Blame
> +
> +        if not lang or not migrations:
> +            print('You must specify --lang and at least one migration module.')
> +            return 1

We should move these constraints into the CommandArguments

::: python/l10n/mach_commands.py:72
(Diff revision 3)
> +
> +            if not localization_dir:
> +                localization_dir = mozpath.join(
> +                    self.topsrcdir,
> +                    self.substs['L10NBASEDIR']
> +                )

I think you copied a bug from the compare-locales mach command :-/.

toolkit/moz.configure sets L10NBASEDIR to an absolute path, and it resolves against the objdir, from all that I can tell, too.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.