391680 - Make the python compare-locales work with suite and calendar

Status: RESOLVED FIXED

(Mozilla Localizations :: Infrastructure, defect)

Description

[Axel Hecht [:Pike]]

The perl version of compare-locales has bugs, which I don't really want to fix. We should fix compare-locales in its python impl to actually work with suite and calendar.

What needs to be done?

First, Mozilla.Paths.modules and .locales needs to be rewritten to not be hardcoded but to work off client.mk.

make -f client.mk echo-variable-LOCALES_toolkit should get one chunk, and the apps should be _app mins the toolkit dirs.

I guess the best way to do this is to actually make modules a subclass of dict, getting the dirs on demand via overloading, probably implementing __missing__() is the way to go.

Then we should actually pass in the application and potentially a set of locales to Mozilla.CompareLocales.compare.

I'd be happy to accept patches for this, I'm not sure how timely I can fix this myself.

Comment 1

[Axel Hecht [:Pike]]

Taking, and differently than initially thought.

Comment 2

[Axel Hecht [:Pike]]

Attachment: don't hardcode browser/mail at too many places

So, I removed the hard-coding of browser and mail in Paths.py. The static dictionaries are now replaced with subclasses of dict which ask client.mk for help on creation time. Thus, you can have more than one of those mappings. I didn't drop the restriction on being in the right working dir in relation to the mozilla/client.mk and mozilla/${app}s/locales/all-locales. The all-locales magic moved out to its own function, too, so that it will only ask for those apps that the current test run is actually interested in.

I left the hard-coded app set of browser and mail for test-locales, that's the script driving http://people.mozilla.com/~axel/status-1.8/, which might stay branch-only, so there's not that much rationale for doing major changes to it now.

Comment 3

[Simon Paquet [:sipaq]]

(In reply to comment #2)
> I left the hard-coded app set of browser and mail for test-locales, that's the
> script driving http://people.mozilla.com/~axel/status-1.8/, which might stay
> branch-only, so there's not that much rationale for doing major changes to it
> now.

Axel, the Calendar team will do its next major release (0.7) and also the next 1-2 releases after that (0.9 and 1.0) from the 1.8 Branch. Therefore as requested before, it would be really cool, if you could adjust that script to include mozilla/calendar/locales as well.

Comment 4

[Robert Kaiser]

Comment on attachment 277166 [details] [diff] [review]
don't hardcode browser/mail at too many places

Why do we special-case toolkit at all here? client.mk should report "netwerk dom toolkit security/manager " for all toolkit-based apps, only special-casing one of those sounds strange to me. Ideally, we don't need any special-casing at all on those.
I'd imaging we'd just add up locales from all apps that contain one dir - this would also smoothen suite and mail both containing editor/ui, I'd hope.

Comment 5

[Axel Hecht [:Pike]]

Special casing toolkit comes from running the tool for both mail and browser. So if there was a missing string in toolkit, test-locales would report one for browser, and one for mail, which looks like two missing strings, and thus overreporting.

client.mk itself actually does include the toolkit dirs for each app, that's why I remove them afterwards, in Paths.Modules.__init__,

+    for k,v in self.iteritems():
+      if k == 'toolkit':
+        continue
+      self[k] = [d for d in v if d not in self['toolkit']]

This goes over all key-value pairs and for all but toolkit, sets the dirs to all dirs that are not in toolkit.
+

Comment 6

[Robert Kaiser]

Couldn't we just go and merge dirs for test-locales that are mentioned multiple times without special-casing toolkit explicitely?

As I said, we'd get into the same for editor/ui when including both suite and mail and we could get more cases in the future, esp. when we might find a solution for extensions.

Comment 7

[Axel Hecht [:Pike]]

Re calendar, I'll report on the overall status of l10n tools after next week. I have loads of stuff in my head, but I'm not sure how much of that is going to land and/or go into production when.

Comment 8

[Axel Hecht [:Pike]]

Re comment 6, I'm not sure. I have been thinking about reporting per dir myself, but I need to invest further thinking in that. But those would be significantly bigger changes.

Comment 9

[Robert Kaiser]

Comment on attachment 277166 [details] [diff] [review]
don't hardcode browser/mail at too many places

OK, I trust you have actually tested the code - I don't know much how it's used, so I can't test it much myself.

Apart from that munging around with toolkit, this looks as good to me as it can (given I don't know python).

Could you file a bug on making this per-directory just so we don't forget it?

Comment 10

[Axel Hecht [:Pike]]

FIXED, thanks for the review.

I'm not sure if per-directory is the right thing to do, yet. It'll stay in the back of my head, as I'm still shoveling things back and forth.