android: Fold all_android.gyp into all.gyp.

android: Fold all_android.gyp into all.gyp.

All other platforms have their targets in all.gyp. Android didn't during
bring-up, but enough stuff works by now that there's no need for the
separate all_android.gyp file any more.

BUG=330631
R=yfriedman@chromium.org
TBR=rsleevi@chromium.org

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

[Nico, 2014-01-23 17:01:53]

ping


On Wed, Jan 22, 2014 at 3:11 PM, <thakis@chromium.org> wrote:

> Reviewers: Torne,
>
> Description:
> android: Fold all_android.gyp into all.gyp.
>
> All other platforms have their targets in all.gyp. Android didn't during
> bring-up, but enough stuff works by now that there's no need for the
> separate all_android.gyp file any more.
>
> BUG=330631
>
> Please review this at https://codereview.chromium.org/139743016/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+196, -200 lines):
>   D build/all_android.gyp
>   M build/all.gyp
>   M build/android/envsetup_functions.sh
>   M net/net.gyp
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Torne, 2014-01-23 21:53:13]

Looks reasonable but I don't really want to approve this myself as I dunno about
all the possible android bot configurations that may be affected.

Yaron, do you know who would be a good reviewer for this? cjhopman is on
vacation...

[Yaron, 2014-01-23 22:41:28]

https://codereview.chromium.org/139743016/diff/910001/build/all.gyp
File build/all.gyp (right):

https://codereview.chromium.org/139743016/diff/910001/build/all.gyp#newcode41
build/all.gyp:41: '../ui/ui_unittests.gyp:ui_unittests',
Unrelated but this seems pretty bizarre. This is also included in
chrome_builder_tests. I wonder if it's just a bad merge from a previous time
where ui_unittests was in a bring up phase of sorts for ios

https://codereview.chromium.org/139743016/diff/910001/build/all.gyp#newcode702
build/all.gyp:702: # on tryserver triggers and load.  Discuss with jrg please.
s/jrg/chrome-infrastructure-team

https://codereview.chromium.org/139743016/diff/910001/build/all.gyp#newcode703
build/all.gyp:703: 'target_name': 'android_builder_tests',
A gclient grep for android_builder_tests doesn't turn up this string outside of
these gyp files. I think this is a historical artifact and we could just add
most of the android-specific targets as a condition of All (fine to do in a
separate change)

https://codereview.chromium.org/139743016/diff/910001/build/android/envsetup_...
File build/android/envsetup_functions.sh (right):

https://codereview.chromium.org/139743016/diff/910001/build/android/envsetup_...
build/android/envsetup_functions.sh:153: unset CHROMIUM_GYP_FILE
Won't this still break developers? E.g. I typically do a gclient sync which runs
build/gyp_chromium (but not envsetup).

[Yaron, 2014-01-23 22:42:17]

https://codereview.chromium.org/139743016/diff/910001/build/android/envsetup_...
File build/android/envsetup_functions.sh (right):

https://codereview.chromium.org/139743016/diff/910001/build/android/envsetup_...
build/android/envsetup_functions.sh:153: unset CHROMIUM_GYP_FILE
On 2014/01/23 22:41:28, Yaron wrote:
> Won't this still break developers? E.g. I typically do a gclient sync which
runs
> build/gyp_chromium (but not envsetup).

What I mean is, should we leave in a stub all_android.gyp that just includes the
main one. After the two weeks or so, then delete the stub?

[Nico, 2014-01-23 23:09:24]

Thanks!

https://codereview.chromium.org/139743016/diff/910001/build/all.gyp
File build/all.gyp (right):

https://codereview.chromium.org/139743016/diff/910001/build/all.gyp#newcode41
build/all.gyp:41: '../ui/ui_unittests.gyp:ui_unittests',
On 2014/01/23 22:41:28, Yaron wrote:
> Unrelated but this seems pretty bizarre. This is also included in
> chrome_builder_tests. I wonder if it's just a bad merge from a previous time
> where ui_unittests was in a bring up phase of sorts for ios

Hm, it's strange that 'All' doesn't depend on chromium_builder_tests – it looks
like "All" currently doesn't include all these tests.

https://codereview.chromium.org/139743016/diff/910001/build/all.gyp#newcode702
build/all.gyp:702: # on tryserver triggers and load.  Discuss with jrg please.
On 2014/01/23 22:41:28, Yaron wrote:
> s/jrg/chrome-infrastructure-team

Done.

https://codereview.chromium.org/139743016/diff/910001/build/all.gyp#newcode703
build/all.gyp:703: 'target_name': 'android_builder_tests',
On 2014/01/23 22:41:28, Yaron wrote:
> A gclient grep for android_builder_tests doesn't turn up this string outside
of
> these gyp files. I think this is a historical artifact and we could just add
> most of the android-specific targets as a condition of All (fine to do in a
> separate change)

Nice! Will look into doing this in a follow-up.

https://codereview.chromium.org/139743016/diff/910001/build/android/envsetup_...
File build/android/envsetup_functions.sh (right):

https://codereview.chromium.org/139743016/diff/910001/build/android/envsetup_...
build/android/envsetup_functions.sh:153: unset CHROMIUM_GYP_FILE
On 2014/01/23 22:42:17, Yaron wrote:
> On 2014/01/23 22:41:28, Yaron wrote:
> > Won't this still break developers? E.g. I typically do a gclient sync which
> runs
> > build/gyp_chromium (but not envsetup).

Aren't folks to envsetup after syncing? If this file adds new DEFINES, they
won't get them.

But sure, I can keep an empty all_android.gyp around too for a while, done :-)

[Yaron, 2014-01-24 01:09:47]

On 2014/01/23 23:09:24, Nico wrote:
> Thanks!
> 
> https://codereview.chromium.org/139743016/diff/910001/build/all.gyp
> File build/all.gyp (right):
> 
> https://codereview.chromium.org/139743016/diff/910001/build/all.gyp#newcode41
> build/all.gyp:41: '../ui/ui_unittests.gyp:ui_unittests',
> On 2014/01/23 22:41:28, Yaron wrote:
> > Unrelated but this seems pretty bizarre. This is also included in
> > chrome_builder_tests. I wonder if it's just a bad merge from a previous time
> > where ui_unittests was in a bring up phase of sorts for ios
> 
> Hm, it's strange that 'All' doesn't depend on chromium_builder_tests – it
looks
> like "All" currently doesn't include all these tests.
> 
> https://codereview.chromium.org/139743016/diff/910001/build/all.gyp#newcode702
> build/all.gyp:702: # on tryserver triggers and load.  Discuss with jrg please.
> On 2014/01/23 22:41:28, Yaron wrote:
> > s/jrg/chrome-infrastructure-team
> 
> Done.
> 
> https://codereview.chromium.org/139743016/diff/910001/build/all.gyp#newcode703
> build/all.gyp:703: 'target_name': 'android_builder_tests',
> On 2014/01/23 22:41:28, Yaron wrote:
> > A gclient grep for android_builder_tests doesn't turn up this string outside
> of
> > these gyp files. I think this is a historical artifact and we could just add
> > most of the android-specific targets as a condition of All (fine to do in a
> > separate change)
> 
> Nice! Will look into doing this in a follow-up.
> 
>
https://codereview.chromium.org/139743016/diff/910001/build/android/envsetup_...
> File build/android/envsetup_functions.sh (right):
> 
>
https://codereview.chromium.org/139743016/diff/910001/build/android/envsetup_...
> build/android/envsetup_functions.sh:153: unset CHROMIUM_GYP_FILE
> On 2014/01/23 22:42:17, Yaron wrote:
> > On 2014/01/23 22:41:28, Yaron wrote:
> > > Won't this still break developers? E.g. I typically do a gclient sync
which
> > runs
> > > build/gyp_chromium (but not envsetup).
> 
> Aren't folks to envsetup after syncing? If this file adds new DEFINES, they
> won't get them.
> 
I don't think so. I think folks who use "cr" always run it but others run it
when they create a new shell.
 
> But sure, I can keep an empty all_android.gyp around too for a while, done :-)

lgtm

[Nico, 2014-01-24 01:13:40]

On Thu, Jan 23, 2014 at 5:09 PM, <yfriedman@chromium.org> wrote:

> On 2014/01/23 23:09:24, Nico wrote:
>
>> Thanks!
>>
>
>  https://codereview.chromium.org/139743016/diff/910001/build/all.gyp
>> File build/all.gyp (right):
>>
>
>  https://codereview.chromium.org/139743016/diff/910001/
>> build/all.gyp#newcode41
>> build/all.gyp:41: '../ui/ui_unittests.gyp:ui_unittests',
>> On 2014/01/23 22:41:28, Yaron wrote:
>> > Unrelated but this seems pretty bizarre. This is also included in
>> > chrome_builder_tests. I wonder if it's just a bad merge from a previous
>> time
>> > where ui_unittests was in a bring up phase of sorts for ios
>>
>
>  Hm, it's strange that 'All' doesn't depend on chromium_builder_tests – it
>>
> looks
>
>> like "All" currently doesn't include all these tests.
>>
>
>  https://codereview.chromium.org/139743016/diff/910001/
>> build/all.gyp#newcode702
>> build/all.gyp:702: # on tryserver triggers and load.  Discuss with jrg
>> please.
>> On 2014/01/23 22:41:28, Yaron wrote:
>> > s/jrg/chrome-infrastructure-team
>>
>
>  Done.
>>
>
>  https://codereview.chromium.org/139743016/diff/910001/
>> build/all.gyp#newcode703
>> build/all.gyp:703: 'target_name': 'android_builder_tests',
>> On 2014/01/23 22:41:28, Yaron wrote:
>> > A gclient grep for android_builder_tests doesn't turn up this string
>> outside
>> of
>> > these gyp files. I think this is a historical artifact and we could
>> just add
>> > most of the android-specific targets as a condition of All (fine to do
>> in a
>> > separate change)
>>
>
>  Nice! Will look into doing this in a follow-up.
>>
>
>
> https://codereview.chromium.org/139743016/diff/910001/
> build/android/envsetup_functions.sh
>
>> File build/android/envsetup_functions.sh (right):
>>
>
>
> https://codereview.chromium.org/139743016/diff/910001/
> build/android/envsetup_functions.sh#newcode153
>
>> build/android/envsetup_functions.sh:153: unset CHROMIUM_GYP_FILE
>> On 2014/01/23 22:42:17, Yaron wrote:
>> > On 2014/01/23 22:41:28, Yaron wrote:
>> > > Won't this still break developers? E.g. I typically do a gclient sync
>>
> which
>
>> > runs
>> > > build/gyp_chromium (but not envsetup).
>>
>
>  Aren't folks to envsetup after syncing? If this file adds new DEFINES,
>> they
>> won't get them.
>>
>
> I don't think so. I think folks who use "cr" always run it but others run
> it
> when they create a new shell.


Huh, good to know :-) What's the process if someone does a change to
envsetup that requires resourcing it? Do they send a "please resource" mail
every time?


>
>
>  But sure, I can keep an empty all_android.gyp around too for a while,
>> done :-)
>>
>
> lgtm
>
> https://codereview.chromium.org/139743016/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2014-01-24 01:14:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/139743016/980001

[Torne, 2014-01-24 01:14:52]

On 2014/01/24 01:13:40, Nico wrote:
> On Thu, Jan 23, 2014 at 5:09 PM, <mailto:yfriedman@chromium.org> wrote:
> 
> > On 2014/01/23 23:09:24, Nico wrote:
> >
> >> Thanks!
> >>
> >
> >  https://codereview.chromium.org/139743016/diff/910001/build/all.gyp
> >> File build/all.gyp (right):
> >>
> >
> >  https://codereview.chromium.org/139743016/diff/910001/
> >> build/all.gyp#newcode41
> >> build/all.gyp:41: '../ui/ui_unittests.gyp:ui_unittests',
> >> On 2014/01/23 22:41:28, Yaron wrote:
> >> > Unrelated but this seems pretty bizarre. This is also included in
> >> > chrome_builder_tests. I wonder if it's just a bad merge from a previous
> >> time
> >> > where ui_unittests was in a bring up phase of sorts for ios
> >>
> >
> >  Hm, it's strange that 'All' doesn't depend on chromium_builder_tests – it
> >>
> > looks
> >
> >> like "All" currently doesn't include all these tests.
> >>
> >
> >  https://codereview.chromium.org/139743016/diff/910001/
> >> build/all.gyp#newcode702
> >> build/all.gyp:702: # on tryserver triggers and load.  Discuss with jrg
> >> please.
> >> On 2014/01/23 22:41:28, Yaron wrote:
> >> > s/jrg/chrome-infrastructure-team
> >>
> >
> >  Done.
> >>
> >
> >  https://codereview.chromium.org/139743016/diff/910001/
> >> build/all.gyp#newcode703
> >> build/all.gyp:703: 'target_name': 'android_builder_tests',
> >> On 2014/01/23 22:41:28, Yaron wrote:
> >> > A gclient grep for android_builder_tests doesn't turn up this string
> >> outside
> >> of
> >> > these gyp files. I think this is a historical artifact and we could
> >> just add
> >> > most of the android-specific targets as a condition of All (fine to do
> >> in a
> >> > separate change)
> >>
> >
> >  Nice! Will look into doing this in a follow-up.
> >>
> >
> >
> > https://codereview.chromium.org/139743016/diff/910001/
> > build/android/envsetup_functions.sh
> >
> >> File build/android/envsetup_functions.sh (right):
> >>
> >
> >
> > https://codereview.chromium.org/139743016/diff/910001/
> > build/android/envsetup_functions.sh#newcode153
> >
> >> build/android/envsetup_functions.sh:153: unset CHROMIUM_GYP_FILE
> >> On 2014/01/23 22:42:17, Yaron wrote:
> >> > On 2014/01/23 22:41:28, Yaron wrote:
> >> > > Won't this still break developers? E.g. I typically do a gclient sync
> >>
> > which
> >
> >> > runs
> >> > > build/gyp_chromium (but not envsetup).
> >>
> >
> >  Aren't folks to envsetup after syncing? If this file adds new DEFINES,
> >> they
> >> won't get them.
> >>
> >
> > I don't think so. I think folks who use "cr" always run it but others run
> > it
> > when they create a new shell.
> 
> 
> Huh, good to know :-) What's the process if someone does a change to
> envsetup that requires resourcing it? Do they send a "please resource" mail
> every time?

Generally no, people just get broken and then send an email asking why their
build doesn't work, and someone suggests starting a new shell and rerunning
envsetup :)

You can take this as yet anotehr reason why we'd like to get rid of envsetup.


> 
> >
> >
> >  But sure, I can keep an empty all_android.gyp around too for a while,
> >> done :-)
> >>
> >
> > lgtm
> >
> > https://codereview.chromium.org/139743016/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[Nico, 2014-01-24 04:32:22]

Committed patchset #12 manually as r246782 (tree was closed).

[whywhat, 2014-01-24 16:05:55]

There're two android_gyp issues for me when I run it with this change in my
client (bots are surprisingly green though), should I fix my setup or submit
https://codereview.chromium.org/144063007 ?

https://codereview.chromium.org/139743016/diff/980001/build/all_android.gyp
File build/all_android.gyp (left):

https://codereview.chromium.org/139743016/diff/980001/build/all_android.gyp#o...
build/all_android.gyp:18: 'type': 'none',
gyp fails for me with 'gyp: Missing 'type' field in target
temporary_android_dummy_target'. should I update gyp?

https://codereview.chromium.org/139743016/diff/980001/build/all_android.gyp
File build/all_android.gyp (right):

https://codereview.chromium.org/139743016/diff/980001/build/all_android.gyp#n...
build/all_android.gyp:14: ],
Shouldn't this bracket have been a }, ?

[Nico, 2014-01-24 16:40:04]

On Fri, Jan 24, 2014 at 8:05 AM, <avayvod@chromium.org> wrote:

> There're two android_gyp issues for me when I run it with this change in my
> client (bots are surprisingly green though), should I fix my setup or
> submit
> https://codereview.chromium.org/144063007 ?
>
>
> https://codereview.chromium.org/139743016/diff/980001/
> build/all_android.gyp
> File build/all_android.gyp (left):
>
> https://codereview.chromium.org/139743016/diff/980001/
> build/all_android.gyp#oldcode18
> build/all_android.gyp:18: 'type': 'none',
> gyp fails for me with 'gyp: Missing 'type' field in target
> temporary_android_dummy_target'. should I update gyp?
>
> https://codereview.chromium.org/139743016/diff/980001/
> build/all_android.gyp
> File build/all_android.gyp (right):
>
> https://codereview.chromium.org/139743016/diff/980001/
> build/all_android.gyp#newcode14
> build/all_android.gyp:14: ],
> Shouldn't this bracket have been a }, ?
>

Yes. bauerb@ fixed this already. It was also missing a 'type': 'none' which
bauerb fixed as well.

But you can just re-source your envsetup file too, this file shouldn't be
needed.


>
> https://codereview.chromium.org/139743016/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nico, 2014-01-24 19:32:32]

https://codereview.chromium.org/139743016/diff/910001/build/all.gyp
File build/all.gyp (right):

https://codereview.chromium.org/139743016/diff/910001/build/all.gyp#newcode703
build/all.gyp:703: 'target_name': 'android_builder_tests',
> Nice! Will look into doing this in a follow-up.

Did look into this. Since android uses GYP_GENERATOR_FLAGS=default_target=All,
all the targets listed in this target would have to move into All, so maybe the
default_target thing should be changed first. Now looking into what that
requires.