Closed
Bug 834176
Opened 11 years ago
Closed 11 years ago
NON_OMNIJAR_FILES is not handled by the new packager
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla21
People
(Reporter: glandium, Assigned: glandium)
References
Details
Attachments
(1 file)
12.85 KB,
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
It turns out at least thunderbird needs to declare additional NON_OMNIJAR_FILES.
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #705803 -
Flags: review?(gps)
Comment 2•11 years ago
|
||
Comment on attachment 705803 [details] [diff] [review] Use NON_OMNIJAR_FILES value in the new packager Review of attachment 705803 [details] [diff] [review]: ----------------------------------------------------------------- Seems pretty straightforward. Just have a few nits. ::: python/mozbuild/mozpack/packager/formats.py @@ +200,5 @@ > ''' > Formatter for the omnijar package format. > ''' > + def __init__(self, copier, omnijar_name, compress=True, optimize=True, > + non_resource=[]): Since this is a list, perhaps it should be non_resources? @@ +258,5 @@ > omnijar archive. > ''' > base = self._get_base(path) > path = mozpack.path.split(mozpack.path.relpath(path, base)) > + if any(mozpack.path.match(path, p.replace('*', '**')) Why do you change search modes here? I'm guessing historical reasons to match the existing $(NON_OMNIJAR_FILES) lists from the apps that define it? Whatever the case, this deserves at least a comment. Should we also consider a follow-up bug to remove this? ::: toolkit/mozapps/installer/l10n-repack.py @@ +182,5 @@ > + parser.add_argument('l10n', > + help='Directory containing the staged langpack') > + parser.add_argument('--non-resource', nargs='+', metavar='PATTERN', > + default=[], > + help='Extra files not to be considered as resources') But I think singular "non-resource" is OK here since the argument can occur multiple times.
Attachment #705803 -
Flags: review?(gps) → review+
Assignee | ||
Comment 3•11 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #2) > Why do you change search modes here? I'm guessing historical reasons to > match the existing $(NON_OMNIJAR_FILES) lists from the apps that define it? > Whatever the case, this deserves at least a comment. Should we also consider > a follow-up bug to remove this? I was pondering doing that at the caller level, but I guess it can stay here until further improvements to the packaging code.
Assignee | ||
Comment 4•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/bf60e2782355
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•