Closed Bug 1133428 Opened 5 years ago Closed 5 years ago

L10n repackages should survive searchplugins missing, even if listed in list.txt

Categories

(SeaMonkey :: Build Config, defect)

SeaMonkey 2.33 Branch
x86
macOS
defect
Not set

Tracking

(seamonkey2.33 fixed, seamonkey2.34 fixed, seamonkey2.35 fixed, seamonkey2.36 fixed)

RESOLVED FIXED
seamonkey2.36
Tracking Status
seamonkey2.33 --- fixed
seamonkey2.34 --- fixed
seamonkey2.35 --- fixed
seamonkey2.36 --- fixed

People

(Reporter: ewong, Assigned: ewong)

References

Details

Attachments

(3 files, 3 obsolete files)

This is a big problem with our repacks.

creativecommons.xml is mentioned in quite a few of our locales' list.txt,
but that file doesn't exist in the searchplugins/ folder.

locales are :
hu, nb-NO, be, fr, uk, es-AR, zh-CN, es-ES, nl, sv-SE, gl, en-GB, lt

I talked to :flod over at #l10n and basically whatever patch that's done
on this bug will fix the repacks.  (This means a push to c-b...)

But in the long run, those locales should be fixed. (i.e. creativecommons.xml
removed from the list.txt in the locales).




+++ This bug was initially created as a clone of Bug #1073212 +++

Right now, if a search plugin in listed in list.txt and neither found in en-US nor in l10n, the build dies.

Let's not do that and make the build more robust.

Doing a warning would still be nice.

I've tried to stumble into what we'd need to change, but I can't even get an unpatched repack working locally on mac, so I sadly won't be of much help.
Attachment #8564909 - Flags: review?(kairo)
Comment on attachment 8564909 [details] [diff] [review]
fix search plugins check

Actually, come to think of this, I do not believe in this actually being a bug. If localizers do not maintain their list.txt correctly, that is an actual bug and I don't feel too good with papering over that in a way that localizers will probably even never see it.
Comment on attachment 8564909 [details] [diff] [review]
fix search plugins check

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

All that said, given that we should match the logic in Firefox and you did just that, the patch is probably still good.
Attachment #8564909 - Flags: review?(kairo) → review+
Comment on attachment 8564909 [details] [diff] [review]
fix search plugins check

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: l10n repacks are busted.
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky):
String changes made by this patch: None
Attachment #8564909 - Flags: approval-comm-release?
Attachment #8564909 - Flags: approval-comm-beta?
Attachment #8564909 - Flags: approval-comm-aurora?
Attachment #8564909 - Flags: approval-comm-release?
Attachment #8564909 - Flags: approval-comm-release+
Attachment #8564909 - Flags: approval-comm-beta?
Attachment #8564909 - Flags: approval-comm-beta+
Attachment #8564909 - Flags: approval-comm-aurora?
Attachment #8564909 - Flags: approval-comm-aurora+
Comment on attachment 8564909 [details] [diff] [review]
fix search plugins check

Pushed to comm-beta:
http://hg.mozilla.org/releases/comm-beta/rev/9b5dc9623403
Attached patch bustage fix (obsolete) — Splinter Review
should not be adding a second suffix.
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Attachment #8567768 - Flags: review?(kairo)
Comment on attachment 8567768 [details] [diff] [review]
bustage fix

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

Let's do target and path Ala http://mxr.mozilla.org/comm-beta/source/mozilla/browser/locales/Makefile.in#72 just incase.
Attachment #8567768 - Flags: review?(kairo)
Attachment #8567768 - Flags: review+
Attachment #8567768 - Flags: approval-comm-beta+
Attachment #8567768 - Flags: approval-comm-aurora+
Attached patch bustage fix (v2) (obsolete) — Splinter Review
Attachment #8567768 - Attachment is obsolete: true
Attachment #8567783 - Flags: review?(bugspam.Callek)
Comment on attachment 8567783 [details] [diff] [review]
bustage fix (v2)

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

Thinking about it more, I'd like http://mxr.mozilla.org/comm-beta/source/mozilla/browser/locales/Makefile.in?mark=78-78#76 to exist here just to be *extra* safe, no need for new patch just land with this fixed.
Attachment #8567783 - Flags: review?(bugspam.Callek)
Attachment #8567783 - Flags: review+
Attachment #8567783 - Flags: approval-comm-beta+
Attachment #8567783 - Flags: approval-comm-aurora+
Attachment #8567783 - Attachment is obsolete: true
Attachment #8567784 - Flags: review?(bugspam.Callek)
Attachment #8567784 - Flags: review?(bugspam.Callek)
Attachment #8567784 - Flags: review+
Attachment #8567784 - Flags: approval-comm-beta+
Attachment #8567784 - Flags: approval-comm-aurora+
Attached patch bustage fix #2 (v1) (obsolete) — Splinter Review
post-push review
Attachment #8567831 - Flags: review?(bugspam.Callek)
Comment on attachment 8567831 [details] [diff] [review]
bustage fix #2 (v1)

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky):
String changes made by this patch:
Attachment #8567831 - Flags: approval-comm-beta?
Attachment #8567831 - Flags: approval-comm-aurora?
(In reply to Edmund Wong (:ewong) from comment #13)
> Comment on attachment 8567831 [details] [diff] [review]
> bustage fix #2 (v1)
> 
> [Approval Request Comment]
> Regression caused by (bug #): 
> User impact if declined:  build bustage
> Testing completed (on m-c, etc.): 
> Risk to taking this patch (and alternatives if risky):
> String changes made by this patch:

keep on forgetting to fill this.
Comment on attachment 8567831 [details] [diff] [review]
bustage fix #2 (v1)

a=me build bustage
Attachment #8567831 - Flags: approval-comm-beta?
Attachment #8567831 - Flags: approval-comm-beta+
Attachment #8567831 - Flags: approval-comm-aurora?
Attachment #8567831 - Flags: approval-comm-aurora+
Comment on attachment 8567831 [details] [diff] [review]
bustage fix #2 (v1)

This was backed out:
http://hg.mozilla.org/releases/comm-beta/rev/c65477473f3a
Attachment #8567831 - Flags: review?(bugspam.Callek)
Attachment #8567831 - Flags: approval-comm-beta+
Attachment #8567831 - Flags: approval-comm-aurora+
Pushed a followup, there was issues because the whole var stuff was below the rules.mk import which was causing them to not get used :/

https://hg.mozilla.org/releases/comm-beta/rev/739988b699a5
https://hg.mozilla.org/releases/comm-beta/rev/61d7d083d05a

got r=ewong on IRC
Attachment #8567831 - Attachment is obsolete: true
Attached patch bustage fix #2Splinter Review
[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: make_pkg bustage
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky):
String changes made by this patch:

This patch was pushed to c-b's SEA_COMM360_20150215_RELBRANCH.  It needs
to be pushed to c-a, c-b and c-r.

all three of these patches (8564909, 8567784 and this) needs to be pushed 
together.
Attachment #8569128 - Flags: review+
Attachment #8569128 - Flags: approval-comm-release?
Attachment #8569128 - Flags: approval-comm-beta?
Attachment #8569128 - Flags: approval-comm-aurora?
Attachment #8569128 - Flags: approval-comm-release?
Attachment #8569128 - Flags: approval-comm-release+
Attachment #8569128 - Flags: approval-comm-beta?
Attachment #8569128 - Flags: approval-comm-beta+
Attachment #8569128 - Flags: approval-comm-aurora?
Attachment #8569128 - Flags: approval-comm-aurora+
Comment on attachment 8564909 [details] [diff] [review]
fix search plugins check

Pushed to comm-beta:
https://hg.mozilla.org/releases/comm-beta/rev/d70432f425d5
Attachment #8569128 - Attachment description: bustage fix #3 → bustage fix #2
Attachment #8567784 - Attachment description: bustage fix (v3) → bustage fix @1 (v3)
Attachment #8567784 - Attachment description: bustage fix @1 (v3) → bustage fix #1 (v3)
The push to c-b from comment #18 was uplifted to c-r so no push to c-r is
necessary.

So closing this.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.