Make packager install and szip .so libraries in assets/ directly

RESOLVED FIXED in Firefox 24

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: nalexander, Assigned: nalexander)

Tracking

Trunk
Firefox 25
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox24 fixed, firefox25 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Updated

5 years ago
Blocks: 873569
(Assignee)

Comment 1

5 years ago
Created attachment 768429 [details] [diff] [review]
Make packager install and szip .so libraries in assets/ directly. r=glandium

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.
Attachment #768429 - Flags: review?(mh+mozilla)
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.
Attachment #768429 - Flags: review?(mh+mozilla) → review+
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/dfc8d955b043
Assignee: nobody → nalexander
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox 25
https://hg.mozilla.org/mozilla-central/rev/dfc8d955b043
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Depends on: 888646
(Assignee)

Comment 5

5 years ago
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.
Attachment #768429 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 6

5 years ago
(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
Attachment #768429 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/0d566d5133be
status-firefox24: --- → fixed
status-firefox25: --- → fixed
You need to log in before you can comment on or make changes to this bug.