1309304 - Move python files for generating search info to a central location

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Kaply [:mkaply]]

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.

Comment 1

[Mike Kaply [:mkaply]]

Attachment: Move search python files to build/search

Comment 2

[Mike Hommey [:glandium]]

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)

Comment 3

[Mike Kaply [:mkaply]]

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

[Mike Hommey [:glandium]]

py_actions don't care where the output goes.

Comment 5

[Mike Kaply [:mkaply]]

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.

Comment 6

[Mike Kaply [:mkaply]]

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

[Mike Hommey [:glandium]]

> 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

Comment 8

[Mike Kaply [:mkaply]]

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 9

[Mike Kaply [:mkaply]]

Attachment: Bug 1309304 - Move search python files to a central location.

Review commit: https://reviewboard.mozilla.org/r/87216/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/87216/

Comment 10

[Mike Hommey [:glandium]]

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

Comment 11

[Mike Kaply [:mkaply]]

Comment on attachment 8802963 [details]
Bug 1309304 - Move search python files to a central location.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/87216/diff/1-2/

Comment 12

[Mike Kaply [:mkaply]]

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

[Mike Hommey [:glandium]]

Comment on attachment 8802963 [details]
Bug 1309304 - Move search python files to a central location.

https://reviewboard.mozilla.org/r/87216/#review88014

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f668770cfb69
Move search python files to a central location. r=glandium

Comment 15

[Phil Ringnalda (:philor)]

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