Bug #908580 was a symptom of this problem and fixed it only partly: submakes uses XULrunner too and they need to know it is. For instance, in bug 908698, TBPL redefines the Xulrunner directory, and therefore the email makefile will fail there too.
Created attachment 795015 [details] [diff] [review] patch v1 github pull request at https://github.com/mozilla-b2g/gaia/pull/11738 In this patch, we export the new XULRUNNER_DIRECTORY variable along with the existing various xulrunner program paths. These paths have been made absolute using make's `abspath` operation. The patch also removes the ./ prefixes to those paths as they're likely useless and prevent from using an absolute path when defining XULRUNNER_DIRECTORY from the command line. --- Makefile | 12 +++++++----- apps/email/Makefile | 15 --------------- 2 files changed, 7 insertions(+), 20 deletions(-) asking review from Alexandre, Andrew for the email bit, and I'd like to have James feedback too. Especially, I'd like someone to test this on MacOS as I don't have that here, and file path operations can bring issues. Thanks
Comment on attachment 795015 [details] [diff] [review] patch v1 This seems like a good idea to avoid having our sub-Makefile try and figure things out that the top-level Makefile knows. Another alternative would be to create an "xulrunner.mk" and let the e-mail app "include ../../xulrunner.mk". I think the increase in complexity is unlikely to be worth the cost, especially since we already can do APP=email in the root Makefile. So I suggest we add the following to the top: === cut here === # We can't figure out XULRUNNERSDK on our own; it's complex and some builders # may want to override our find logic (ex: TBPL), so let's just leave it up to # the root Makefile. If you know what you're doing, you can manually define # XULRUNNERSDK and XPCSHELLSDK on the command line. ifndef XULRUNNERSDK $(error This Makefile needs to be run by the root gaia makefile. Use APP=email) endif === r=asuth with that change, assuming it works :)
Attachment #795015 - Flags: review?(bugmail) → review+
James Burke also setup the Makefile immediately, so he might have some thoughts. Oh, and let me revise my r=asuth to be something along the lines of what I propose. Mainly I want us to error out if you try and run the Makefile from within our subdir, and explain a little context so we're never tempted to copy the logic back down into our Makefile again.
Comment on attachment 795015 [details] [diff] [review] patch v1 Hey Yuren, if you can review this before Alexandre, please do :)
Attachment #795015 - Flags: review?(yurenju.mozilla)
Andrew> I think I'll use your code as-is, thanks ! (the other possibility is to check for this in the target using bash, which I find less readable) Alexandre, Yuren> I'd like to replace "make" with "$(MAKE)" in the "app-makefiles" target, so that it will just work for people that use "gmake" instead of "make". WHat do you think ?
sounds good for gmake user on mac/bsd :-)
I'll review the PR tomorrow morning since I'm leaving from office.
Comment on attachment 795015 [details] [diff] [review] patch v1 Looks good.
Attachment #795015 - Flags: review?(yurenju.mozilla) → review+
Created attachment 795884 [details] [diff] [review] patch v2 Carrying r=yurenju,asuth Uses $(MAKE) instead of "make" when calling submake in the apps (I decided to not change the other "make" call, as I don't know what it's for). Will try this patch on Windows and MacOS X before landing.
Doesn't impact mainline dogfooding per recent smoketests.
No longer blocks: 884399
This patch will be integrated in the new patch for bug 906316.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.