551322 - explicit flag labeling a Jetpack-based extension

Status: RESOLVED WONTFIX

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

Description

[Myk Melez [:myk] [@mykmelez]]

There should be an explicit flag labeling a Jetpack-based extension, so Firefox and AMO have to introspect XPIs to determine which ones are Jetpack-based (f.e. when Firefox is determining whether or not it must be restarted after installing the extension).

Perhaps this is as simple as an install.rdf triple?

Comment 1

[Atul Varma [:atul]]

Sweet. The install.rdf file that's used as a template for Jetpack-based extensions is at python-lib/cuddlefish/app-extension/install.rdf, so just adding the triple there should be enough, I think.

If we want to include what version of the SDK made the extension, it will require some more complexity. However, for such metadata I recommend just looking at the harness-options.json file in the root directory of the extension, as it contains metadata on what all packages are included in the XPI--this is generally more useful than what version of the SDK was used to build the XPI, I think.

Comment 2

[Justin Scott [:fligtar]]

From AMO's point of view, it doesn't really matter to us what this TRIPLE is called or whether it has the version number in it. If you think it's a good idea for us to store the SDK version number in our db for easy filtering or something down the road that we haven't thought of yet, that's doable.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

This could be something like:

  <em:type>64</em:type>

Where "64" is the type value for a no-restart Jetpack-based extension, which Firefox currently ignores but will eventually treat specially.  But perhaps this affects AMO's and Firefox's treatment of the extension in undesirable ways?  It also conflates "no-restart" with "Jetpack-based", which is problematic, as traditional extensions will be able to take advantage of the no-restart support, and we want to use this to generate metrics on how many extensions are Jetpack-based.

So instead we could do something like:

  <em:jetpack>true</em:jetpack>

fligtar, mossop, thoughts?

Comment 4

[Dave Townsend [:mossop]]

(In reply to comment #3)
> This could be something like:
> 
>   <em:type>64</em:type>
> 
> Where "64" is the type value for a no-restart Jetpack-based extension, which
> Firefox currently ignores but will eventually treat specially.  But perhaps
> this affects AMO's and Firefox's treatment of the extension in undesirable
> ways?  It also conflates "no-restart" with "Jetpack-based", which is
> problematic, as traditional extensions will be able to take advantage of the
> no-restart support, and we want to use this to generate metrics on how many
> extensions are Jetpack-based.
> 
> So instead we could do something like:
> 
>   <em:jetpack>true</em:jetpack>

It depends on what behaviors we want to see changed based on the existence of this property. I presume at a minimum a nicer install experience, but in order for that to be true we would have to do some verification that the package really is safe script with a real Jetpack SDK.

Comment 5

[Atul Varma [:atul]]

AFAIK, we don't actually need any behaviors changed based on the existence of this property--it's just a flag that tells us whether or not the XPI was built with the Jetpack SDK, really.  The really easy way to do this is to look for a file called 'harness-options.json' alongside the install.rdf, which fligtar said they could deal with--was just the case that adding something to install.rdf would make it even easier for AMO to detect.

Comment 6

[Justin Scott [:fligtar]]

I think this might be 2 things, as mentioned above. AMO will want to know whether the add-on is restart-less, regardless of whether it's built with Jetpack, to display the UI flag. But we also want to know whether it's built with Jetpack for ease of reviewing later on.

So we can either have the 2 flags or AMO can look for harness-options.json permanently, but we'll definitely need the no-restart flag for non-Jetpacks.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #6)
> I think this might be 2 things, as mentioned above. AMO will want to know
> whether the add-on is restart-less, regardless of whether it's built with
> Jetpack, to display the UI flag. But we also want to know whether it's built
> with Jetpack for ease of reviewing later on.

Yup, this is absolutely right.  I'm guessing the restartless flag will get shaken out as part of the EM rewrite, so let's focus this bug on the flag for letting AMO know that an extension was built using Jetpack.

fligtar: if it's just as easy for AMO to look for harness-options.json, then we can leave it at that and wontfix this bug; but if it's easier for AMO to have an install.rdf flag, then let's add one, for which my latest thinking is that we have an em:jetpack flag whose value can be anything:

  <em:jetpack>{}</em:jetpack>

AMO can then treat this as a boolean flag, whose presence indicates that the extension was built using Jetpack.  Then, in the future, if we need to provide more information (f.e. whether it was built using the SDK or FlightDeck, we can include it in the value (perhaps as JSON).

How does that sound?

Comment 8

[Atul Varma [:atul]]

Note that harness-options.json actually already contains a lot of metadata about the jetpack-based extension, such as what third-party packages (including version info) it is comprised of, etc.  The layout of harness-options.json unfortunately isn't documented yet, but it's likely that if AMO wants lots of information about the addon's composition, it will either have to look in harness-options.json in the future or the relevant information will need to be duplicated from this file and copied into the install.rdf.

Comment 9

[Dave Townsend [:mossop]]

I think it'd be best to just keep jetpack specific info in the jetpack specific file. If you do need to add a property to install.rdf purely for AMO's purposes then I think for purity you should use a different namespace for the property, jp: or something.

Comment 10

[Atul Varma [:atul]]

If we have to make a new namespace, I would really like it to have a fairly easy to remember identifier like "http://jetpack.mozillalabs.com/v1". I can never remember XHTML's or XUL's. :(

Comment 11

[Justin Scott [:fligtar]]

If this bug is solely for Jetpack, then sure, we can just use the existing file.

Comment 12

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #11)
> If this bug is solely for Jetpack, then sure, we can just use the existing
> file.

Ok, great!  In that case, I'm wontfixing this bug.  Instead, AMO will use the presence of harness-options.json to determine that an extension is Jetpack-based, and if it needs additional information, it'll introspect that file to obtain it (and if the info it needs is not in that file, we'll add it to it).

The issue of how an extension notifies AMO and Firefox that it is restartless can then be worked out as part of the EM rewrite.

Comment 13

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".