Last Comment Bug 721625 - Linux Repacks broken after Bug 719659
: Linux Repacks broken after Bug 719659
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All Linux
: -- normal (vote)
: ---
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
: Gregory Szorc [:gps]
Mentors:
: 721637 722036 (view as bug list)
Depends on:
Blocks: 719659
  Show dependency treegraph
 
Reported: 2012-01-26 19:56 PST by Justin Wood (:Callek)
Modified: 2012-01-28 07:28 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Probable patch (2.89 KB, patch)
2012-01-26 20:57 PST, Jeff Walden [:Waldo] (remove +bmo to email)
respindola: review-
Details | Diff | Splinter Review

Description Justin Wood (:Callek) 2012-01-26 19:56:53 PST
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] (remove +bmo to email) 2012-01-26 20:57:49 PST
Created attachment 592047 [details] [diff] [review]
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 Ted Mielczarek [:ted.mielczarek] 2012-01-27 04:37:50 PST
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 3 Justin Wood (:Callek) 2012-01-27 05:37:39 PST
*** Bug 721637 has been marked as a duplicate of this bug. ***
Comment 4 Justin Wood (:Callek) 2012-01-27 05:40:12 PST
(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) 2012-01-27 06:23:27 PST
> 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) 2012-01-27 06:27:03 PST
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) 2012-01-27 07:01:51 PST
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) 2012-01-27 07:13:56 PST
(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) 2012-01-27 07:15:39 PST
(fixed by backout)
Comment 10 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2012-01-27 07:35:55 PST
> 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 2012-01-27 08:30:56 PST
(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 Ted Mielczarek [:ted.mielczarek] 2012-01-27 09:22:33 PST
As originally filed this said "SeaMonkey repacks", which is what Rafael was referring to.
Comment 13 Chris Cooper [:coop] 2012-01-28 07:28:49 PST
*** Bug 722036 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.