Note: There are a few cases of duplicates in user autocompletion which are being worked on.

configure error with sed 4.3

RESOLVED FIXED in Firefox 51

Status

()

Core
Build Config
RESOLVED FIXED
7 months ago
3 months ago

People

(Reporter: bugzilla, Assigned: bagder)

Tracking

51 Branch
mozilla53
Points:
---

Firefox Tracking Flags

(firefox50 wontfix, firefox51 fixed, firefox52 fixed, firefox53 fixed, firefox-esr45 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 months ago
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

Updated

7 months ago
Attachment #8824503 - Attachment is patch: true
Attachment #8824503 - Attachment mime type: text/x-patch → text/plain

Updated

7 months ago
Component: Untriaged → Build Config
Product: Firefox → Core

Updated

7 months ago
Attachment #8824503 - Flags: review?(mh+mozilla)
(Reporter)

Comment 1

7 months ago
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?
(Assignee)

Comment 2

7 months ago
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"`

Updated

7 months ago
Duplicate of this bug: 1329252

Updated

7 months ago
Duplicate of this bug: 1329394
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.
Attachment #8824503 - Flags: review?(mh+mozilla) → review-
(Assignee)

Comment 6

7 months ago
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.
Attachment #8825307 - Flags: review?(mh+mozilla)
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.
Attachment #8825307 - Flags: review?(mh+mozilla) → review+
(Assignee)

Updated

7 months ago
Keywords: checkin-needed

Comment 8

6 months ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ebcbf47a83e7
add bracket for sed 4.3 compliance, r=glandium
Keywords: checkin-needed

Updated

6 months ago
Attachment #8824503 - Attachment is obsolete: true

Updated

6 months ago
status-firefox50: --- → affected
status-firefox51: --- → affected
status-firefox52: --- → affected
status-firefox53: --- → affected
status-firefox-esr45: --- → affected

Comment 9

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ebcbf47a83e7
Status: UNCONFIRMED → RESOLVED
Last Resolved: 6 months ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
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?
(Assignee)

Comment 11

6 months ago
(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.
(Assignee)

Comment 12

6 months ago
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]:
Attachment #8825307 - Flags: approval-mozilla-release?
Attachment #8825307 - Flags: approval-mozilla-beta?
Attachment #8825307 - Flags: approval-mozilla-aurora?
Please also get this on esr.
Flags: needinfo?(daniel)
(Assignee)

Comment 14

6 months ago
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.
Flags: needinfo?(daniel)
Attachment #8825307 - Flags: approval-mozilla-esr45?
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.
Attachment #8825307 - Flags: approval-mozilla-release?
Attachment #8825307 - Flags: approval-mozilla-release-
Attachment #8825307 - Flags: approval-mozilla-beta?
Attachment #8825307 - Flags: approval-mozilla-beta+
Attachment #8825307 - Flags: approval-mozilla-aurora?
Attachment #8825307 - Flags: approval-mozilla-aurora+
oops.  s/not//.  That's what I get for submitting halfway through editing the comment.
https://hg.mozilla.org/releases/mozilla-aurora/rev/5164c0e84b2d
status-firefox52: affected → fixed
https://hg.mozilla.org/releases/mozilla-beta/rev/5fb0834eba17
status-firefox51: affected → fixed
Assignee: nobody → daniel

Updated

6 months ago
Duplicate of this bug: 1331362
status-firefox50: affected → wontfix

Comment 20

6 months ago
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
Attachment #8825307 - Flags: approval-mozilla-esr45? → approval-mozilla-esr45+
https://hg.mozilla.org/releases/mozilla-esr45/rev/571b48abf054895d21d896be638f48338d87e864
status-firefox-esr45: affected → fixed

Updated

3 months ago
Duplicate of this bug: 1355946
You need to log in before you can comment on or make changes to this bug.