Closed Bug 961745 Opened 6 years ago Closed 6 years ago

Find a way to build Firefox mulet

Categories

(Firefox OS Graveyard :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
2.0 S1 (9may)

People

(Reporter: ochameau, Assigned: ochameau)

References

Details

(Whiteboard: [systemsfe][priority])

Attachments

(1 file, 14 obsolete files)

4.29 KB, patch
mshal
: review+
Details | Diff | Splinter Review
"Firefox mulet" is mostly Firefox but built with most b2g flags turned on, like MOZ_B2G, but not only. We need to find a (sane) way to build such product that is a merge of b2g/ and browser/.
Whiteboard: [systemsfe]
Attached patch patch (obsolete) — Splinter Review
Some tweaks to ensure that content permission handler from b2g/
isn't overloaded by the one from browser/

This patch is full of hacks, someone with build system background
should figure out how to make a mulet sanely!
Do we plan to provide shims for device APIs which will not be available on desktop (such as telephony, messaging, etc.)?
That's not a short term goal for me or my team. The challenge here is to build Firefox with all b2g flags, and make b2g work inside on firefox with a parity with b2g desktop (same features/bugs/limitations). TBH, that's already a vast project.

In parralel various shims have been hacked into gaia build system, but that's really hard to maintain as it is only enabled when running gaia on Firefox desktop with a quite hacky firefox addon.
The "mulet" will allow us to first get rid of shims for API enabled on b2g desktop, but not on Firefox.

Hopefully, with this environment unification, it will encourage everyone to contribute to m-c and implement real fallback directly into the platform instead of having shims enabled only on some particular setup.

Having said that, if someone have cycles to work on shims/fallbacks, that would be a really great contribution!
Sounds god.  I added this for discussion to the next week's DOM/WebAPI work week.
Attached patch patch (obsolete) — Splinter Review
updated, rebased. Also try to ship b2g-l10n package,
used, at least, by aboutCertError.xhtml.
Attachment #8364628 - Attachment is obsolete: true
https://tbpl.mozilla.org/?tree=Try&rev=73f969823c87

This try build says that "./mach build package" fails:
0:08.44 Traceback (most recent call last):
 0:08.44   File "/mnt/desktop/gecko/toolkit/mozapps/installer/packager.py", line 375, in <module>
 0:08.44     main()
 0:08.44   File "/mnt/desktop/gecko/toolkit/mozapps/installer/packager.py", line 367, in main
 0:08.44     args.source, gre_path, base)
 0:08.44   File "/mnt/desktop/gecko/toolkit/mozapps/installer/packager.py", line 148, in precompile_cache
 0:08.44     errors.fatal('Error while running startup cache precompilation')
 0:08.44   File "/mnt/desktop/gecko/python/mozbuild/mozpack/errors.py", line 101, in fatal
 0:08.44     self._handle(self.FATAL, msg)
 0:08.44   File "/mnt/desktop/gecko/python/mozbuild/mozpack/errors.py", line 96, in _handle
 0:08.44     raise ErrorMessage(msg)
 0:08.44 mozpack.errors.ErrorMessage: Error: Error while running startup cache precompilation

And prevents tbpl to expose tarball and see how it goes on win/mac.
Comment on attachment 8370079 [details] [diff] [review]
patch

Review of attachment 8370079 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/components/moz.build
@@ +46,5 @@
> +# When building the mulet, this moz.build is being
> +# referenced from browser/moz.build, which set
> +# DIST_SUBDIR='browser' whereas we don't expect
> +# to put b2g components in a sub dir
> +DIST_SUBDIR = ''

Without this, all b2g components are put in a sub folder.
For example, ErrorPage.jsm would not be at:
  resource://gre/modules/ErrorPage.jsm
  objdir/dist/bin/modules/ErrorPage.jsm
But instead, here:
  resource:///modules/ErrorPage.jsm
  objdir/dist/bin/browser/modules/ErrorPage.jsm

::: browser/app/Makefile.in
@@ +6,5 @@
>  
>  PREF_JS_EXPORTS = $(srcdir)/profile/firefox.js \
>  		  $(NULL)
>  
> +PREF_JS_EXPORTS += $(topsrcdir)/b2g/app/b2g.js

That's the only think we pull from b2g/app/ folder,
we may also want to use b2g desktop icons, and there is a bunch of stuff inside b2g/app/Makefile.in. LIBS flags may be usefull, and also User agent override is implemented there.

::: browser/components/BrowserComponents.manifest
@@ +46,2 @@
>  contract @mozilla.org/content-permission/prompt;1 {d8903bf6-68d5-4e97-bcd1-e4d3012f721a}
> +#endif

I do this, otherwise, the content permission prompt component of Firefox overloads the b2g one from b2g/components/. For some unknown reason, BrowserComponents.manifest takes the lead of B2GComponents.manifest from b2g/components...
Some other xpcom may be overloaded like that.

::: browser/components/moz.build
@@ +34,2 @@
>  EXTRA_PP_COMPONENTS += [
> +    'BrowserComponents.manifest',

That's useless, I though it would change the order/priority.

::: browser/confvars.sh
@@ +68,5 @@
>  # Enable exact rooting on desktop.
>  JSGC_USE_EXACT_ROOTING=1
>  
> +
> +MOZ_B2G=1

I think that's useless given the AC_DEFINE(MOZ_B2G) in configure.sh
This is an updated version that removes the useless parts, and guards the usefl against the presence of MOZ_MULET. We also add a --enable-mulet flag only valid with the browser app, that forces MOZ_B2G.
Attachment #8370079 - Attachment is obsolete: true
Comment on attachment 8373984 [details] [diff] [review]
0001-Bug-961745-Adding-enable-mulet-and-MOZ_MULET.patch

Review of attachment 8373984 [details] [diff] [review]:
-----------------------------------------------------------------

I pushed a rebased-against-last-master mulet branch:
  https://github.com/ochameau/mozilla-central/tree/mulet
with your patch, and removed the two unecessary modifications.

::: browser/components/moz.build
@@ +38,1 @@
>  EXTRA_PP_COMPONENTS += [

^

::: browser/confvars.sh
@@ +68,4 @@
>  # Enable exact rooting on desktop.
>  JSGC_USE_EXACT_ROOTING=1
>  
> +

^
Updating patch after Alexandre's rebase.
Attachment #8373984 - Attachment is obsolete: true
(In reply to Alexandre Poirot (:ochameau) from comment #6)
> https://tbpl.mozilla.org/?tree=Try&rev=73f969823c87
> 
> This try build says that "./mach build package" fails:
> 0:08.44 Traceback (most recent call last):
>  0:08.44   File "/mnt/desktop/gecko/toolkit/mozapps/installer/packager.py",
> line 375, in <module>
>  0:08.44     main()
>  0:08.44   File "/mnt/desktop/gecko/toolkit/mozapps/installer/packager.py",
> line 367, in main
>  0:08.44     args.source, gre_path, base)
>  0:08.44   File "/mnt/desktop/gecko/toolkit/mozapps/installer/packager.py",
> line 148, in precompile_cache
>  0:08.44     errors.fatal('Error while running startup cache precompilation')
>  0:08.44   File "/mnt/desktop/gecko/python/mozbuild/mozpack/errors.py", line
> 101, in fatal
>  0:08.44     self._handle(self.FATAL, msg)
>  0:08.44   File "/mnt/desktop/gecko/python/mozbuild/mozpack/errors.py", line
> 96, in _handle
>  0:08.44     raise ErrorMessage(msg)
>  0:08.44 mozpack.errors.ErrorMessage: Error: Error while running startup
> cache precompilation
> 
> And prevents tbpl to expose tarball and see how it goes on win/mac.

I have a quick fix for this, like if join() does not correctly handle an empty set of arguments. I'm currently checking why we call it with an empty set.
Evidences for now would suggest that OS.Constants.Path.profileDir is undefined
It is undefined because of an exception in lazyPathGetter ...

 0:12.74 Exception::[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/osfile/osfile_async_front.jsm :: lazyPathGetter/< :: line 97"  data: no]lazyPathGetter('profileDir', 'ProfD') -> undefined
 0:12.95 Exception::[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/osfile/osfile_async_front.jsm :: lazyPathGetter/< :: line 97"  data: no]lazyPathGetter('profileDir', 'ProfD') -> undefined
 0:13.65 Exception::[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/osfile/osfile_async_front.jsm :: lazyPathGetter/< :: line 97"  data: no]lazyPathGetter('profileDir', 'ProfD') -> undefined
 0:13.73 Exception::[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/osfile/osfile_async_front.jsm :: lazyPathGetter/< :: line 97"  data: no]lazyPathGetter('profileDir', 'ProfD') -> undefined
 0:13.75 Exception::[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/osfile/osfile_async_front.jsm :: lazyPathGetter/< :: line 97"  data: no]lazyPathGetter('profileDir', 'ProfD') -> undefined
 0:14.21 Exception::[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/osfile/osfile_async_front.jsm :: lazyPathGetter/< :: line 97"  data: no]lazyPathGetter('profileDir', 'ProfD') -> undefined
 0:14.22 Exception::[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/osfile/osfile_async_front.jsm :: lazyPathGetter/< :: line 97"  data: no]lazyPathGetter('profileDir', 'ProfD') -> undefined
 0:14.27 Exception::[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/osfile/osfile_async_front.jsm :: lazyPathGetter/< :: line 97"  data: no]lazyPathGetter('profileDir', 'ProfD') -> undefined
 0:14.37 Exception::[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/osfile/osfile_async_front.jsm :: lazyPathGetter/< :: line 97"  data: no]lazyPathGetter('profileDir', 'ProfD') -> undefined
 0:14.65 Exception::[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/osfile/osfile_async_front.jsm :: lazyPathGetter/< :: line 97"  data: no]lazyPathGetter('profileDir', 'ProfD') -> undefined
Looks like I experience those errors also when building a browser app without mulet enabled. And while I do see the same errors, we do not load DownloadsAPI.jsm and thus do no call to OS.Path.join() with one value being undefined. I think this code is just buggy but I can't explain why mulet would make it visible and not b2gdesktop.
The original errors during package are about l10n key errors in 'gcli.jsm' which is firefox specific. (i.e. not shipped in b2g desktop)
(In reply to Alexandre Poirot (:ochameau) from comment #15)
> The original errors during package are about l10n key errors in 'gcli.jsm'
> which is firefox specific. (i.e. not shipped in b2g desktop)

I do see the same behavior regarding OS.Constants.Path, also.

So meanwhile I've enabled a quick hack until I dig further and sent to try: https://tbpl.mozilla.org/?tree=Try&rev=2b5766d11d21
Attached file package.mulet.log (obsolete) —
Output of make -f client.mk package with mulet
Attached file package.browser.log (obsolete) —
Output of make -f client.mk package without mulet
Both attachments exposes lines with:
Exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/osfile/osfile_async_front.jsm :: lazyPathGetter/< :: line 97"  data: no]TypeError: i is undefined
Try tentative with the mulet code but not enabling --enable-mulet: https://tbpl.mozilla.org/?tree=Try&rev=2b5766d11d21

We see breakage on B2G Emulator :(, but I can't understand why yet.

I sent a new Try in which I force enable mulet: https://tbpl.mozilla.org/?tree=Try&rev=b9f8a3585ce1
Okay so now we have breakage during l10n-check step.
I've added a hack for l10n-check to pass, the resulting Try is running at: https://tbpl.mozilla.org/?tree=Try&rev=6cbfd3e40c8c

We'll need some help from pike probably to make it not a hack.
I'm starting to get some green build finally!
https://tbpl.mozilla.org/?tree=Try&rev=b39fd7ad3730
Okay, looks some methods in OS.Path implems for Unix and Windows are not happy during the packaging step: https://tbpl.mozilla.org/php/getParsedLog.php?id=34683445&tree=Try
 - OS.Path.join() exposes TypeError when some arguments are |undefined|
 - OS.Path.winGetDrive() exposes TypeError when the path is |undefined|

It seems that in the context of this |make package| target those behavior are expected: as far as I could understand the code [http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/precompile_cache.js#81] which is executed, it just does a Cu.import() to get precompiled code.
Depends on: 973538
New try pending at: https://tbpl.mozilla.org/?tree=Try&rev=e777f7668671

For now it seems to be a bit better
Please find attached a patch that:
 - adds a --enable-mulet config flag
 - defines MOZ_MULET=1 and MOZ_B2G=1 when this flag is set

There is a new pending try to make sure we don't break things https://tbpl.mozilla.org/?tree=Try&rev=59a4c77b9a17
Attachment #8374061 - Attachment is obsolete: true
Attachment #8389140 - Flags: review?(gps)
Comment on attachment 8389140 [details] [diff] [review]
0001-Bug-961745-Adding-enable-mulet-and-MOZ_MULET.patch

Seems like we have an issue :(
Attachment #8389140 - Flags: review?(gps)
$ strace -ff ./mach build package 2>&1 | egrep '\/tmp\/tmp.*\.zip'
[pid 22585] open("/tmp/tmpL6rB2v.zip", O_RDWR|O_CREAT|O_EXCL|O_NOFOLLOW, 0600) = 5
[pid 22585] unlink("/tmp/tmpL6rB2v.zip") = 0
[pid 22619] access("/tmp/tmpL6rB2v.zip", F_OK) = -1 ENOENT (No such file or directory)
[pid 22619] access("/tmp/tmpL6rB2v.zip", F_OK) = -1 ENOENT (No such file or directory)
[pid 22619] open("/tmp/tmpL6rB2v.zip", O_RDWR|O_CREAT, 0664) = 13
[pid 22619] access("/tmp/tmpL6rB2v.zip", F_OK) = 0
[pid 22619] open("/tmp/tmpL6rB2v.zip", O_RDONLY) = 13
[pid 22585] open("/tmp/tmpL6rB2v.zip", O_RDONLY) = 5
[pid 22585] stat("/tmp/tmpL6rB2v.zip", {st_mode=S_IFREG|0664, st_size=4417895, ...}) = 0
[pid 22585] unlink("/tmp/tmpL6rB2v.zip") = 0
[pid 22585] open("/tmp/tmpbQmwOY.zip", O_RDWR|O_CREAT|O_EXCL|O_NOFOLLOW, 0600) = 5
[pid 22585] unlink("/tmp/tmpbQmwOY.zip") = 0
[pid 22633] access("/tmp/tmpbQmwOY.zip", F_OK) = -1 ENOENT (No such file or directory)
[pid 22585] open("/tmp/tmpbQmwOY.zip", O_RDONLY) = -1 ENOENT (No such file or directory)
[pid 22585] stat("/tmp/tmpbQmwOY.zip", 0x7fff181f16a0) = -1 ENOENT (No such file or directory)
[pid 22567] write(1, " 0:13.56 IOError: [Errno 2] No s"..., 76 0:13.56 IOError: [Errno 2] No such file or directory: '/tmp/tmpbQmwOY.zip'
Updated version of the patch. Fixes the breakage locally, new try pending: https://tbpl.mozilla.org/?tree=Try&rev=4aaca07ed9c2.

The bug was because I used some python code in a Makefile for a condition on MOZ_MULET :)
Attachment #8389140 - Attachment is obsolete: true
Attachment #8389299 - Flags: review?(gps)
I won't be able to review this until next week. Try :mshal or :glandium if you need a review sooner.
Target Milestone: --- → 1.4 S4 (28mar)
Comment on attachment 8389299 [details] [diff] [review]
0001-Bug-961745-Adding-enable-mulet-and-MOZ_MULET.patch

Review of attachment 8389299 [details] [diff] [review]:
-----------------------------------------------------------------

Having browser/ reference code under b2g/ is a bit wonky. It feels very wrong to me.

Some context.

Directories in the tree/build system typically belong to one of Gecko/core or an application (such as browser/ or b2g/).

The general pattern for components that are utilized by multiple applications is to have them live inside the Gecko/core parts. Because of the way packaging works, this means they can't live inside the application directory (browser/, b2g/). Well, technically it means that the directory doesn't chain to an application's moz.build file. This is related to the DIST_SUBDIR hack in this patch. A common dumping ground for these cross-application components is toolkit/.

Anyway, I would highly prefer one of the following solutions:

1) Move Mulet (the source code) out of b2g/ and into toolkit/ or wherever.
2) Move the DIRS/PARALLEL_DIRS reference of mulet (wherever it is - I don't see it in my tree) into toolkit/toolkit.mozbuild (making it part of Gecko).

I don't fully grok what Mulet is. Perhaps I'm misunderstanding the request here.

::: browser/moz.build
@@ +31,5 @@
> +    PARALLEL_DIRS += [
> +        '../b2g/chrome',
> +        '../b2g/components',
> +        '../b2g/locales'
> +    ]

This is very wrong because it will include lots of b2g components in the desktop browser that shouldn't be there.
Attachment #8389299 - Flags: review?(gps) → review-
The mulet is the french word for mule. That explain part of this project goal.
Instead of mixing a horse with a donkey, we would like to mix b2g desktop with Firefox.

So it is about replacing b2g-desktop with a special Firefox build, called Mulet, which has:
- all desktop b2g-specific flags turned on (like MOZ_B2G and most likely various other),
- ships all b2g-specific components that currently works on desktop (so we do want b2g/components),
- also ships b2g/chrome so that we can run FirefoxOS/gaia into Firefox. The code in b2g/chrome and most importantly shell.js is the key glue that plugs the system app with all toolkit/platform code.

That to allow getting rid of b2g-desktop and replace it with this Mulet.
We are going this to offer a unique and devtool-enabled environment on desktop.
b2g desktop sucks as there is no devtools on it, and it competes with Firefox.
Gaia developers are used to develop gaia in Firefox nightly, but Firefox runs Gaia in a very poor mode and it is being done with many hacks that are really hard to maintain.

Having said that, I have to agree with you that this patch is very hacky (I'm the author of the few hacks it contains...). But I wish you could help us finding a way to give birth to this mulet without messing with the build system!
See comment 32. If the Mulet is still unclear to you, we can take some time on vidyo to chat about it.
Feel free also to involve anyone that may help or should be involved in such build system discussion.
I'm also planning to write a blogpost about the mulet to clarify this project to everyone.
Flags: needinfo?(gps)
Whiteboard: [systemsfe] → [systemsfe][priority]
Target Milestone: 1.4 S4 (28mar) → 1.4 S5 (11apr)
Rebased.
Attachment #8389299 - Attachment is obsolete: true
Attachment #8404641 - Flags: review?(gps)
Depends on: 981664
Jonas wrote a dev-b2g group post about the mulet. https://groups.google.com/forum/?hl=en#!topic/mozilla.dev.b2g/2WUg10993OM 
This should answer some of your questions.
Target Milestone: 1.4 S5 (11apr) → 1.4 S6 (25apr)
Blocks: 997096
Comment on attachment 8404641 [details] [diff] [review]
0001-Bug-961745-Adding-enable-mulet-and-MOZ_MULET.patch

Michael is looking at this.
Attachment #8404641 - Flags: review?(gps) → review?(mshal)
Attached patch mulet-application.patch (obsolete) — Splinter Review
I agree with gps' concerns - given the hackyness required in browser/ and b2g/, this seems like it requires a different solution. Attached is a patch to try to make mulet a separate top-level application - since you used the 'browser' application as the main starting point, I copied these files from there into mulet/ and made a few modifications:

 - confvars.sh, adding MOZ_B2G=1 (so it isn't needed in configure.in)
 - app.mozbuild, adding the two b2g directories to recurse into
 - build.mk (I think I copied this from b2g/ originally)

Some concerns I have with this approach:

1) Does this even work for you? I'm not familiar enough with mulet to really know if it was built properly. I was able to at least compile and run 'make check' using this mozconfig:

ac_add_options --enable-signmar
ac_add_options --enable-profiling
ac_add_options --disable-elf-hack # --enable-elf-hack conflicts with --enable-profiling

# Nightlies only since this has a cost in performance
ac_add_options --enable-js-diagnostics

ac_add_options --enable-mulet

# This will overwrite the default of stripping everything and keep the symbol table.
# This is useful for profiling and debugging and only increases the package size
# by 2 MBs.
STRIP_FLAGS="--strip-debug"

ac_add_options --enable-application=mulet

Though the --enable-mulet shouldn't be necessary anymore, since it uses --enable-application=mulet

2) I'm not sure what approval is needed to actually create a top-level application - I think you may need to go through bsmedberg? And it probably needs a better justification than "mshal thought the patch would look nicer".

3) The files in mulet/ are my best-guesses based on what I understood your patch to be doing. If this is actually a valid route to go down, you'll need to go through them and make sure it has everything you need (and remove extra stuff that shouldn't be there). Again my only cursory knowledge of mulet limits me here.
Attachment #8411439 - Flags: feedback?(jonas)
Comment on attachment 8404641 [details] [diff] [review]
0001-Bug-961745-Adding-enable-mulet-and-MOZ_MULET.patch

Please see #c37
Attachment #8404641 - Flags: review?(mshal) → review-
The main concern I have is that as people change browser/confvars.sh, they'll forget to make the same change to browser/confvars.sh.

Could we make mulet/confvars.sh source browser/confvars.sh?
Attachment #8411439 - Flags: feedback?(jonas) → feedback?(poirot.alex)
Attachment #8374405 - Attachment is obsolete: true
Attachment #8374406 - Attachment is obsolete: true
Attachment #8404641 - Attachment is obsolete: true
Comment on attachment 8411439 [details] [diff] [review]
mulet-application.patch

Review of attachment 8411439 [details] [diff] [review]:
-----------------------------------------------------------------

Sounds good to me, I just didn't knew how to do that.
I share the same concern about confvars.sh, we really don't want to maintain any copy of b2g/browser file.
But I have an additional patch on top of this one that proves we can source it.

You patch only miss two important things to make it work:
 - AC_SUBST(MOZ_MULET) in configure.in
 - b2g/app/b2g.js hack in browser/app/Makefile.in
  Again... I don't know what we could do from the app.mozbuild to inject b2g.js without hacking browser Makefile.

Otherwise, with you patch and these additional fixes,
I succesfully build a Mulet with just:
  ac_add_options --enable-application=mulet
Attachment #8411439 - Flags: feedback?(poirot.alex) → feedback+
Attached patch fix for mulet application (obsolete) — Splinter Review
Here is the additional patch
Comment on attachment 8411862 [details] [diff] [review]
fix for mulet application

Any idea for b2g/app/b2g.js?
Attachment #8411862 - Flags: feedback?(mshal)
Comment on attachment 8411862 [details] [diff] [review]
fix for mulet application

Personally I am not so concerned with the b2g/app/b2g.js line living in browser/app/Makefile.in. Given what mulet is, I think we have to expect some of that :).

I think this addresses the main hacks I was concerned about - the DIST_SUBDIR='' thing in b2g/components/moz.build is gone, and the recursion from browser into ../b2g is gone too (right?)

The main roadblock of getting approval for a top-level application still needs to be addressed, of course, but I am happy with how this is shaping up.
Attachment #8411862 - Flags: feedback?(mshal) → feedback+
Ochameau: We discussed on irc and came to the conclusion that the easiest thing to do here is to make the mulet product live in something like "/b2g/dev" rather than in "/mulet". Introducing a new top-level directory is more work, and mulet is quite b2g specific so it makes sense to have it live somewhere under /b2g.

If you put together a rolled up patch that does that then I think we have something that's ready to ship.
Here is a merge of the two patches, with mulet moved to b2g/dev.

This patch is the very first step to start building it.
There is most likely some tweaks to add.
For example, we will most likely also want to pull the b2g/confvars.sh.

The only thing required in mozconfig is:
  ac_add_options --enable-application=b2g/dev
Attachment #8411439 - Attachment is obsolete: true
Attachment #8411862 - Attachment is obsolete: true
Comment on attachment 8412167 [details] [diff] [review]
Introduce b2g/dev application to build a mulet. r=mshal,fabrice

Something like this?
Are we ready to land if I get reviews from mshal and fabrice?

(I'm waiting it to build sucessfully before asking reviews)
Attachment #8412167 - Flags: feedback?(jonas)
Assignee: lissyx+mozillians → poirot.alex
Comment on attachment 8412167 [details] [diff] [review]
Introduce b2g/dev application to build a mulet. r=mshal,fabrice

"It works".

I don't know how much the mulet works with just this patch,
so you may want to try my branch, I just pushed with this patch (and many others)
  https://github.com/ochameau/mozilla-central/commits/mulet

To test, you have to build with this in your mozconfig:
  ac_add_options --enable-application=b2g/dev
And start firefox-bin with a gaia profile, then if shell.html isn't automatically opened, open manually chrome://b2g/content/shell.html
Attachment #8412167 - Flags: review?(mshal)
Attachment #8412167 - Flags: review?(fabrice)
Comment on attachment 8412167 [details] [diff] [review]
Introduce b2g/dev application to build a mulet. r=mshal,fabrice

Review of attachment 8412167 [details] [diff] [review]:
-----------------------------------------------------------------

This looks great

::: b2g/dev/confvars.sh
@@ +5,5 @@
> +
> +MOZ_B2G=1
> +MOZ_MULET=1
> +
> +. ../browser/confvars.sh

shouldn't this be ". ../../browser/confvars.sh"?
Attachment #8412167 - Flags: feedback?(jonas) → feedback+
Comment on attachment 8412167 [details] [diff] [review]
Introduce b2g/dev application to build a mulet. r=mshal,fabrice

Review of attachment 8412167 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, but we really need Mike's review.
Attachment #8412167 - Flags: review?(fabrice) → review+
(In reply to Jonas Sicking (:sicking) from comment #49)
> ::: b2g/dev/confvars.sh
> @@ +5,5 @@
> > +
> > +MOZ_B2G=1
> > +MOZ_MULET=1
> > +
> > +. ../browser/confvars.sh
> 
> shouldn't this be ". ../../browser/confvars.sh"?

Yes I know this is weird but that works, whereas ../../browser/confvars.sh doesn't.
It doesn't make sense to me given how confvars.sh is evaluated:
  http://mxr.mozilla.org/mozilla-central/source/configure.in#4128
    . "${srcdir}/${MOZ_BUILD_APP}/confvars.sh"
(In reply to Alexandre Poirot (:ochameau) from comment #51)
> (In reply to Jonas Sicking (:sicking) from comment #49)
> > ::: b2g/dev/confvars.sh
> > @@ +5,5 @@
> > > +
> > > +MOZ_B2G=1
> > > +MOZ_MULET=1
> > > +
> > > +. ../browser/confvars.sh
> > 
> > shouldn't this be ". ../../browser/confvars.sh"?
> 
> Yes I know this is weird but that works, whereas ../../browser/confvars.sh
> doesn't.
> It doesn't make sense to me given how confvars.sh is evaluated:
>   http://mxr.mozilla.org/mozilla-central/source/configure.in#4128
>     . "${srcdir}/${MOZ_BUILD_APP}/confvars.sh"

Then I'd say that we rather want to source it the same way, with an absolute path and not a relative one.
(In reply to Alexandre Poirot (:ochameau) from comment #51)
> Yes I know this is weird but that works, whereas ../../browser/confvars.sh
> doesn't.
> It doesn't make sense to me given how confvars.sh is evaluated:
>   http://mxr.mozilla.org/mozilla-central/source/configure.in#4128
>     . "${srcdir}/${MOZ_BUILD_APP}/confvars.sh"

Sourcing a shell script is apparently done relative to the cwd, not relative to where the script is. The working directory is m-c/objdir/, so m-c/objdir/../browser/confvars.sh picks up the correct file (it's not m-c/b2g/app/../../browser/confvars.sh, like you might expect). I agree that a absolute path would be more straightforward:

. ${srcdir}/browser/confvars.sh
Comment on attachment 8412167 [details] [diff] [review]
Introduce b2g/dev application to build a mulet. r=mshal,fabrice

>diff --git a/b2g/dev/build.mk b/b2g/dev/build.mk
>new file mode 100644
>index 0000000..ac82714
>--- /dev/null
>+++ b/b2g/dev/build.mk
>@@ -0,0 +1,31 @@
>+# This Source Code Form is subject to the terms of the Mozilla Public
>+# License, v. 2.0. If a copy of the MPL was not distributed with this
>+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
>+
>+include $(topsrcdir)/toolkit/mozapps/installer/package-name.mk
>+
>+installer:
>+	@$(MAKE) -C browser/installer installer
>+
>+package:
>+	@$(MAKE) -C browser/installer
>+
>+install::
>+	@echo "Mulet can't be installed directly."
>+	@exit 1
>+
>+upload::
>+	@$(MAKE) -C browser/installer upload
>+
>+ifdef ENABLE_TESTS
>+# Implemented in testing/testsuite-targets.mk
>+
>+mochitest-browser-chrome:
>+	$(RUN_MOCHITEST) --browser-chrome
>+	$(CHECK_TEST_ERROR)
>+
>+mochitest:: mochitest-browser-chrome
>+
>+.PHONY: mochitest-browser-chrome
>+endif

Have you verified that all of these targets are necessary / correct for mulet? In particular, are you sure that they should call browser/installer and not b2g/installer? If so that's fine, I just want to avoid cargo-culting if possible.

>+. ../browser/confvars.sh

I agree with #c52 - using ${srcdir} would be easier to understand.
Attachment #8412167 - Flags: review?(mshal) → review+
Target Milestone: 1.4 S6 (25apr) → 2.0 S1 (9may)
- use absolute path for confvars
- include browser/build.mk:
  I think we just want firefox targets.

The patch ends up even simplier...
Attachment #8412167 - Attachment is obsolete: true
Attachment #8413995 - Flags: review?(mshal)
Comment on attachment 8413995 [details] [diff] [review]
Introduce b2g/dev application to build a mulet

Review of attachment 8413995 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/dev/confvars.sh
@@ +6,5 @@
> +MOZ_B2G=1
> +MOZ_MULET=1
> +
> +. ../browser/confvars.sh
> +. ${srcdir}/browser/confvars.sh

You only want the second of these two lines, right?
yes...
Attachment #8413995 - Attachment is obsolete: true
Attachment #8413995 - Flags: review?(mshal)
Attachment #8414020 - Flags: review?(mshal)
Comment on attachment 8414020 [details] [diff] [review]
Introduce b2g/dev application to build a mulet

Looks good!
Attachment #8414020 - Flags: review?(mshal) → review+
\o/ I've no idea what could possibly break with such small patch, but just to be sure:
  https://tbpl.mozilla.org/?tree=Try&rev=8cee48179f10
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1f9bd75bb714
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.