Linux: move use_system_harfbuzz logic out of the main file.

Linux: move use_system_harfbuzz logic out of the main file.

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

[Ryan Sleevi, 2013-04-05 18:15:55]

Rather than using 'system' as the directory name, could you use something like
dist? Or some other name?

My goal with avoiding 'system' is that 'system.gyp' is part of the main Chromium
workflow, and so it may be confusing to have system/ there.

I'm not terribly picky about the name (or maybe I am and don't realize it), but
mostly I'm looking to find something that may be able to broadcast "here's
something you don't have to update every time".

Perhaps a README and an OWNERS to go with your new directory, if expressing this
via naming gets complicated?

[Paweł Hajdan Jr., 2013-04-05 21:06:58]

PTAL

I think this will not encourage most people to even look at it.

IMO README or OWNERS would be an overkill here.

[Ryan Sleevi, 2013-04-05 21:13:50]

I think a one or two sentence README would be vastly helpful.

From the point of source archeology, think about what it means a year down the
road, where the changes to these .gyp files are less about extracting them (like
this commit, which cleanly links to the bug) and more about making sure they're
compliant.

Neither your bug description nor your commit description provide any clear
context for why this directory is being created or the stuff we talked about,
that's why I think it's important to be clearer.

I haven't reviewed the Python script, but otherwise LGTM.

[Paweł Hajdan Jr., 2013-04-05 22:18:17]

Darin, could you do third_party OWNERS review?

This is an implementation I came up with after our last discussion. I'd prefer
to do it this way - if you have any concerns, let's discuss this offline if
possible.

[darin (slow to review), 2013-04-08 20:25:42]

This is an interesting approach.  Another idea might be to do this at the DEPS
level.  If we could optionally DEPS in a version of third_party/harfbuzz that
just uses a system library, then the only variance would be in the .gclient
file.  Did you consider such an approach?

Perhaps one downside of this is that it would cause all third_party modules that
you wish to replace at buildtime to be imported using DEPS.

[Paweł Hajdan Jr., 2013-04-08 20:36:49]

On 2013/04/08 20:25:42, darin wrote:
> This is an interesting approach.  Another idea might be to do this at the DEPS
> level.  If we could optionally DEPS in a version of third_party/harfbuzz that
> just uses a system library, then the only variance would be in the .gclient
> file.  Did you consider such an approach?

I did not think about gclient/DEPS based approach, but it would not really work
- because the decision is made at tarball generation time (users of this
mechanism should not need to even know about gclient - while we're very used to
it, for average distro developer tarball is the way to obtain sources, not a
direct source control checkout). Not all users of the tarball necessarily want
this, and then different users may want different options (which also provides a
trivial workaround in case some of them are broken for some time).

> Perhaps one downside of this is that it would cause all third_party modules
that
> you wish to replace at buildtime to be imported using DEPS.

Right - in fact, harfbuzz-ng which is done in this CL would be affected, since
it's not fetched via DEPS.

I think the current approach is fine for the use case I have in mind - is it
also fine for you?

[darin (slow to review), 2013-04-22 19:11:33]

LGTM

I really like how this keeps the main harfbuzz gyp file clean.

https://codereview.chromium.org/13732002/diff/10001/build/linux/unbundle/README
File build/linux/unbundle/README (right):

https://codereview.chromium.org/13732002/diff/10001/build/linux/unbundle/READ...
build/linux/unbundle/README:16: libraries is the norm.
This is a good description of "why" this code exists, but it probably would be
nice to include a "how" section, so it is clear how one might actually use this
module.  You might consider adding some sample command lines that show how to
use the python script, etc.?

[Paweł Hajdan Jr., 2013-04-22 21:06:47]

Committed patchset #4 manually as r195590 (presubmit successful).

[Paweł Hajdan Jr., 2013-04-22 21:07:18]

https://codereview.chromium.org/13732002/diff/10001/build/linux/unbundle/README
File build/linux/unbundle/README (right):

https://codereview.chromium.org/13732002/diff/10001/build/linux/unbundle/READ...
build/linux/unbundle/README:16: libraries is the norm.
On 2013/04/22 19:11:33, darin wrote:
> This is a good description of "why" this code exists, but it probably would be
> nice to include a "how" section, so it is clear how one might actually use
this
> module.  You might consider adding some sample command lines that show how to
> use the python script, etc.?

Good point. Done. Thanks!

[sL1pKn07, 2013-05-01 05:26:40]

one question, if for example don't build with certain library, for exaple, in
archlinux don't build if use system re2 (not in repositories) in  or libwebp
(without VP9 support)...

how to use then? 

like this?
build/linux/unbundle/replace_gyp_files.py -Duse_system_libwebp=0

greetings

[sL1pKn07, 2013-05-01 05:39:47]

> archlinux don't build if use system re2 (not in repositories) in  or libwebp
> (without VP9 support)...

sorry, bad example libwebp, no need vp9 xddd, but r2 is a good exaple

chromium use r2 version 2012-06-20, and in AUR (user repository, like ubuntu
ppa) upload 2013-01-15, fail build with this