844068 - Drop localizing metro

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Axel Hecht [:Pike]]

We should remove the metro directory from the l10n setup.

It's clearly not anywhere close to be part of Firefox localization

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

I agree with that

Comment 2

[Matt Brubeck (:mbrubeck)]

Yes, I agree.  We can re-enable it when we are ready to freeze strings and go to Aurora.

Comment 3

[Axel Hecht [:Pike]]

Attachment: maybe good enough

Matt, this might be the best option now. Are you a good person to review?

This change only disables metro in compare-locales, and the dashboard and stuff.

This one is needed for sure, I'm not sure what happens if we'd remove/comment out

	@$(MAKE) -C ../metro/locales AB_CD=$* XPI_NAME=locale-$*

in browser/locales/Makefile.in. I expect things to break horribly there.

This patch is going to switch off l10n-merge for existing files in our localizations, though, which means that localized builds will break over time on metro. But I think that's OK. I can't come up with a low-risk solution here.

Given that metro is off on aurora anyway (?), we want this to land on aurora, too.

Comment 4

[Matt Brubeck (:mbrubeck)]

Comment on attachment 717163 [details] [diff] [review]
maybe good enough

Looks good to me.  Asking gavin just to get a Firefox peer's eyes on this, or in case there's anyone else he things should check it.

And yes, if this works we should definitely land it on Aurora.

Comment 5

[Mike Hommey [:glandium]]

(In reply to Axel Hecht [:Pike] from comment #3)
> Created attachment 717163 [details] [diff] [review]
> maybe good enough
> 
> Matt, this might be the best option now. Are you a good person to review?
> 
> This change only disables metro in compare-locales, and the dashboard and
> stuff.
> 
> This one is needed for sure, I'm not sure what happens if we'd
> remove/comment out
> 
> 	@$(MAKE) -C ../metro/locales AB_CD=$* XPI_NAME=locale-$*
> 
> in browser/locales/Makefile.in. I expect things to break horribly there.

You'd basically break metro on nightly (en-US) builds.

> This patch is going to switch off l10n-merge for existing files in our
> localizations, though, which means that localized builds will break over
> time on metro. But I think that's OK. I can't come up with a low-risk
> solution here.

Does changing l10n.ini actually impact the l10n merge happening when doing make -C metro/locales?

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 717163 [details] [diff] [review]
maybe good enough

I really have no opinion here - I don't think you need anyone but Axel to sign off on this :)

Comment 7

[Axel Hecht [:Pike]]

(In reply to Mike Hommey [:glandium] from comment #5)
> (In reply to Axel Hecht [:Pike] from comment #3)
> > Created attachment 717163 [details] [diff] [review]
> > maybe good enough
> > 
> > Matt, this might be the best option now. Are you a good person to review?
> > 
> > This change only disables metro in compare-locales, and the dashboard and
> > stuff.
> > 
> > This one is needed for sure, I'm not sure what happens if we'd
> > remove/comment out
> > 
> > 	@$(MAKE) -C ../metro/locales AB_CD=$* XPI_NAME=locale-$*
> > 
> > in browser/locales/Makefile.in. I expect things to break horribly there.
> 
> You'd basically break metro on nightly (en-US) builds.

Well, I've copied that out of the libs-% target, that shouldn't affect en-US.

For existing users, though, it'd drop doing l10n fallback on a per-file basis to en-US, and wouldn't register any chrome for metro.

Or....would it, with the new packager? Makes me wonder, can we abuse the packager to keep the en-US files and chrome reg lines in?

> > This patch is going to switch off l10n-merge for existing files in our
> > localizations, though, which means that localized builds will break over
> > time on metro. But I think that's OK. I can't come up with a low-risk
> > solution here.
> 
> Does changing l10n.ini actually impact the l10n merge happening when doing
> make -C metro/locales?

Yes, as it's not creating the merged files for files that exist in locales, but have missing entries. ysod-worthy here, but it wouldn't break the build for sure.

Comment 8

[Axel Hecht [:Pike]]

Comment on attachment 717163 [details] [diff] [review]
maybe good enough

Asking Mike for a real review, though.

Comment 9

[Mike Hommey [:glandium]]

(In reply to Axel Hecht [:Pike] from comment #7)
> Or....would it, with the new packager? Makes me wonder, can we abuse the
> packager to keep the en-US files and chrome reg lines in?

The new packager removes all localized chrome before importing the new locale, but it could surely be modified to only drop the localized categories where we do have data from the new locale.

> > Does changing l10n.ini actually impact the l10n merge happening when doing
> > make -C metro/locales?
> 
> Yes, as it's not creating the merged files for files that exist in locales,
> but have missing entries. ysod-worthy here, but it wouldn't break the build
> for sure.

So, in fact, wouldn't removing it from l10n.ini without removing it from libs-%:: break the build because of the missing files for JarMaker?

Comment 10

[Axel Hecht [:Pike]]

Missing files get picked up from en-US. That's an optimization I did in l10n-merge, 'mergedir', 'localedir', 'en-US'; so that I don't have to copy files around for nothing. mergedir is going to be empty for metro, though.

Re packager, I suggest we wait with that until we have real problems? "category" could also be chrome package? And then we're having desktop "browser", but not metro "browser", not sure if that's easy to digest.

If we really don't need *any* of the stuff that's coming through browser/locales/jar.mn, we can probably just switch it off in libs-%, and leave the browser package on metro as en-US only.

Comment 11

[Mike Hommey [:glandium]]

(In reply to Axel Hecht [:Pike] from comment #10)
> Missing files get picked up from en-US. That's an optimization I did in
> l10n-merge, 'mergedir', 'localedir', 'en-US'; so that I don't have to copy
> files around for nothing. mergedir is going to be empty for metro, though.

Ok, so while l10n.ini affects the content of the mergedir, it doesn't affect the rest of the process, so it will still pick missing files from en-US?

Comment 12

[Axel Hecht [:Pike]]

Yep. Caveat: it'll also pick files from l10n if the exist, so as our current localizations bitrot away, the localized builds will break. I'll reach out to the community to get their stuff moved away or thrown away, given that we can't really tell how appropriate their current work is going to be at the end.

Comment 13

[Axel Hecht [:Pike]]

Landed on central, https://hg.mozilla.org/mozilla-central/rev/10f28efcb0b7, as DONTBUILD as this only affects l10n builds, which don't get build for en-US check-ins anyway.

Comment 14

[Axel Hecht [:Pike]]

Comment on attachment 717163 [details] [diff] [review]
maybe good enough

[Approval Request Comment]
Bug caused by (feature/regressing bug #): landing metro with l10n strings, but the strings are ready for l10n yet
User impact if declined: 
Testing completed (on m-c, etc.): landed on mozilla-central, is going to be in tomorrow's nightlies
Risk to taking this patch (and alternatives if risky): existing localizations bitrot on purpose
String or UUID changes made by this patch: only string removals in effect

Comment 15

[Axel Hecht [:Pike]]

(In reply to Axel Hecht [:Pike] from comment #14)
> Comment on attachment 717163 [details] [diff] [review]
> maybe good enough
> 
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #): landing metro with l10n strings,
> but the strings are ready for l10n yet
> User impact if declined: 
> Testing completed (on m-c, etc.): landed on mozilla-central, is going to be
> in tomorrow's nightlies
> Risk to taking this patch (and alternatives if risky): existing
> localizations bitrot on purpose
> String or UUID changes made by this patch: only string removals in effect

Note that bug 841919 disabled metro for aurora, and this patch is actually a missing piece in doing so.

Comment 16

[Alex Keybl [:akeybl]]

Approving for Aurora 21 and tracking for 22 in case this is needed again.

Comment 17

[Axel Hecht [:Pike]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4fe41db7c3dd, landed on aurora.

Comment 18

[Axel Hecht [:Pike]]

As we landed this on central, marking fixed22, too.