gyp: generate "everything" and "most" targets instead of "all"

gyp: generate "everything" and "most" targets instead of "all"
"make all" at the toplevel now chains to "make everything"

BUG=http://code.google.com/p/skia/issues/detail?id=932
Committed: https://code.google.com/p/skia/source/detail?r=6116

[epoger, 2012-10-12 18:51:13]

At patchset 2, I have confirmed that this works properly on my Mac.  The
toplevel "make" commands listed in
https://sites.google.com/site/skiadocs/user-documentation/quick-start-guides/mac
work exactly as before: "make gm", "make bench", "make all", "make clean", etc.

I believe it will work on Linux, but I haven't tried yet.

I know this will require some changes to make.py to keep the Windows build
working, but I would like to get the CL reviewed as it stands now and go from
there.

[EricB, 2012-10-12 19:00:07]

+djsollen@

https://codereview.appspot.com/6651064/diff/2001/skia.gyp
File skia.gyp (right):

https://codereview.appspot.com/6651064/diff/2001/skia.gyp#newcode14
skia.gyp:14: 'target_name': 'most',
Will we get in trouble for having multiple targets with the same name?

[epoger, 2012-10-12 19:06:27]

https://codereview.appspot.com/6651064/diff/2001/skia.gyp
File skia.gyp (right):

https://codereview.appspot.com/6651064/diff/2001/skia.gyp#newcode14
skia.gyp:14: 'target_name': 'most',
On 2012/10/12 19:00:07, EricB wrote:
> Will we get in trouble for having multiple targets with the same name?

Do you mean the appearance of "most" targets, both here and in gyp/most.gyp? 
(And same for "everything")

I think we could avoid that, and still keep full functionality, by changing the
guts of this file to read:

  'targets': [
    {
      'target_name': 'alltargets',
      'type': 'none',
      'dependencies': [
        'gyp/everything.gyp:everything',
        'gyp/most.gyp:most',
      ],
    },
  ],

[EricB, 2012-10-12 19:10:43]

https://codereview.appspot.com/6651064/diff/2001/skia.gyp
File skia.gyp (right):

https://codereview.appspot.com/6651064/diff/2001/skia.gyp#newcode14
skia.gyp:14: 'target_name': 'most',
On 2012/10/12 19:06:28, epoger wrote:
> On 2012/10/12 19:00:07, EricB wrote:
> > Will we get in trouble for having multiple targets with the same name?
> 
> Do you mean the appearance of "most" targets, both here and in gyp/most.gyp? 
> (And same for "everything")
> 
> I think we could avoid that, and still keep full functionality, by changing
the
> guts of this file to read:
> 
>   'targets': [
>     {
>       'target_name': 'alltargets',
>       'type': 'none',
>       'dependencies': [
>         'gyp/everything.gyp:everything',
>         'gyp/most.gyp:most',
>       ],
>     },
>   ],
> 

I think that's better.  If I recall correctly, the catalyst of this whole
discussion was the fact that ninja was complaining about multiple "all" targets,
although maybe that only applies to targets in the same directory?

[tfarina1, 2012-10-12 19:18:51]

https://codereview.appspot.com/6651064/diff/2001/gyp/everything.gyp
File gyp/everything.gyp (right):

https://codereview.appspot.com/6651064/diff/2001/gyp/everything.gyp#newcode13
gyp/everything.gyp:13: 'target_name': 'everything',
is this needed? if you want to build the debugger can't you just say:

ninja -C out/Debug debugger?

[tfarina1, 2012-10-12 19:24:35]

The problem I see here is that skia is fragmented in a bunch of small
static_libraries (animator, core, effects, freetype, gpu, opts, images, ports,
utils, views, etc...)

What we need something like:

{
  'target_name': 'All',                                                      
  'type': 'none',                                                            
  'dependencies': [                                                          
    animator.gyp:animator,
    views.gyp:views,
    core.gyp:core,
    etc,
  ],
},

[tfarina1, 2012-10-12 19:26:25]

moving myself to the CC list again.

[epoger, 2012-10-12 19:26:34]

https://codereview.appspot.com/6651064/diff/2001/gyp/everything.gyp
File gyp/everything.gyp (right):

https://codereview.appspot.com/6651064/diff/2001/gyp/everything.gyp#newcode13
gyp/everything.gyp:13: 'target_name': 'everything',
On 2012/10/12 19:18:51, tfarina1 wrote:
> is this needed? if you want to build the debugger can't you just say:
> 
> ninja -C out/Debug debugger?

Something like that.  But again, I'm trying to keep it where you can just run
"make all" from trunk and BOOM it builds everything.

https://codereview.appspot.com/6651064/diff/2001/skia.gyp
File skia.gyp (right):

https://codereview.appspot.com/6651064/diff/2001/skia.gyp#newcode14
skia.gyp:14: 'target_name': 'most',
On 2012/10/12 19:10:44, EricB wrote:
> On 2012/10/12 19:06:28, epoger wrote:
> > On 2012/10/12 19:00:07, EricB wrote:
> > > Will we get in trouble for having multiple targets with the same name?
> > 
> > Do you mean the appearance of "most" targets, both here and in gyp/most.gyp?

> > (And same for "everything")
> > 
> > I think we could avoid that, and still keep full functionality, by changing
> the
> > guts of this file to read:
> > 
> >   'targets': [
> >     {
> >       'target_name': 'alltargets',
> >       'type': 'none',
> >       'dependencies': [
> >         'gyp/everything.gyp:everything',
> >         'gyp/most.gyp:most',
> >       ],
> >     },
> >   ],
> > 
> 
> I think that's better.  If I recall correctly, the catalyst of this whole
> discussion was the fact that ninja was complaining about multiple "all"
targets,
> although maybe that only applies to targets in the same directory?

I made the change in patchset 3, and it seems to work fine.

[tfarina1, 2012-10-12 19:33:28]

https://codereview.appspot.com/6651064/diff/2001/gyp/most.gyp
File gyp/most.gyp (right):

https://codereview.appspot.com/6651064/diff/2001/gyp/most.gyp#newcode30
gyp/most.gyp:30: # Local Variables:
Unrelated to this patch, but still...

Can we removed this in a follow up? See the following patches for a background.

http://codereview.chromium.org/7310019
http://codereview.chromium.org/7300011

[EricB, 2012-10-12 19:34:56]

This is looking good at patch set 3.  I don't like that we have to use names
like "everything" and "most" in addition to "all," but I guess it's necessary.

[epoger, 2012-10-18 05:03:48]

Ready for review at patchset 5!

In patchset 4, I updated my local source tree, resolving conflicts that came
from https://code.google.com/p/skia/source/detail?r=5936 .

In patchset 5, I added changes to make.py to keep the Windows toplevel "make"
build in sync with the other platforms.

As of patchset 5, I have confirmed that "make", "make all", "make most", "make
clean" all behave as expected on Linux, Mac, and Windows.

NOTE: With this change, "make all" actually attempts to build everything!  "make
all" fails on my Windows desktop, because I don't have QT installed... so some
users who are used to "make all" succeeding may now see it fail.  I think this
means that the BuildAllOtherTargets step will start failing on some of our
buildbots... we may want to replace "make all" in that step with "make most" (or
just "make", which has the same effect), or else install QT on all the
buildbots...

[EricB, 2012-10-18 12:02:20]

On 2012/10/18 05:03:48, epoger wrote:
> Ready for review at patchset 5!
> 
> In patchset 4, I updated my local source tree, resolving conflicts that came
> from https://code.google.com/p/skia/source/detail?r=5936 .
> 
> In patchset 5, I added changes to make.py to keep the Windows toplevel "make"
> build in sync with the other platforms.
> 
> As of patchset 5, I have confirmed that "make", "make all", "make most", "make
> clean" all behave as expected on Linux, Mac, and Windows.
> 
> NOTE: With this change, "make all" actually attempts to build everything! 
"make
> all" fails on my Windows desktop, because I don't have QT installed... so some
> users who are used to "make all" succeeding may now see it fail.  I think this
> means that the BuildAllOtherTargets step will start failing on some of our
> buildbots... we may want to replace "make all" in that step with "make most"
(or
> just "make", which has the same effect), or else install QT on all the
> buildbots...

LGTM.  FWIW, since we're adding "most" and "everything", I think we should be
building them on the bots, if maybe not at first.  That would include the
debugger, which we really should be building.  I guess that means we need to
install QT on the bots, *or* we have one bot (on each platform?) which builds
the debugger so that not every machine needs QT.  Here's my proposed plan:

1. Write up a quick change to run "make most" instead of "make all" on the bots.

2. Commit both changes at the "same time" and restart the master to pick them
up.

3. Decide which machines should have QT installed, install it, and add "make
everything" to some set of bots.

[epoger, 2012-10-18 13:26:47]

On 2012/10/18 12:02:20, EricB wrote:
> On 2012/10/18 05:03:48, epoger wrote:
> > Ready for review at patchset 5!
> > 
> > In patchset 4, I updated my local source tree, resolving conflicts that came
> > from https://code.google.com/p/skia/source/detail?r=5936 .
> > 
> > In patchset 5, I added changes to make.py to keep the Windows toplevel
"make"
> > build in sync with the other platforms.
> > 
> > As of patchset 5, I have confirmed that "make", "make all", "make most",
"make
> > clean" all behave as expected on Linux, Mac, and Windows.
> > 
> > NOTE: With this change, "make all" actually attempts to build everything! 
> "make
> > all" fails on my Windows desktop, because I don't have QT installed... so
some
> > users who are used to "make all" succeeding may now see it fail.  I think
this
> > means that the BuildAllOtherTargets step will start failing on some of our
> > buildbots... we may want to replace "make all" in that step with "make most"
> (or
> > just "make", which has the same effect), or else install QT on all the
> > buildbots...
> 
> LGTM.  FWIW, since we're adding "most" and "everything", I think we should be
> building them on the bots, if maybe not at first.  That would include the
> debugger, which we really should be building.  I guess that means we need to
> install QT on the bots, *or* we have one bot (on each platform?) which builds
> the debugger so that not every machine needs QT.  Here's my proposed plan:
> 
> 1. Write up a quick change to run "make most" instead of "make all" on the
bots.
> 
> 2. Commit both changes at the "same time" and restart the master to pick them
> up.
> 
> 3. Decide which machines should have QT installed, install it, and add "make
> everything" to some set of bots.

Sounds good.  I'll actually stretch it out a tiny bit more, just to avoid race
conditions... I will knock out steps 1-3 as quickly as possible; #4 will happen
"some other time".  OK?

1. Write up CL #2 that adds a "most" target to the build, and commit it.

2. Write up CL #3 that changes the bots' BuildAllOtherTargets step to BuildMost,
commit it, and restart the master.

3. Commit CL #1 (this one) that changes "make all" to chain to "make everything"

4 (later on). Decide which machines should have QT installed, install it, and
add "make
everything" to some set of bots.

[EricB, 2012-10-18 13:29:00]

SGTM


On Thu, Oct 18, 2012 at 9:26 AM, <epoger@google.com> wrote:

> On 2012/10/18 12:02:20, EricB wrote:
>
>> On 2012/10/18 05:03:48, epoger wrote:
>> > Ready for review at patchset 5!
>> >
>> > In patchset 4, I updated my local source tree, resolving conflicts
>>
> that came
>
>> > from
https://code.google.com/p/**skia/source/detail?r=5936<https://code.google.com....
>> >
>> > In patchset 5, I added changes to make.py to keep the Windows
>>
> toplevel "make"
>
>> > build in sync with the other platforms.
>> >
>> > As of patchset 5, I have confirmed that "make", "make all", "make
>>
> most", "make
>
>> > clean" all behave as expected on Linux, Mac, and Windows.
>> >
>> > NOTE: With this change, "make all" actually attempts to build
>>
> everything!
>
>> "make
>> > all" fails on my Windows desktop, because I don't have QT
>>
> installed... so some
>
>> > users who are used to "make all" succeeding may now see it fail.  I
>>
> think this
>
>> > means that the BuildAllOtherTargets step will start failing on some
>>
> of our
>
>> > buildbots... we may want to replace "make all" in that step with
>>
> "make most"
>
>> (or
>> > just "make", which has the same effect), or else install QT on all
>>
> the
>
>> > buildbots...
>>
>
>  LGTM.  FWIW, since we're adding "most" and "everything", I think we
>>
> should be
>
>> building them on the bots, if maybe not at first.  That would include
>>
> the
>
>> debugger, which we really should be building.  I guess that means we
>>
> need to
>
>> install QT on the bots, *or* we have one bot (on each platform?) which
>>
> builds
>
>> the debugger so that not every machine needs QT.  Here's my proposed
>>
> plan:
>
>  1. Write up a quick change to run "make most" instead of "make all" on
>>
> the bots.
>
>  2. Commit both changes at the "same time" and restart the master to
>>
> pick them
>
>> up.
>>
>
>  3. Decide which machines should have QT installed, install it, and add
>>
> "make
>
>> everything" to some set of bots.
>>
>
> Sounds good.  I'll actually stretch it out a tiny bit more, just to
> avoid race conditions... I will knock out steps 1-3 as quickly as
> possible; #4 will happen "some other time".  OK?
>
> 1. Write up CL #2 that adds a "most" target to the build, and commit it.
>
> 2. Write up CL #3 that changes the bots' BuildAllOtherTargets step to
> BuildMost, commit it, and restart the master.
>
> 3. Commit CL #1 (this one) that changes "make all" to chain to "make
> everything"
>
> 4 (later on). Decide which machines should have QT installed, install
>
> it, and add "make
> everything" to some set of bots.
>
>
https://codereview.appspot.**com/6651064/<https://codereview.appspot.com/6651...
>

[epoger, 2012-10-23 15:30:32]

Eric / any other interested parties: please look at patchset 7 and the comments
below.  I think it's ready to commit now...

On 2012/10/18 13:26:47, epoger wrote:
> Sounds good.  I'll actually stretch it out a tiny bit more, just to avoid race
> conditions... I will knock out steps 1-3 as quickly as possible; #4 will
happen
> "some other time".  OK?
> 
> 1. Write up CL #2 that adds a "most" target to the build, and commit it.

#1 committed as https://code.google.com/p/skia/source/detail?r=5999
 
> 2. Write up CL #3 that changes the bots' BuildAllOtherTargets step to
BuildMost,
> commit it, and restart the master.

#2 committed as https://code.google.com/p/skia/source/detail?r=6000

> 3. Commit CL #1 (this one) that changes "make all" to chain to "make
everything"

#3 is ready to commit (IMHO) at patchset 7 in this CL.

> 4 (later on). Decide which machines should have QT installed, install it, and
> add "make
> everything" to some set of bots.

#4: filed https://code.google.com/p/skia/issues/detail?id=948 ('add "make all"
(and thus QT libraries) to some or all buildbots') to track

[EricB, 2012-10-23 15:54:34]

On 2012/10/23 15:30:32, epoger wrote:
> Eric / any other interested parties: please look at patchset 7 and the
comments
> below.  I think it's ready to commit now...
> 
> On 2012/10/18 13:26:47, epoger wrote:
> > Sounds good.  I'll actually stretch it out a tiny bit more, just to avoid
race
> > conditions... I will knock out steps 1-3 as quickly as possible; #4 will
> happen
> > "some other time".  OK?
> > 
> > 1. Write up CL #2 that adds a "most" target to the build, and commit it.
> 
> #1 committed as https://code.google.com/p/skia/source/detail?r=5999
>  
> > 2. Write up CL #3 that changes the bots' BuildAllOtherTargets step to
> BuildMost,
> > commit it, and restart the master.
> 
> #2 committed as https://code.google.com/p/skia/source/detail?r=6000
> 
> > 3. Commit CL #1 (this one) that changes "make all" to chain to "make
> everything"
> 
> #3 is ready to commit (IMHO) at patchset 7 in this CL.
> 
> > 4 (later on). Decide which machines should have QT installed, install it,
and
> > add "make
> > everything" to some set of bots.
> 
> #4: filed https://code.google.com/p/skia/issues/detail?id=948 ('add "make all"
> (and thus QT libraries) to some or all buildbots') to track

LGTM at patch set 7.