528413 - sed used in non-portable way in configure-related -- breaks on Solaris 10

Status: VERIFIED FIXED

(MailNews Core :: Build Config, defect)

Description

[Jeff Wieland]

User-Agent:       Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.8.1.23) Gecko/20090908 SeaMonkey/1.1.18 Firefox/2.0
Build Identifier: http://releases.mozilla.org/pub/mozilla.org/seamonkey/releases/2.0/source/seamonkey-2.0.source.tar.bz2

When I compile SeaMonkey 2.0 from source on Solaris 10, it identifies
itself as:

$ seamonkey2 -v
Mozilla SeaMonkey 2.1a1pre, Copyright (c) 1998-2009 mozilla.org


Reproducible: Always

Steps to Reproduce:
1.  Build SeaMonkey 2.0 on Solaris 10

Actual Results:  
$ seamonkey2 -v
Mozilla SeaMonkey 2.1a1pre, Copyright (c) 1998-2009 mozilla.org

Expected Results:  
$ seamonkey2 -v
Mozilla SeaMonkey 2.0, Copyright (c) 1998-2009 mozilla.org

There are three instances of the following code:

MOZILLA_BRANCH_VERSION=`echo ${MOZILLA_VERSION} | sed -e 's/^\([0-9]\.[0-9]\.[0-9]\).*/\1/;'`

found in suite/confvars.sh, mail/confvars.sh, and configure.in

For instance from suite/confvars.sh

# Needed for the mozilla-central build side of the system.
MOZILLA_BRANCH_VERSION=`echo ${MOZILLA_VERSION} | sed -e 's/\(^[0-9]\.[0-9]\.[0-9]\).*/\1/;'`
if test "$MOZILLA_BRANCH_VERSION" = "1.9.1"; then
  MOZ_APP_VERSION=`cat $topsrcdir/$MOZ_BUILD_APP/config/version-191.txt`
else
  MOZ_APP_VERSION=`cat $topsrcdir/$MOZ_BUILD_APP/config/version.txt`
fi

Assuming that MOZILLA_VERSION is set to 1.9.1.4 from the contents of
milestone.txt file, on Solaris 10 I can run:

$ echo 1.9.1.4 | sed -e 's/\(^[0-9]\.[0-9]\.[0-9]\).*/\1/;'
1.9.1.4
$

If I use the GNU version of sed:

$ echo 1.9.1.4 | /opt/gnu-coreutils/bin/sed -e \ 's/\(^[0-9]\.[0-9]\.[0-9]\).*/\1/;'
1.9.1
$

So it appears to be relying on a behavior that grep on Solaris
does not have.

This may not cover all of the cases, but from what's in the
milestone.txt file now, one could use the following:

MOZILLA_BRANCH_VERSION=`echo ${MOZILLA_VERSION} | cut -f3 -d.`

I have the GNU core utilities installed in a separate directory,
and prepending that to my patch allows the correct version to
be found.  These would not be found on a standard Solaris 10 
system.

Comment 1

[Jeff Wieland]

Note that this affects Thunderbird as well as Seamonkey.

Comment 2

[Phil Ringnalda (:philor)]

Yeah, but the significant thing is that it doesn't affect Firefox - if it's not in mozilla-central, it's not a Core bug.

Comment 3

[Jeff Wieland]

Thanks -- that makes sense.  I haven't submitted a bug here in years.

Comment 4

[Ginn Chen]

The 3 instance are not the same.

I fixed it for configure.in of comm-central.
See Bug 480515.

You should apply the patch to confvars.sh.

Comment 5

[Philip Chee]

So basically we need to backport Bug 480515 to comm-1.9.1.

Comment 6

[Mark Banner (:standard8)]

(In reply to comment #5)
> So basically we need to backport Bug 480515 to comm-1.9.1.

No, we need to do the fix of bug 480515 (which was to comm-central's configure.in) to the confvars.sh files.

Comment 7

[Jens Hatlak (:InvisibleSmiley)]

Attachment: caret first

Patch applies cleanly on both c-c and c-191.

Jeff, please check whether this works for you (just to be sure).

Mark, feel free to pass on the review request if need be (the bug 480515 attachment was reviewed by KaiRo who is on vacation until the end of the month); AFAIK you have the authority to review both TB and SM parts which I combined in the patch.

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 412282 [details] [diff] [review]
caret first

r=Standard8. Please land on comm-central.

I need to figure out some things on the Thunderbird side, as to when this can/should land, so I'll do both the approvals in a couple of days.

Comment 9

[Jeff Wieland]

This works for me.  Thanks!

Comment 10

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 412282 [details] [diff] [review]
caret first

http://hg.mozilla.org/comm-central/rev/7303c85436dc

Comment 11

[Stefan [:stefanh]]

Jens, is this fixed on trunk? If it is, please resolve it as fixed.

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 412282 [details] [diff] [review]
caret first

a=Standard8 for comm-1.9.1 (both SM + TB parts).

I'm pushing a set of things to comm-1.9.1 anyway and want to make sure this gets in, so will push it in a few minutes.

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 412282 [details] [diff] [review]
caret first

Checked in:
http://hg.mozilla.org/releases/comm-1.9.1/rev/963e984965dd

Comment 14

[Mark Banner (:standard8)]

Checked in for build 2 of RC1:
http://hg.mozilla.org/releases/comm-1.9.1/rev/621fbfb5b48c

Comment 15

[Ludovic Hirlimann [:Usul]]

Brian can you comment on the fact that you can build out of the box and that the patch is applied an works on sun - as I don't have a sun box to build on to verify the bug ?

Comment 16

[Ludovic Hirlimann [:Usul]]

Brian ping for comment

Comment 17

[Ludovic Hirlimann [:Usul]]

(In reply to comment #16)
> Brian ping for comment
Asking Ginn , can you confirm that building on solaris doesn't require sed tricks ?

Comment 18

[Ginn Chen]

I built comm-1.9.1 and thunderbird --version shows

 Thunderbird 3.0.1pre, Copyright (c) 1998-2010 mozilla.org

So I think it is fixed.