Fix the coverage build which used to complain about the missing pyautolib target.

Fix the coverage build which used to complain about the missing pyautolib target.
In the case pyautolib shouldn't be built, define a fake pyautolib target. This lets other targets
to unconditionally depend on pyautolib.

BUG=307018
R=thakis@chromium.org

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

[Alexander Potapenko, 2013-10-18 12:20:17]

PTAL

[Nico, 2013-10-18 14:03:06]

https://codereview.chromium.org/29013002/diff/1/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/29013002/diff/1/chrome/chrome_tests.gypi#newc...
chrome/chrome_tests.gypi:2930: ['OS=="linux" and enable_automation==1 and '
indent 2 more

https://codereview.chromium.org/29013002/diff/1/chrome/chrome_tests.gypi#newc...
chrome/chrome_tests.gypi:2930: ['OS=="linux" and enable_automation==1 and '
Instead of adding these checks everywhere something depends on pyautolib, I
think it's better to change things that target pyautolib always exists: As an
empty type none target in the cases it currently doesn't exist in, and as a
normal target else.

[Alexander Potapenko, 2013-10-18 14:16:58]

https://codereview.chromium.org/29013002/diff/1/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/29013002/diff/1/chrome/chrome_tests.gypi#newc...
chrome/chrome_tests.gypi:2930: ['OS=="linux" and enable_automation==1 and '
On 2013/10/18 14:03:06, Nico wrote:
> indent 2 more

The previous indentation is actually wrong. Fixed it.

https://codereview.chromium.org/29013002/diff/1/chrome/chrome_tests.gypi#newc...
chrome/chrome_tests.gypi:2930: ['OS=="linux" and enable_automation==1 and '
On 2013/10/18 14:03:06, Nico wrote:
> Instead of adding these checks everywhere something depends on pyautolib, I
> think it's better to change things that target pyautolib always exists: As an
> empty type none target in the cases it currently doesn't exist in, and as a
> normal target else.

Nice idea. Done.

[Nico, 2013-10-18 14:18:58]

https://codereview.chromium.org/29013002/diff/50001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/29013002/diff/50001/chrome/chrome_tests.gypi#...
chrome/chrome_tests.gypi:2836: }
To not repeat the target name, other places write it like

  {
    'target_name': 'mytarget'
    'conditions': [
      ['is_foo', {
        'type': 'none',
      }, {
        'type': 'shared_library',
        'sources': …
      }],
    ]
  }

[Alexander Potapenko, 2013-10-18 14:24:24]

On 2013/10/18 14:18:58, Nico wrote:
> https://codereview.chromium.org/29013002/diff/50001/chrome/chrome_tests.gypi
> File chrome/chrome_tests.gypi (right):
> 
>
https://codereview.chromium.org/29013002/diff/50001/chrome/chrome_tests.gypi#...
> chrome/chrome_tests.gypi:2836: }
> To not repeat the target name, other places write it like
> 
>   {
>     'target_name': 'mytarget'
>     'conditions': [
>       ['is_foo', {
>         'type': 'none',
>       }, {
>         'type': 'shared_library',
>         'sources': …
>       }],
>     ]
>   }
There's another target, webrtc_test_tools, under the same is_foo condition.
We'll have to repeat the condition twice (once for each target) if we choose to
use it inside the target declaration. That said, I'm not sure this is going to
simplify the code.

[Nico, 2013-10-18 14:29:26]

On Fri, Oct 18, 2013 at 7:24 AM, <glider@chromium.org> wrote:

> On 2013/10/18 14:18:58, Nico wrote:
>
>> https://codereview.chromium.**org/29013002/diff/50001/**
>>
chrome/chrome_tests.gypi<https://codereview.chromium.org/29013002/diff/50001/chrome/chrome_tests.gypi>
>> File chrome/chrome_tests.gypi (right):
>>
>
>
> https://codereview.chromium.**org/29013002/diff/50001/**
>
chrome/chrome_tests.gypi#**newcode2836<https://codereview.chromium.org/29013002/diff/50001/chrome/chrome_tests.gypi#newcode2836>
>
>> chrome/chrome_tests.gypi:2836: }
>> To not repeat the target name, other places write it like
>>
>
>     {
>>      'target_name': 'mytarget'
>>      'conditions': [
>>        ['is_foo', {
>>          'type': 'none',
>>        }, {
>>          'type': 'shared_library',
>>          'sources': …
>>        }],
>>      ]
>>    }
>>
> There's another target, webrtc_test_tools, under the same is_foo condition.
> We'll have to repeat the condition twice (once for each target) if we
> choose to
> use it inside the target declaration. That said, I'm not sure this is
> going to
> simplify the code.
>

From what I can tell, once the pyautolib target always exists, the
webrtc_test_tools target can just move out of the condition, right?


>
>
https://codereview.chromium.**org/29013002/<https://codereview.chromium.org/2...
>

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

[Nico, 2013-10-18 14:30:46]

…and you can also make the dependency on pyautolib in build/all.gyp
unconditional.


On Fri, Oct 18, 2013 at 7:29 AM, Nico Weber <thakis@chromium.org> wrote:

> On Fri, Oct 18, 2013 at 7:24 AM, <glider@chromium.org> wrote:
>
>> On 2013/10/18 14:18:58, Nico wrote:
>>
>>> https://codereview.chromium.**org/29013002/diff/50001/**
>>>
chrome/chrome_tests.gypi<https://codereview.chromium.org/29013002/diff/50001/chrome/chrome_tests.gypi>
>>> File chrome/chrome_tests.gypi (right):
>>>
>>
>>
>> https://codereview.chromium.**org/29013002/diff/50001/**
>>
chrome/chrome_tests.gypi#**newcode2836<https://codereview.chromium.org/29013002/diff/50001/chrome/chrome_tests.gypi#newcode2836>
>>
>>> chrome/chrome_tests.gypi:2836: }
>>> To not repeat the target name, other places write it like
>>>
>>
>>     {
>>>      'target_name': 'mytarget'
>>>      'conditions': [
>>>        ['is_foo', {
>>>          'type': 'none',
>>>        }, {
>>>          'type': 'shared_library',
>>>          'sources': …
>>>        }],
>>>      ]
>>>    }
>>>
>> There's another target, webrtc_test_tools, under the same is_foo
>> condition.
>> We'll have to repeat the condition twice (once for each target) if we
>> choose to
>> use it inside the target declaration. That said, I'm not sure this is
>> going to
>> simplify the code.
>>
>
> From what I can tell, once the pyautolib target always exists, the
> webrtc_test_tools target can just move out of the condition, right?
>
>
>>
>>
https://codereview.chromium.**org/29013002/<https://codereview.chromium.org/2...
>>
>
>

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

[Alexander Potapenko, 2013-10-18 14:39:45]

On 2013/10/18 14:30:46, Nico wrote:
> …and you can also make the dependency on pyautolib in build/all.gyp
> unconditional.
Done

[Nico, 2013-10-18 15:21:07]

Thanks, lgtm!

Much nicer :-)

https://codereview.chromium.org/29013002/diff/160001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/29013002/diff/160001/chrome/chrome_tests.gypi...
chrome/chrome_tests.gypi:2610: '-Wno-self-assign',
Since you're here, optional additional change: You can move this into the
unconditional xcode_settings block earlier in the target (the one setting
OTHER_LDFLAGS) since all platforms that use xcode_settings use clang.

You can also delete the cflags block 2 lines below since the same flag is
already set unconditionally further up.

This is optional, and fine to do in a separate as well if you want to land this
first (you can TBR the followup to me if you decide to do it.)

[Alexander Potapenko, 2013-10-18 15:25:55]

https://codereview.chromium.org/29013002/diff/160001/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

https://codereview.chromium.org/29013002/diff/160001/chrome/chrome_tests.gypi...
chrome/chrome_tests.gypi:2610: '-Wno-self-assign',
On 2013/10/18 15:21:07, Nico wrote:
> Since you're here, optional additional change: You can move this into the
> unconditional xcode_settings block earlier in the target (the one setting
> OTHER_LDFLAGS) since all platforms that use xcode_settings use clang.
> 
> You can also delete the cflags block 2 lines below since the same flag is
> already set unconditionally further up.
> 
> This is optional, and fine to do in a separate as well if you want to land
this
> first (you can TBR the followup to me if you decide to do it.)

I wonder if there are bots that cover this path and will let me check I don't
screw up the flags. Let me fix that in a separate CL.

[commit-bot: I haz the power, 2013-10-18 15:28:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/glider@chromium.org/29013002/160001

[commit-bot: I haz the power, 2013-10-18 22:04:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/glider@chromium.org/29013002/160001

[commit-bot: I haz the power, 2013-10-19 06:45:47]

Retried try job too often on win7_aura for step(s) app_list_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[Alexander Potapenko, 2013-10-21 16:37:56]

Committed patchset #4 manually as r229837 (presubmit successful).