892193 - Make --strip-sdk the default

Status: RESOLVED FIXED

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

Description

[Erik Vold [:erikvold] (please needinfo? me)]

For more info see: https://groups.google.com/forum/#!topic/mozilla-labs-jetpack/NkDBaNk2GdA

Comment 1

[Nicholas Nethercote [inactive]]

Will this save memory if you have multiple JetPack add-ons installed?

Comment 2

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Nicholas Nethercote [:njn] from comment #1)
> Will this save memory if you have multiple JetPack add-ons installed?

Eventually yes, at the moment I don't think so.

Comment 3

[Erik Vold [:erikvold] (please needinfo? me)]

Dave, fyi this is really hard to do right.  Switching the default here means lots of other arguments become redundant, and removing them breaks a bunch of tests, so those tests need to be fully understood and corrected, or removed if appropriate.  I'm not sure if Alex has the bandwidth for this, but we should at least have him review the code.  Maybe we can do this in Paris?

Comment 4

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

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

Pointer to Github pull-request

Comment 5

[Erik Vold [:erikvold] (please needinfo? me)]

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

Well I can't find anything wrong. I tried this on Fennec too.  I'll make followup bugs about --no-strip-xpi and --force-use-bundled-sdk being redundant.

Comment 6

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/cf811ad2d1f91505dfcb840c48ddb52905bfe109
Merge pull request #1142 from Gozala/bug/strip-sdk@892193

Bug 892193 - Make strip-sdk a default. r=@erikvold

Comment 7

[Dave Townsend [:mossop]]

This is fixed now, right?

Comment 8

[Dave Townsend [:mossop]]

Backed this out as it caused bug 903172 on tinderbox. With strip-sdk as the default we don't package the files needed by tests.

Comment 9

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

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

Pointer to Github pull-request

Comment 10

[Erik Vold [:erikvold] (please needinfo? me)]

I'm curious, why was the data folder a problem?

Comment 11

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

I have talked about this in the weekly meeting (probably there are notes of this too). But main issue is that if you `--strip-sdk` SDK is stripped which also includes it's data folder. Making data of the sdk special for tests was my original approach which end up being a rabbit hole. CFX does lot

Comment 12

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

I have talked about this in the weekly meeting (probably there are notes of this too). But main issue is that if you `--strip-sdk` SDK is stripped which also includes it's data folder. Making data of the sdk special for tests was my original approach which end up being a rabbit hole. CFX is full of workarounds
that sniff for command 'test' to do things differently. Finally this was also colliding with the module overload feature that overrides modules folder, but no data.

After discussions we thought that getting rid of this special data folder was a best immediate solution to
unblock shipment of 1.5. Also doing many changes to CFX did not seem like a good idea because of CFX.js.

Comment 13

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/94a67d16d26f6d124d5db821fff9ad18a41ce980
Merge pull request #1268 from Gozala/bug/no-data@892193

Bug 892193 - Make --strip-sdk the default r=@erikvold