543038 - pymake can't build mobile-browser

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

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

[(not currently active) Ted Mielczarek]

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.

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

Comment 3

[(not currently active) Ted Mielczarek]

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

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

(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

[(not currently active) Ted Mielczarek]

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

Comment 6

[Brad Lassey [:blassey] (use needinfo?)]

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

[(not currently active) Ted Mielczarek]

For the record, we sorted out the redit failure, it needs a $(BIN_SUFFIX) or $(HOST_BIN_SUFFIX).

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch v.2

this also has a fix for a build bustage on windows desktop

Comment 9

[Axel Hecht [:Pike]]

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

[(not currently active) Ted Mielczarek]

That'd also be fine.

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

pushed http://hg.mozilla.org/mobile-browser/rev/47b658f2a87d

Comment 12

[Axel Hecht [:Pike]]

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

[(not currently active) Ted Mielczarek]

I think it's Mac, actually.

Brad: just replace the "echo -n" with printf. (printf doesn't put a newline in by default.)