746131 - CFX in JS - Craft an SDK addon called by cfx in python in order to install the xpi

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Alexandre Poirot [:ochameau]]

It would be the very first step of CFX in JS development:
Craft an SDK addon that will allow to implement CFX workflow steps in Javascript.

We basically need to figure out:
 - How to build this SDK addon required by CFX to work. We can easily face Chicken or the Egg issue. CFX needs this addon in order to work, and this addon is built by CFX.
 - Where we are going to host this addon. In addon-sdk repository? If yes, where? Or are we going to host it in a dedicated repo?
 - What is the safiest way to communicated between CFX in Python and this addon. Have to work on every platforms and environnements. (I'm thinking loud about tinderbox!)

Comment 1

[Alexandre Poirot [:ochameau]]

Work in progress:
https://github.com/ochameau/addon-sdk/compare/master...cfx.js

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 410

WIP patch in order to give some visiblity to this work.
Feedback is welcomed, but I'm still waiting before asking review.

Comment 3

[Alexandre Poirot [:ochameau]]

Self hosted or in addon-sdk repository?

For an addon developer, it isn't really valuable to download sources of cfx.js.
The developer just need the xpi of it. So that there is no much value in hosting cfx.js in current addon-sdk repository for developers.
We may just host cfx.js in its own repository, like addon builder helper addon. It allows to make a smaller SDK and decouple SDK modules from tools that live around them.
But it comes with some disavantages. For example it is a bit harder, or at least, it takes some additional time to develop cfx.js when some modifications are needed in SDK modules. Just because we have to handle two git repositories and use a very specific addon-sdk revision for some cfx.js changes.
It will require some work on tbpl in order to run cfx.js too.

Comment 4

[Alexandre Poirot [:ochameau]]

Comment on attachment 617750 [details]
Pull request 410

I've updated the pull request. I subtly changed the focus of it.

Instead of installing the xpi, I'm building it instead.
In fact, it didn't make much sense to order cfxpy to install cfxjs in order to just install the user addon. It even complefy cfxpy implementation for mobile.
As it would have to behave first as an headless application to build the xpi,
and then as a firefox addon in order to install the user addon.
So I'd prefer keeping addon installation in python until we are able to launch firefox directly from javascript/cfxjs.

This patch introduce cfxjs, still in addon-sdk repository. I haven't received any feedback about this, but I'm still open to land it in its own repo.
cfxjs now behaves as an headless xulrunner application.
I've crafted a xulrunner app template here:
  https://github.com/ochameau/addon-sdk/tree/5c5d8ec22592899d6ba482bdd4623395be92598d/cfx/xulrunner-app
It basically execute this CommandLineHandler.js xpcom which fake an install of the addon living next to him: cfx.js. So that we can build cfxjs as a regular addon and just have to put it in this directory in order to have it working as a xulrunner command line app!

Then you may take a look at cfxjs README:
  https://github.com/ochameau/addon-sdk/blob/5c5d8ec22592899d6ba482bdd4623395be92598d/cfx/README.md
In order to see how it works.
To summarize, it looks for an environnement variable which contains a path to a JSON file containing necessary information to run a command.
In this patch, it is only using one command that build an xpi.
Note that xpi.js file is pythonish. It was made on purpose in order to implement same thing than python code. I'm open to iterate on this now that it is working and covered by unit tests.

This patch is backward compatible. I only missing simple pref implementation and some minor stuff like files blacklist. I've done my best to implement something doing exactly the same thing than python code.

Now we have various options: 
- review and land this,
- eventually continue and do the same for packages parser and manifest building,
- wait for the new packageless manifest implementation in js before reviewing/landing the whole thing.

I'd say that we can land this first as the piece of code which is going to change for the new manifest is quite small. I think that we will only have to change this:
https://github.com/ochameau/addon-sdk/blob/5c5d8ec22592899d6ba482bdd4623395be92598d/cfx/xpi.js#L108-145
And then, just wait for the new manifest built in js.

Comment 5

[Alexandre Poirot [:ochameau]]

Daniel, we will have to synchronize us on this bug. When this is going to land, you will need a firefox/xulrunner binary on Addon builder servers which execute cfx, in order to be able to run "cfx xpi".

Comment 6

[Alexandre Poirot [:ochameau]]

Pushed to try:
https://tbpl.mozilla.org/?tree=Try&rev=edd75b6f345c

Comment 7

[Alexandre Poirot [:ochameau]]

Comment on attachment 617750 [details]
Pull request 410

All tests now pass on tinderbox!!
This patch is now ready to be reviewed. Unfortunately I had to rebase it because of various issues with git :( (I wasn't able to rebase a revision before pushing it, I unfortunately commited a huge xpi file that I didn't want to put in git history)

It should now be 99.99% the same than python implementation. I've added preferences and file filtering implementation. So that we can land it to master. But I'm still open to landing it in a branch if you think it is a better option.

The only cfx behavior modification is for mobile development. You will now have to pass the path to ADB via `--adb` option, instead of `--binary`. That's because you may pass a custom pass to firefox binary by using --binary in order to build the xpi by cfx.js.

All existing python tests are still running but against JS implementation.

Otherwise I kept the python xpi building implementation as it can be handy while working on cfx.js. cfx.js is a regular sdk addon that needs to be built by cfx and installed here: `cfx/xulrunner-app/cfx.xpi`. So if anything goes wrong with cfx.js you can enable python implementation in xpi.py in order to build it. We can remove it during next cfx.js steps as it won't work with new manifest.

Comment 8

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 617750 [details]
Pull request 410

Please hold on landing this for a bit, details are in pull request.

Comment 9

[Alexandre Poirot [:ochameau]]

Closing this bug as everything got reviewed and landed in cfx-js upstream repo.