1412751 - Missing multilocale.json in non-packaged builds

Status: RESOLVED FIXED

(Core :: Internationalization, enhancement)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

In bug 1362617 we added ./dist/bin/res/multilocale.json, but we added it as a package step.

That means that if a developer does `./mach build` on mozilla-central and then try `./mach run` he'll launch Firefox without `multilocale.json`.

We need to either make sure that we can operate in a runtime without it, or add it to the build step as well.

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Pike, do you have a preference?

Comment 2

[Axel Hecht [:Pike]]

Having it always be there gives less chance for surprises, so I'd favor to add a build step.

Comment 3

[Nick Alexander :nalexander [he/him]]

(In reply to Axel Hecht [:Pike] from comment #2)
> Having it always be there gives less chance for surprises, so I'd favor to
> add a build step.

I have a mild preference for this too.  Man, this has been a crazy rabbit's hole.  Stay strong, Gandalf!

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

Michael - where should I put the dependency (I only need on `multilocale.json`) for the build target?

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1412751 - Move the multilocale.json generation to python and add to moz.build.

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

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8923481 [details]
Bug 1412751 - Move the multilocale.json generation to python and add to moz.build.

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

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

I know this is not what we'd like to do, but I wrote a simple ugly hack that does the job.

I duplicated the `multilocale.json` target from packager.mk to toolkit/locales/Makefile.in and added it as a dependency on `libs`.

The interesting trick is that in there, I could not use the `$(DIST)/$(RESPATH)/res` because RESPATH/BINPATH are undefined. So I used the `FINAL_TARGET`.

Maybe the solution will be to extract this code to a separate `mk` linked from both packager.mk and toolkit/locales/Makefile.in? If we can close the gap between RESPATH/BINPATH and FINAL_TARGET.

Leaving it off to :mshal who said he'll take a look.

Comment 8

[Michael Shal [:mshal]]

Attachment: gen-multilocale.patch

For things that are required for 'mach build', we prefer to have them in moz.build so that we can eventually build with a non-make backend. Packaging and l10n aren't part of that initial work, so it's ok for that stuff to be in the Makefiles. As such, I'd like to use a GENERATED_FILES script in toolkit/locales to generate the file during build if possible. We can also call the script from the package and l10n entry-points in packager.mk so that if the file format changes, it's just done in one place.

This patch should show what I'm intending. I believe it maintains the existing multilocale.json files that we have during package and installers-%, and generates an 'en-US' multilocale.json during build. Is en-US only the intended result for 'mach build'? Or does it need to be tied into locale settings somehow?

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

I think that for now it's ok if it is just en-US in 'mach build'. Eventually we may want to tie it to MOZ_CHROME_MULTILOCALE.

Comment 10

[Axel Hecht [:Pike]]

Comment on attachment 8923575 [details] [diff] [review]
gen-multilocale.patch

Review of attachment 8923575 [details] [diff] [review]:
-----------------------------------------------------------------

I like the general idea, just wondering about c-c.

::: toolkit/mozapps/installer/packager.mk
@@ +200,4 @@
>  # and places it in dist/bin/res - it should be used when packaging a build.
>  multilocale.json: LOCALES?=$(MOZ_CHROME_MULTILOCALE)
>  multilocale.json:
> +	$(call py_action,file_generate,$(topsrcdir)/toolkit/locales/gen_multilocale.py main $(MULTILOCALE_DIR)/multilocale.json $(MDDEPDIR)/multilocale.json.pp $(ALL_LOCALES))

Does this work for comm-central, in both configurations?

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8923575 [details] [diff] [review]
gen-multilocale.patch

The patch works as expected on build, package, repackage and `MOZ_CHROME_MULTILOCALE package`! Thank you :mshal!

Comment 12

[Zibi Braniecki [:zbraniecki][:gandalf]]

> Does this work for comm-central, in both configurations?

@mshal> gandalf: I think so? I'm not up-to-date on how c-c works, but as long as it recurses into toolkit/locales of the m-c tree it should build for it (with the default en-US output)

Jorg, we identified a weakness of the previous approach and :mshal wrote a patch on top of mine that gets the build step executed as part of build as well.
We believe it should work in c-c out of the box, but can you verify please?

All you need it apply the patch, `rm objdir/dist/bin/res/multilocale.json`, `./mach build` and check if `objdir/dist/bin/res/multilocale.json` is there and has just `["en-US"]`. Thanks!

Comment 13

[Jorg K (CEST = GMT+2)]

There was no multilocale.json on the system when I started. Following the steps from comment #12 I now have two:
C:\mozilla-source\comm-central\obj-i686-pc-mingw32\dist\bin\res\multilocale.json
C:\mozilla-source\comm-central\obj-i686-pc-mingw32\toolkit\locales\multilocale.json
Both are the same and have
{"locales": ["en-US"]}

Comment 14

[Zibi Braniecki [:zbraniecki][:gandalf]]

Thank you! :)

Comment 15

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8923575 [details] [diff] [review]
gen-multilocale.patch

Review of attachment 8923575 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/locales/gen_multilocale.py
@@ +9,5 @@
> +
> +def main(output, *locales):
> +    locales = list(locales) or ['en-US']
> +    locales_output = ['"%s"' % l for l in locales]
> +    print('{"locales": [%s]}' % (', '.join(locales_output)), file=output)

Please try using json.dump for this. I know this is a direct translation of the Makefile, but doing this by hand shouldn't be necessary in python.

Comment 16

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8923481 [details]
Bug 1412751 - Move the multilocale.json generation to python and add to moz.build.

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

Comment 17

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8923481 [details]
Bug 1412751 - Move the multilocale.json generation to python and add to moz.build.

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

Comment 18

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8923481 [details]
Bug 1412751 - Move the multilocale.json generation to python and add to moz.build.

https://reviewboard.mozilla.org/r/194626/#review200214

Thanks for updating this, looks good!

::: toolkit/locales/gen_multilocale.py:13
(Diff revision 4)
> +import sys
> +
> +
> +def main(output, *locales):
> +    locales = list(locales)
> +    if "en-US" not in locales:

Isn't this is equivalent to `if not locales:`, because we either make sure "en-US" is present in packager.mk or call this from moz.build without arguments? Anyway, I don't think it does any harm, but it's a little different from the last iteration.

Comment 19

[Zibi Braniecki [:zbraniecki][:gandalf]]

Yeah I made this change on purpose. We currently have a guard that adds en-US in makefile, but decided to add it here as well, esp that it covers the empty list scenario too.

Sorry for not writing a note about it!

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dcc8d6c79629
Move the multilocale.json generation to python and add to moz.build. r=chmanchester

Comment 21

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/dcc8d6c79629

Comment 22

[Jorg K (CEST = GMT+2)]

Looks like this broke out build:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=952762964f408e33d060dd1110cac0626ae5fa51
Linux 32, Mac and and Windows 8 at time of writing, Linux 64 is busted due to bug 1413257.

We get:
IOError: [Errno 2] No such file or directory: 'c:/builds/moz2_slave/tb-c-cen-w64-ntly-000000000000/build/toolkit/locales/gen_multilocale.py' 
IOError: [Errno 2] No such file or directory: '/builds/slave/tb-c-cen-lx-ntly-0000000000000/build/toolkit/locales/gen_multilocale.py'

Tom, could you also please take a look.

Comment 23

[Jorg K (CEST = GMT+2)]

From the log:

make[3]: Entering directory `/builds/slave/tb-c-cen-lx-ntly-0000000000000/build/objdir-tb/mail/installer'
/builds/slave/tb-c-cen-lx-ntly-0000000000000/build/objdir-tb/_virtualenv/bin/python -m mozbuild.action.file_generate /builds/slave/tb-c-cen-lx-ntly-0000000000000/build/toolkit/locales/gen_multilocale.py main ../../dist/bin/res/multilocale.json .deps/multilocale.json.pp  en-US
Traceback (most recent call last):
  File "/tools/python27/lib/python2.7/runpy.py", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/tools/python27/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/builds/slave/tb-c-cen-lx-ntly-0000000000000/build/mozilla/python/mozbuild/mozbuild/action/file_generate.py", line 112, in <module>
    sys.exit(main(sys.argv[1:]))
  File "/builds/slave/tb-c-cen-lx-ntly-0000000000000/build/mozilla/python/mozbuild/mozbuild/action/file_generate.py", line 51, in main
    with open(script, 'r') as fh:
IOError: [Errno 2] No such file or directory: '/builds/slave/tb-c-cen-lx-ntly-0000000000000/build/toolkit/locales/gen_multilocale.py'
make[3]: *** [multilocale.json] Error 1
make[3]: Leaving directory `/builds/slave/tb-c-cen-lx-ntly-0000000000000/build/objdir-tb/mail/installer'

Comment 24

[Zibi Braniecki [:zbraniecki][:gandalf]]

I'm sorry Jorg, but I do not know what is the cause of the problem.

Python seems to report missing `get_multilocale.py` file in ./toolkit/locales, but the file is there: https://hg.mozilla.org/mozilla-central/file/tip/toolkit/locales/gen_multilocale.py

It may be best to file a new bug about c-c at this point :(

Comment 25

[Jorg K (CEST = GMT+2)]

Thanks. I filed bug 1413581.