Closed
Bug 1309304
Opened 8 years ago
Closed 8 years ago
Move python files for generating search info to a central location
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox52 fixed)
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: mkaply, Unassigned)
Details
Attachments
(2 files)
I originally put some python files I need for generating search files into browser/locales, but not only will they be needed for mobile, but they'll be needed for Thunderbird and seamonkey as well. So moving them to build/search.
Reporter | ||
Comment 1•8 years ago
|
||
Reporter | ||
Updated•8 years ago
|
Attachment #8799955 -
Flags: review?(mh+mozilla)
Comment 2•8 years ago
|
||
Comment on attachment 8799955 [details] [diff] [review] Move search python files to build/search Review of attachment 8799955 [details] [diff] [review]: ----------------------------------------------------------------- Move them to python/mozbuild/mozbuild/action, and use $(call py_action) (see other uses of that in the tree)
Attachment #8799955 -
Flags: review?(mh+mozilla) → review-
Reporter | ||
Comment 3•8 years ago
|
||
I can do that easily with the searchjson since it is in a target, but the generated search plugins list is directed to standard out as part of the initial processing of the makefile. I don't see any way to switch that to a target or to use actions in that case...
Comment 4•8 years ago
|
||
py_actions don't care where the output goes.
Reporter | ||
Comment 5•8 years ago
|
||
Based on my testing, py_actions only work in targets. So for instance this line in the makefile: https://dxr.mozilla.org/mozilla-central/source/browser/locales/Makefile.in#64 SEARCHPLUGINS_FILENAMES := $(shell $(PYTHON) $(srcdir)/searchplugins.py $(srcdir)/search/list.json $(AB_CD)) Can't be replaced with a py_action. We're creating the list of names early so it can be passed to preprocessing.
Reporter | ||
Comment 6•8 years ago
|
||
Also see: https://bug1300201.bmoattachments.org/attachment.cgi?id=8796588 Where for mobile we will need to generate the list of names early.
Comment 7•8 years ago
|
||
> Can't be replaced with a py_action.
Sure it can
$ ./mach build browser/locales/echo-variable-SEARCHPLUGINS_FILENAMES
0:00.12 /usr/bin/make -C /home/glandium/gecko-push/obj-x86_64-pc-linux-gnu -j4 -s backend
0:00.16 /usr/bin/make -C browser/locales -j4 -s echo-variable-SEARCHPLUGINS_FILENAMES
0:00.20 google yahoo-en-CA amazondotcom bing wikipedia yahoo google-nocodes ddg twitter
0:00.23 Your build was successful!
$ patch -p1 <<\EOF
diff --git a/browser/locales/Makefile.in b/browser/locales/Makefile.in
index c8db2bc..d5e2bac 100644
--- a/browser/locales/Makefile.in
+++ b/browser/locales/Makefile.in
@@ -61,7 +61,7 @@ STUB_HOOK = $(NSINSTALL) -D '$(ABS_DIST)/$(PKG_INST_PATH)'; \
$(NULL)
endif
-SEARCHPLUGINS_FILENAMES := $(shell $(PYTHON) $(srcdir)/searchplugins.py $(srcdir)/search/list.json $(AB_CD))
+SEARCHPLUGINS_FILENAMES := $(shell $(call py_action,searchplugins,$(srcdir)/search/list.json $(AB_CD)))
SEARCHPLUGINS_PATH := .deps/generated_$(AB_CD)
SEARCHPLUGINS_TARGET := libs searchplugins
SEARCHPLUGINS := $(foreach plugin,$(addsuffix .xml,$(SEARCHPLUGINS_FILENAMES)),$(or $(wildcard $(call EN_US_OR_L10N_FILE,searchplugins/$(plugin))),$(warning Missing searchplugin: $(plugin))))
EOF
patching file browser/locales/Makefile.in
$ mv browser/locales/searchplugins.py python/mozbuild/mozbuild/action/
$ ./mach build browser/locales/echo-variable-SEARCHPLUGINS_FILENAMES
(...)
0:17.94 /usr/bin/make -C browser/locales -j4 -s echo-variable-SEARCHPLUGINS_FILENAMES
0:17.98 google yahoo-en-CA amazondotcom bing wikipedia yahoo google-nocodes ddg twitter
Reporter | ||
Comment 8•8 years ago
|
||
Ah. I was trying to call the py_action directly. Didn't think about putting the shell in front of it. Awesome. I'll get a patch together.
Comment hidden (mozreview-request) |
Comment 10•8 years ago
|
||
mozreview-review |
Comment on attachment 8802963 [details] Bug 1309304 - Move search python files to a central location. https://reviewboard.mozilla.org/r/87216/#review87200 Missing .py in the file name for output_searchplugins_list
Attachment #8802963 -
Flags: review?(mh+mozilla)
Comment hidden (mozreview-request) |
Reporter | ||
Comment 12•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8802963 [details] Bug 1309304 - Move search python files to a central location. https://reviewboard.mozilla.org/r/87216/#review87200 Doh. Fixed.
Comment 13•8 years ago
|
||
mozreview-review |
Comment on attachment 8802963 [details] Bug 1309304 - Move search python files to a central location. https://reviewboard.mozilla.org/r/87216/#review88014
Attachment #8802963 -
Flags: review?(mh+mozilla) → review+
Comment 14•8 years ago
|
||
Pushed by mozilla@kaply.com: https://hg.mozilla.org/integration/autoland/rev/f668770cfb69 Move search python files to a central location. r=glandium
Comment 15•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f668770cfb69
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Assignee | ||
Updated•6 years ago
|
Component: Build Config → General
Product: Firefox → Firefox Build System
Updated•5 years ago
|
Target Milestone: Firefox 52 → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•