Linux: make cups (common unix printing system) dependency optional.

Linux: make cups (common unix printing system) dependency optional.

This is upstreaming a Gentoo Linux patch.

TEST=none
BUG=none

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

[Mark Mentovai, 2010-06-18 14:36:25]

I don’t know what this has to do with target_arch, but that was already there.

LG.

[Paweł Hajdan Jr., 2010-06-21 06:29:10]

+maruel (it's printing-related, Linux)

sanjeevr/scottbyer: ping

I'd like to get an "LGTM" from at least one person more familiar with our
printing code. It's not obvious whether making dependency on cups optional won't
interfere with your plans.

[M-A Ruel, 2010-06-21 13:54:06]

Is it only needed for chrome for chromeos?

[sanjeevr, 2010-06-21 17:40:27]

Sorry for the delay, this slipped under my radar. Can I get a little more
context? I don't see any issues with the specific change but what scenarios are
you making this change for?

http://codereview.chromium.org/2858014/diff/1/2
File chrome/chrome.gyp (right):

http://codereview.chromium.org/2858014/diff/1/2#newcode1119
chrome/chrome.gyp:1119: ['use_cups==1 and OS=="linux" and chromeos==0 and
target_arch!="arm"', {
Perhaps it might make more sense to set use_cups to 1 only if the other
conditions are true. SO then you can only use "use_cups == 1" here.

[Paweł Hajdan Jr., 2010-06-21 17:49:10]

On Mon, Jun 21, 2010 at 19:40, <sanjeevr@chromium.org> wrote:

> Sorry for the delay, this slipped under my radar. Can I get a little more
> context? I don't see any issues with the specific change but what scenarios
> are
> you making this change for?
>

Gentoo Linux users have complained about cups being pulled in as a
dependency.


> http://codereview.chromium.org/2858014/diff/1/2
> File chrome/chrome.gyp (right):
>
> http://codereview.chromium.org/2858014/diff/1/2#newcode1119
> chrome/chrome.gyp:1119: ['use_cups==1 and OS=="linux" and chromeos==0
> and target_arch!="arm"', {
> Perhaps it might make more sense to set use_cups to 1 only if the other
> conditions are true. SO then you can only use "use_cups == 1" here.


Sure, I can do that. I'm waiting for more comments to see whether you're
generally going to accept the change. I'm happy with either way.

If it turns out to only be needed for ChromeOS build, we just wouldn't need
the variable at all. What do you think?


>  http://codereview.chromium.org/2858014/show
>

[sanjeevr, 2010-06-21 18:07:59]

So the idea is to use CUPS as the print system for the Linux cloud print proxy
where CUPS is available. For ChromeOS, the entire cloud print proxy makes no
sense and we need to exclude the entire proxy code (not just the CUPS part). 

Similarly on any Linux system without CUPS, there is no point in including any
of the cloud print proxy code at all. That is a todo for us (to exclude the
Cloud Print Proxy code is no CUPS is available).

To summarize, I am OK with this change (if you could make that one tweak to the
gyp file I had mentioned in my previous comment).

On 2010/06/21 17:49:10, Paweł Hajdan Jr. wrote:
> On Mon, Jun 21, 2010 at 19:40,
<https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=sanjeevr@chromium.org>
wrote:
> 
> > Sorry for the delay, this slipped under my radar. Can I get a little more
> > context? I don't see any issues with the specific change but what scenarios
> > are
> > you making this change for?
> >
> 
> Gentoo Linux users have complained about cups being pulled in as a
> dependency.
> 
> 
> > http://codereview.chromium.org/2858014/diff/1/2
> > File chrome/chrome.gyp (right):
> >
> > http://codereview.chromium.org/2858014/diff/1/2#newcode1119
> > chrome/chrome.gyp:1119: ['use_cups==1 and OS=="linux" and chromeos==0
> > and target_arch!="arm"', {
> > Perhaps it might make more sense to set use_cups to 1 only if the other
> > conditions are true. SO then you can only use "use_cups == 1" here.
> 
> 
> Sure, I can do that. I'm waiting for more comments to see whether you're
> generally going to accept the change. I'm happy with either way.
> 
> If it turns out to only be needed for ChromeOS build, we just wouldn't need
> the variable at all. What do you think?
> 
> 
> >  http://codereview.chromium.org/2858014/show
> >
>

[Paweł Hajdan Jr., 2010-06-21 19:48:51]

Patch updated, please take another look.

[sanjeevr, 2010-06-21 23:42:16]

LGTM