Enable -fPIC by default on x64

Enable -fPIC by default on x64

Get rid of linux_fpic gyp variable.

BUG=26625
TEST=all linux builds should compile

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

[Evan Martin, 2010-08-09 22:46:36]

Given my below comment, perhaps it'd make sense to -fPIC always on
library=shared_library in the common gypi.

http://codereview.chromium.org/3135001/diff/1/4
File gpu/demos/demos.gyp (left):

http://codereview.chromium.org/3135001/diff/1/4#oldcode13
gpu/demos/demos.gyp:13: # unless linux_fpic is 1.
I think you should leave in enough of this comment for it to be clear why we're
disabling the demos on ia32.

I wonder if removing this option (which allows fpic on 32bit) means we'll break
pepper on 32?

[Nirnimesh, 2010-08-09 23:05:00]

http://codereview.chromium.org/3135001/diff/1/4
File gpu/demos/demos.gyp (left):

http://codereview.chromium.org/3135001/diff/1/4#oldcode13
gpu/demos/demos.gyp:13: # unless linux_fpic is 1.
On 2010/08/09 22:46:36, Evan Martin wrote:
> I think you should leave in enough of this comment for it to be clear why
we're
> disabling the demos on ia32.
> 
> I wonder if removing this option (which allows fpic on 32bit) means we'll
break
> pepper on 32?

The previous conditional has pepper enabled on 32-bit.
I'm not adding -fPIC to 32bit. Should I?

[Evan Martin, 2010-08-09 23:08:41]

On 2010/08/09 23:05:00, Nirnimesh wrote:
> > I wonder if removing this option (which allows fpic on 32bit) means we'll
> break
> > pepper on 32?
> 
> The previous conditional has pepper enabled on 32-bit.
> I'm not adding -fPIC to 32bit. Should I?

I was concerned that people working on nacl were relying on linux_fpic to work
on it on 32 bit.  Maybe you should find someone from there to review?  I usually
send everything to sehr@google.com.

I also am concerned about the comments in the bug about ffmpeg breaking.

[Nirnimesh, 2010-08-09 23:23:42]

+sehr to make sure this is correct for nacl.

I've removed all references to linux_fpic, and am not enabling more 32 bit
targets than already was, and have checked on my x64 box where ffmpeg seems to
build fine.

[Evan Martin, 2010-08-09 23:32:24]

LGTM as long as:
- you get LGTM from nacl
- you watch the perf bots and report back.  :)

[Nirnimesh, 2010-08-10 18:41:47]

Evan, Sehr appears to be on vacation. Do you know someone else from NaCl?

[apatrick, 2010-08-10 18:44:53]

On 2010/08/10 18:41:47, Nirnimesh wrote:
> Evan, Sehr appears to be on vacation. Do you know someone else from NaCl?

I suggest neb@, he's working on this.

[Nirnimesh, 2010-08-10 18:47:28]

neb, could you have a look to make sure this is fine for NaCl please?

[neb, 2010-08-10 19:25:25]

On 2010/08/10 18:47:28, Nirnimesh wrote:
> neb, could you have a look to make sure this is fine for NaCl please?

To the best of my knowledge, NaCl doesn't use linux_fpic in its build, and
should be fine to build with -fPIC. I think you should ask Bennet.

[neb, 2010-08-10 19:25:58]

He's bsy@

[Nirnimesh, 2010-08-10 20:27:45]

Bennet, do you see any problems with this on NaCl?

[bsy, 2010-08-11 03:57:22]

On 2010/08/10 20:27:45, Nirnimesh wrote:
> Bennet, do you see any problems with this on NaCl?

neb is right; we don't use linux_fpic in our gyp build.

[Nirnimesh, 2010-08-11 18:43:02]

Looks like the coast is clear. Going to commit this. Thanks all.

[alokp, 2010-08-11 19:52:52]

http://codereview.chromium.org/3135001/diff/4001/5003
File gpu/demos/demos.gyp (right):

http://codereview.chromium.org/3135001/diff/4001/5003#newcode9
gpu/demos/demos.gyp:9: ['OS=="linux" and (target_arch=="x64" or
target_arch=="arm")', {
This is wrong. It reverses the original logic. Now pepper demos are disabled on
all platforms except linux 64 bit and arm. Now that -fPIC is enable on those
platforms there is no need for enable_pepper_demos variable or it should always
be 1.

[alokp, 2010-08-11 19:56:14]

http://codereview.chromium.org/3135001/diff/4001/5002
File chrome/chrome_tests.gypi (left):

http://codereview.chromium.org/3135001/diff/4001/5002#oldcode343
chrome/chrome_tests.gypi:343: #
http://code.google.com/p/chromium/issues/detail?id=18337
These should be cleaned up too. We have a lot of disabled stuff just due to not
having -fPIC.

http://codereview.chromium.org/3135001/diff/4001/5002#oldcode352
chrome/chrome_tests.gypi:352: ['target_arch!="x64" and target_arch!="arm" and
OS=="win"', {
ditto

[Nirnimesh, 2010-08-11 20:06:28]

On 2010/08/11 19:52:52, alokp wrote:
> http://codereview.chromium.org/3135001/diff/4001/5003
> File gpu/demos/demos.gyp (right):
> 
> http://codereview.chromium.org/3135001/diff/4001/5003#newcode9
> gpu/demos/demos.gyp:9: ['OS=="linux" and (target_arch=="x64" or
> target_arch=="arm")', {
> This is wrong. It reverses the original logic. Now pepper demos are disabled
on
> all platforms except linux 64 bit and arm. Now that -fPIC is enable on those
> platforms there is no need for enable_pepper_demos variable or it should
always
> be 1.

I see. Fixing it. Thanks for pointing.

[Nirnimesh, 2010-08-11 21:31:38]

Fixing building pepper demos

http://codereview.chromium.org/3135001/diff/4001/5002
File chrome/chrome_tests.gypi (left):

http://codereview.chromium.org/3135001/diff/4001/5002#oldcode343
chrome/chrome_tests.gypi:343: #
http://code.google.com/p/chromium/issues/detail?id=18337
On 2010/08/11 19:56:14, alokp wrote:
> These should be cleaned up too. We have a lot of disabled stuff just due to
not
> having -fPIC.

I'm not sure I understand the gyp fu used here. There's no
copy_npapi_test_plugin target in webkit/webkit.gyp. Merely removing this
conditional and moving  to the dependencies section above did not work. Someone
familiar with this should advise.

http://codereview.chromium.org/3135001/diff/4001/5002#oldcode352
chrome/chrome_tests.gypi:352: ['target_arch!="x64" and target_arch!="arm" and
OS=="win"', {
On 2010/08/11 19:56:14, alokp wrote:
> ditto

This is supposed to run on win only. Not removing this conditional.

http://codereview.chromium.org/3135001/diff/4001/5003
File gpu/demos/demos.gyp (right):

http://codereview.chromium.org/3135001/diff/4001/5003#newcode9
gpu/demos/demos.gyp:9: ['OS=="linux" and (target_arch=="x64" or
target_arch=="arm")', {
On 2010/08/11 19:52:52, alokp wrote:
> This is wrong. It reverses the original logic. Now pepper demos are disabled
on
> all platforms except linux 64 bit and arm. Now that -fPIC is enable on those
> platforms there is no need for enable_pepper_demos variable or it should
always
> be 1.

Done.

[alokp, 2010-08-11 21:52:14]

lgtm