Linux: Autodetect and change build flags when using gcc 4.4.

Linux: Autodetect and change build flags when using gcc 4.4.

This runs g++ -dumpversion (or $CXX) to establish the compiler version
and set the gyp gcc_version variable automatically.

BUG=25209
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=30824

[Evan Martin, 2009-10-28 19:26:13]

This looks pretty good to me -- just a couple minor adjustments

http://codereview.chromium.org/339046/diff/1/3
File build/compiler_version.py (right):

http://codereview.chromium.org/339046/diff/1/3#newcode23
Line 23: p1 = subprocess.Popen(compiler, stdout=subprocess.PIPE)
You can use shell=True to avoid the split above; that makes this work even when
your command line is even more complicated, like including quotes.

Can you give this variable a name like "pipe"?  We frown on abbreviations.

http://codereview.chromium.org/339046/diff/1/3#newcode25
Line 25: result = re.findall(re.compile(r"(\d+)\.(\d+)\..*"), gcc_output)
Could you make this only look at the first line?

http://codereview.chromium.org/339046/diff/1/3#newcode32
Line 32: return os.environ[env]
You can use
  return os.environ.get(env, "")
instead of this try/except stuff, or even just inline that into the code below

Maybe you could use None instead of "" for clarity

http://codereview.chromium.org/339046/diff/1/4
File chrome/chrome.gyp (right):

http://codereview.chromium.org/339046/diff/1/4#newcode4682
Line 4682: # building mru_cache_unittest.cc.
This makes me sad.

I guess this belongs in a different change though?

[Craig, 2009-10-29 15:41:32]

Thanks for the comments - I think it looks a bit better now. Please take another
look.

Let me know if you want me to split the strict aliasing bit out separately
please.

http://codereview.chromium.org/339046/diff/1/3
File build/compiler_version.py (right):

http://codereview.chromium.org/339046/diff/1/3#newcode23
Line 23: p1 = subprocess.Popen(compiler, stdout=subprocess.PIPE)
On 2009/10/28 19:26:13, Evan Martin wrote:
> You can use shell=True to avoid the split above; that makes this work even
when
> your command line is even more complicated, like including quotes.
> 
> Can you give this variable a name like "pipe"?  We frown on abbreviations.

Done.

http://codereview.chromium.org/339046/diff/1/3#newcode25
Line 25: result = re.findall(re.compile(r"(\d+)\.(\d+)\..*"), gcc_output)
On 2009/10/28 19:26:13, Evan Martin wrote:
> Could you make this only look at the first line?

Done.

http://codereview.chromium.org/339046/diff/1/3#newcode32
Line 32: return os.environ[env]
On 2009/10/28 19:26:13, Evan Martin wrote:
> You can use
>   return os.environ.get(env, "")
> instead of this try/except stuff, or even just inline that into the code below
> 
> Maybe you could use None instead of "" for clarity

Done.

http://codereview.chromium.org/339046/diff/1/4
File chrome/chrome.gyp (right):

http://codereview.chromium.org/339046/diff/1/4#newcode4682
Line 4682: # building mru_cache_unittest.cc.
On 2009/10/28 19:26:13, Evan Martin wrote:
> This makes me sad.
> 
> I guess this belongs in a different change though?

I can split it out if you want. I thought it was sort of the same thing
"automatically choosing the right options based on compiler version". I thought
it was better btw. to only enforce no-strict-aliasing on the unit tests target
here than setting the existing no_strict_aliasing variable which turns it on
everywhere.

I should actually file a bug with the glibc people if I can wrap my head around
the error ... I think agl and deanm looked at it at some point and that's when
the "global" no_strict_aliasing in common.gypi was added.

[Craig, 2009-11-02 18:04:33]

On 2009/10/29 15:41:32, Craig wrote:
> Thanks for the comments - I think it looks a bit better now. Please take
another
> look.
> 
> Let me know if you want me to split the strict aliasing bit out separately
> please.

I have removed the chrome.gyp change now ... I will land that separately.

[Evan Martin, 2009-11-02 18:07:22]

LGTM, whatever you decide wrt my comment

http://codereview.chromium.org/339046/diff/3001/4002
File build/compiler_version.py (right):

http://codereview.chromium.org/339046/diff/3001/4002#newcode26
Line 26: except Exception, e:
Should we complain more loudly when something goes wrong?
Like maybe we should print it to stderr while still returning "".

[Craig, 2009-11-03 16:06:33]

On 2009/11/02 18:07:22, Evan Martin wrote:
> LGTM, whatever you decide wrt my comment

Your comments are always good :) I'm printing the command line and the exception
to stderr now on failure. Thank you.

> http://codereview.chromium.org/339046/diff/3001/4002
> File build/compiler_version.py (right):
> 
> http://codereview.chromium.org/339046/diff/3001/4002#newcode26
> Line 26: except Exception, e:
> Should we complain more loudly when something goes wrong?
> Like maybe we should print it to stderr while still returning "".

[Evan Martin, 2009-11-03 16:16:31]

LGTM