721625 - Linux Repacks broken after Bug 719659

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Justin Wood (:Callek)]

For the past few days Linux nightly/dep builds for SeaMonkey have been busted in configure.

Key Differences between SeaMonkey and Firefox linux repacks here:

* SeaMonkey does NOT use mozconfigs here, firefox does.
* SeaMonkey ends up with gcc 4.1.1 Firefox gets the mozconfig set 4.5-something.

A bisect confirmed it, and a config.log proved it, that we are regressed from 719659 and is due to an unrecognized CFLAG, this is also hidden by the fact that after these CFLAGS are set, we don't have a real "is these CFLAGS valid" check before the next check(s) here.

Snippet from config.log:

configure:18483: /tools/gcc/bin/gcc -c  -std=gnu99 -fgnu89-inline -fno-strict-aliasing  conftest.c 1
>&5
cc1: error: unrecognized command line option "-fgnu89-inline"
configure: failed program was:
#line 18475 "configure"
#include "confdefs.h"

#include <curl/curl.h>
int main() {

; return 0; }

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Probable patch

I don't have gcc 4.1 to test this with, so I used these as CC/CXX to attempt to simulate it:

[jwalden@wheres-wally src]$ cat /tmp/cc-or-fail /tmp/cxx-or-fail 
#!/bin/bash

if [[ "$@" = *fgnu89-inline* ]]; then
  exit 1
else
  gcc $@
fi
#!/bin/bash

if [[ "$@" = *fgnu89-inline* ]]; then
  exit 1
else
  g++ $@
fi


The resulting command lines seemed not to include the offending bits in them (and did with clang and gcc 4.6 in my distro), so I think this should do the job.

Comment 2

[(not currently active) Ted Mielczarek]

I honestly don't feel like we should be spending time making things work with GCC 4.1. We should just blacklist compilers that old.

Comment 4

[Justin Wood (:Callek)]

(In reply to Ted Mielczarek [:ted, :luser] from comment #2)
> I honestly don't feel like we should be spending time making things work
> with GCC 4.1. We should just blacklist compilers that old.

After seeing Bug 721637 this is not just a SeaMonkey issue.

Ted, the issue isn't about making "things" work with GCC 4.1, they already do, this is just about _repacks_ which would work if configure wouldn't barf unnecessarily. All we build in repacks is libmar and its dependants, whose resulting binaries are not shipped, and only used to create the update snippets.

Comment 5

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

> Ted, the issue isn't about making "things" work with GCC 4.1, they already
> do, this is just about _repacks_ which would work if configure wouldn't barf
> unnecessarily. All we build in repacks is libmar and its dependants, whose
> resulting binaries are not shipped, and only used to create the update
> snippets.

In which case why do you care which compiler is used? Just update to at least gcc 4.2.

Comment 6

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Comment on attachment 592047 [details] [diff] [review]
Probable patch

If if we must support such an old compiler, this snippet is definitely too big now to be duplicated. Please move it to a new .m4 file in ./build/autoconf.

Comment 7

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

Adding checks for features that exist since gcc 4.2 is terrible precedence. We should really not go there. I have spent too much time looking at code and checks for old compiler (even cfront).

What I think we should do is add 

os.environ["CC"] = /tools/gcc-4.5-0moz3/bin/gcc
os.environ["CXX"] = /tools/gcc-4.5-0moz3/bin/g++

just before http://mxr.mozilla.org/build/source/buildbotcustom/process/factory.py#2989

If for some reason we *can't* do it, then my next preference would be reverting this patch. The warnings might make sure we fix this again and be sure to not include gcc 4.1 check on the fix.

If all else fails, the code should be moved to a .m4 file. The file should have a big comment saying it should be removed immediately after 10 is released and we should have a bug open for removing it.

Comment 8

[Justin Wood (:Callek)]

(In reply to Rafael Ávila de Espíndola (:espindola) from comment #7)
> If for some reason we *can't* do it,

Rather than hardcoding things like compiler in Buildbot scripts, our fix would be mozconfigs-in-source-tree (so we don't affect other trains) But as said, SeaMonkey does not have that support yet, and won't before sometime during the next train.

> then my next preference would be
> reverting this patch. The warnings might make sure we fix this again and be
> sure to not include gcc 4.1 check on the fix.

Done: 
https://hg.mozilla.org/mozilla-central/rev/1a08877de7ed
https://hg.mozilla.org/mozilla-central/rev/cdf89e1937eb (merge cset)

Comment 9

[Justin Wood (:Callek)]

(fixed by backout)

Comment 10

[Rafael Ávila de Espíndola (:espindola) (not reading bugmail)]

> Rather than hardcoding things like compiler in Buildbot scripts, our fix
> would be mozconfigs-in-source-tree (so we don't affect other trains) But as
> said, SeaMonkey does not have that support yet, and won't before sometime
> during the next train.

I find it depressing that a side project having a broken system finds itself in the right of harming mozilla for its benefit.

Comment 11

[Alexander L. Slovesnik]

(In reply to Rafael Ávila de Espíndola (:espindola) from comment #10)
> I find it depressing that a side project having a broken system finds itself
> in the right of harming mozilla for its benefit.
Firefox and Thunderbird l10n builds are not "Mozilla's side project". I find it depressing that nobody took any notice that they've been broken for 3 days after Bug 719659 has been checked in.

Comment 12

[(not currently active) Ted Mielczarek]

As originally filed this said "SeaMonkey repacks", which is what Rafael was referring to.