Closed
Bug 1445582
Opened 6 years ago
Closed 6 years ago
output_searchplugins_list errors should be fatal
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox-esr52 fixed, firefox59 wontfix, firefox60 fixed, firefox61 fixed)
RESOLVED
FIXED
mozilla61
People
(Reporter: jcristau, Assigned: jcristau)
References
Details
Attachments
(1 file)
The 52.7.0esr Italian build suffered from bug 1445278. That error should have been fatal.
Comment hidden (mozreview-request) |
Comment 2•6 years ago
|
||
mozreview-review |
Comment on attachment 8958753 [details] Bug 1445582 - error out when the searchplugins list is empty or a plugin is missing https://reviewboard.mozilla.org/r/227676/#review233412 ::: browser/locales/Makefile.in:42 (Diff revision 1) > MOZ_INSTALLER_PATH=$(topsrcdir)/browser/installer/windows > > -SEARCHPLUGINS_FILENAMES := $(shell $(call py_action,output_searchplugins_list,$(srcdir)/search/list.json $(AB_CD))) > +SEARCHPLUGINS_FILENAMES := $(or $(shell $(call py_action,output_searchplugins_list,$(srcdir)/search/list.json $(AB_CD))), $(error Missing search plugins)) > SEARCHPLUGINS_PATH := .deps/generated_$(AB_CD) > SEARCHPLUGINS_TARGET := libs searchplugins > SEARCHPLUGINS := $(foreach plugin,$(addsuffix .xml,$(SEARCHPLUGINS_FILENAMES)),$(or $(wildcard $(srcdir)/searchplugins/$(plugin)),$(warning Missing searchplugin: $(plugin)))) Should this also be an error rather than a warning? ::: browser/locales/Makefile.in:43 (Diff revision 1) > > -SEARCHPLUGINS_FILENAMES := $(shell $(call py_action,output_searchplugins_list,$(srcdir)/search/list.json $(AB_CD))) > +SEARCHPLUGINS_FILENAMES := $(or $(shell $(call py_action,output_searchplugins_list,$(srcdir)/search/list.json $(AB_CD))), $(error Missing search plugins)) > SEARCHPLUGINS_PATH := .deps/generated_$(AB_CD) > SEARCHPLUGINS_TARGET := libs searchplugins > SEARCHPLUGINS := $(foreach plugin,$(addsuffix .xml,$(SEARCHPLUGINS_FILENAMES)),$(or $(wildcard $(srcdir)/searchplugins/$(plugin)),$(warning Missing searchplugin: $(plugin)))) > # Some locale-specific search plugins may have preprocessor directives, but the Not really related to this bug, but now that all the search plugins are in the tree, I don't think we still need preprocessing.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8958753 [details] Bug 1445582 - error out when the searchplugins list is empty or a plugin is missing https://reviewboard.mozilla.org/r/227676/#review233412 > Should this also be an error rather than a warning? Doesn't sound like a bad idea. Changed now.
Updated•6 years ago
|
status-firefox59:
--- → wontfix
status-firefox60:
--- → affected
status-firefox61:
--- → affected
status-firefox-esr52:
--- → affected
Assignee | ||
Updated•6 years ago
|
Attachment #8958753 -
Flags: review?(core-build-config-reviews)
Updated•6 years ago
|
Updated•6 years ago
|
Attachment #8958753 -
Flags: review?(core-build-config-reviews) → review?(nfroyd)
Comment 5•6 years ago
|
||
mozreview-review |
Comment on attachment 8958753 [details] Bug 1445582 - error out when the searchplugins list is empty or a plugin is missing https://reviewboard.mozilla.org/r/227676/#review234808 I don't have a ton of familiarity with this code, but erroring out earlier rather than later seems like a good strategy.
Attachment #8958753 -
Flags: review?(nfroyd) → review+
Pushed by jcristau@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9bdac68266e2 error out when the searchplugins list is empty or a plugin is missing r=froydnj
Comment 7•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9bdac68266e2
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Assignee | ||
Comment 8•6 years ago
|
||
Looks like there are no uplift approval flags here, so using the whiteboard.
Whiteboard: [checkin-needed-beta]
Comment 9•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/031af54f082d
Whiteboard: [checkin-needed-beta]
Comment 10•6 years ago
|
||
uplift |
https://hg.mozilla.org/releases/mozilla-esr52/rev/5678bfed1348
You need to log in
before you can comment on or make changes to this bug.
Description
•