make it possible to disable the use of libpci on linux again

make it possible to disable the use of libpci on linux again

When CL 430573002 landed, it forced the use of libpci on linux, but
many embedded systems lack this library (and probably lack a pci bus
altogether). Let's add a new config variable to make it possible to
disable the use of libpci.

BUG=398416
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289262

[Mostyn Bramley-Moore, 2014-08-11 07:35:56]

@dnicoara, Zhenyao, piman: how does this small fixup look?  (I'm not
particularly familiar with gn yet...)

[dnicoara, 2014-08-11 13:38:01]

I think a bit of code surgery is still required since this is going to break on
a clean build. gpu_info_collector_linux.cc includes "library_loaders/libpci.h"
which is a generated header file in the libpci action target.

On a side note, the libpci target includes libpci libraries only if
"linux_link_pci==1". I wonder if that would be enough for your needed? Of
course, this would mean that the header files would still need to be present
when compiling, but libpci wouldn't be needed at runtime.

[Mostyn Bramley-Moore, 2014-08-11 14:17:02]

On 2014/08/11 13:38:01, dnicoara wrote:
> I think a bit of code surgery is still required since this is going to break
on
> a clean build. gpu_info_collector_linux.cc includes "library_loaders/libpci.h"
> which is a generated header file in the libpci action target.

Ah right- I had forgotten that I was using a .gypi file to exclude
gpu_info_collector_linux.cc in favor of a local version.

> On a side note, the libpci target includes libpci libraries only if
> "linux_link_pci==1". I wonder if that would be enough for your needed? Of
> course, this would mean that the header files would still need to be present
> when compiling, but libpci wouldn't be needed at runtime.

I would prefer a clean stub, with all the libpci use inside a target that's only
built when linux_link_pci==1.  I'll dig around a bit more to see if I can find a
nice solution.

[dnicoara, 2014-08-11 14:23:21]

On 2014/08/11 14:17:02, Mostyn Bramley-Moore wrote:
> On 2014/08/11 13:38:01, dnicoara wrote:
> > I think a bit of code surgery is still required since this is going to break
> on
> > a clean build. gpu_info_collector_linux.cc includes
"library_loaders/libpci.h"
> > which is a generated header file in the libpci action target.
> 
> Ah right- I had forgotten that I was using a .gypi file to exclude
> gpu_info_collector_linux.cc in favor of a local version.
> 
> > On a side note, the libpci target includes libpci libraries only if
> > "linux_link_pci==1". I wonder if that would be enough for your needed? Of
> > course, this would mean that the header files would still need to be present
> > when compiling, but libpci wouldn't be needed at runtime.
> 
> I would prefer a clean stub, with all the libpci use inside a target that's
only
> built when linux_link_pci==1.  I'll dig around a bit more to see if I can find
a
> nice solution.

I don't know what the original intent of "linux_link_pci" was, but perhaps it
can be removed if there's a use_libpci flag?

[Mostyn Bramley-Moore, 2014-08-11 14:31:59]

> I don't know what the original intent of "linux_link_pci" was, but perhaps it
> can be removed if there's a use_libpci flag?

I would like to do that too, but I'll have to look through the code history some
more first.

[Mostyn Bramley-Moore, 2014-08-12 14:39:57]

I think I have a simple solution in patchset 5, which makes it possible to build
linux without ozone and without x11, and therefore avoid libpci.  Do ozone
targets always have libpci?  If not, I can do some slightly larger refactoring.

In patchset 6, I added some documentation to clarify what the linux_link_*
settings do (added Paweł for review).

[Paweł Hajdan Jr., 2014-08-12 14:49:38]

Please do NOT remove linux_link_* options.

[Paweł Hajdan Jr., 2014-08-12 14:49:47]

LGTM

[dnicoara, 2014-08-12 16:12:53]

libpci should be optional for Ozone targets, though I haven't gotten around to
figuring out what the "correct" way to make it optional is. I'm not too worried
right now since Ozone doesn't need to link against libpci, so it can gracefully
return early if it determines there's no PCI support.

lgtm with nit. You don't need to worry about it right now.

https://codereview.chromium.org/462433002/diff/100001/gpu/config/BUILD.gn
File gpu/config/BUILD.gn (right):

https://codereview.chromium.org/462433002/diff/100001/gpu/config/BUILD.gn#new...
gpu/config/BUILD.gn:5: import("//build/config/features.gni")
I don't think you need this anymore.

[Mostyn Bramley-Moore, 2014-08-12 18:18:46]

On 2014/08/12 14:49:38, Paweł Hajdan Jr. wrote:
> Please do NOT remove linux_link_* options.

I won't, I realised what they did and added comments to make it more obvious.

On 2014/08/12 16:12:53, dnicoara wrote:
> libpci should be optional for Ozone targets, though I haven't gotten around to
> figuring out what the "correct" way to make it optional is. I'm not too
worried
> right now since Ozone doesn't need to link against libpci, so it can
gracefully
> return early if it determines there's no PCI support.

I might be able to help out with that in a later CL if you give me some details
on what configurations you want Ozone to support.

@Zhenyao: can you please give this OWNERS approval?

https://codereview.chromium.org/462433002/diff/100001/gpu/config/BUILD.gn
File gpu/config/BUILD.gn (right):

https://codereview.chromium.org/462433002/diff/100001/gpu/config/BUILD.gn#new...
gpu/config/BUILD.gn:5: import("//build/config/features.gni")
On 2014/08/12 16:12:52, dnicoara wrote:
> I don't think you need this anymore.

Done.

[dnicoara, 2014-08-12 18:52:02]

On 2014/08/12 18:18:46, Mostyn Bramley-Moore wrote:
> On 2014/08/12 14:49:38, Paweł Hajdan Jr. wrote:
> > Please do NOT remove linux_link_* options.
> 
> I won't, I realised what they did and added comments to make it more obvious.
> 
> On 2014/08/12 16:12:53, dnicoara wrote:
> > libpci should be optional for Ozone targets, though I haven't gotten around
to
> > figuring out what the "correct" way to make it optional is. I'm not too
> worried
> > right now since Ozone doesn't need to link against libpci, so it can
> gracefully
> > return early if it determines there's no PCI support.
> 
> I might be able to help out with that in a later CL if you give me some
details
> on what configurations you want Ozone to support.

Thank you! So far only with and without libpci support, hence why I'm not
worried.

[Zhenyao Mo, 2014-08-12 22:11:26]

I am not gpu/ owner. LGTM anyway.

[Mostyn Bramley-Moore, 2014-08-12 22:16:22]

Ah, right.  Adding sievers for gpu/ approval.

[piman, 2014-08-13 05:44:09]

lgtm

[Mostyn Bramley-Moore, 2014-08-13 07:29:46]

The CQ bit was checked by mostynb@opera.com

[commit-bot: I haz the power, 2014-08-13 07:30:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mostynb@opera.com/462433002/120001

[commit-bot: I haz the power, 2014-08-13 10:35:43]

Change committed as 289262