CFX should recognize modules shipped with platform

RESOLVED FIXED in Firefox 21

Status

Add-on SDK
General
P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: irakli, Assigned: ochameau)

Tracking

(Blocks: 1 bug)

unspecified
1.14
x86
Mac OS X
Dependency tree / graph

Firefox Tracking Flags

(firefox20 unaffected, firefox21+ fixed)

Details

Attachments

(10 attachments, 2 obsolete attachments)

165 bytes, text/html
irakli
: review+
Details
165 bytes, text/html
irakli
: review+
Details
165 bytes, text/html
irakli
: review+
Details
165 bytes, text/html
irakli
: review+
Details
165 bytes, text/html
ochameau
: review+
Details
165 bytes, text/html
ochameau
: review+
Details
165 bytes, text/html
ochameau
: review+
Details
165 bytes, text/html
ochameau
: review+
Details
165 bytes, text/html
irakli
: review+
Details
165 bytes, text/html
irakli
: review+
Details
CFX needs to recognize modules shipped with a firefox and create appropriate entries in generated manifest file.
Blocks: 731779, 787346
Depends on: 793932

Updated

5 years ago
Priority: -- → P1
Depends on: 809581
Blocks: 826933
(Assignee)

Updated

5 years ago
Assignee: nobody → poirot.alex
(Assignee)

Comment 1

5 years ago
Created attachment 714084 [details]
Pull request 788 - part 1 - update manifest to match js normalization

Currently, javascript code normalize the manifest generated by python.
That makes everything more complex as python and javascript side have different kind of manifest.
This patch allows to have only one by updating python code.
Attachment #714084 - Flags: review?(rFobic)
(Assignee)

Comment 2

5 years ago
Created attachment 714086 [details]
Pull request 789 - part 2 - use absolute path in tests

Currently, various tests are using "magic" path. 
We shouldn't as it rely on deprecated search path we would like to get rid of and that will make it painfull to run tests without manifest.
Attachment #714086 - Flags: review?(rFobic)

Comment 3

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

https://github.com/mozilla/addon-sdk/commit/a4303c2542ed91b17b35fec510e6add44fcc51c3
Merge pull request #788 from ochameau/manifest-v2

Bug 793925: Simplify manifest to match javascript normalization r=@gozala
Attachment #714084 - Flags: review?(rFobic) → review+
Depends on: 841766

Comment 4

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

https://github.com/mozilla/addon-sdk/commit/8d8500b336f83d2ab70898330af167c21568531f
Merge pull request #789 from ochameau/abs-test-paths

Bug 793925 - Update test to use absolute path to sdk modules r=@gozala
Attachment #714086 - Flags: review?(rFobic) → review+
tracking-firefox21: --- → ?

Updated

5 years ago
tracking-firefox21: ? → +

Updated

5 years ago
status-firefox20: --- → unaffected
(Assignee)

Comment 5

5 years ago
Created attachment 715195 [details]
Pull request 798 - part 4 - move test-url to test addon
Attachment #715195 - Flags: review?(rFobic)
(Assignee)

Comment 6

5 years ago
Created attachment 715199 [details]
Pull request 799 - part 5 - move test-layout to its own test addon
(Assignee)

Comment 7

5 years ago
Created attachment 715231 [details]
Pull request 787 - part 3

Final part, that I'm planning to rebase on top of part 4 and 5 before merging, in order to run all tests.
Attachment #715231 - Flags: review?(rFobic)
Comment on attachment 715231 [details]
Pull request 787 - part 3

Please make sure to address comments in pull before landing this.
Attachment #715231 - Flags: review?(rFobic) → review+
Attachment #715195 - Flags: review?(rFobic) → review+
Attachment #715199 - Flags: review+

Comment 9

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

https://github.com/mozilla/addon-sdk/commit/a7d86d24e2a0739b72e78b3d59bde75c0ae4d22b
Bug 793925: part 4 - move test-url related to packed/unpacked addons to dedicated test addon

https://github.com/mozilla/addon-sdk/commit/dd666dabca216e1b90f9064d40faf0bcc576e5b7
Merge pull request #798 from ochameau/test-url

Bug 793925: part 4 - move test-url related to packed/unpacked addons to dedicated test addon r=@gozala

Comment 10

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

https://github.com/mozilla/addon-sdk/commit/b57bbed3873ccea67399d4bab38fa69ea6b1c59d
Bug 793925: part 5 - Move test-layout-change to a test addon, so that it will be executed exactly like a regular addon.

https://github.com/mozilla/addon-sdk/commit/698e850f7c594335b85bfb3aed0fcfc1d470c346
Merge pull request #799 from ochameau/test-layout

Bug 793925: part 5 - Move test-layout-change to a test addon, so that it will be executed exactly like a regular addon. r=@gozala
(Assignee)

Comment 11

5 years ago
Created attachment 715465 [details]
Pull request 801 - part 3 - use absolute path for test modules

Landing Pull request 787 in chunks.
Carry over r+ from it.
Attachment #715231 - Attachment is obsolete: true
Attachment #715465 - Flags: review+

Comment 12

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

https://github.com/mozilla/addon-sdk/commit/879a04f0ed879aa4b2e558b8b17c60d0240e2ba2
Bug 793925: part 3 - Use absolute path for tests and load each of them in its own loader as the main module.

https://github.com/mozilla/addon-sdk/commit/7c4bd7f0beb33af47af417980623db2720c2431d
Merge pull request #801 from ochameau/793925-part3

Bug 793925: part 3 - Use absolute path for tests and load each of them in its own loader as the main module. r=@gozala
(Assignee)

Comment 13

5 years ago
Created attachment 715480 [details]
Pull request 802 - part 6 - Simplify manifest path by dropping package name

Carry over r+ from pull request 787.
This part allows to have paths in manifest that match the one on filesystem from the lib/ folder, so that we will be able to use it to find system modules on fs without any heuristic.
Attachment #715480 - Flags: review+

Comment 14

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

https://github.com/mozilla/addon-sdk/commit/85431ccd52e3a36c96af297d37efc0640d8504cc
Bug 793925: part 6 - Simplify manifest path by stripping package name in order to match path of the module from lib folder.

https://github.com/mozilla/addon-sdk/commit/f6cb16746fb61acca92deddee72500527cb08377
Merge pull request #802 from ochameau/793925-part6

Bug 793925: part 6 - Simplify manifest path by stripping package name in order to match path of the module from lib folder. r=@gozala
(Assignee)

Comment 15

5 years ago
Created attachment 715494 [details]
Pull request 803 - part 7 - move sdk/test to test in order to avoid magic for require(test)

Carry over r+ from it.

require(test) currently works thanks to the mapping file. We would like it to work without manifest. So that we should move it to root modules folder.
Attachment #715494 - Flags: review+

Comment 16

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

https://github.com/mozilla/addon-sdk/commit/377ca19392a8337ecfd79340db34ee2b6453dd39
Bug 793925: part 7 - move sdk/test to test so that we can do require(test) without magic.

https://github.com/mozilla/addon-sdk/commit/6d978926e32d40a9b66e26294d10adfce2b008c8
Merge pull request #803 from ochameau/793925-part7

Bug 793925: part 7 - move sdk/test to test so that we can do require(test) without magic. r=@gozala
(Assignee)

Comment 17

5 years ago
Created attachment 715519 [details]
Pull request 804 - part 8 - fix relative path handling of system modules

Carry over r+ from pull request 787.

This patch allows to fix various issues about the handling of system module where it isn't possible to load a module with a relative path.
Attachment #715519 - Flags: review+

Comment 18

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

https://github.com/mozilla/addon-sdk/commit/a7ed5c079a876e48c6ff4432811afee385c64864
Bug 793925: part 8 - Fix handling of relative path for system/out-of-manifest modules

https://github.com/mozilla/addon-sdk/commit/34d45c535319cfdf03935af79d1e7c3ac8611dca
Merge pull request #804 from ochameau/793925-part8

Bug 793925: part 8 - Fix handling of relative path for system/out-of-manifest modules r=@gozala
(Assignee)

Comment 19

5 years ago
For the sake of clarity (sorry about all these PR), here is the summary of all changeset that will mostliky have to be uplifted, in their order to landing:

part 1
https://github.com/mozilla/addon-sdk/commit/a4303c2542ed91b17b35fec510e6add44fcc51c3
part 2
https://github.com/mozilla/addon-sdk/commit/8d8500b336f83d2ab70898330af167c21568531f
part 4
https://github.com/mozilla/addon-sdk/commit/dd666dabca216e1b90f9064d40faf0bcc576e5b7
part 5
https://github.com/mozilla/addon-sdk/commit/698e850f7c594335b85bfb3aed0fcfc1d470c346
part 3
https://github.com/mozilla/addon-sdk/commit/7c4bd7f0beb33af47af417980623db2720c2431d
part 6
https://github.com/mozilla/addon-sdk/commit/f6cb16746fb61acca92deddee72500527cb08377
part 7
https://github.com/mozilla/addon-sdk/commit/6d978926e32d40a9b66e26294d10adfce2b008c8
part 8
https://github.com/mozilla/addon-sdk/commit/34d45c535319cfdf03935af79d1e7c3ac8611dca

All remaining work is visible at: https://github.com/mozilla/addon-sdk/pull/787
And only affect cfx and boostrap.js code.

So we should be good to push part 1 to 8 to mozilla-central.
(Assignee)

Comment 20

5 years ago
I tested all these patches applied to a custom firefox build and I confirm that it actually works!

So we should land these patches on stabilization and mozilla-central.
I cherry picked them on stabilization on this branch:
  https://github.com/ochameau/addon-sdk/tree/stab-793925
(it can help you to fix minor conflicts)

Here is how to test this work:
+ Checkout following branch in /mozilla-central/addon-sdk/source:
https://github.com/ochameau/addon-sdk/tree/use-mc-modules-rebase-stab
It contains the previous one plus the additional cfx modification, all based on top of stabilization.

+ Rebuild firefox

+ Build an addon with this SDK with the -m option: cfx xpi -m

+ Open the xpi, see that:
-> harness-options.json doesn't have manifest entries for SDK modules
-> resources/ folder only contains addon module

+ Install it on your custom firefox build and see the addon working.
I've uplifted the latest master to inbound in bug 842762
Depends on: 842762
(Assignee)

Comment 22

5 years ago
Created attachment 715919 [details]
Pull request 808 - fix 3rd party packages support

We broke 3rd party package support by no longer mapping 3rd party packages lib folder to the loader.
Attachment #715919 - Flags: review?(rFobic)
(Assignee)

Comment 23

5 years ago
Created attachment 716039 [details]
Pull request 787 - final part - add cfx options to control sdk module inclusion in the xpi

Here is the final patch that doesn't have to be uplifted to aurora as the modified files aren't shipped in final firefox binary.

Introduce 3 new cfx options:
  -o/--override-modules: use sdk modules from your local sdk repository clone instead of firefox ones,
  --strip-sdk: prevent from shipping sdk modules in the xpi,
  --force-xpi-sdk: force the usage of xpi modules instead of firefox ones
(incompatible with -o and --strip-sdk)

Important implementation details:
Bootstrap.js checks for flags in manifest in order to decide to use or not xpi sdk modules so that if something goes badly wrong, we will still be able to force using xpi modules.
First, if --strip-sdk is passed, the addon will never try to access xpi sdk modules but try to look into firefox even on FF 21-.
Then, if --strip-sdk is omitted, it will either use firefox module if we are on FF 21+ -or- in any firefox version if we used the --force-xpi-sdk option.
Finally, if we pass -o or --override-modules, we will use sdk modules from your local SDK clone, even if we pass or not --strip-sdk.

All these options are mainly designed for SDK contributors, I don't see why a developer would use any of them except if they start modifying SDK modules.

I'm considering writting tests for these new options in a followup patch.
Attachment #716039 - Flags: review?(rFobic)
Attachment #715919 - Flags: review?(rFobic) → review+

Comment 24

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

https://github.com/mozilla/addon-sdk/commit/62eae520ba9fc45e0953e69d7659088602cf0c3a
Bug 793925 - Fix support of 3rd party packages.

https://github.com/mozilla/addon-sdk/commit/d6a76a2896c590beff686a44bafaeda6f69f9e89
Merge pull request #808 from ochameau/793925-3rd-party-packages

Bug 793925 - Fix support of 3rd party packages. r=@gozala
(Assignee)

Comment 25

5 years ago
New changeset to push to stabilization:
  https://github.com/mozilla/addon-sdk/commit/d6a76a2896c590beff686a44bafaeda6f69f9e89
(Assignee)

Comment 26

5 years ago
Created attachment 716120 [details]
716039: Pull request 787 - final part - add cfx options to control sdk module inclusion in the xpi
Attachment #716039 - Attachment is obsolete: true
Attachment #716039 - Flags: review?(rFobic)
Attachment #716120 - Flags: review?(rFobic)
Comment on attachment 716120 [details]
716039: Pull request 787 - final part - add cfx options to control sdk module inclusion in the xpi

With comments addressed.
Attachment #716120 - Flags: review?(rFobic) → review+
(Assignee)

Updated

5 years ago
Target Milestone: --- → 1.14

Comment 28

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

https://github.com/mozilla/addon-sdk/commit/606dd17a6efe2e0fedbaf7b79b2d4bebfeb86da8
Merge pull request #788 from ochameau/manifest-v2

Bug 793925: Simplify manifest to match javascript normalization r=@gozala
(cherry picked from commit 457f683c1da57de83f3f721e61ef0c9696b3616f)

https://github.com/mozilla/addon-sdk/commit/16ac371215298980da39dbc99113f4c5ad96863b
Merge pull request #789 from ochameau/abs-test-paths

Bug 793925 - Update test to use absolute path to sdk modules r=@gozala
Conflicts:
	test/windows/test-firefox-windows.js
(cherry picked from commit 9b726da9fd9b9110c0c433c477b87938c4c284f1)

https://github.com/mozilla/addon-sdk/commit/a0f4a31330e188bb1c1be5cfe5db90a2327ae238
Merge pull request #798 from ochameau/test-url

Bug 793925: part 4 - move test-url related to packed/unpacked addons to dedicated test addon r=@gozala
(cherry picked from commit b58d805d26b57a5547351f19779d5a003830b38b)

https://github.com/mozilla/addon-sdk/commit/51fefbd82d186710006556347df12d6e75c81e70
Merge pull request #799 from ochameau/test-layout

Bug 793925: part 5 - Move test-layout-change to a test addon, so that it will be executed exactly like a regular addon. r=@gozala
(cherry picked from commit 034c5c9adea1ebd9f99e994aede8a58d9b637ae5)

https://github.com/mozilla/addon-sdk/commit/28ec8277179334f4a83742099318f2e7c0f9ce3c
Merge pull request #801 from ochameau/793925-part3

Bug 793925: part 3 - Use absolute path for tests and load each of them in its own loader as the main module. r=@gozala
(cherry picked from commit 7c060641c35bfa167d9acbdf9a3e1e1309277f39)

https://github.com/mozilla/addon-sdk/commit/cd39593025a204cbf70abe705ded274fe5ec7463
Merge pull request #802 from ochameau/793925-part6

Bug 793925: part 6 - Simplify manifest path by stripping package name in order to match path of the module from lib folder. r=@gozala
Conflicts:
	test/private-browsing/windows.js
	test/test-private-browsing.js
(cherry picked from commit 0b7b6cae65480343d402f8e1e76c8a44ed3ac75d)

https://github.com/mozilla/addon-sdk/commit/21f662a61bc02124250c2c3dadee39658d99aff0
Merge pull request #803 from ochameau/793925-part7

Bug 793925: part 7 - move sdk/test to test so that we can do require(test) without magic. r=@gozala
(cherry picked from commit 4fcb1757cd170ff6fcd51fb99058a91022b09faa)

https://github.com/mozilla/addon-sdk/commit/5985813007192a851de7b76d6fcfc0a11eb038ff
Merge pull request #804 from ochameau/793925-part8

Bug 793925: part 8 - Fix handling of relative path for system/out-of-manifest modules r=@gozala
(cherry picked from commit 0953c247573aed93f4ec1f2cece16357e345a656)

Comment 29

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

https://github.com/mozilla/addon-sdk/commit/da96d2374d78f4927497edae3c4fe6cbad92043c
Merge pull request #808 from ochameau/793925-3rd-party-packages

Bug 793925 - Fix support of 3rd party packages. r=@gozala(cherry picked from commit d6a76a2896c590beff686a44bafaeda6f69f9e89)

Comment 30

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

https://github.com/mozilla/addon-sdk/commit/62eae520ba9fc45e0953e69d7659088602cf0c3a
Bug 793925 - Fix support of 3rd party packages.

https://github.com/mozilla/addon-sdk/commit/d6a76a2896c590beff686a44bafaeda6f69f9e89
Merge pull request #808 from ochameau/793925-3rd-party-packages
Blocks: 843638

Comment 31

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

https://github.com/mozilla/addon-sdk/commit/2359879f440badc21e16e1decf4efd993c3e96bc
Bug 793925: final part - CFX should recognize modules shipped with platform. r=@gozala
Introduce 3 new cfx options:
-o/--override-modules: use sdk modules from your repository instead of
firefox ones
--strip-sdk: prevent from shipping sdk modules in the xpi
--force-use-bundled-sdk: force the usage of xpi modules instead of firefox
ones
(incompatible with -o and --strip-sdk)

https://github.com/mozilla/addon-sdk/commit/a4dac4ac74dbc841e3052912dbc8a756c31f2031
Merge pull request #787 from ochameau/use-mc-modules

Bug 793925: final part - CFX should recognize modules shipped with platform r=@gozala

Comment 32

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

https://github.com/mozilla/addon-sdk/commit/2359879f440badc21e16e1decf4efd993c3e96bc
Bug 793925: final part - CFX should recognize modules shipped with platform. r=@gozala

https://github.com/mozilla/addon-sdk/commit/a4dac4ac74dbc841e3052912dbc8a756c31f2031
Merge pull request #787 from ochameau/use-mc-modules

Comment 33

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

https://github.com/mozilla/addon-sdk/commit/ff84cd66edbc697125fe49a4ac70366f2c662586
Merge pull request #787 from ochameau/use-mc-modules

Bug 793925: final part - CFX should recognize modules shipped with platform r=@gozala(cherry picked from commit a4dac4ac74dbc841e3052912dbc8a756c31f2031)
(Assignee)

Comment 34

5 years ago
Looks like everything is now fixed and landed on master and stabilization.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Blocks: 847418
status-firefox21: --- → fixed
You need to log in before you can comment on or make changes to this bug.