cfx xpi started skipping the "defaults" dir from the template

RESOLVED FIXED in 1.3

Status

Add-on SDK
General
P2
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Nickolay_Ponomarev, Assigned: warner)

Tracking

unspecified
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
This only affects old-style ("xul") extensions (bug 641215). To reproduce you need to run cfx xpi with a non-default template that has default preferences ('addon.xpi!/defaults/preferences/*.js').

bug 686008 in https://github.com/mozilla/addon-sdk/commit/d98f1d0af8bdb97e3c8af2b7f1a80f027fc67126 changed the directory filter for template from [".svn", ".hg", ".git"] to [".git", ".svn", ".hg", "defaults"], the latter was only used for the data/ subtree before.

I couldn't find any reason behind ignoring "defaults" inside the data/ directory either - blame points to a huge checkin for bug 627607 <https://github.com/mozilla/addon-sdk/commit/a020fa100b84415724c98803d4a8772752ef7fbf> and there's no reference to any discussion about that.

Is there any reason "defaults" cannot be removed from the common filter list?
Brian, was there any reason for including "defaults"?
Priority: -- → P2
Whiteboard: [triage:followup]
(Assignee)

Comment 2

6 years ago
No, having "defaults" in there was a mistake. What happened was this:

* back in 0.3, https://github.com/mozilla/addon-sdk/commit/af3410c00c7b8bced45b6e7bddc016bac7c8809f
  started removing "defaults" from 'cfx xpi' because the app-extension
  template at that time had a defaults/preferences/prefs.js
  (https://github.com/mozilla/addon-sdk/blob/af3410c00c7b8bced45b6e7bddc016bac7c8809f/python-lib/cuddlefish/app-extension/defaults/preferences/prefs.js)
  that turned on browser.dom.window.dump.enabled and javascript.options.strict,
  and we didn't want these debug options in generated XPIs (only for 'cfx run').

* later (between 0.9 and 1.0), nickolay's
  4a9dc79eaec02a936821b26dd474e36abc6dc551 moved them out of an on-disk file
  and into a 'cfx run'-specific profile addition, and the next patch stopped
  stripping "defaults" during 'cfx xpi'.

* later (also between 0.9 and 1.0), when I rewrote the manifest-generation
  code (a020fa100b84415724c98803d4a8772752ef7fbf), I copied the list of
  IGNORED_DIRS blindly from the old 'cfx xpi' code (xpi.py) into manifest.py,
  including "defaults". The parent of that commit included nickolay's fix and
  did not strip "defaults", but somehow I managed to base it on an old
  version (I started that branch before 0.9, so somewhere in the middle of
  merging and rebasing I failed to incorporate the xpi.py change)

* 1.0 shipped with a manifest.py that stripped "defaults" and an xpi.py that
  did not

* then last month, my d98f1d0af8bdb97e3c8af2b7f1a80f027fc67126 commit
  factored out the two directory-stripping routines and resolved the
  difference by removing "defaults" from both.

Anyways, the fix is easy.. will attach a patch momentarily.
(Assignee)

Comment 3

6 years ago
Created attachment 566956 [details] [diff] [review]
stop removing 'defaults' when building an XPI
Assignee: nobody → warner-bugzilla
Status: NEW → ASSIGNED
Attachment #566956 - Flags: review?(rFobic)
Comment on attachment 566956 [details] [diff] [review]
stop removing 'defaults' when building an XPI

Review of attachment 566956 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!
Attachment #566956 - Flags: review?(rFobic) → review+
(Assignee)

Comment 5

6 years ago
Landed in https://github.com/mozilla/addon-sdk/commit/77d8a82430b4edf02a637ba531ca5b1359abad56
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3
(Reporter)

Comment 6

6 years ago
Brian, thanks for the quick fix!
Whiteboard: [triage:followup]
You need to log in before you can comment on or make changes to this bug.