1147217 - Improve l10n repack error message when locale doesn't contain necessary files

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Improve l10n repack error message when locale doesn't contain necessary files

Comment 2

[Mike Hommey [:glandium]]

Attachment: Improve l10n repack error message when locale doesn't contain necessary files

Comment 3

[Mike Hommey [:glandium]]

Attachment: Improve l10n repack error message when locale doesn't contain necessary files

Comment 4

[Michael Shal [:mshal]]

Comment on attachment 8582795 [details] [diff] [review]
Improve l10n repack error message when locale doesn't contain necessary files

>+            if base not in l10n_paths:
>+                errors.fatal("Locale doesn't contain %s/" % base)
>+                continue
>+            if e.name not in l10n_paths[base]:
>+                errors.fatal("Locale doesn't have a manifest entry for '%s'" %
>+                    e.name)
>+                continue

Why did you need to add the continues here? It looks like errors.fatal() should raise an exception.

Comment 5

[Mike Hommey [:glandium]]

(In reply to Michael Shal [:mshal] from comment #4)
> Comment on attachment 8582795 [details] [diff] [review]
> Improve l10n repack error message when locale doesn't contain necessary files
> 
> >+            if base not in l10n_paths:
> >+                errors.fatal("Locale doesn't contain %s/" % base)
> >+                continue
> >+            if e.name not in l10n_paths[base]:
> >+                errors.fatal("Locale doesn't have a manifest entry for '%s'" %
> >+                    e.name)
> >+                continue
> 
> Why did you need to add the continues here? It looks like errors.fatal()
> should raise an exception.

errors.accumulate(), which is used in the caller, inhibits those exceptions, accumulates the errors, and then throws an aggregated exception with all the errors.

Comment 6

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b23c836ff69a

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b23c836ff69a

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5c4cacd09c9c

Comment 9

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8582795 [details] [diff] [review]
Improve l10n repack error message when locale doesn't contain necessary files

I thought we could get away without it, but it seems we need to uplift this one for bug 1147207 to cleanly apply. Please see that bug for approval comment.

Comment 10

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8582795 [details] [diff] [review]
Improve l10n repack error message when locale doesn't contain necessary files

Scratch that, you've already uplifted it. Sorry I missed it!