Add src/printing to the GN build.

Add src/printing to the GN build.

BUG=
R=thestig@chromium.org

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

[Lei Zhang, 2014-06-02 20:59:32]

https://codereview.chromium.org/311603003/diff/1/build/config/linux/BUILD.gn
File build/config/linux/BUILD.gn (right):

https://codereview.chromium.org/311603003/diff/1/build/config/linux/BUILD.gn#...
build/config/linux/BUILD.gn:183: # Note: in GYP this runs libgcrypt-config with
some horrible sed stuff. This
In http://crrev.com/271563, I removed the libgcrypt dependency, but forgot to
remove this. Since we no longer depend on libgcrypt, you can omit this. I'll go
ahead and delete it from the gyp version in a separate CL.

[brettw, 2014-06-02 22:32:39]

On 2014/06/02 20:59:32, Lei Zhang wrote:
> https://codereview.chromium.org/311603003/diff/1/build/config/linux/BUILD.gn
> File build/config/linux/BUILD.gn (right):
> 
>
https://codereview.chromium.org/311603003/diff/1/build/config/linux/BUILD.gn#...
> build/config/linux/BUILD.gn:183: # Note: in GYP this runs libgcrypt-config
with
> some horrible sed stuff. This
> In http://crrev.com/271563, I removed the libgcrypt dependency, but forgot to
> remove this. Since we no longer depend on libgcrypt, you can omit this. I'll
go
> ahead and delete it from the gyp version in a separate CL.

Thanks a lot! I was particularly confused by the using_cups condition in
liggcrypt

[Lei Zhang, 2014-06-02 22:37:02]

https://codereview.chromium.org/311603003/diff/1/printing/cups_config_helper.py
File printing/cups_config_helper.py (right):

https://codereview.chromium.org/311603003/diff/1/printing/cups_config_helper....
printing/cups_config_helper.py:15:
https://bugs.launchpad.net/ubuntu/+source/cupsys/+bug/163704
I just noticed this too. On my Linux machine, cups-config --libs outputs:

-lcups -L/usr/lib/x86_64-linux-gnu -Wl,-Bsymbolic-functions -Wl,-z,relro
-lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lgnutls -L/lib/x86_64-linux-gnu
-lgcrypt -lz -lpthread -lcrypt -lm

which looks fairly sane, so maybe we can get rid of this file altogether as
well? Would that make the enabling GN for printing/ easier?

[brettw, 2014-06-02 22:53:25]

https://codereview.chromium.org/311603003/diff/1/printing/cups_config_helper.py
File printing/cups_config_helper.py (right):

https://codereview.chromium.org/311603003/diff/1/printing/cups_config_helper....
printing/cups_config_helper.py:15:
https://bugs.launchpad.net/ubuntu/+source/cupsys/+bug/163704
I actually need this for GN to get the libs the way I want (unless we want to
hardcode the list of dependencies, which I'm also OK doing). I need to strip the
"-l"s and format as a GN list.

[Lei Zhang, 2014-06-02 23:33:27]

https://codereview.chromium.org/311603003/diff/1/build/config/linux/BUILD.gn
File build/config/linux/BUILD.gn (right):

https://codereview.chromium.org/311603003/diff/1/build/config/linux/BUILD.gn#...
build/config/linux/BUILD.gn:183: # Note: in GYP this runs libgcrypt-config with
some horrible sed stuff. This
Please remove this.

https://codereview.chromium.org/311603003/diff/1/printing/cups_config_helper.py
File printing/cups_config_helper.py (right):

https://codereview.chromium.org/311603003/diff/1/printing/cups_config_helper....
printing/cups_config_helper.py:15:
https://bugs.launchpad.net/ubuntu/+source/cupsys/+bug/163704
On 2014/06/02 22:53:26, brettw wrote:
> I actually need this for GN to get the libs the way I want (unless we want to
> hardcode the list of dependencies, which I'm also OK doing). I need to strip
the
> "-l"s and format as a GN list.

Ok, I'll make a pass through this file later and clean it up a bit.

https://codereview.chromium.org/311603003/diff/1/printing/cups_config_helper....
printing/cups_config_helper.py:83: print '['
We should sanity check and make sure |lib| starts with "-l".
run_cups_config('--libs') doesn't guarantee all the returned values in |flags|
start with '-l'.

Otherwise, this block could have been shortened to:

print '[\n%s\n]' % '\n'.join('"%s"' % lib[2:] for lib in flags)

[brettw, 2014-06-03 23:36:35]

I forgot to actually include the new file in the patch. Do you want to look at
printing/BUILD.gn?

[Lei Zhang, 2014-06-03 23:48:29]

lgtm assuming the |use_cups| variable is equivalent to the same conditional in
gyp.

https://codereview.chromium.org/311603003/diff/20001/printing/BUILD.gn
File printing/BUILD.gn (right):

https://codereview.chromium.org/311603003/diff/20001/printing/BUILD.gn#newcode13
printing/BUILD.gn:13: use_cups = (is_linux || is_mac)
In GN, is there an equivalent to build/common.gypi where this is all defined?

As is, will |use_cups| be true for CrOS? It evaluates to 0 in gyp for CrOS.

https://codereview.chromium.org/311603003/diff/20001/printing/BUILD.gn#newcod...
printing/BUILD.gn:149: if (mac_sdk_version == "10.9") {
can this be combined with the previous line?

https://codereview.chromium.org/311603003/diff/20001/printing/cups_config_hel...
File printing/cups_config_helper.py (right):

https://codereview.chromium.org/311603003/diff/20001/printing/cups_config_hel...
printing/cups_config_helper.py:14: TODO(evan): remove me once
Not that Evan will remove this any time soon, but you should probably update the
comments here so this file doesn't accidentally get removed.

[brettw, 2014-06-04 17:09:38]

https://codereview.chromium.org/311603003/diff/20001/printing/BUILD.gn
File printing/BUILD.gn (right):

https://codereview.chromium.org/311603003/diff/20001/printing/BUILD.gn#newcode13
printing/BUILD.gn:13: use_cups = (is_linux || is_mac)
On 2014/06/03 23:48:28, Lei Zhang wrote:
> In GN, is there an equivalent to build/common.gypi where this is all defined?

You can do that kind of thing, but this is the only place in the build this is
necessary and we shouldn't make globals unless necessary.

> As is, will |use_cups| be true for CrOS? It evaluates to 0 in gyp for CrOS.

Nope, I fixed this.

[Lei Zhang, 2014-06-04 17:37:08]

lgtm

[brettw, 2014-06-04 18:09:50]

Committed patchset #3 manually as r274865 (presubmit successful).