Move src/gpu/gles2_conform_test out of src-internal

Move src/gpu/gles2_conform_test out of src-internal

We directly check them into src/gpu/gles2_conform_support

BUG=325536
TEST=gpu builders
R=kbr@chromium.org, piman@chromium.org

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

[Zhenyao Mo, 2013-12-04 23:05:07]

Please review.

I tried to minimize the changes, so I did only two things

1) copy and check in the src-internal files to gpu/gles2_conform_support - they
were originally put into gpu/gles2_conform_test by src-internal/DEPS

2) modify the corresponding paths from gles2_conform_test to
gles2_conform_support

[Zhenyao Mo, 2013-12-04 23:06:01]

I could probably combine the gles2_conform_test.gyp and
gles2_conform_support.gyp, but decided to leave it to a follow-up CL.

[piman, 2013-12-04 23:20:06]

https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/gl...
File gpu/gles2_conform_support/gles2_conform_test.gyp (right):

https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/gl...
gpu/gles2_conform_support/gles2_conform_test.gyp:8:
'../../third_party/gles2_conform/gles2_conform.gypi',
How does this work with an external checkout?

[Ken Russell (switch to Gerrit), 2013-12-04 23:20:41]

On 2013/12/04 23:20:06, piman wrote:
>
https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/gl...
> File gpu/gles2_conform_support/gles2_conform_test.gyp (right):
> 
>
https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/gl...
> gpu/gles2_conform_support/gles2_conform_test.gyp:8:
> '../../third_party/gles2_conform/gles2_conform.gypi',
> How does this work with an external checkout?

These sources only build if you have access to the internal sources.

[Ken Russell (switch to Gerrit), 2013-12-04 23:21:28]

On 2013/12/04 23:20:41, Ken Russell wrote:
> On 2013/12/04 23:20:06, piman wrote:
> >
>
https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/gl...
> > File gpu/gles2_conform_support/gles2_conform_test.gyp (right):
> > 
> >
>
https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/gl...
> > gpu/gles2_conform_support/gles2_conform_test.gyp:8:
> > '../../third_party/gles2_conform/gles2_conform.gypi',
> > How does this work with an external checkout?
> 
> These sources only build if you have access to the internal sources.

Although, if this will prevent gclient runhooks from running without
src-internal present, that is a problem.

[piman, 2013-12-04 23:24:15]

I don't think it's meaningful to have a target we can't build. While it
helps greppability, that's just it. People can't test their fixes or submit
try jobs.


On Wed, Dec 4, 2013 at 3:21 PM, <kbr@chromium.org> wrote:

> On 2013/12/04 23:20:41, Ken Russell wrote:
>
>> On 2013/12/04 23:20:06, piman wrote:
>> >
>>
>
> https://codereview.chromium.org/105733002/diff/1/gpu/
> gles2_conform_support/gles2_conform_test.gyp
>
>> > File gpu/gles2_conform_support/gles2_conform_test.gyp (right):
>> >
>> >
>>
>
> https://codereview.chromium.org/105733002/diff/1/gpu/
> gles2_conform_support/gles2_conform_test.gyp#newcode8
>
>> > gpu/gles2_conform_support/gles2_conform_test.gyp:8:
>> > '../../third_party/gles2_conform/gles2_conform.gypi',
>> > How does this work with an external checkout?
>>
>
>  These sources only build if you have access to the internal sources.
>>
>
> Although, if this will prevent gclient runhooks from running without
> src-internal present, that is a problem.
>
>
> https://codereview.chromium.org/105733002/
>

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

[Zhenyao Mo, 2013-12-04 23:26:20]

https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/gl...
File gpu/gles2_conform_support/gles2_conform_test.gyp (right):

https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/gl...
gpu/gles2_conform_support/gles2_conform_test.gyp:8:
'../../third_party/gles2_conform/gles2_conform.gypi',
On 2013/12/04 23:20:06, piman wrote:
> How does this work with an external checkout?

This is behind a variable:
https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&...

But let me test runhooks as kbr suggested

[Zhenyao Mo, 2013-12-04 23:27:25]

On 2013/12/04 23:24:15, piman wrote:
> I don't think it's meaningful to have a target we can't build. While it
> helps greppability, that's just it. People can't test their fixes or submit
> try jobs.

This target is part of the gpu bots, and once we make it tree closers, they will
be on the default try jobs (and hopefully also CQ bots).

> 
> 
> On Wed, Dec 4, 2013 at 3:21 PM, <mailto:kbr@chromium.org> wrote:
> 
> > On 2013/12/04 23:20:41, Ken Russell wrote:
> >
> >> On 2013/12/04 23:20:06, piman wrote:
> >> >
> >>
> >
> > https://codereview.chromium.org/105733002/diff/1/gpu/
> > gles2_conform_support/gles2_conform_test.gyp
> >
> >> > File gpu/gles2_conform_support/gles2_conform_test.gyp (right):
> >> >
> >> >
> >>
> >
> > https://codereview.chromium.org/105733002/diff/1/gpu/
> > gles2_conform_support/gles2_conform_test.gyp#newcode8
> >
> >> > gpu/gles2_conform_support/gles2_conform_test.gyp:8:
> >> > '../../third_party/gles2_conform/gles2_conform.gypi',
> >> > How does this work with an external checkout?
> >>
> >
> >  These sources only build if you have access to the internal sources.
> >>
> >
> > Although, if this will prevent gclient runhooks from running without
> > src-internal present, that is a problem.
> >
> >
> > https://codereview.chromium.org/105733002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[piman, 2013-12-04 23:54:31]

On Wed, Dec 4, 2013 at 3:27 PM, <zmo@chromium.org> wrote:

> On 2013/12/04 23:24:15, piman wrote:
>
>> I don't think it's meaningful to have a target we can't build. While it
>> helps greppability, that's just it. People can't test their fixes or
>> submit
>> try jobs.
>>
>
> This target is part of the gpu bots, and once we make it tree closers,
> they will
> be on the default try jobs (and hopefully also CQ bots).


But I think we should make the parts that depend on base/ to be at least
compilable by folks, so that they can test their changes, no?

On Wed, Dec 4, 2013 at 3:21 PM, <mailto:kbr@chromium.org> wrote:
>>
>
>  > On 2013/12/04 23:20:41, Ken Russell wrote:
>> >
>> >> On 2013/12/04 23:20:06, piman wrote:
>> >> >
>> >>
>> >
>> > https://codereview.chromium.org/105733002/diff/1/gpu/
>> > gles2_conform_support/gles2_conform_test.gyp
>> >
>> >> > File gpu/gles2_conform_support/gles2_conform_test.gyp (right):
>> >> >
>> >> >
>> >>
>> >
>> > https://codereview.chromium.org/105733002/diff/1/gpu/
>> > gles2_conform_support/gles2_conform_test.gyp#newcode8
>> >
>> >> > gpu/gles2_conform_support/gles2_conform_test.gyp:8:
>> >> > '../../third_party/gles2_conform/gles2_conform.gypi',
>>
>> >> > How does this work with an external checkout?
>> >>
>> >
>> >  These sources only build if you have access to the internal sources.
>> >>
>> >
>> > Although, if this will prevent gclient runhooks from running without
>> > src-internal present, that is a problem.
>> >
>> >
>> > https://codereview.chromium.org/105733002/
>> >
>>
>
>  To unsubscribe from this group and stop receiving emails from it, send an
>>
> email
>
>> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>
>
>
>
> https://codereview.chromium.org/105733002/
>

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

[Zhenyao Mo, 2013-12-05 00:02:37]

I see your point.  What about with internal_gles2_conform_tests=1, we
do what we did (internal gles2 tests), and with
internal_gles2_conform_tests=0, we just build with a dummy test?

On Wed, Dec 4, 2013 at 3:54 PM, Antoine Labour <piman@chromium.org> wrote:
>
>
>
> On Wed, Dec 4, 2013 at 3:27 PM, <zmo@chromium.org> wrote:
>>
>> On 2013/12/04 23:24:15, piman wrote:
>>>
>>> I don't think it's meaningful to have a target we can't build. While it
>>> helps greppability, that's just it. People can't test their fixes or
>>> submit
>>> try jobs.
>>
>>
>> This target is part of the gpu bots, and once we make it tree closers,
>> they will
>> be on the default try jobs (and hopefully also CQ bots).
>
>
> But I think we should make the parts that depend on base/ to be at least
> compilable by folks, so that they can test their changes, no?
>
>>> On Wed, Dec 4, 2013 at 3:21 PM, <mailto:kbr@chromium.org> wrote:
>>
>>
>>> > On 2013/12/04 23:20:41, Ken Russell wrote:
>>> >
>>> >> On 2013/12/04 23:20:06, piman wrote:
>>> >> >
>>> >>
>>> >
>>> > https://codereview.chromium.org/105733002/diff/1/gpu/
>>> > gles2_conform_support/gles2_conform_test.gyp
>>> >
>>> >> > File gpu/gles2_conform_support/gles2_conform_test.gyp (right):
>>> >> >
>>> >> >
>>> >>
>>> >
>>> > https://codereview.chromium.org/105733002/diff/1/gpu/
>>> > gles2_conform_support/gles2_conform_test.gyp#newcode8
>>> >
>>> >> > gpu/gles2_conform_support/gles2_conform_test.gyp:8:
>>> >> > '../../third_party/gles2_conform/gles2_conform.gypi',
>>>
>>> >> > How does this work with an external checkout?
>>> >>
>>> >
>>> >  These sources only build if you have access to the internal sources.
>>> >>
>>> >
>>> > Although, if this will prevent gclient runhooks from running without
>>> > src-internal present, that is a problem.
>>> >
>>> >
>>> > https://codereview.chromium.org/105733002/
>>> >
>>
>>
>>> To unsubscribe from this group and stop receiving emails from it, send an
>>
>> email
>>>
>>> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>
>>
>>
>>
>> https://codereview.chromium.org/105733002/
>
>

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

[piman, 2013-12-05 00:20:25]

On Wed, Dec 4, 2013 at 4:02 PM, Zhenyao Mo <zmo@chromium.org> wrote:

> I see your point.  What about with internal_gles2_conform_tests=1, we
> do what we did (internal gles2 tests), and with
> internal_gles2_conform_tests=0, we just build with a dummy test?
>

That sounds excellent.

Antoine


>
> On Wed, Dec 4, 2013 at 3:54 PM, Antoine Labour <piman@chromium.org> wrote:
> >
> >
> >
> > On Wed, Dec 4, 2013 at 3:27 PM, <zmo@chromium.org> wrote:
> >>
> >> On 2013/12/04 23:24:15, piman wrote:
> >>>
> >>> I don't think it's meaningful to have a target we can't build. While it
> >>> helps greppability, that's just it. People can't test their fixes or
> >>> submit
> >>> try jobs.
> >>
> >>
> >> This target is part of the gpu bots, and once we make it tree closers,
> >> they will
> >> be on the default try jobs (and hopefully also CQ bots).
> >
> >
> > But I think we should make the parts that depend on base/ to be at least
> > compilable by folks, so that they can test their changes, no?
> >
> >>> On Wed, Dec 4, 2013 at 3:21 PM, <mailto:kbr@chromium.org> wrote:
> >>
> >>
> >>> > On 2013/12/04 23:20:41, Ken Russell wrote:
> >>> >
> >>> >> On 2013/12/04 23:20:06, piman wrote:
> >>> >> >
> >>> >>
> >>> >
> >>> > https://codereview.chromium.org/105733002/diff/1/gpu/
> >>> > gles2_conform_support/gles2_conform_test.gyp
> >>> >
> >>> >> > File gpu/gles2_conform_support/gles2_conform_test.gyp (right):
> >>> >> >
> >>> >> >
> >>> >>
> >>> >
> >>> > https://codereview.chromium.org/105733002/diff/1/gpu/
> >>> > gles2_conform_support/gles2_conform_test.gyp#newcode8
> >>> >
> >>> >> > gpu/gles2_conform_support/gles2_conform_test.gyp:8:
> >>> >> > '../../third_party/gles2_conform/gles2_conform.gypi',
> >>>
> >>> >> > How does this work with an external checkout?
> >>> >>
> >>> >
> >>> >  These sources only build if you have access to the internal sources.
> >>> >>
> >>> >
> >>> > Although, if this will prevent gclient runhooks from running without
> >>> > src-internal present, that is a problem.
> >>> >
> >>> >
> >>> > https://codereview.chromium.org/105733002/
> >>> >
> >>
> >>
> >>> To unsubscribe from this group and stop receiving emails from it, send
> an
> >>
> >> email
> >>>
> >>> to mailto:chromium-reviews+unsubscribe@chromium.org.
> >>
> >>
> >>
> >>
> >> https://codereview.chromium.org/105733002/
> >
> >
>

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

[Ken Russell (switch to Gerrit), 2013-12-05 00:40:14]

Overall this looks good. Antoine's suggestion to compile in a dummy test sounds
really useful though.

https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/RE...
File gpu/gles2_conform_support/README (right):

https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/RE...
gpu/gles2_conform_support/README:1: To run OpenGL ES 2.0 conformance tests, do
the following:
Is it worth adding a note that these require access to Google-internal sources?

https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/ge...
File gpu/gles2_conform_support/generate_gles2_conform_tests.py (right):

https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/ge...
gpu/gles2_conform_support/generate_gles2_conform_tests.py:2: # Copyright (c)
2012 The Chromium Authors. All rights reserved.
May be worth updating copyright year.

[Zhenyao Mo, 2013-12-05 01:48:31]

https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/RE...
File gpu/gles2_conform_support/README (right):

https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/RE...
gpu/gles2_conform_support/README:1: To run OpenGL ES 2.0 conformance tests, do
the following:
On 2013/12/05 00:40:15, Ken Russell wrote:
> Is it worth adding a note that these require access to Google-internal
sources?

Done.

https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/ge...
File gpu/gles2_conform_support/generate_gles2_conform_tests.py (right):

https://codereview.chromium.org/105733002/diff/1/gpu/gles2_conform_support/ge...
gpu/gles2_conform_support/generate_gles2_conform_tests.py:2: # Copyright (c)
2012 The Chromium Authors. All rights reserved.
On 2013/12/05 00:40:15, Ken Russell wrote:
> May be worth updating copyright year.

Done here and all other newly added files

[Zhenyao Mo, 2013-12-05 01:50:56]

This is tested with 

1) a workspace with src-internal and with intenral_gles2_conform_tests=1

2) a workspace without src-internal (and by default
internal_gles2_conform_tests=0)

[Ken Russell (switch to Gerrit), 2013-12-05 02:22:14]

LGTM. Excellent work. Thank you for taking care of this.

[piman, 2013-12-05 02:28:02]

lgtm

[commit-bot: I haz the power, 2013-12-05 02:49:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zmo@chromium.org/105733002/50001

[commit-bot: I haz the power, 2013-12-05 03:07:30]

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

[commit-bot: I haz the power, 2013-12-05 03:18:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zmo@chromium.org/105733002/70001

[commit-bot: I haz the power, 2013-12-05 03:33:57]

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

[Zhenyao Mo, 2013-12-05 21:41:16]

Committed patchset #5 manually as r239037 (presubmit successful).

[Nico, 2014-08-01 22:41:28]

https://codereview.chromium.org/105733002/diff/70001/gpu/gles2_conform_suppor...
File gpu/gles2_conform_support/gles2_conform_test.gyp (right):

https://codereview.chromium.org/105733002/diff/70001/gpu/gles2_conform_suppor...
gpu/gles2_conform_support/gles2_conform_test.gyp:146: '-Wno-return-type',
OS==mac doesn't use cflags, this block has no effect.

https://codereview.chromium.org/105733002/diff/70001/gpu/gles2_conform_suppor...
gpu/gles2_conform_support/gles2_conform_test.gyp:149: 'LD': 'clang++',
Why does this need to change the linker?

https://codereview.chromium.org/105733002/diff/70001/gpu/gles2_conform_suppor...
gpu/gles2_conform_support/gles2_conform_test.gyp:158: '-Wno-return-type',
We try to not disable warnings for non-3rd-party code. Why is all this here? Can
we not fix the code instead?