Note: There are a few cases of duplicates in user autocompletion which are being worked on.
Last Comment Bug 1329272 - configure error with sed 4.3
: configure error with sed 4.3
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: 51 Branch
: Unspecified Unspecified
-- normal (vote)
: mozilla53
Assigned To: Daniel Stenberg [:bagder]
:
: Gregory Szorc [:gps]
Mentors:
: 1329252 1329394 1331362 1355946 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2017-01-06 10:48 PST by bugzilla
Modified: 2017-04-12 14:13 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
fixed
fixed
fixed
fixed


Attachments
sed.patch (948 bytes, patch)
2017-01-06 10:48 PST, bugzilla
mh+mozilla: review-
Details | Diff | Splinter Review
0001-bug-1329272-add-bracket-for-sed-4.3-compliance-r-gla.patch (1.18 KB, patch)
2017-01-09 23:25 PST, Daniel Stenberg [:bagder]
mh+mozilla: review+
jcristau: approval‑mozilla‑aurora+
jcristau: approval‑mozilla‑beta+
jcristau: approval‑mozilla‑release-
rkothari: approval‑mozilla‑esr45+
Details | Diff | Splinter Review

Description User image bugzilla 2017-01-06 10:48:30 PST
Created attachment 8824503 [details] [diff] [review]
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 User image bugzilla 2017-01-06 18:34:39 PST
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 User image Daniel Stenberg [:bagder] 2017-01-09 05:02:03 PST
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 3 User image Gregory Szorc [:gps] 2017-01-09 12:29:22 PST
*** Bug 1329252 has been marked as a duplicate of this bug. ***
Comment 4 User image Gregory Szorc [:gps] 2017-01-09 12:30:48 PST
*** Bug 1329394 has been marked as a duplicate of this bug. ***
Comment 5 User image Mike Hommey [:glandium] 2017-01-09 12:38:32 PST
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 User image Daniel Stenberg [:bagder] 2017-01-09 23:25:26 PST
Created attachment 8825307 [details] [diff] [review]
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 User image Mike Hommey [:glandium] 2017-01-09 23:38:50 PST
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 User image Pulsebot 2017-01-10 06:34:35 PST
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ebcbf47a83e7
add bracket for sed 4.3 compliance, r=glandium
Comment 9 User image Wes Kocher (:KWierso) 2017-01-10 18:14:27 PST
https://hg.mozilla.org/mozilla-central/rev/ebcbf47a83e7
Comment 10 User image Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] 2017-01-10 22:52:26 PST
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 User image Daniel Stenberg [:bagder] 2017-01-10 22:58:34 PST
(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 User image Daniel Stenberg [:bagder] 2017-01-12 02:26:05 PST
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 User image Mike Hommey [:glandium] 2017-01-12 04:29:19 PST
Please also get this on esr.
Comment 14 User image Daniel Stenberg [:bagder] 2017-01-12 04:34:58 PST
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 User image Julien Cristau [:jcristau] 2017-01-12 05:39:57 PST
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 User image Julien Cristau [:jcristau] 2017-01-12 05:56:54 PST
oops.  s/not//.  That's what I get for submitting halfway through editing the comment.
Comment 17 User image Carsten Book [:Tomcat] - PTO - back July 23rd 2017-01-12 06:50:22 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/5164c0e84b2d
Comment 18 User image Carsten Book [:Tomcat] - PTO - back July 23rd 2017-01-12 07:18:44 PST
https://hg.mozilla.org/releases/mozilla-beta/rev/5fb0834eba17
Comment 19 User image Mike Hommey [:glandium] 2017-01-16 13:46:32 PST
*** Bug 1331362 has been marked as a duplicate of this bug. ***
Comment 20 User image Ritu Kothari (:ritu) 2017-01-17 11:08:15 PST
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 22 User image Andrew Young 2017-04-12 14:13:34 PDT
*** Bug 1355946 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.