900504 - l10n packaging of gaia broken, doesn't pick up content for apps

Status: RESOLVED FIXED

(Firefox OS Graveyard :: GonkIntegration, defect)

Description

[Axel Hecht [:Pike]]

The localization scheme for apps is not working right now.

The root cause boils down to Makefile setting the GAIA_APPDIRS to absolute paths, which trump all file path manipulations including the targets in multilocale.py.

I'm not sure how one would fix this, I got lost in the relationships between gaia apps that we localize and all the distribution stuff around it.

This only affects master, aka 1.2, thus requesting koi?. This does not affect v1-train.

Comment 1

[Axel Hecht [:Pike]]

This may very well be a regression from bug 848604.

Comment 2

[Axel Hecht [:Pike]]

Yuren, can you own this, or find an owner for this?

Comment 3

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Sure, it's my bad.

Comment 4

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Hi Pike, could you provide STRs? it would be helpful for fixing it.

Comment 5

[Axel Hecht [:Pike]]

https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Building#Building_multilocale has docs, you'll need to actually clone some hg repos with languages outside of gaia. Italian and Polish would be good candidates.

Once you've set up the repos and variables, make profile or debug-profile, as usual.

Comment 7

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

I have investigated this issue, I plan to add a new argument --gaia to store GAIA_DIR and find the correct path for *.ini.

Comment 8

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Attachment: github PR: https://github.com/mozilla-b2g/gaia/pull/11598

Stas, could you help to review this PR? thanks!

Comment 9

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Hi Pike, Do you know who can review this or could you?

Looks Stas is pretty busy.

Comment 10

[Staś Małolepszy :stas]

I'm sorry, I was indeed busy but I'll review this today.

Comment 11

[Staś Małolepszy :stas]

Comment on attachment 792010 [details] [review]
github PR: https://github.com/mozilla-b2g/gaia/pull/11598

r- because of two problems that I found:

1.manifest.webapp files need the same logic. Right now, it's os.path.join(source, locale, os.path.dirname(manifest_file), ...) and manifest_file is an absolute path, so source and locale are ignored.

2. relative path is correct for apps, but incorrect for anything in shared/

For example:

target /home/stas/moz/b2g/gaia/apps/system/locales/system.el.properties
relative apps/system/locales/system.el.properties

but:

target shared/locales/branding/branding.el.properties
relative branding.el.properties

target shared/locales/permissions/permissions.el.properties
relative ns/permissions.el.properties

target shared/locales/date/date.el.properties
relative .el.properties

target shared/locales/tz/tz.el.properties
relative properties

This can be fixed in the Makefile by making shared be absolute as well, like this:

-GAIA_LOCALE_SRCDIRS=shared $(GAIA_APPDIRS)
+GAIA_LOCALE_SRCDIRS=$(CURDIR)/shared $(GAIA_APPDIRS)

Comment 12

[Staś Małolepszy :stas]

Attachment: Replacement patch

Yuren, what do you think about this approach instead?

Comment 13

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 795320 [details] [diff] [review]
Replacement patch

Looks good, let's land it :D
r=yurenju

Comment 14

[Staś Małolepszy :stas]

Landed on master: https://github.com/mozilla-b2g/gaia/commit/50b3e3ae7cff0db311cb98cd6b83d6650e9dd584.  Thanks for the review, Yuren!

Comment 15

[Ed Morley [:emorley]]

This broke Windows B2G desktop builds:
https://tbpl.mozilla.org/php/getParsedLog.php?id=27051712&tree=Mozilla-Central
https://tbpl.mozilla.org/php/getParsedLog.php?id=27051563&tree=Mozilla-Central

Reverted:
https://github.com/mozilla-b2g/gaia/commit/599214a0f41eece076dc83cd85f5b27f8cfe67f2

Comment 16

[Ed Morley [:emorley]]

(In reply to Ed Morley (Away 23rd-26th August) [:edmorley UTC+1] from comment #15)
> This broke Windows B2G desktop builds:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=27051712&tree=Mozilla-
> Central
> https://tbpl.mozilla.org/php/getParsedLog.php?id=27051563&tree=Mozilla-
> Central
> 
> Reverted:
> https://github.com/mozilla-b2g/gaia/commit/
> 599214a0f41eece076dc83cd85f5b27f8cfe67f2

Ok turns out to have been something else (see bug 908691), this can reland (sorry for the noise).

Comment 17

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

so, maybe, there are 2 different errors, and the error outlined by Ed is actually caused by this bug...

Comment 18

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

root cause is $(CURDIR) looks like C:\\a\\b\\c, but we got path from find_files in multilocales.py is something like C:\a\b\c, so we got a wrong path when use make_relative().

Comment 19

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Attachment: github PR: https://github.com/mozilla-b2g/gaia/pull/11598

Updated pull request, but I'm not sure if this is a good idea... I detect OS platform in make_relative() to modify gaia path.

Comment 20

[Staś Małolepszy :stas]

Comment on attachment 796529 [details] [review]
github PR: https://github.com/mozilla-b2g/gaia/pull/11598

I don't have a Windows machine to test this, but it looks good to me.  I don't want to spend too much time fixing this file because ultimately, I'd like to see it rewritten into JavaScript anyways.

r=stas

Comment 21

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

I'll re-write it into javascript soon :-)

Comment 22

[Staś Małolepszy :stas]

Yuren, do you want me to merge the pull request, or will you do it?

Comment 23

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

I'll do it, let me do some final check.

Comment 24

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

merged again.
https://github.com/mozilla-b2g/gaia/commit/98b82e7574d52ba331add18c617e08d8784ccf4a

Comment 25

[Ryan VanderMeulen [:RyanVM]]

John, can you please assist with the uplift? :)

Comment 26

[John Ford [:jhford] CET/CEST Berlin Time]

There is a small merge conflict in the Makefile, please resolve that and push to v1.2

Comment 27

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

this commit was landed before branching v1.2, so we don't need merge again.

Comment 28

[Dylan Oliver [:doliver]]

Marking status as 1.2 fixed, per comment #27.

Comment 29

[Ed Morley [:emorley]]

(To get this out of the saved search for uplift)