en-US dictionaries and searchplugins are shipped in l10n repacks

RESOLVED FIXED in mozilla21

Status

RESOLVED FIXED
6 years ago
9 months ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

Trunk
mozilla21
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Comment 1

6 years ago
Created attachment 705510 [details] [diff] [review]
Correctly handle non chrome directories when doing l10n-repack

This changes how l10n-repack handles non chrome directories, such that files with the same name are replaced, files only in l10n are added, and files only in the original build are removed.
Attachment #705510 - Flags: review?(gps)
Comment on attachment 705510 [details] [diff] [review]
Correctly handle non chrome directories when doing l10n-repack

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

Just some style nits.

::: toolkit/mozapps/installer/l10n-repack.py
@@ +125,5 @@
>              for e in [e for e in f if e.localized]:
>                  f.remove(e)
> +        if p in paths:
> +            path = paths[p]
> +            if path:

path = p.paths.get(p, None)
if path:

@@ +132,2 @@
>                  files = [f for p, f in l10n_finder.find(path)]
> +                if len(files) == 0:

if not len(files)

@@ +132,3 @@
>                  files = [f for p, f in l10n_finder.find(path)]
> +                if len(files) == 0:
> +                    if not base in NON_CHROME:

if base not in NON_CHROME:
Attachment #705510 - Flags: review?(gps) → review+
(Assignee)

Comment 3

6 years ago
(In reply to Gregory Szorc [:gps] from comment #2)
> Comment on attachment 705510 [details] [diff] [review]
> Correctly handle non chrome directories when doing l10n-repack
> 
> Review of attachment 705510 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Just some style nits.
> 
> ::: toolkit/mozapps/installer/l10n-repack.py
> @@ +125,5 @@
> >              for e in [e for e in f if e.localized]:
> >                  f.remove(e)
> > +        if p in paths:
> > +            path = paths[p]
> > +            if path:
> 
> path = p.paths.get(p, None)
> if path:

Note that paths[p] == None is treated differently from p not in paths.
(Assignee)

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
(Assignee)

Updated

6 years ago
Depends on: 834432
(Assignee)

Comment 5

6 years ago
Backed out because of bug 834432:
https://hg.mozilla.org/mozilla-central/rev/169e0492b03b
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 6

6 years ago
Created attachment 706142 [details] [diff] [review]
Correctly handle non chrome directories when doing l10n-repack

This combines the now backed out patch from this bug and the patch from bug 834432.
Attachment #706142 - Flags: review?(gps)
(Assignee)

Updated

6 years ago
Attachment #705510 - Attachment is obsolete: true
Comment on attachment 706142 [details] [diff] [review]
Correctly handle non chrome directories when doing l10n-repack

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

I lied about 4 hours. I remember looking at most of this last night. Not sure why I didn't hit r+ then.

Would be really nice to have test coverage of l10n repacks. These break way too often...
Attachment #706142 - Flags: review?(gps) → review+
(Assignee)

Comment 8

6 years ago
Thanks for the review. I agree we need test coverage for these. But let's already try to fix them :)

https://hg.mozilla.org/mozilla-central/rev/52c92a6c6e24
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Depends on: 835132

Updated

9 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.