647693 - no way to migrate an AMO-hosted existing traditional extension (with UUID) to jetpack (with Program ID)

Status: RESOLVED FIXED

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

Description

[[:philipp]]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0) Gecko/20100101 Firefox/4.0
Build Identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0) Gecko/20100101 Firefox/4.0

i have a traditional extension hosted on AMO - which has a GUID  specified in the <em:id>/em:id> element of its install.rdf file.
now i want to migrate that extension's development to the new addon-sdk, which automatically signs the .xpi-file & assigns a Jetpack-/Program ID (derived from a private key) which is also set as <em:id>. when i'm trying to upload the xpi file generated by the addon-sdk as a new version of my addon to AMO, i get the error: "UUID doesn't match add-on."

There should be a way to assign a Jetpack-/Program ID to an existing extension, in order for end users getting a new extension-version built by the addon sdk via auto-update.

Reproducible: Always

Comment 1

[Jorge Villalobos [:jorgev] (he/him)]

As a workaround, you should be able to change the ID to whatever you want after the add-on is built.

Comment 2

[[:philipp]]

i originally tried this, but got an error claiming the archive is corrupted when i attempted to install it in firefox afterwards (therefore i originally thouhgt, there is some kind of signing mechanism involved). now i've tried it again and it's working as you said - thanks for the tip!

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I'm going through bugs in Bugzilla that haven't been touched in a few months, and this one hasn't been used since April.

I've hit this bug a bunch personally while writing add-ons and would like to see something done about it. The workaround does work, but you have to do the manual change every time before submitting new versions to AMO, which is a serious pain. (Plus, I don't know if changing to a non-jetpack ID messes with AMO's detection of Jetpack-based add-ons...)

CCing clouserw to see what he thinks. Is there any way for AMO to allow changing the add-on ID it has registered for the add-on? Even if you had to go into and do a one-time edit to the add-on's listing to register the new ID, it'd be easier than doing a manual edit to the add-on each time prior to uploading it to AMO.

If it can't/won't technically be done, feel free to WONTFIX the bug. :)

Comment 4

[Piotr Zalewa [:zalun]]

We are working on syncing Addons on Builder and AMO.

Comment 5

[Wil Clouser [:clouserw]]

All the private key signing stuff is gone, if I remember correctly, so that shouldn't be a problem anymore.  As Piotr said, we're working on syncing the sites.

Comment 6

[Daniel Buchner [:dbuc]]

(In reply to madperson from comment #0)
> User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0) Gecko/20100101
> Firefox/4.0
> Build Identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0) Gecko/20100101
> Firefox/4.0
> 
> i have a traditional extension hosted on AMO - which has a GUID  specified
> in the <em:id>/em:id> element of its install.rdf file.
> now i want to migrate that extension's development to the new addon-sdk,
> which automatically signs the .xpi-file & assigns a Jetpack-/Program ID
> (derived from a private key) which is also set as <em:id>. when i'm trying
> to upload the xpi file generated by the addon-sdk as a new version of my
> addon to AMO, i get the error: "UUID doesn't match add-on."
> 
> There should be a way to assign a Jetpack-/Program ID to an existing
> extension, in order for end users getting a new extension-version built by
> the addon sdk via auto-update.
> 
> Reproducible: Always

Is this an Add-on Builder-specific thing, or something that the SDK does on it's own?

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I think it's an AMO thing. 

When you upload an addon to AMO, after the first upload, the addon can not change its ID (used in various pings for updates and probably other things). So when the addon gets ported to Jetpack and gets a randomly assigned jid, the ported addon is rejected as a new version of the addon when you try to upload it to AMO.

I'm not sure if it's possible to carry over the original ID into the SDK, it might not work correctly for various modules expecting a jid value.

Comment 8

[[:philipp]]

when you're putting the original UUID into package.json & try creating the extension via the SDK with 'cfx xpi', the process fails with the following error message (i've tried it with addon-sdk 1.1):

Traceback (most recent call last):
  File "C:\addon-sdk-1.1\bin\cfx", line 29, in <module> cuddlefish.run()
  File "C:\addon-sdk-1.1\python-lib\cuddlefish\__init__.py", line 676, in run include_dep_tests=options.dep_tests
  File "C:\addon-sdk-1.1\python-lib\cuddlefish\packaging.py", line 277, in generate_build_for_target validate_resource_hostname(prefix)
  File "C:\addon-sdk-1.1\python-lib\cuddlefish\packaging.py", line 85, in validate_resource_hostname raise ValueError('invalid resource hostname: %s' % name)
ValueError: invalid resource hostname: {XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX}-

Comment 9

[Wil Clouser [:clouserw]]

This sounds like the SDK is changing the IDs so I think this is an SDK bug.

Comment 10

[[:philipp]]

there are two ways to resolve it...
- either AMO allows to asign a jetpack-id to an already existing traditional extension with a UUID (at the moment it just states in the edit section of an addon: "The UUID of your add-on is specified in its install manifest and uniquely identifies it. You cannot change your UUID once it has been submitted.")
- or the sdk accepts the UUID-format as id key in the package.json file of a jetpack addon without throwing the error message cited in my previous post when attempting to build a xpi file.

Comment 11

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

This is an SDK bug.

We intend for addon developers to be able to migrate their traditional addons to the SDK, including retaining their traditional addon IDs, so AMO recognizes SDK-built packages as updated versions of the same addons.

Currently, we support traditional email-style IDs ([e-mail]), and we should support UUID-style IDs too, but the code that converts an ID into a hostname for use in resource: URLs <https://github.com/mozilla/addon-sdk/blob/master/python-lib/cuddlefish/__init__.py#L594-597> is not stripping the curly brackets in which such IDs are typically enclosed (as seen in comment 8).

This seems easy to fix, and we should just do it and then cherry-pick it to the stabilization branch for the next release.  I'll work on a patch on Monday if @warner doesn't beat me to it (he's PTO today, so I have a fighting chance!).

Comment 12

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

Attachment: wip v1: fixes problem, no test

This works in my manual testing.  But I'm having trouble figuring out how to test it automatically.  Brian, any suggestions for how best to fit a test for this into the cfx tests?

Comment 13

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

Attachment: patch v1: fixes problem

Here's a patch with a test for the added functionality, which has been factored into a separate function to make it easier to test.

@warner and I discussed testing this patch on IRC yesterday by packaging a sample addon and then checking the names of the directories inside it, but such a test would be poorly isolated from unrelated changes, whereas the test in this patch is well isolated to the specific functional area being changed.

Comment 14

[Brian Warner [:warner :bwarner]]

Comment on attachment 574994 [details] [diff] [review]
patch v1: fixes problem

looks ok. Are you aware of the "strip()" method on python strings, and how you can tell it what exactly to strip? You could use unique_prefix = unique_prefix.strip("{}"). The effect wouldn't be quite the same as the regexp you've got there (which only removes a single { from the front, a single } from the back, and only if the bits inbetween are strictly a lower-case UUID), but it might be easier to read.

You could also do =unique_prefix.lstrip("{").rstrip("}") to get closer to the regexp, although it'd still be willing to turn i.e. "{{{not-a-uuid" into "not-a-uuid".

Also, consider if you want the UUIDs to be strictly lower-case. If not, you'll need to change those a-f to a-fA-F .

But, good enough in my book.

Comment 15

[[github robot]]

Commit pushed to https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/544ba62d3301bd062d882ff198cce77180ea2099
fix bug 647693 - no way to migrate an AMO-hosted existing traditional extension (with UUID) to jetpack (with program ID) - by stripping enclosing curly braces from UUIDs before attempting to use them as resource: URL hostnames; r=@warner

Comment 16

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

(In reply to Brian Warner [:warner :bwarner] from comment #14)
> Are you aware of the "strip()" method on python strings, and how
> you can tell it what exactly to strip?

I am now!  And I suspected something like that existed.  But I thought it better to be stricter and more targeted about what we strip, since stripping is destructive.  I'm happy to loosen the rules if it seems better to do so, however, f.e. if someone identifies an additional use case that the current rule doesn't satisfy.


> Also, consider if you want the UUIDs to be strictly lower-case. If not,
> you'll need to change those a-f to a-fA-F .

I want to strip UUIDs case-insensitively, and an earlier version included A-F, but then I realized that unique_prefix is lowercased before it is stripped, so that isn't necessary.