1465731 - mozjs fails to build on armv6

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Guillaume GARDET]

Attachment: fix_armv6_build.patch

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.

Comment 1

[ovidiu boca[:Ovidiu]]

This is with the latest Firefox version? If not please please download it from here: https://www.mozilla.org/ro/firefox/new/

Comment 2

[Lars T Hansen [:lth]]

Looks like 52 ESR.

Comment 3

[Guillaume GARDET]

Yes, this is 52 ESR.

Comment 4

[Nicolas B. Pierron [:nbp]]

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.

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction#Step_4_Get_your_code_reviewed

Comment 5

[Guillaume GARDET]

Attachment: Patch with 'From' and 'Subject' lines

Comment 6

[Guillaume GARDET]

Anything blocking?

Comment 7

[Lars T Hansen [:lth]]

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.

Comment 8

[Mike Hommey [:glandium]]

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.

Comment 9

[Guillaume GARDET]

Attachment: Simpler patch, as requested

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/494eb5db5648
Fix armv6 build. r=glandium

Comment 11

[Stefan Hindli [:stefan_hindli]]

Backed out changeset 494eb5db5648 (Bug 1465731) for bustages in the MOZBUILD file

https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed,busted,exception&classifiedState=unclassified&fromchange=d4a99f4598435e66d76dedd72558a494c73687fc&selectedJob=203307980

https://treeherder.mozilla.org/logviewer.html#?job_id=203307980&repo=mozilla-inbound&lineNumber=1611

Comment 12

[Stefan Hindli [:stefan_hindli]]

Disregard last comment since bustages were caused

Comment 13

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c3191c16f0a
Backed out changeset 494eb5db5648 for bustages in the MOZBUILD file

Comment 14

[Mike Hommey [:glandium]]

Err, my fault, 1s replaces on the first line, but the first line doesn't contain __ARM_ARCH_. The original patch was actually fine.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/34ab0ef2419d
Fix armv6 build. r=glandiumd

Comment 16

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/34ab0ef2419d