1056136 - 2.1 simulator builds fail to run on Firefox 31

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Simulator, defect)

Description

[Alexandre Poirot [:ochameau]]

Bug 1042239 introduced an exception when trying to use 2.1 builds on Firefox release (and most likely also 32 and 33).

Error loading bootstrap.js for [e-mail]: [Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: resource://gre/modules/addons/XPIProvider.jsm -> file:///C:/Users/Alex/AppData/Roaming/Mozilla/Firefox/Profiles/tf3q5grh.dl/extensions/[e-mail]/bootstrap.js :: <TOP_LEVEL> :: line 29"  data: no] Stack trace: resource://gre/modules/addons/XPIProvider.jsm -> file:///C:/Users/Alex/AppData/Roaming/Mozilla/Firefox/Profiles/tf3q5grh.dl/extensions/[e-mail]/bootstrap.js:29 < resource://gre/modules/addons/XPIProvider.jsm:4136 < XPI_loadBootstrapScope()@resource://gre/modules/addons/XPIProvider.jsm:4136 < XPI_callBootstrapMethod()@resource://gre/modules/addons/XPIProvider.jsm:4205 < AI_startInstall/<()@resource://gre/modules/addons/XPIProvider.jsm:5594 < TaskImpl_run()@resource://gre/modules/Task.jsm:298 < Handler.prototype.process()@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:863 < this.PromiseWalker.walkerLoop()@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:742 < <file:unknown>

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Get rid of SDK dependency for simulator addon.

https://tbpl.mozilla.org/?tree=Try&rev=74ab6cf3c1d3

Comment 2

[Paul Rouget [:paul]]

So... if I'm not mistaken, this bug will also fix bug 1056059, right?

Comment 3

[Alexandre Poirot [:ochameau]]

(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #2)
> So... if I'm not mistaken, this bug will also fix bug 1056059, right?

Hum yes, that should fix it for newer 2.1 builds, if that's really related to bug 1042239.

Comment 4

[Alexandre Poirot [:ochameau]]

Comment on attachment 8475945 [details] [diff] [review]
Get rid of SDK dependency for simulator addon.

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

What do you think about such move?
It looks like going backward, but it should be much more stable...

(Note that bootstrap.js comes from adbhelper which itself comes mostly from an existing bootstrap.js living in m-c)

There is a few hacks like the appinfo.label thing in main.js, but we may be able to tweak that.

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8475945 [details] [diff] [review]
Get rid of SDK dependency for simulator addon.

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

The seems like the path overall.  See various nits below.

Is b2g/simulator/package.json still used after this?  If not, we should remove it.

::: b2g/simulator/bootstrap.js
@@ +2,5 @@
> +const Cu = Components.utils;
> +
> +Cu.import("resource://gre/modules/Services.jsm");
> +
> +// Usefull piece of code from :bent

Nit: Usefull -> Useful

::: b2g/simulator/install.rdf.in
@@ +22,5 @@
> +    <!-- Front End MetaData -->
> +    <em:name>@ADDON_NAME@</em:name>
> +    <em:description>@ADDON_DESCRIPTION@</em:description>
> +    <em:creator>Myk Melez (https://github.com/mykmelez)</em:creator>
> +    

Nit: Trailing whitespace

::: b2g/simulator/lib/main.js
@@ +10,1 @@
>  const System = require("sdk/system");

Do you still need this anymore?

::: b2g/simulator/lib/simulator-process.js
@@ +17,2 @@
>  const URL = require("sdk/url");
> +const Subprocess = require("sdk/system/child_process/subprocess");

This is available back to Fx 31?

If so, you should delete b2g/simulator/packages.

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to J. Ryan Stinnett [:jryans] from comment #5)
> The seems like the path overall.

Meant to say "this seems like a good path overall".  This will keeps us more isolated from SDK changes in the future.

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: Get rid of SDK dependency for simulator addon

Michael, Could you help us review the build_xpi.py modification?
The big difference here is that I'm now crafting a good old bootstrapped addon
instead of using cfx to build an SDK addon.

Comment 8

[Alexandre Poirot [:ochameau]]

(In reply to J. Ryan Stinnett [:jryans] from comment #5)
> Comment on attachment 8475945 [details] [diff] [review]
> Get rid of SDK dependency for simulator addon.
> 
> Review of attachment 8475945 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The seems like the path overall.  See various nits below.
> 
> Is b2g/simulator/package.json still used after this?  If not, we should
> remove it.

Removed!

> 
> ::: b2g/simulator/bootstrap.js
> @@ +2,5 @@
> > +const Cu = Components.utils;
> > +
> > +Cu.import("resource://gre/modules/Services.jsm");
> > +
> > +// Usefull piece of code from :bent
> 
> Nit: Usefull -> Useful

Removed!

> 
> ::: b2g/simulator/install.rdf.in
> @@ +22,5 @@
> > +    <!-- Front End MetaData -->
> > +    <em:name>@ADDON_NAME@</em:name>
> > +    <em:description>@ADDON_DESCRIPTION@</em:description>
> > +    <em:creator>Myk Melez (https://github.com/mykmelez)</em:creator>
> > +    
> 
> Nit: Trailing whitespace

Removed!

> 
> ::: b2g/simulator/lib/main.js
> @@ +10,1 @@
> >  const System = require("sdk/system");
> 
> Do you still need this anymore?

Removed!

> 
> ::: b2g/simulator/lib/simulator-process.js
> @@ +17,2 @@
> >  const URL = require("sdk/url");
> > +const Subprocess = require("sdk/system/child_process/subprocess");
> 
> This is available back to Fx 31?
> 
> If so, you should delete b2g/simulator/packages.

Removed!

It landed in March from bug 900288. I don't have any branch calendar to know exactly in which Fx version it appeared. But it seems available in release branch:
http://mxr.mozilla.org/mozilla-release/source/addon-sdk/source/lib/sdk/system/child_process/subprocess.js

Comment 9

[Michael Shal [:mshal]]

(In reply to Alexandre Poirot [:ochameau] from comment #7)
> Michael, Could you help us review the build_xpi.py modification?
> The big difference here is that I'm now crafting a good old bootstrapped
> addon
> instead of using cfx to build an SDK addon.

Hmm, I am not familiar with this script at all. Is there someone else who can help review?

Comment 10

[Alexandre Poirot [:ochameau]]

:gps did the original build peer review, but I'm wondering how useful it is to make you guys review this script 
as it looks like a lot of internal machinery to the simulator addon.

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8476670 [details] [diff] [review]
Get rid of SDK dependency for simulator addon

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

I only reviewed build_xpi.py. I'm not the right person to look at the rest.

This patch doesn't get r+ until it does zip writing properly.

::: b2g/simulator/build_xpi.py
@@ +138,5 @@
> +    add_file_to_zip(xpi_path, manifest, "install.rdf")
> +    add_file_to_zip(xpi_path, os.path.join(srcdir, "bootstrap.js"), "bootstrap.js")
> +    add_file_to_zip(xpi_path, options_file, "options.xul")
> +    add_file_to_zip(xpi_path, os.path.join(srcdir, "icon.png"), "icon.png")
> +    add_file_to_zip(xpi_path, os.path.join(srcdir, "icon64.png"), "icon64.png")

Please use mozpack.mozjar.JarWriter with optimize=False for producing XPIs. It does things like use a constant value for mtimes so the output is idempotent. This is stuff `cfx xpi` (hopefully) did behind the scenes.

Comment 12

[Alexandre Poirot [:ochameau]]

Attachment: Get rid of SDK dependency for simulator addon

Use JarWriter instead of zip module.

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 8479950 [details] [diff] [review]
Get rid of SDK dependency for simulator addon

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

r+ only covers build_xpi.py and is conditional on passing 'rb' to open().

::: b2g/simulator/build_xpi.py
@@ +70,5 @@
>              relpath = os.path.join(dir_relpath, filename)
>              if relpath in blacklist:
>                  continue
> +            path = os.path.normpath(os.path.join(pathInZip, relpath))
> +            file = open(os.path.join(dirpath, filename))

Python opens files in text mode by default. This means line endings may get normalized. You should explicitly open via open(path, 'rb').

@@ +75,5 @@
> +            mode = os.stat(os.path.join(dirpath, filename)).st_mode
> +            jar.add(path.encode("ascii"), file, mode=mode)
> +
> +def add_file_to_zip(jar, path, pathInZip):
> +    file = open(path)

ditto

Comment 14

[Alexandre Poirot [:ochameau]]

Attachment: Get rid of SDK dependency for simulator addon

https://tbpl.mozilla.org/?tree=Try&rev=852af223cfce

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/87ac83b75957

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/87ac83b75957