Move python files for generating search info to a central location

RESOLVED FIXED in Firefox 52

Status

()

Firefox
Build Config
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mkaply, Unassigned)

Tracking

50 Branch
Firefox 52
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
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

2 years ago
Created attachment 8799955 [details] [diff] [review]
Move search python files to build/search
(Reporter)

Updated

2 years ago
Attachment #8799955 - Flags: review?(mh+mozilla)
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

2 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...
py_actions don't care where the output goes.
(Reporter)

Comment 5

2 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

2 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.
> 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

2 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

2 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

2 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

2 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

2 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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f668770cfb69
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
You need to log in before you can comment on or make changes to this bug.