Create a JSM with a constructor which would populate a bootstrap.js with the logic necessary to run jetpacks

RESOLVED FIXED

Status

defect
P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: evold, Assigned: irakli)

Tracking

(Blocks 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Irakli and I were talking about the issue where AMO has to hash our bootstraps in order to accept them, which they may only be willing to do once per month making it difficult to coordinate bootstrap.js changes.

Irakli came up with an idea to export all of the logic that needs to be in the bootstrap to a jsm which provides a constructor which would populate the bootstrap.js global with the functions that it requires (startup, shutdown, etc..)

This will also allow us to update legacy add-ons bootstraps (in the future), without requiring the users to rebuild.
OS: Mac OS X → All
Hardware: x86 → All
Priority: -- → P1
Assignee: nobody → evold
Comment on attachment 8533958 [details] [review]
Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1741

Rejecting this, as we discussed I'm going to submit a pull for this.
Attachment #8533958 - Flags: review?(rFobic) → review-
Posted file v1 (obsolete) —
Attachment #8535292 - Flags: review?(evold)
Comment on attachment 8535292 [details] [review]
v1

There is a new moduleURI pref added which should be added in a separate bug.  Also now that bug 1108785 has landed we should merge the master branch into this one, then include the new bootstrap.js file in the root of the addon-sdk repo.  I think that the test suite will fail in this case, because the `mainPath` entry is not switched with `jpm test` is used.
Depends on: 1108785
Attachment #8535292 - Flags: review?(evold) → review-
(In reply to Erik Vold [:erikvold] (please needinfo? me) from comment #4)
> Comment on attachment 8535292 [details] [review]
> v1
> 
> There is a new moduleURI pref added which should be added in a separate bug.

I assume you're talking about `mountURI` here, if so that's fine I can remove that line and send another pull for with that line.

> Also now that bug 1108785 has landed we should merge the master branch into
> this one, then include the new bootstrap.js file in the root of the
> addon-sdk repo.  I think that the test suite will fail in this case, because
> the `mainPath` entry is not switched with `jpm test` is used.

Please note that this bootstrap.js is not actually used by JPM so landing it won't really break tests as it won't affect jpm in any ways until JPM starts using it. So I disagree that merging master into this will make any difference.

As of `mainPath` I responded in pull request it's not gone it's still there you probably missed it.


BTW I also think that some changes in nightly broke JPM as on my machine module tests no longer pass.
Posted file v2
Ok I have updated pull request:

1. I merged master into it.
2. Removed `mountURI` prefs.

I believe I have addressed rest comments by replies in pulls.
Attachment #8535292 - Attachment is obsolete: true
Attachment #8536849 - Flags: review?(evold)
Comment on attachment 8536849 [details] [review]
v2

looks good, thanks!
Attachment #8536849 - Flags: review?(evold) → review+
Assignee: evold → rFobic

Comment 8

5 years ago
Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/bc51bc2087790678441426162af068b24511e43b
Merge pull request #1742 from Gozala/bootstrap

Bug 1075541 - Implement addon/bootstrap as shared module. r=@erikvold
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1031575
Depends on: 1102107
You need to log in before you can comment on or make changes to this bug.