1424171 - Support localized files with different relative path in l10n-central than in source tree.

Status: RESOLVED FIXED

(Thunderbird :: Build Config, enhancement)

Description

[Tom Prince [:tomprince]]

As part of moving comm-central to taskcluster, we are reworking it to support building with comm-central checked out in a subdirectory of mozilla-central. This means that files are in e.g. `comm/mail` in the source directory, but in `mail/` in l10n-central. 

There are a number of places where this inconsistency causes problems with the build system:

- config/config.mk / mozbuild.actions.jar_marker:
  - jar_maker has logic to find localized files based on the topsrcdir and relativesrcdir, which get bassed in from the make variables of the same names. Those values don't work for a comm-central checked out inside mozilla-central. If instead, the top of the comm-central checkout and the relative path from there is passed, then things appear to work correctly. I'm not sure what the best way to address this is. One option would be to have a level indirection in what gets passed to jar_maker, and override this inside comm-central locale Makefiles. Or, the logic in jar_maker could be expanded to allow different relative paths for the source and l10n-central directories.

- toolkit/locales/l10n.mk / compare-locales:
  - We need to pass in some defines from MOZ_APP_NAME/locales/Makefile.in to l10n.toml to support both source layouts (at least until some time after 57)
  - We also need some way parameterize the basepath in l10n.toml

- (future) mozbuild LOCALIZED_FILES et al support similar to the above

Comment 1

[Axel Hecht [:Pike]]

(In reply to Tom Prince [:tomprince] from comment #0)
> - toolkit/locales/l10n.mk / compare-locales:
>   - We need to pass in some defines from MOZ_APP_NAME/locales/Makefile.in to
> l10n.toml to support both source layouts (at least until some time after 57)

I would think that passing `-Dmozilla=../` should work.

>   - We also need some way parameterize the basepath in l10n.toml

I don't think that's necessary.

Comment 2

[Tom Prince [:tomprince]]

(In reply to Axel Hecht [:Pike] from comment #1)
> (In reply to Tom Prince [:tomprince] from comment #0)
> > - toolkit/locales/l10n.mk / compare-locales:
> >   - We need to pass in some defines from MOZ_APP_NAME/locales/Makefile.in to
> > l10n.toml to support both source layouts (at least until some time after 57)
> 
> I would think that passing `-Dmozilla=../` should work.

Yep. Just need a way to thread those options from comm/mail/locales/{Makefile.in,moz.build} to compare-locales
 
> >   - We also need some way parameterize the basepath in l10n.toml
> 
> I don't think that's necessary.

Ah. I had missed the trick of keeping the basepath the same, and using `..` for the path to mozilla. I was trying to change the basepath and adjust both other paths.

Comment 3

[Tom Prince [:tomprince]]

Attachment: Bug 1424171: Allow controlling the base a relative directories used for looking up localized files;

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

Comment 4

[Tom Prince [:tomprince]]

Attachment: Bug 1424171: Allow passing defines to compare-locales;

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

Comment 5

[Axel Hecht [:Pike]]

Comment on attachment 8936761 [details]
Bug 1424171: Allow passing defines to compare-locales;

https://reviewboard.mozilla.org/r/207476/#review213384

Comment 6

[Tom Prince [:tomprince]]

Attachment: Bug 1424171: Control locale dirs.

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

Comment 7

[Tom Prince [:tomprince]]

Attachment: Bug 1424171: Pass mozilla dir to compare-locales.

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

Comment 8

[Tom Prince [:tomprince]]

Comment on attachment 8936760 [details]
Bug 1424171: Allow controlling the base a relative directories used for looking up localized files;

https://reviewboard.mozilla.org/r/207472/#review214526

I'm not at all attached to the names. And this might be something that should be in `moz.build`, but I'm not sure what shape it would take there, suggestions welcome.

https://reviewboard.mozilla.org/r/208792/ shows how this would be used.

Comment 9

[Tom Prince [:tomprince]]

Comment on attachment 8938075 [details]
Bug 1424171: Control locale dirs.

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

Comment 10

[Tom Prince [:tomprince]]

Comment on attachment 8938076 [details]
Bug 1424171: Pass mozilla dir to compare-locales.

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

Comment 11

[(not currently active) Ted Mielczarek]

Comment on attachment 8936760 [details]
Bug 1424171: Allow controlling the base a relative directories used for looking up localized files;

https://reviewboard.mozilla.org/r/207474/#review216868

I'm always really hesitant to make changes to the l10n repack code because it's so fragile. In this case I think everything should work the same in the Firefox case, so this should be OK. I am planning to find time to rewrite all of this code in the near future, so hopefully when I do so we can make it work for you in a cleaner fashion.

Comment 12

[(not currently active) Ted Mielczarek]

Comment on attachment 8936761 [details]
Bug 1424171: Allow passing defines to compare-locales;

https://reviewboard.mozilla.org/r/207476/#review216872

Comment 13

[Tom Prince [:tomprince]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f1ddb4acee0f37a4a087b573878f7e9a83554834

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bd11f296e5ed
Allow controlling the base a relative directories used for looking up localized files; r=ted
https://hg.mozilla.org/integration/autoland/rev/54015b4eb2ed
Allow passing defines to compare-locales; r=Pike,ted

Comment 15

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/bd11f296e5ed
https://hg.mozilla.org/mozilla-central/rev/54015b4eb2ed

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/32ad15ba47d5
Control locale dirs.
https://hg.mozilla.org/comm-central/rev/bd596856ab37
Pass mozilla dir to compare-locales.