Switch to use in-tree version of compare-locales

RESOLVED FIXED

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Pike, Assigned: Callek)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(firefox46 fixed, firefox50 fixed)

Details

Attachments

(5 attachments, 4 obsolete attachments)

58 bytes, text/x-review-board-request
Pike
: review+
Details
58 bytes, text/x-review-board-request
jlund
: review+
Details
58 bytes, text/x-review-board-request
gps
: review+
Details
58 bytes, text/x-review-board-request
jlund
: review+
Details
58 bytes, text/x-review-board-request
rail
: review+
Details
(Reporter)

Description

3 years ago
Let's use the in-tree version of compare-locales for Desktop, and Android.

This is the less-aggressive path than originally proposed in https://groups.google.com/d/msg/mozilla.dev.builds/0koWKXx7arg/izvBHzy8nmwJ

I don't know what it'd take for SeaMonkey, tbh. For Thunderbird, we'll want bug 1154448 in the tree, though.
(Reporter)

Comment 1

3 years ago
Created attachment 8705715 [details]
MozReview Request: bug 1223385, refactor mach helper into LocalesMixin, r?rail

Review commit: https://reviewboard.mozilla.org/r/30161/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30161/
Attachment #8705715 - Flags: review?(rail)
(Reporter)

Comment 2

3 years ago
Created attachment 8705716 [details]
MozReview Request: bug 1223385, use in-tree compare-locales in mozharness, r?rail

Review commit: https://reviewboard.mozilla.org/r/30163/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30163/
Attachment #8705716 - Flags: review?(rail)
(Reporter)

Comment 3

3 years ago
Created attachment 8705717 [details]
MozReview Request: bug 1223385, use in-tree compare-locales in Makefiles, r?gps

Also fix that the default merge dir in the mach command creates a directory
that's the merge make target, and thus keeps that make target from actually
running.

Review commit: https://reviewboard.mozilla.org/r/30165/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30165/
Attachment #8705717 - Flags: review?(gps)
(Reporter)

Updated

3 years ago
Assignee: nobody → l10n

Comment 4

3 years ago
Comment on attachment 8705715 [details]
MozReview Request: bug 1223385, refactor mach helper into LocalesMixin, r?rail

https://reviewboard.mozilla.org/r/30161/#review26861
Attachment #8705715 - Flags: review?(rail) → review+

Updated

3 years ago
Attachment #8705716 - Flags: review?(rail) → review+

Comment 5

3 years ago
Comment on attachment 8705716 [details]
MozReview Request: bug 1223385, use in-tree compare-locales in mozharness, r?rail

https://reviewboard.mozilla.org/r/30163/#review26865

::: testing/mozharness/mozharness/mozilla/l10n/locales.py:164
(Diff revision 1)
> -        status = self.run_command(command, error_list=compare_locales_error_list,
> +        status = self._mach(command, env, error_list=compare_locales_error_list,

Much easier to read! Thanks.
Comment on attachment 8705717 [details]
MozReview Request: bug 1223385, use in-tree compare-locales in Makefiles, r?gps

https://reviewboard.mozilla.org/r/30165/#review26903

Thank you for explaining the default merge dir change in the commit message, as I would have been scratching my head otherwise.
Attachment #8705717 - Flags: review?(gps) → review+
(Reporter)

Updated

3 years ago
Keywords: checkin-needed
Version: 38 Branch → Trunk

Comment 8

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/47e2dfac99d5
https://hg.mozilla.org/mozilla-central/rev/a575f9a6f0d2
https://hg.mozilla.org/mozilla-central/rev/a87a27864bb8
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
(Reporter)

Comment 9

3 years ago
Created attachment 8707849 [details] [diff] [review]
adding back abs_compare_locales_dir to fix bustage

Nightlies are busted :-(

https://treeherder.mozilla.org/logviewer.html#?job_id=3079720&repo=mozilla-central is the log.

The removal of the dirs entry was premature, we should add it back so that clobber find the compare-locales checkout that mozharness still does.

We should remove those in a follow-up.

Updated

3 years ago
Blocks: 1239707
(Reporter)

Comment 14

3 years ago
Backed this out because I've used mozilla-central as try enough.

This patch turned out to be mozharness more than anything else, so I'm moving this bug over.

I think the patches are an OK start, but this patch needs someone that can actually hack on mozharness, and I can't even get it to run locally. Thus I'm unassigning myself.

Maybe the last problem was that the merge dirs for chrome-% were wrong in the multilocale fennec build. Maybe that wasn't the last problem :-/

We do have things in the code today, notably https://dxr.mozilla.org/mozilla-central/source/dom/locales/en-US/chrome/netErrorApp.dtd, which is breaking the currently used compare-locales parser. That's riding with 45, https://hg.mozilla.org/mozilla-central/rev/b957faece566. So we're already late, sadly.
Assignee: l10n → nobody
Status: RESOLVED → REOPENED
Component: Build Config → Mozharness
Product: Firefox → Release Engineering
QA Contact: jlund
Resolution: FIXED → ---
Target Milestone: Firefox 46 → ---
Version: Trunk → ---
(Reporter)

Comment 15

3 years ago
PS: desktop nightlies work without https://hg.mozilla.org/mozilla-central/rev/f59d1f7941b0, i.e., within the bootstrap environment, but not with the plain query_env().
Nick, can you take a look?
Flags: needinfo?(nthomas)
Not sure how far I'll get in mach land, but I'll have a look around.
Assignee: nobody → nthomas
Flags: needinfo?(nthomas)
(Reporter)

Updated

3 years ago
Blocks: 1277505
(Assignee)

Updated

3 years ago
Assignee: nthomas → bugspam.Callek
(Assignee)

Comment 18

3 years ago
Created attachment 8769273 [details]
Bug 1223385 - Remove unused extract-bookmarks.py.

Review commit: https://reviewboard.mozilla.org/r/63240/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/63240/
Attachment #8769273 - Flags: review?(l10n)
Attachment #8769274 - Flags: review?(jlund)
Attachment #8769275 - Flags: review?(gps)
Attachment #8769276 - Flags: review?(jlund)
Attachment #8769277 - Flags: review?(rail)
(Assignee)

Comment 19

3 years ago
Created attachment 8769274 [details]
Bug 1223385 - remove b2g_desktop_multilocale script due to being unused (dougt-encouraged).

Review commit: https://reviewboard.mozilla.org/r/63242/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/63242/
(Assignee)

Comment 20

3 years ago
Created attachment 8769275 [details]
Bug 1223385 - use in-tree compare-locales in Makefiles,

Also fix that the default merge dir in the mach command creates a directory
that's the merge make target, and thus keeps that make target from actually
running.

Review commit: https://reviewboard.mozilla.org/r/63244/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/63244/
(Assignee)

Comment 21

3 years ago
Created attachment 8769276 [details]
Bug 1223385 - Make sure nothing in mozharness uses (or clones) compare-locales manually.

Review commit: https://reviewboard.mozilla.org/r/63246/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/63246/
(Assignee)

Comment 22

3 years ago
Created attachment 8769277 [details]
Bug 1223385 - Don't clone the older copy of compare-locales anywhere.

Review commit: https://reviewboard.mozilla.org/r/63248/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/63248/
(Assignee)

Updated

3 years ago
Attachment #8707849 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8705717 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8705716 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8705715 - Attachment is obsolete: true
(Reporter)

Updated

3 years ago
Attachment #8769273 - Flags: review?(l10n) → review+
(Reporter)

Comment 23

3 years ago
Comment on attachment 8769273 [details]
Bug 1223385 - Remove unused extract-bookmarks.py.

https://reviewboard.mozilla.org/r/63240/#review60046

Removal is cool, it was only useful when we converted bookmarks.html to bookmarks.ini back in the old old days.

Comment 24

3 years ago
Comment on attachment 8769277 [details]
Bug 1223385 - Don't clone the older copy of compare-locales anywhere.

https://reviewboard.mozilla.org/r/63248/#review60054
Attachment #8769277 - Flags: review?(rail) → review+
(Assignee)

Comment 25

3 years ago
Fwiw, I did a try push at: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d6c071826f9

Also of note though is the buildbot l10n stuff will likely die due to timeouts (it has that problem), but if its timeout-dead that is still a good sign I didn't break it.
Comment on attachment 8769274 [details]
Bug 1223385 - remove b2g_desktop_multilocale script due to being unused (dougt-encouraged).

https://reviewboard.mozilla.org/r/63242/#review60056
Attachment #8769274 - Flags: review?(jlund) → review+
Comment on attachment 8769276 [details]
Bug 1223385 - Make sure nothing in mozharness uses (or clones) compare-locales manually.

https://reviewboard.mozilla.org/r/63246/#review60060

<p>what does this mean for these: https://dxr.mozilla.org/mozilla-central/search?q=path%3Amozharness%2Fconfigs+compare-locales&amp;redirect=true ?</p>
Attachment #8769276 - Flags: review?(jlund) → review+
(Assignee)

Comment 28

3 years ago
(In reply to Jordan Lund (:jlund) from comment #27)
> <p>what does this mean for these:
> https://dxr.mozilla.org/mozilla-central/
> search?q=path%3Amozharness%2Fconfigs+compare-locales&amp;redirect=true ?</p>

See the last patch in the series (already reviewed by rail)
Comment on attachment 8769275 [details]
Bug 1223385 - use in-tree compare-locales in Makefiles,

https://reviewboard.mozilla.org/r/63244/#review60688
Attachment #8769275 - Flags: review?(gps) → review+

Comment 30

3 years ago
Pushed by Callek@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f0116af2fd7
Remove unused extract-bookmarks.py. r=Pike
https://hg.mozilla.org/integration/mozilla-inbound/rev/fcb193036063
remove b2g_desktop_multilocale script due to being unused (dougt-encouraged). r=jlund
https://hg.mozilla.org/integration/mozilla-inbound/rev/5278700efd2d
use in-tree compare-locales in Makefiles, r=gps
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e4044707462
Make sure nothing in mozharness uses (or clones) compare-locales manually. r=jlund
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9964faafa19
Don't clone the older copy of compare-locales anywhere. r=rail
You need to log in before you can comment on or make changes to this bug.