Adding use_system_libusb option for third_party/libusb

Adding use_system_libusb option for third_party/libusb

There are build and distribution configurations under which packaged libraries
cannot be included. The use_system_libusb option allows these platforms (Linux,
currently) to use the pre-installed libusb.

BUG=124803
TEST=none


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

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

[sadrul, 2012-05-09 19:14:18]

Rubber-stamp LGTM

[commit-bot: I haz the power, 2012-05-09 19:16:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gdk@chromium.org/10332075/1

[commit-bot: I haz the power, 2012-05-09 21:17:14]

Change committed as 136127

[Lei Zhang, 2012-05-09 23:31:15]

check_licenses.py change LGTM.

[Paweł Hajdan Jr., 2012-05-10 06:36:14]

Drive-by (nice to see this done; the patch looks really good, just some minor
comments).

LGTM

https://chromiumcodereview.appspot.com/10332075/diff/10001/chrome/browser/usb...
File chrome/browser/usb/usb_service.cc (right):

https://chromiumcodereview.appspot.com/10332075/diff/10001/chrome/browser/usb...
chrome/browser/usb/usb_service.cc:78: // TODO(gdk): Once there is a reasonable
expectation that platforms will use
Note: there is. Gentoo Linux doesn't even have any older libusb.

Depending on whether libusb_handle_events is broken with 1.0.9, or
libusb_handle_events_completed is not present in older versions, the more
compatible one should be kept (don't worry about the system lib on Ubuntu; if
your bundled libusb is 1.0.9, feel free to use 1.0.9 call).

https://chromiumcodereview.appspot.com/10332075/diff/10001/third_party/libusb...
File third_party/libusb/libusb.gyp (right):

https://chromiumcodereview.appspot.com/10332075/diff/10001/third_party/libusb...
third_party/libusb/libusb.gyp:8: 'required_libusb_version': '1.0.0',
nit: This is not so much needed, because the latest version is 1.0.9 now, and
using pkg-config file for libusb-1.0 will do the right thing.

Feel free to keep it if you want, I rather prefer removing it (up to you).

[Garret Kelly, 2012-05-10 17:44:01]

http://codereview.chromium.org/10332075/diff/10001/chrome/browser/usb/usb_ser...
File chrome/browser/usb/usb_service.cc (right):

http://codereview.chromium.org/10332075/diff/10001/chrome/browser/usb/usb_ser...
chrome/browser/usb/usb_service.cc:78: // TODO(gdk): Once there is a reasonable
expectation that platforms will use
I'll leave the older call for compatibility's sake, it's not going to be removed
from the libusb API at any point.

http://codereview.chromium.org/10332075/diff/10001/third_party/libusb/libusb.gyp
File third_party/libusb/libusb.gyp (right):

http://codereview.chromium.org/10332075/diff/10001/third_party/libusb/libusb....
third_party/libusb/libusb.gyp:8: 'required_libusb_version': '1.0.0',
On 2012/05/10 06:36:15, Paweł Hajdan Jr. wrote:
> nit: This is not so much needed, because the latest version is 1.0.9 now, and
> using pkg-config file for libusb-1.0 will do the right thing.
> 
> Feel free to keep it if you want, I rather prefer removing it (up to you).

Done.

[commit-bot: I haz the power, 2012-05-10 17:46:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gdk@chromium.org/10332075/11110

[commit-bot: I haz the power, 2012-05-10 19:38:50]

Try job failure for 10332075-11110 (retry) (retry) on linux_rel for step
"browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-05-10 19:44:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gdk@chromium.org/10332075/11110

[commit-bot: I haz the power, 2012-05-10 20:08:27]

Try job failure for 10332075-11110 (retry) on android for steps "Compile,
build".
It's a second try, previously, steps "Compile, build" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android&nu...

[commit-bot: I haz the power, 2012-05-10 20:18:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gdk@chromium.org/10332075/11110

[commit-bot: I haz the power, 2012-05-10 22:52:59]

Change committed as 136421

[Paweł Hajdan Jr., 2012-05-11 10:06:05]

Garret, thank you for landing the patch. It seems that there is just last bit of
work remaining to do. Gentoo users are reporting problems, see
https://bugs.gentoo.org/show_bug.cgi?id=413251#c4

Looking at chromium/src/chrome/chrome_browser_extensions.gypi, I think it needs
a dependency on libusb.gyp, so that USE_SYSTEM_LIBUSB symbol is defined there.

You can test for those cases by temporarily removing the bundled headers from
local checkout and building with -Duse_system_libusb=1 . Gentoo removes bundled
headers to ensure they are not used, and detect cases like this, which would
otherwise result in silent headers mix-up.

Please add the missing dependency on libusb.gyp in
chromium/src/chrome/chrome_browser_extensions.gypi and any other places where
it's missing (if any). Thanks!

[Garret Kelly, 2012-05-11 15:32:46]

Hi Paweł,

> Garret, thank you for landing the patch. It seems that there is just last bit
of
> work remaining to do. Gentoo users are reporting problems, see
> https://bugs.gentoo.org/show_bug.cgi?id=413251#c4
> 
> Looking at chromium/src/chrome/chrome_browser_extensions.gypi, I think it
needs
> a dependency on libusb.gyp, so that USE_SYSTEM_LIBUSB symbol is defined there.
> 
> You can test for those cases by temporarily removing the bundled headers from
> local checkout and building with -Duse_system_libusb=1 . Gentoo removes
bundled
> headers to ensure they are not used, and detect cases like this, which would
> otherwise result in silent headers mix-up.
> 
> Please add the missing dependency on libusb.gyp in
> chromium/src/chrome/chrome_browser_extensions.gypi and any other places where
> it's missing (if any). Thanks!

Ah, I hadn't even thought to do that. Thanks for pointing this out. I'm fixing
it right now.

Garret