Last Comment Bug 746124 - (cfx.js) CFX in Javascript
: CFX in Javascript
Product: Add-on SDK
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: P1 normal (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
Depends on: native-jetpack 638249 746131 772681 783070 903029
Blocks: 745877
  Show dependency treegraph
Reported: 2012-04-17 06:31 PDT by Alexandre Poirot [:ochameau]
Modified: 2013-12-17 20:03 PST (History)
7 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

Pointer to Github pull request: (373 bytes, text/html)
2013-06-13 12:07 PDT, Irakli Gozalishvili [:irakli] [:gozala] [@gozala]
evold: review+
Pointer to Github pull request: (358 bytes, text/html)
2013-06-13 17:51 PDT, Irakli Gozalishvili [:irakli] [:gozala] [@gozala]
evold: review+

Description Alexandre Poirot [:ochameau] 2012-04-17 06:31:45 PDT
Tracking bug in order to follow progress around "CFX in JS" feature:

Preliminary discussion happened on etherpad:

This work can easily be splitted in multiple, eventually parallel steps:
 - Be able to execute Javascript code hosted in an addon from cfx in python
Then we would be able do implement following CFX workflow steps in JS:
 - Install and execute an Addon
 - Generate the XPI out of the manifest
 - Compute the manifest out of packages/modules
 - Read/seek for packages/modules on filesystem
 - Interpret command line options
 - Build a command line application with mozilla-platform/Firefox
 - Write shell/bash scripts in order to setup cfx environnement (equivalent of source bin/activate)

Depending on which order we are going to land these steps, this work will allow to improve Addon-builder-helper addon by building xpi on client side. Same thing apply if we want to provide a GUI instead of the existing cfx command line interface. 
We can decide that 4 last steps don't really worth it or are less urgent that improving Addon-builder and execute them later on.

All these steps can be analysed and prototyped in parallel, but they will most likely have to land in this order.

Last but not least, we will have to synchronize this work with:
 - packageless feature
 - Addon SDK as an Addon
 - Ship SDK via AMO

Packageless can directly be implemented in JS, it would just require first steps of this work to be landed.
Ship SDK via AMO task can be simplified if we get completely rid of python dependency. We won't have to bootstrap, nor interact with python environnement from an Addon.
Comment 1 Irakli Gozalishvili [:irakli] [:gozala] [@gozala] 2013-06-13 12:07:04 PDT
Created attachment 762210 [details]
Pointer to Github pull request:

Pointer to Github pull-request
Comment 2 [github robot] 2013-06-13 12:09:10 PDT
Commit pushed to master at
Merge pull request #646 from Gozala/jep/cfx-js

Bug 746124 - Implement nodejs compatible `fs` and `path` modules.
Comment 3 Irakli Gozalishvili [:irakli] [:gozala] [@gozala] 2013-06-13 17:51:51 PDT
Created attachment 762424 [details]
Pointer to Github pull request:

Pointer to Github pull-request
Comment 4 [github robot] 2013-06-14 11:42:38 PDT
Commit pushed to master at
Merge pull request #1032 from Gozala/jep/cfx-js

Bug 746124 - Fix windows specific issues in FS module. r=@erikvold
Comment 5 [github robot] 2013-06-23 12:44:45 PDT
Commits pushed to australis at
Merge pull request #646 from Gozala/jep/cfx-js
Merge pull request #1032 from Gozala/jep/cfx-js
Comment 6 Wes Kocher (:KWierso) 2013-07-01 19:40:37 PDT
So, when this was uplifted in today's merge to mozilla-inbound, the path tests failed on only OSX:

TEST-UNEXPECTED-FAIL | tests/test-path.test path | 
  actual="/builds/slave/talos-slave/test/build/" - 0 == 2
TEST-INFO | Traceback (most recent call last):
  File "resource://gre/modules/commonjs/sdk/timers.js", line 31, in notify
    callback.apply(null, args);
  File "resource://gre/modules/commonjs/sdk/deprecated/unit-test.js", line 294, in null
    timer.setTimeout(function() { onDone(self); }, 0);
  File "resource://gre/modules/commonjs/sdk/deprecated/unit-test.js", line 470, in null
  File "resource://gre/modules/commonjs/sdk/deprecated/unit-test.js", line 452, in runNextTest
    self.start({test: test, onDone: runNextTest});
  File "resource://gre/modules/commonjs/sdk/deprecated/unit-test.js", line 483, in start
  File "resource://gre/modules/commonjs/test.js", line 72, in null
  File "resource://extensions.modules.f90e1ffb-5306-4834-89d0-b14d5b874846-at-jetpack.commonjs.path.tests/test-path.js", line 235, in exports["test path"]
    assert.equal(failures.length, 0, failures.join(''));
  File "resource://gre/modules/commonjs/sdk/test/assert.js", line 125, in equal
    operator: "=="
  File "resource://gre/modules/commonjs/sdk/test/assert.js", line 89, in fail;
  File "resource://gre/modules/commonjs/sdk/deprecated/unit-test.js", line 84, in fail
    this.console.testMessage(false, false,, message);
  File "resource://gre/modules/commonjs/sdk/test/harness.js", line 523, in testMessage

Not sure why it only failed on inbound and not the Jetpack tree, though...
Comment 7 Dave Townsend [:mossop] 2013-07-02 09:27:50 PDT
At a guess this is a bug that only occurs when using the modules in Firefox
Comment 8 [github robot] 2013-07-03 20:52:15 PDT
Commit pushed to master at
Merge pull request #1080 from Gozala/hotfix/path

Bug 746124 - Pull updates for path module from upstream node.
Comment 9 Wes Kocher (:KWierso) 2013-07-03 21:59:37 PDT
So, the try run looked pretty green, so I merged that pull request to master up in comment 8.

The Jetpack tree's tests then ran, and came back all orange:

TBPL's parser isn't catching the error, but it's down near the bottom of the logs:
Testing all packages...
Testing all available packages: /builds/slave/talos-slave/test/addonsdk-poller/addon-sdk-0f7d8b1df10d.
Testing /builds/slave/talos-slave/test/addonsdk-poller/addon-sdk-0f7d8b1df10d...
Traceback (most recent call last):
  File "bin/cfx", line 33, in <module>
  File "/builds/slave/talos-slave/test/addonsdk-poller/addon-sdk-0f7d8b1df10d/python-lib/cuddlefish/", line 617, in run
    test_all(env_root, defaults=options.__dict__)
  File "/builds/slave/talos-slave/test/addonsdk-poller/addon-sdk-0f7d8b1df10d/python-lib/cuddlefish/", line 397, in test_all
    test_all_packages(env_root, defaults)
  File "/builds/slave/talos-slave/test/addonsdk-poller/addon-sdk-0f7d8b1df10d/python-lib/cuddlefish/", line 490, in test_all_packages
  File "/builds/slave/talos-slave/test/addonsdk-poller/addon-sdk-0f7d8b1df10d/python-lib/cuddlefish/", line 763, in run
  File "/builds/slave/talos-slave/test/addonsdk-poller/addon-sdk-0f7d8b1df10d/python-lib/cuddlefish/", line 657, in build_manifest, test_filter_re)
  File "/builds/slave/talos-slave/test/addonsdk-poller/addon-sdk-0f7d8b1df10d/python-lib/cuddlefish/", line 228, in build
    tme = self.process_module(tmi)
  File "/builds/slave/talos-slave/test/addonsdk-poller/addon-sdk-0f7d8b1df10d/python-lib/cuddlefish/", line 410, in process_module
    them_me = self.find_req_for(mi, reqname, looked_in, locations)
  File "/builds/slave/talos-slave/test/addonsdk-poller/addon-sdk-0f7d8b1df10d/python-lib/cuddlefish/", line 464, in find_req_for
    raise BAD("too many ..")
cuddlefish.manifest.BadModuleIdentifier: too many .. in require(../system) from ModuleInfo [addon-sdk tests test-path] (/builds/slave/talos-slave/test/addonsdk-poller/addon-sdk-0f7d8b1df10d/test/test-path.js, None)

program finished with exit code 1

Testing with cfx locally, I see this error, too.

Playing around with the require statement in test-path.js, if I change it from ../system to sdk/system on just that one line, all tests pass for me locally.

I'm going to push to try and see what happens:

Since I'll be gone for the next five days and won't be able to see the results of this until I get back, I'll just back out pull request 1080 tonight to get things back to mostly green on the jetpack tree.

If anyone gets to this over the next few days and sees that my new try run is green, you could reland 1080 and 1081 together and see how that works.
Comment 10 Wes Kocher (:KWierso) 2013-07-03 22:00:53 PDT
Looking at the test results with pull 1081 included, all of the tests that match the filter "path" (100 of them) pass, but most print "undefined". I assume that's what should be printed for these?
Comment 11 Wes Kocher (:KWierso) 2013-07-03 22:13:44 PDT
And the backout of pr1080:
Comment 12 Wes Kocher (:KWierso) 2013-07-03 22:18:59 PDT
(In reply to Wes Kocher (:KWierso) from comment #9)

> Playing around with the require statement in test-path.js, if I change it
> from ../system to sdk/system on just that one line, all tests pass for me
> locally.

I should add that I only changed it in test-path.js, not the identical require() in path.js ...
Comment 13 Wes Kocher (:KWierso) 2013-07-03 23:39:19 PDT

I give up.
Comment 14 Irakli Gozalishvili [:irakli] [:gozala] [@gozala] 2013-07-15 09:57:37 PDT
I don't think this still requires needinfo from me.
Comment 15 Erik Vold [:erikvold] (please needinfo? me) 2013-12-17 20:03:17 PST
I'm going to wontfix this, as far as I could tell the plan here was to have js build bootstrap extensions from jetpacks, in bug 915376 we are instead going to make the whole building setup redundant.

Note You need to log in before you can comment on or make changes to this bug.