This is a follow-up to Bug 873569, specifically https://bugzilla.mozilla.org/show_bug.cgi?id=873569#c27. That ticket makes each |make package| copy .so files into $(STAGE), szip them, and then move them into $(STAGE)/assets. szip is very slow, so each iteration takes a long time. We'd like to have |make package| copy the .so files into $(STAGE)/assets directly, so the szip is in-place and not repeated due to newer timestamps. There's a good deal of work in progress on Bug 873569; this ticket is good Bugzilla hygiene.

Hi glandium, I broke this out into a separate ticket; your last review is at https://bugzilla.mozilla.org/show_bug.cgi?id=873569#c51. Thanks for pushing me towards more idiomatic Python; I think your guidance leaves it much improved. I addressed the multiple warnings by throwing ValueError in the helper functions and catching once. There's a try build with this and the companion Bug 887115 working away at https://tbpl.mozilla.org/?tree=Try&rev=d172e53562f5.

Comment on attachment 768429 [details] [diff] [review] Make packager install and szip .so libraries in assets/ directly. r=glandium Review of attachment 768429 [details] [diff] [review]: ----------------------------------------------------------------- ::: python/mozbuild/mozpack/packager/__init__.py @@ +107,5 @@ > + ''' > + try: > + name, options = Component._split_component_and_options(string) > + except ValueError: > + errors.fatal('Malformed manifest: invalid name or options found') You should use the string you gave in the exception. @@ +150,5 @@ > str = str.strip() > if not str or str.startswith(';'): > return > if str.startswith('[') and str.endswith(']'): > + if str == '[]' or re.search(r'[\[\]]', str[1:-1]): You're explicitly excluding [ and ] in Component, so you can lift that re.search.

Comment on attachment 768429 [details] [diff] [review] Make packager install and szip .so libraries in assets/ directly. r=glandium [Approval Request Comment] Bug caused by (feature/regressing bug #): Regression from Bug 873569, tracked by Bug 895442. User impact if declined: continued broken Android nightly repacks on Aurora and above. Testing completed (on m-c, etc.): This has been on m-c for weeks; I meant to request uplift after it demonstrated its value and forgot. Risk to taking this patch (and alternatives if risky): This messes with Android packaging, so there's a slim possibility of discovering additional packaging/l10n repacking differences between Aurora and Nightly. We could revert Bug 873569 in Aurora and above, but that seems riskier, since new uplifts would have landed on top of 873569. String or IDL/UUID changes made by this patch: none.

(In reply to Nick Alexander :nalexander from comment #5) > Comment on attachment 768429 [details] [diff] [review] > Make packager install and szip .so libraries in assets/ directly. r=glandium > > [Approval Request Comment] > Bug caused by (feature/regressing bug #): Regression from Bug 873569, > tracked by Bug 895442. > > User impact if declined: continued broken Android nightly repacks on Aurora > and above. > > Testing completed (on m-c, etc.): This has been on m-c for weeks; I meant to > request uplift after it demonstrated its value and forgot. > > Risk to taking this patch (and alternatives if risky): This messes with > Android packaging, so there's a slim possibility of discovering additional > packaging/l10n repacking differences between Aurora and Nightly. We could > revert Bug 873569 in Aurora and above, but that seems riskier, since new > uplifts would have landed on top of 873569. > > String or IDL/UUID changes made by this patch: none. I forgot to say: the real value of this is uplifting Bug 887115; I wouldn't like to uplift 887115 without this in place.

Comment on attachment 768429 [details] [diff] [review] Make packager install and szip .so libraries in assets/ directly. r=glandium approving to go with bug 887115