Closed Bug 879563 Opened 11 years ago Closed 11 years ago

Add option for installing additional extensions in DEBUG mode

Categories

(Firefox OS Graveyard :: Gaia, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: eeejay, Assigned: eeejay)

References

()

Details

Attachments

(1 file, 1 obsolete file)

There are tools out there that are useful for Gaia development in the form of Firefox extensions. It would be cool if the Gaia build system could pull in those tools and install them in the profile.
This idea was first mentioned in bug #875040.
Attachment #758285 - Flags: review?(poirot.alex)
Comment on attachment 758285 [details] [diff] [review]
Added option for installing external addons

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

I'm not sure we want a dependency to AMO as addons may be hosted outside of AMO.
Also, we need fine control of addon version in gaia in order to handle eventual incompatibility over time.

Here is two examples reflecting these two cases:
  v1-train may not work with very last version of the addon,
  or gaia master may only work with addon not being reviewed yet on AMO.

I tend to think a list of xpi URL would be enough.
Then it is only matter of downloading xpis and copying them in $(EXT_DIR).
(You are uncompressing addons in your patch, is this really required?)
https://github.com/mozilla-b2g/gaia/blob/master/Makefile#L457
(In reply to Alexandre Poirot (:ochameau) from comment #3)
> Comment on attachment 758285 [details] [diff] [review]
> Added option for installing external addons
> 
> Review of attachment 758285 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm not sure we want a dependency to AMO as addons may be hosted outside of
> AMO.
> Also, we need fine control of addon version in gaia in order to handle
> eventual incompatibility over time.
> 
> Here is two examples reflecting these two cases:
>   v1-train may not work with very last version of the addon,
>   or gaia master may only work with addon not being reviewed yet on AMO.
> 
> I tend to think a list of xpi URL would be enough.

Instead of 'amo', you could provide 'url' for a direct download url. The nice thing about support an amo slug/id is that it would always be the latest stable version.

> Then it is only matter of downloading xpis and copying them in $(EXT_DIR).
> (You are uncompressing addons in your patch, is this really required?)
> https://github.com/mozilla-b2g/gaia/blob/master/Makefile#L457

From the experiments that I have done, I have found that the xpi/directory needs to match the addon id/guid. And if an addon specifies unpack, it needs to be uncompressed.

If I am wrong, then that is good news! Bug I couldn't figure that out.
I see how handy it is to always be on the latest version, but we can't introduce something in the repo that fetch a random external dependency.
We need to control at gaia repo level that this particular gaia revision is compatible with this particular addon version.
If a developer want to use a bleeding edge version, he should do it with its own 
personal settings. May be we should allow a custom build/custom-addons.json?

I see (at least) two options here: 
 - use absolute URL
 - let developer file a custom-addons file with its own versioning choice. (that would make it not shipped by default which kind of sucks)
 - an hybrid version where we set an URL and allow developer to override this with its custom addons setting.

Otherwise, I didn't knew about the id requirement, but I tested and verified in addon manager, you are right. Then I imagine you are also right about the unpack story...
(In reply to Alexandre Poirot (:ochameau) from comment #5)
> I see how handy it is to always be on the latest version, but we can't
> introduce something in the repo that fetch a random external dependency.
> We need to control at gaia repo level that this particular gaia revision is
> compatible with this particular addon version.
> If a developer want to use a bleeding edge version, he should do it with its
> own 
> personal settings. May be we should allow a custom build/custom-addons.json?
> 
> I see (at least) two options here: 
>  - use absolute URL
>  - let developer file a custom-addons file with its own versioning choice.
> (that would make it not shipped by default which kind of sucks)
>  - an hybrid version where we set an URL and allow developer to override
> this with its custom addons setting.
> 

How about additional-extensions.json being version controlled in git. And the user could create another file called custom-additional-extensions.json (that we would add to .gitignore), that would pull in things the user specifies.

The one we provide would use absolute URLs, but I could keep in the AMO id option for the custom files.

I could change the JSON format to be a top-level dictionary instead of a list, so that we could do dict.update() to override the bundled JSON with the user specified one.
(In reply to Eitan Isaacson [:eeejay] from comment #6)
> How about additional-extensions.json being version controlled in git. And
> the user could create another file called custom-additional-extensions.json
> (that we would add to .gitignore), that would pull in things the user
> specifies.
> 
> The one we provide would use absolute URLs, but I could keep in the AMO id
> option for the custom files.
> 
> I could change the JSON format to be a top-level dictionary instead of a
> list, so that we could do dict.update() to override the bundled JSON with
> the user specified one.

Sounds good!
This adds an option for a custom-extensions.json file to allow overriding.
Attachment #758285 - Attachment is obsolete: true
Attachment #758285 - Flags: review?(poirot.alex)
Attachment #760467 - Flags: review?(poirot.alex)
Comment on attachment 760467 [details] [diff] [review]
Bug 879563 - Added option for installing external addons

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

Thanks for the modifications!

::: Makefile
@@ +304,5 @@
>  	@$(call run-js-command, offline-cache)
>  	@echo "Done"
>  
> +# Get additional extensions
> +profile/installed-extensions.json: build/additional-extensions.json $(wildcard .build/custom-extensions.json)

Is `wildcard` any usefull here? 
Can't we just depend on `build/custom-extensions.json`?

@@ +305,5 @@
>  	@echo "Done"
>  
> +# Get additional extensions
> +profile/installed-extensions.json: build/additional-extensions.json $(wildcard .build/custom-extensions.json)
> +ifeq ($(BROWSER),1)

`BROWSER` has been very recently renamed to `DESKTOP`.
Attachment #760467 - Flags: review?(poirot.alex) → review+
Thanks!

(In reply to Alexandre Poirot (:ochameau) from comment #9)
> Comment on attachment 760467 [details] [diff] [review]
> Bug 879563 - Added option for installing external addons
> 
> Review of attachment 760467 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks for the modifications!
> 
> ::: Makefile
> @@ +304,5 @@
> >  	@$(call run-js-command, offline-cache)
> >  	@echo "Done"
> >  
> > +# Get additional extensions
> > +profile/installed-extensions.json: build/additional-extensions.json $(wildcard .build/custom-extensions.json)
> 
> Is `wildcard` any usefull here? 
> Can't we just depend on `build/custom-extensions.json`?
> 

A hard dependency would require that file to exist. We should only depend on it if the file exists (and rebuild that target when it is modified). Using a whildcard assures that there is no match if it does not exist, and thus, not a dependency.

> @@ +305,5 @@
> >  	@echo "Done"
> >  
> > +# Get additional extensions
> > +profile/installed-extensions.json: build/additional-extensions.json $(wildcard .build/custom-extensions.json)
> > +ifeq ($(BROWSER),1)
> 
> `BROWSER` has been very recently renamed to `DESKTOP`.

I rebased and updated.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.