648787 - checking for private key significantly slows down cfx run

Status: RESOLVED FIXED

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

Description

[Nickolay_Ponomarev]

preflight.py:check_for_privkey takes around 0.7s (with another 0.7s spent in manifest generation code on which bug 615060 is already filed).

The code that takes this long is intended to catch a failure case:

https://github.com/mozilla/addon-sdk/blob/master/python-lib/cuddlefish/preflight.py#L160
    # make sure we have the privkey: this catches the case where developer B
    # copies an add-on from developer A and then (accidentally) tries to
    # publish it without replacing the JID

    sk = check_for_privkey(keydir, config["id"], stderr)
    if not sk and err_if_privkey_not_found:
        return False, False

and as far as I can see, after bug 593651 was fixed, the result of this rather expensive check is not even used.

I suggest a simple change: to only check_for_privkey() if err_if_privkey_not_found is actually True.

Comment 1

[Nickolay_Ponomarev]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/144

Pointer to Github pull-request

Comment 2

[Brian Warner [:warner :bwarner]]

That change sounds fine to me.. I'll land it now. (landed in https://github.com/mozilla/addon-sdk/commit/8f549d5ef6263980f06b46fafc0be39b24bb4ad7)

The check takes a long time because it's validating the contents of the private key file (reading the private key, deriving the public key, comparing it against the JID), and deriving the public key involves a lot of math. We could probably implement a weaker check that just looks for the existence of the file (but assumes that the contents are still valid).

We should probably think a bit more about key management: the discussion in bug 593651 was about giving sample code a form of JID that could be used fewer hassles (but less-well-defined ownership) than real JIDs. I don't think we decided to give up on private keys altogether, which is a path that e.g. removing the call to check_for_privkey() would start us down.

Comment 3

[Brian Warner [:warner :bwarner]]

Comment on attachment 524884 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/144

this was merged last april.. forgot to close out the r? flag