Closed Bug 1465731 Opened Last year Closed Last year

mozjs fails to build on armv6


(Firefox Build System :: General, defect)

52 Branch
Not set


(firefox64 fixed)

Tracking Status
firefox64 --- fixed


(Reporter: guillaume.gardet, Assigned: guillaume.gardet)



(1 file, 2 obsolete files)

Attached file fix_armv6_build.patch (obsolete) —
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.62 Safari/537.36

Steps to reproduce:

While trying to build mozjs52 for armv6, you get an error, because ARM_ARCH is not set to 6, but to '6\n6'

Actual results:

ARM_ARCH setting is broken in build/autoconf/arch.m4 file, when multiple lines matches the pattern.

Expected results:

So, we can just use the first line by adding '| head -n 1' for example. See attachment.
This is with the latest Firefox version? If not please please download it from here:
Component: Untriaged → JavaScript Engine
Flags: needinfo?(guillaume.gardet)
Product: Firefox → Core
Attachment #8982154 - Attachment mime type: text/x-patch → text/plain
Looks like 52 ESR.
Yes, this is 52 ESR.
Flags: needinfo?(guillaume.gardet)
Thanks for contributing a patch, but you might want to add you name and a proper subject as part of the commit, in order to get credit back on your contribution.
Component: JavaScript Engine → General
Product: Core → Firefox Build System
Attachment #8982154 - Attachment is obsolete: true
Anything blocking?
Comment on attachment 8992292 [details] [diff] [review]
Patch with 'From' and 'Subject' lines

Needs review from a build peer.  Note the code is the same in current FF, so if the patch is correct we may want to consider more than ESR52.
Attachment #8992292 - Flags: review?(mh+mozilla)
Comment on attachment 8992292 [details] [diff] [review]
Patch with 'From' and 'Subject' lines

Review of attachment 8992292 [details] [diff] [review]:

::: firefox-52.6.0esr.orig/build/autoconf/arch.m4
@@ +217,4 @@
>    AC_MSG_CHECKING(ARM version support in compiler)
>    dnl Determine the target ARM architecture (5 for ARMv5, v5T, v5E, etc.; 6 for ARMv6, v6K, etc.)
> +  ARM_ARCH=`${CC-cc} ${CFLAGS} -dM -E - < /dev/null | sed -n 's/.*__ARM_ARCH_\([[0-9]][[0-9]]*\).*/\1/p' | head -n 1`

It would be better to just add a 1 before the s in the sed command.
Attachment #8992292 - Flags: review?(mh+mozilla)
Attached patch Simpler patch, as requested (obsolete) — Splinter Review
Attachment #8992292 - Attachment is obsolete: true
Attachment #9013980 - Flags: review?(mh+mozilla)
Attachment #9013980 - Flags: review?(mh+mozilla) → review+
Assignee: nobody → guillaume.gardet
Keywords: checkin-needed
Closed: Last year
Resolution: --- → INCOMPLETE
Disregard last comment since bustages were caused
Ever confirmed: true
Resolution: INCOMPLETE → ---
Backout by
Backed out changeset 494eb5db5648 for bustages in the MOZBUILD file
Err, my fault, 1s replaces on the first line, but the first line doesn't contain __ARM_ARCH_. The original patch was actually fine.
Attachment #8992292 - Attachment is obsolete: false
Attachment #8992292 - Flags: review+
Attachment #9013980 - Attachment is obsolete: true
Flags: needinfo?(guillaume.gardet)
Closed: Last yearLast year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.