1329272 - configure error with sed 4.3

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[bugzilla]

Attachment: sed.patch

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20170104201326

Steps to reproduce:

Build firefox 50.1.0, 51.0b12 with sed 4.3 installed


Actual results:

errors out with:
sed: character class syntax is [[:space:]], not [:space:]


Expected results:

work

Comment 1

[bugzilla]

The patch is not enough. You get a similar problem in js/src/old-configure that is generated during the build. Problem with autoconf-2.13?

Comment 2

[Daniel Stenberg [:bagder]]

The double '[' is not enough for some reason for that first [[:space:]] occurrence. If I instead use _three_ brackets on both sides there my sed 4.3 no longer complains:

    version=`sed -n 's/^[[[:space:]]]*#[[:space:]]*define[[:space:]][[:space:]]*U_ICU_VERSION_MAJOR_NUM[[:space:]][[:space:]]*\([0-9][0-9]*\)[[:space:]]*$/\1/p' "$icudir/common/unicode/uvernum.h"`

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 8824503 [details] [diff] [review]
sed.patch

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

The patch needs to be applied to build/autoconf/icu.m4.

Comment 6

[Daniel Stenberg [:bagder]]

Attachment: 0001-bug-1329272-add-bracket-for-sed-4.3-compliance-r-gla.patch

Here's the patch I used to fix the build for me.

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8825307 [details] [diff] [review]
0001-bug-1329272-add-bracket-for-sed-4.3-compliance-r-gla.patch

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

We'll need this on all branches, including esr.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ebcbf47a83e7
add bracket for sed 4.3 compliance, r=glandium

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ebcbf47a83e7

Comment 10

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

I got this problem on m-r too.
I made a proxy script for sed like this:

#! /bin/sh
echo 'sed' "$@" > /tmp/sed.log
/bin/sed "$@"

And I got this:

sed -n s/^[:space:]*#[[:space:]]*define[[:space:]][[:space:]]*U_ICU_VERSION_MAJOR_NUM[[:space:]][[:space:]]*\([0-9][0-9]*\)[[:space:]]*$/^A/p /home/wcpan/local/src/firefox/intl/icu/source/common/unicode/uvernum.h

It looks like the first pair of brackets was comsumed in the first place.
Maybe the root cause is not sed?

Comment 11

[Daniel Stenberg [:bagder]]

(In reply to Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] from comment #10)

> Maybe the root cause is not sed?

Correct, the *cause* is not sed, but sed up until before 4.3 still worked fine with that input. 4.3 started to instead complain on the bad pattern and exit with a message.

The fix that now is in m-c makes sure that the first pair ends up with two brackets on either side of :space: and should work with 4.3 and older sed versions.

Comment 12

[Daniel Stenberg [:bagder]]

Comment on attachment 8825307 [details] [diff] [review]
0001-bug-1329272-add-bracket-for-sed-4.3-compliance-r-gla.patch

Approval Request Comment
[Feature/Bug causing the regression]: 1329272 
[User impact if declined]: Failed builds for users having sed 4.3 installed.
[Is this code covered by automated tests?]: Not yet, but will in the future once the buildbots upgrade to sed 4.3
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no 
[Why is the change risky/not risky?]: it is a small change that carries low risk
[String changes made/needed]:

Comment 13

[Mike Hommey [:glandium]]

Please also get this on esr.

Comment 14

[Daniel Stenberg [:bagder]]

Comment on attachment 8825307 [details] [diff] [review]
0001-bug-1329272-add-bracket-for-sed-4.3-compliance-r-gla.patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: Causes build problems for users with sed 4.3 (or later, presumably) installed
Fix Landed on Version: nightly so far, being suggested for uplift to "all" branches
Risk to taking this patch (and alternatives if risky): Very small risk.
String or UUID changes made by this patch: 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 15

[Julien Cristau [:jcristau]]

Comment on attachment 8825307 [details] [diff] [review]
0001-bug-1329272-add-bracket-for-sed-4.3-compliance-r-gla.patch

build fix for new sed, aurora52+ and beta51+.  There won't be a dot release for this, so not dropping approval-mozilla-release.

Comment 16

[Julien Cristau [:jcristau]]

oops.  s/not//.  That's what I get for submitting halfway through editing the comment.

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5164c0e84b2d

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5fb0834eba17

Comment 20

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8825307 [details] [diff] [review]
0001-bug-1329272-add-bracket-for-sed-4.3-compliance-r-gla.patch

Seems like a safe fix, let's include it in ESR45.7

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-esr45/rev/571b48abf054895d21d896be638f48338d87e864