Closed Bug 892098 Opened 8 years ago Closed 7 years ago

add argument to include updateKey in install.rdf

Categories

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

defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: johndoe_fakestreet, Unassigned)

References

Details

Attachments

(2 files, 3 obsolete files)

User Agent: Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20100101 Firefox/21.0 (Beta/Release)
Build ID: 20130511120803

Steps to reproduce:

if you hosting the update.rdf yourself, you have to put the updateKey into the install.rdf of the firefox addon. with this patch, you can provide the update-key as an argument and it's include in the install.rdf ( like --update-url):
cfx --update-key=MIGf.... xpi
remove my useless comments from the patch
Attachment #773558 - Attachment is obsolete: true
Is doing this through the command line the right option? Seems like it might be better put into package.json
Priority: -- → P3
yeah, you're right. The updateKey isn't changed very often. I will submit a new patch for that.
But doesn't the same hold for the update-url? It's also a command line argument.
(In reply to johndoe_fakestreet from comment #3)
> yeah, you're right. The updateKey isn't changed very often. I will submit a
> new patch for that.
> But doesn't the same hold for the update-url? It's also a command line
> argument.

I think it does, I'm not sure why it was added to the command line but I think it makes more sense in package.json too.

Irakli does this make sense to you?
Flags: needinfo?(rFobic)
Putting both in `package.json` makes sense. Unfortunately I can't recall reasons why cli argument was chosen in first place, probably that decision predates me.
Flags: needinfo?(rFobic)
the patch removes the argument '--update-url' and adds the keys 'updateKey' and 'updateURL' to the package.json. I've also insert some docs to 'package-spec.md'.
do you include my patch? Can I help in any way, so this will happen?
Comment on attachment 780562 [details] [diff] [review]
0001-add-keys-updateURL-and-updateKey-to-package.json.patch

Erik can you please take this one, trying to make sure I finish all pending task before my upcoming PTO
Attachment #780562 - Flags: review?(rFobic) → review?(evold)
Comment on attachment 780562 [details] [diff] [review]
0001-add-keys-updateURL-and-updateKey-to-package.json.patch

The patch needs tests.  Let us know if you need/want help with that.
Attachment #780562 - Flags: review?(evold) → review-
Sure. I add tests, but it will take time until the end of next week, because I'm on holiday for some days.
If you have comments or notes about things which I got wrong totally, please let me know.
*ping, someone working on it?
Erik, what is the status here?
Flags: needinfo?(evold)
(In reply to Dave Townsend (:Mossop) from comment #14)
> Erik, what is the status here?

Oh I didn't know there was a change here, it looks the new patch needs a review.
Flags: needinfo?(evold)
Comment on attachment 804945 [details] [diff] [review]
adds a test for updateKey and updateURL to test_rdf.py

Ok it looks like this test is merely testing that our python function which generates the install.rdf will include the desired xml string (if the xml valid or not).  We need some more complete testing here, such that if the `updateURL` and `updateKey` keys are found in the package.json file then the generate install.rdf has the correct nodes.

One way to do this would be to check that the Addon.permissions https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/Add-on_Manager/Addon from the AddonManager https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/Add-on_Manager/AddonManager says that the addon is able to upgrade, or parse the install.rdf in the generated xpi for the updateURL/updateKey values.

I would suggest using a test addon like those found here https://github.com/mozilla/addon-sdk/tree/master/test/addons
Attachment #804945 - Flags: review?(evold) → review-
here is my unit test patch for updateKey and updateURL.

First I tried to implement your suggestion and use AddonManger in firefox, but I noticed that the (permissions & AddonManager.PERM_CAN_UPGRADE) is always true, whether or not there is an em:updatKey and/or em:updateURL in the install.rdf. The same thing holds for 'providesUpdatesSecurely'.

So in the end I added an unit test to python-lib/cuddlefish/tests/test_linker.py. It seems to be the right place for tests which builds and checks the xpi.
Attachment #804945 - Attachment is obsolete: true
*ping
(In reply to johndoe_fakestreet from comment #17)
> Created attachment 8341274 [details] [diff] [review]
> 0001-add-unit-tests-for-keys-updateKey-and-updateURL-in-p.patch
> 
> here is my unit test patch for updateKey and updateURL.
> 
> First I tried to implement your suggestion and use AddonManger in firefox,
> but I noticed that the (permissions & AddonManager.PERM_CAN_UPGRADE) is
> always true, whether or not there is an em:updatKey and/or em:updateURL in
> the install.rdf. The same thing holds for 'providesUpdatesSecurely'.
> 
> So in the end I added an unit test to
> python-lib/cuddlefish/tests/test_linker.py. It seems to be the right place
> for tests which builds and checks the xpi.

Hmm well there is news since last November, which is that we are working on killing as much python code as possible now.  So we'll need an addon test like the one I mentioned in comment 16.  I see the issue you mentioned in comment 17 now though, so I'm not sure how best to resolve this at the moment..

The good news is that in bug 915376 this bug will be resolved for free, so I think it'd be best to either close this bug as a dup of bug 915376 or make sure we resolve this bug with tests that will work after bug 915376 is resolved.

I'm sorry about the confusion here, it was difficult to get agreement on bug 915376 for sometime..
Comment on attachment 8341274 [details] [diff] [review]
0001-add-unit-tests-for-keys-updateKey-and-updateURL-in-p.patch

So it looks like we'll need a test addon which will work after bug 915376 is resolved (see comment 19 for more info).  I think that this may work for a test addon:

Components.utils.import("resource://gre/modules/AddonManager.jsm");
AddonManager.getAddonByID(require('self').id, function(aAddon) {
  assert.ok(aAddon.updateKey);
});
Attachment #8341274 - Flags: review?(evold) → review-
Sorry we won't be releasing any new versions of cfx, jpm is the replacement https://www.npmjs.com/package/jpm
Blocks: cfx.py
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
(In reply to Erik Vold [:erikvold] (please needinfo? me) from comment #21)
> Sorry we won't be releasing any new versions of cfx, jpm is the replacement
> https://www.npmjs.com/package/jpm

The release of a new version is not needed I think, but to ease the time until jpm is ready, could be put one of the patches in HEAD?
You need to log in before you can comment on or make changes to this bug.