Snapshoting improvements for building with gyp under windows....

Snapshoting improvements for building with gyp under windows.
Added google_update and install utils.


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

[bradn, 2009-03-05 18:51:19]

More of chrome building under windows.
Still missing stuff at the link.

[Mark Mentovai, 2009-03-05 19:34:43]

First pass of comments, this mostly looks good but I'm concerned about
cross-platform-ism.  You can feed it to a try server to make sure that it's OK
on the Mac.

http://codereview.chromium.org/39204/diff/1/4
File chrome/chrome.gyp (right):

http://codereview.chromium.org/39204/diff/1/4#newcode86
Line 86: '../third_party/zlib/zlib.gyp:zlib',
Ah, we always mess this up on the Mac because if we don't declare it we can
still get the system-provided zlib.h.

Do we need this directly in common, or did we miss a direct_dependent_settings
somewhere?

http://codereview.chromium.org/39204/diff/1/4#newcode541
Line 541: 'browser/history/history_indexer.idl',
I don't see an .idl rule, do we need one?  What's the meaning of this without
one?

http://codereview.chromium.org/39204/diff/1/4#newcode812
Line 812: 'browser/views/frame/glass_browser_frame_view.cc',
sort

http://codereview.chromium.org/39204/diff/1/4#newcode1247
Line 1247: 'target_name': 'views',
views certainly isn't built on the Mac.  Is it built on Linux?  We should put
the whole lib in a conditional or something.

http://codereview.chromium.org/39204/diff/1/4#newcode1421
Line 1421: 'target_name': 'plugin',
Same for plugin, we don't build this on the Mac yet, do we build it on Linux?

http://codereview.chromium.org/39204/diff/1/2
File chrome/installer/util/prebuild/util_prebuild.gyp (right):

http://codereview.chromium.org/39204/diff/1/2#newcode1
Line 1: {
Shouldn't we just have chrome/installer/installer.gyp with multiple targets?  Or
even just put the targets into chrome.gyp?

[bradn, 2009-03-06 01:36:59]

I've had some trouble on the try server (and it was down last night).
I won't submit until it succeeds.

http://codereview.chromium.org/39204/diff/1/4
File chrome/chrome.gyp (right):

http://codereview.chromium.org/39204/diff/1/4#newcode86
Line 86: '../third_party/zlib/zlib.gyp:zlib',
On 2009/03/05 19:34:44, Mark Mentovai wrote:
> Ah, we always mess this up on the Mac because if we don't declare it we can
> still get the system-provided zlib.h.
> 
> Do we need this directly in common, or did we miss a direct_dependent_settings
> somewhere?

This is directly in common. I've added some comments.

http://codereview.chromium.org/39204/diff/1/4#newcode541
Line 541: 'browser/history/history_indexer.idl',
On 2009/03/05 19:34:44, Mark Mentovai wrote:
> I don't see an .idl rule, do we need one?  What's the meaning of this without
> one?

I was relying on the built-in midl rule in vstudio. I've moved this into a
condition.

http://codereview.chromium.org/39204/diff/1/4#newcode812
Line 812: 'browser/views/frame/glass_browser_frame_view.cc',
On 2009/03/05 19:34:44, Mark Mentovai wrote:
> sort

Done.

http://codereview.chromium.org/39204/diff/1/4#newcode1247
Line 1247: 'target_name': 'views',
On 2009/03/05 19:34:44, Mark Mentovai wrote:
> views certainly isn't built on the Mac.  Is it built on Linux?  We should put
> the whole lib in a conditional or something.

views seems to be windows only I've moved it into a conditional.

http://codereview.chromium.org/39204/diff/1/4#newcode1421
Line 1421: 'target_name': 'plugin',
On 2009/03/05 19:34:44, Mark Mentovai wrote:
> Same for plugin, we don't build this on the Mac yet, do we build it on Linux?
plugin appears to be built on linux, but I'll let sgk turn it on when he's
ready.
I've also moved it to a conditional.

http://codereview.chromium.org/39204/diff/1/2
File chrome/installer/util/prebuild/util_prebuild.gyp (right):

http://codereview.chromium.org/39204/diff/1/2#newcode1
Line 1: {
On 2009/03/05 19:34:44, Mark Mentovai wrote:
> Shouldn't we just have chrome/installer/installer.gyp with multiple targets? 
Or
> even just put the targets into chrome.gyp?

I kind of like the gyp files spread out in general and more chopped up. But if
you feel strongly we could move these around. This one in particular, however,
is a bit of a pain. The batch file + python makes relative path assumptions. I'd
rather not muck with it right now (since it's in active use as is.) I'd be ok
with moving the util one up into installer.gyp if you think that's best. I
haven't gotten to installer itself yet, since it is separable. Thoughts?

[Mark Mentovai, 2009-03-06 17:36:10]

Seems mostly fine now, just a couple of comments.  LGTM with these changes if it
passes a Mac tryserver.

We can move the stuff around between files later, I agree that the ones you
chose to break out from chrome.gyp don't strictly need to be in chrome.gyp, and
we've got time to think about how things are laid out and change our minds.

http://codereview.chromium.org/39204/diff/1003/14
File chrome/chrome.gyp (right):

http://codereview.chromium.org/39204/diff/1003/14#newcode86
Line 86: '../third_party/zlib/zlib.gyp:zlib',  # Used directly by unzip below.
Comment wasn't actually necessary (although it doesn't hurt anything), I was
just reflecting on why I always seemed to omit this one.

http://codereview.chromium.org/39204/diff/1003/14#newcode1165
Line 1165: ['OS!="win"', {
This can become an "else" clause of the above condition now.

http://codereview.chromium.org/39204/diff/1003/14#newcode1426
Line 1426: ['OS!="win"', {
me too

http://codereview.chromium.org/39204/diff/1003/14#newcode1631
Line 1631: 'sources!': [
Since we really want to exclude everything other than
test/unit/run_all_unittests.cc, maybe sources/ would be better.

[bradn, 2009-03-06 23:02:11]

http://codereview.chromium.org/39204/diff/1003/14
File chrome/chrome.gyp (right):

http://codereview.chromium.org/39204/diff/1003/14#newcode86
Line 86: '../third_party/zlib/zlib.gyp:zlib',  # Used directly by unzip below.
On 2009/03/06 17:36:10, Mark Mentovai wrote:
> Comment wasn't actually necessary (although it doesn't hurt anything), I was
> just reflecting on why I always seemed to omit this one.

Ok I'll drop it since it is more clutter.

http://codereview.chromium.org/39204/diff/1003/14#newcode1165
Line 1165: ['OS!="win"', {
On 2009/03/06 17:36:10, Mark Mentovai wrote:
> This can become an "else" clause of the above condition now.

Done.

http://codereview.chromium.org/39204/diff/1003/14#newcode1426
Line 1426: ['OS!="win"', {
On 2009/03/06 17:36:10, Mark Mentovai wrote:
> me too

Done.

http://codereview.chromium.org/39204/diff/1003/14#newcode1631
Line 1631: 'sources!': [
On 2009/03/06 17:36:10, Mark Mentovai wrote:
> Since we really want to exclude everything other than
> test/unit/run_all_unittests.cc, maybe sources/ would be better.

Done.