Closed
Bug 543038
Opened 13 years ago
Closed 13 years ago
pymake can't build mobile-browser
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: blassey, Assigned: blassey)
Details
Attachments
(1 file, 1 obsolete file)
3.14 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
it currently chokes on calling redit, saying the command returns 127 http://mxr.mozilla.org/mobile-browser/source/app/Makefile.in#150 and on our search plugins packaging: /bin/bash: dpkg-architecture: command not found /bin/bash: -c: line 0: syntax error near unexpected token `(' /bin/bash: -c: line 0: `for line in "locale/en-US/browser/searchplugins/amazondotcom.xml ($(plugin).xml "locale/en-US/br owser/searchplugins/google.xml ($(plugin).xml "locale/en-US/browser/searchplugins/twitter.xml ($(plugin).xml "locale/en- US/browser/searchplugins/wikipedia.xml ($(plugin).xml"); do \' c:\mozilla-central\objdir-wm-standalone\mobile\locales\Makefile:76:0: command 'for line in "locale/en-US/browser/searchp lugins/amazondotcom.xml ($(plugin).xml "locale/en-US/browser/searchplugins/google.xml ($(plugin).xml "locale/en-US/brows er/searchplugins/twitter.xml ($(plugin).xml "locale/en-US/browser/searchplugins/wikipedia.xml ($(plugin).xml"); do \ echo " $line" >> tmp-search.jar.mn; \ done' failed, return code 258 http://mxr.mozilla.org/mobile-browser/source/locales/Makefile.in#73 there may be other failure points
Comment 1•13 years ago
|
||
For that particular case, try replacing it with: $(foreach plugin,$(SEARCH_PLUGINS),echo "locale/$(AB_CD)/browser/searchplugins/$(notdir $(plugin)).xml ($(plugin).xml)" >> tmp-search.jar.mn;) This will still result in a shell invocation in pymake, but it's not terrible. The alternative looks like this: http://mxr.mozilla.org/mozilla-central/source/config/rules.mk#149 so something like: define ADD_SEARCH_PLUGIN echo "$1" >> $2 endef # do not remove the blank line! searchplugins:: @echo "$(AB_CD).jar:" > tmp-search.jar.mn $(foreach plugin,$(SEARCH_PLUGINS),$(call,ADD_SEARCH_PLUGIN,locale/$(AB_CD)/browser/searchplugins/$(notdir $(plugin)).xml ($(plugin).xml),tmp-search.jar.mn)) This should not result in shell invocation in pymake.
Assignee | ||
Comment 2•13 years ago
|
||
Assignee: nobody → bugmail
Attachment #429924 -
Flags: review?(mark.finkle)
Comment 3•13 years ago
|
||
Comment on attachment 429924 [details] [diff] [review] patch >diff --git a/app/Makefile.in b/app/Makefile.in >--- a/app/Makefile.in >+++ b/app/Makefile.in >- $(REDIT_PATH)/redit $(DIST)/bin/$(APP_BINARY) $(srcdir)/$(APP_ICON).ico >+ $(shell $(REDIT_PATH)/redit $(DIST)/bin/$(APP_BINARY) $(srcdir)/$(APP_ICON).ico) You sure about this? This will run this command (possibly at makefile parse time), then execute the stdout as a shell command in the rule. >diff --git a/locales/Makefile.in b/locales/Makefile.in >--- a/locales/Makefile.in >+++ b/locales/Makefile.in >@@ -61,29 +61,30 @@ SUBMAKEFILES += \ >-searchplugins:: >- @echo "$(AB_CD).jar:" > tmp-search.jar.mn >- @for line in $(foreach plugin,$(SEARCH_PLUGINS),"locale/$(AB_CD)/browser/searchplugins/$(notdir $(plugin)).xml ($(plugin).xml)"); do \ >- echo " $$line" >> tmp-search.jar.mn; \ >- done >+tmp-search.jar.mn:: >+ @echo -n "$(AB_CD).jar:" > $@ >+ printf "$(foreach plugin,$(SEARCH_PLUGINS),$(subst __PLUGIN_SUBST__,$(plugin), \n locale/$(AB_CD)/browser/searchplugins/__PLUGIN_SUBST__.xml (__PLUGIN_SUBST__.xml)))" >> $@ >+ @echo >> $@ Did my suggestions above not work? This seems really complicated, and it still doesn't save you a shell invocation (since you have redirection there, pymake needs to invoke sh).
Assignee | ||
Comment 4•13 years ago
|
||
(In reply to comment #3) > (From update of attachment 429924 [details] [diff] [review]) > >diff --git a/app/Makefile.in b/app/Makefile.in > >--- a/app/Makefile.in > >+++ b/app/Makefile.in > >- $(REDIT_PATH)/redit $(DIST)/bin/$(APP_BINARY) $(srcdir)/$(APP_ICON).ico > >+ $(shell $(REDIT_PATH)/redit $(DIST)/bin/$(APP_BINARY) $(srcdir)/$(APP_ICON).ico) > > You sure about this? This will run this command (possibly at makefile parse > time), then execute the stdout as a shell command in the rule. perhaps this worked for me because it wasn't a clobber build. Got any suggestions? > Did my suggestions above not work? This seems really complicated, and it still > doesn't save you a shell invocation (since you have redirection there, pymake > needs to invoke sh). I couldn't get your code to work, did you test it?
Comment 5•13 years ago
|
||
Can you show me the exact error that redit line gives? I tested my code in this sample Makefile: ALL = 1 2 3 4 5 $(shell rm a) a: $(foreach x,$(ALL),echo $(strip $(x)) >> $@;) invoking gnu make shows: make: Entering directory `/tmp/foo' echo 1 >> a; echo 2 >> a; echo 3 >> a; echo 4 >> a; echo 5 >> a; make: Leaving directory `/tmp/foo' and the file 'a' contains the expected output. (using pymake produces the same output.)
Assignee | ||
Comment 6•13 years ago
|
||
redit failure:
c:\mozilla-central\objdir-x86\mobile\app\Makefile:195:0$ c:/mozilla-central/objdir-x86/dist/bin/redit ../../dist/bin/fen
nec.exe ../../../mobile/app/mobile.ico
[Error 2] The system cannot find the file specified
c:\mozilla-central\objdir-x86\mobile\app\Makefile:195:0: command 'c:/mozilla-central/objdir-x86/dist/bin/redit ../../dis
t/bin/fennec.exe ../../../mobile/app/mobile.ico' failed, return code -127
c:\mozilla-central\config\rules.mk:736:0: command 'c:/mozilla-build/python25/python.exe c:/pymake/make.py libs' failed,
return code 2
> Did my suggestions above not work? This seems really complicated, and it still
> doesn't save you a shell invocation (since you have redirection there, pymake
> needs to invoke sh).
btw... do you think its overly complicated because of the subst? That is to work around an apparent bug in pymake that it only substitutes the first occurrence of $(plugin). This works with regular make bug fails for pymake:
printf "$(foreach plugin,$(SEARCH_PLUGINS), \n locale/$(AB_CD)/browser/searchplugins/$(plugin).xml ($(plugin).xml))" >> $@
Comment 7•13 years ago
|
||
For the record, we sorted out the redit failure, it needs a $(BIN_SUFFIX) or $(HOST_BIN_SUFFIX).
Assignee | ||
Comment 8•13 years ago
|
||
this also has a fix for a build bustage on windows desktop
Attachment #429924 -
Attachment is obsolete: true
Attachment #430109 -
Flags: review?(mark.finkle)
Attachment #429924 -
Flags: review?(mark.finkle)
Updated•13 years ago
|
Attachment #430109 -
Flags: review?(mark.finkle) → review+
Comment 9•13 years ago
|
||
For the record, at some point we might end up doing similar code for Firefox for searchplugins. Blunt question, should we just code that piece of tmp-search.jar.mn creation in a python script?
Comment 10•13 years ago
|
||
That'd also be fine.
Assignee | ||
Comment 11•13 years ago
|
||
pushed http://hg.mozilla.org/mobile-browser/rev/47b658f2a87d
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 12•13 years ago
|
||
echo -n isn't cross-platform. I don't remember which platform busted on that, but I do recall having to patch that in favor of printf, IIRC.
Comment 13•13 years ago
|
||
I think it's Mac, actually. Brad: just replace the "echo -n" with printf. (printf doesn't put a newline in by default.)
Updated•5 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•