636769 - Support specifying add-ons by URL in addition to local path

Status: VERIFIED FIXED

(Testing :: Mozbase, defect)

Description

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Currently add-ons must be specified by local path, however it would be really useful to be able to specify a URL and have Mozmill take care of downloading the add-on for installation.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2] (away July 24th - August 16th)]

It can base on bug 636772 which we are planning to fix soon for our own purpose in the automation scripts. Should be a simple port afterward.

Comment 2

[Jeff Hammel]

Are we talking about the URL of the AMO page (e.g.):

https://addons.mozilla.org/en-US/firefox/addon/adblock-plus/

or the URL of the installation?:

https://addons.mozilla.org/firefox/downloads/latest/1865/addon-1865-latest.xpi

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2] (away July 24th - August 16th)]

We can't be that magic to determine the target XPI file, so yes it means the URL of the XPI itself.

Comment 4

[Jeff Hammel]

Sounds good; we can screen-scrape but since we're stuck with python 2.4 and don't want to add external dependencies I don't really want to do this as our one case of parsing XML in mozmill (reading install.rdf) is horribly broken.  I'll just assume the xpi URL, that is fair enough.

Comment 5

[Jeff Hammel]

Attachment: Support specifying add-ons by URL in addition to local path

Works, ABICT.  I have mixed emotions about it though.  While it is very nice to be able to specify a URL, it will result in downloading the addon each time which is a serious lag. But its 2.0+ so I guess I cant argue with that

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2] (away July 24th - August 16th)]

Jeff, remember that it has to be a list. Users can specify multiple add-ons via the command line.

Comment 7

[Jeff Hammel]

Please look at the Profile __init__ method:

        # handle addon installation
        self.addons_installed = []
        self.addons = addons or []
        for addon in self.addons:
            self.install_addon(addon)

The list is a non-issue.  We should probably move the magical putting addons in a directory up at this level as the purpose of install_addon should, mostly, install a single addon.  Since a URL cannot be a directory of addons this should work fine.

Comment 8

[Heather Arthur [:harth]]

Comment on attachment 521672 [details] [diff] [review]
Support specifying add-ons by URL in addition to local path

looks good, works great. A couple notes:

>+        for prefix in ('http://', 'https://', 'ftp://'):
>+            if path.startswith(prefix):

I'd hate to do a for loop when all we need is an if statement. Why not use urlparse.
We might as well support file:// urls, or any type of valid url really.

We should think about providing some feedback like "downloading baaddon.xpi" because there is a serious pause as it downloads and the user might not know what's going on.

Comment 9

[Jeff Hammel]

I was thinking of just doing `if '://' in path:` and hope its something that urllib2 supports.  That would be shorter.  I specifically picked these three as I know that urllib2 supports them.  For instance..... gopher://host.net/.... ? ssh:// ? Probably not.

Comment 10

[Heather Arthur [:harth]]

(In reply to comment #9)
> I was thinking of just doing `if '://' in path:` and hope its something that
> urllib2 supports.  That would be shorter.  I specifically picked these three as
> I know that urllib2 supports them.  For instance..... gopher://host.net/.... ?
> ssh:// ? Probably not.

yeah, that works.

Comment 11

[Jeff Hammel]

pushed to master with the `if '://' ...` check: https://github.com/mozautomation/mozmill/commit/bbd071ea8c16dee812d819d8c808c6be3bbcba0e 

but if we lose any of our gopher users, i won't be happy ;)

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2] (away July 24th - August 16th)]

Looks good. Thanks for the patch!