Skia fails to build on debian armel since bug 777614

RESOLVED FIXED in Firefox 24

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

24 Branch
mozilla34
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox24 fixed, firefox25 fixed, firefox26 fixed, firefox31 wontfix, firefox32 fixed, firefox33 fixed, firefox34 fixed, firefox-esr31 fixed)

Details

(Whiteboard: [qa-])

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Skia r5539 brought some armv6+ instructions without counterpart for older arm. There's also a #if SK_ARM_ARCH == 6 that should really be a <= 6.
(Assignee)

Comment 1

5 years ago
Created attachment 785360 [details] [diff] [review]
Fix Skia for ARM v4t

Note the #ifdef for pld is technically wrong, because it's supported on ARMv5TE, but i didn't want to go through hoops to do this right, considering apart from me, probably nobody cares, and I'm targetting ARMv4T anyways.
Attachment #785360 - Flags: review?(tterribe)
(Assignee)

Updated

5 years ago
Assignee: nobody → mh+mozilla
(Assignee)

Comment 2

5 years ago
Created attachment 790567 [details] [diff] [review]
Fix Skia for ARM v4t

With a typo fix.
Attachment #790567 - Flags: review?(tterribe)
(Assignee)

Updated

5 years ago
Attachment #785360 - Attachment is obsolete: true
Attachment #785360 - Flags: review?(tterribe)
Comment on attachment 790567 [details] [diff] [review]
Fix Skia for ARM v4t

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

r=me, but see one question below.

::: gfx/skia/patches/0026-Bug-901208-Fix-ARM-v4t.patch
@@ +9,5 @@
> +                   "blo     2f                           \n\t"
> +                   "and     r4, r3, #0x0000f8            \n\t"
> +                   "and     r5, r3, #0x00fc00            \n\t"
> +                   "and     r6, r3, #0xf80000            \n\t"
> ++#if SK_ARM_ARCH >= 6

This is safe, so I have no objection, but isn't this what SK_ARM_HAS_EDSP is for?
Attachment #790567 - Flags: review?(tterribe) → review+
(Assignee)

Comment 4

5 years ago
(In reply to Timothy B. Terriberry (:derf) from comment #3)
> This is safe, so I have no objection, but isn't this what SK_ARM_HAS_EDSP is
> for?

Indeed, thanks.

Comment 6

5 years ago
https://hg.mozilla.org/mozilla-central/rev/0bb8a5f5724e
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
(Assignee)

Comment 7

5 years ago
Comment on attachment 790567 [details] [diff] [review]
Fix Skia for ARM v4t

[Approval Request Comment]
I'd like this to land for the next ESR to avoid carrying the patch around for a year.
Bug caused by last skia upstream import
User impact if declined: Failure to build on the debian armel port
Testing completed (on m-c, etc.): Tested on Firefox 23 and 24 on debian.
Risk to taking this patch (and alternatives if risky): NPoTB
String or IDL/UUID changes made by this patch: None
Attachment #790567 - Flags: approval-mozilla-beta?
Attachment #790567 - Flags: approval-mozilla-aurora?
Comment on attachment 790567 [details] [diff] [review]
Fix Skia for ARM v4t

Please make sure that any Builds that may be impacted by this patch are generated once the patch lands on Beta.
Attachment #790567 - Flags: approval-mozilla-beta?
Attachment #790567 - Flags: approval-mozilla-beta+
Attachment #790567 - Flags: approval-mozilla-aurora?
Attachment #790567 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/f4ed497d794b
https://hg.mozilla.org/releases/mozilla-beta/rev/ad1f0ae291d7
status-firefox24: --- → fixed
status-firefox25: --- → fixed
status-firefox26: --- → fixed
Assuming no verification needed here. Please add the verifyme keyword and remove the [qa-] whiteboard tag to request verification.
Whiteboard: [qa-]
(Assignee)

Updated

4 years ago
Status: RESOLVED → REOPENED
status-firefox31: --- → affected
status-firefox32: --- → affected
status-firefox33: --- → affected
status-firefox34: --- → affected
Resolution: FIXED → ---
(Assignee)

Comment 12

4 years ago
Created attachment 8466837 [details] [diff] [review]
Fix Skia for ARM v4t, refreshed against current trunk

This is the same patch as what landed in Fx26, but was left out in bug 910754.
(Assignee)

Comment 13

4 years ago
Comment on attachment 8466837 [details] [diff] [review]
Fix Skia for ARM v4t, refreshed against current trunk

Carrying over r+
Attachment #8466837 - Attachment description: Fix Skia for ARM v4t → Fix Skia for ARM v4t, refreshed against current trunk
Attachment #8466837 - Flags: review+
(Assignee)

Comment 14

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/f1a641e3d9dd
Whiteboard: [qa-] → [qa-][npotb][checkin-needed-aurora][checkin-needed-beta][checkin-needed-esr31]
Target Milestone: mozilla26 → ---
https://hg.mozilla.org/releases/mozilla-aurora/rev/747541380bbb
https://hg.mozilla.org/releases/mozilla-beta/rev/aa124023125a
https://hg.mozilla.org/releases/mozilla-esr31/rev/216554269ce2
status-firefox31: affected → wontfix
status-firefox32: affected → fixed
status-firefox33: affected → fixed
status-firefox34: affected → fixed
status-firefox-esr31: --- → fixed
Whiteboard: [qa-][npotb][checkin-needed-aurora][checkin-needed-beta][checkin-needed-esr31] → [qa-]
https://hg.mozilla.org/mozilla-central/rev/f1a641e3d9dd
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.