Add Ozone configuration to facilitate management of external platforms

Add Ozone configuration to facilitate management of external platforms

With the old GYP files, out-of-tree builds of external Ozone platforms
used to be possible. In practice, external Ozone platforms are really
handled as forks of chromium. In order to facilitate the management of
such forks, we introduce a separate ozone_extra.gni configuration file
where fork maintainers can list external platforms.

R=spang@chromium.org
TEST=Restore the ui/ozone/platform/caca directory removed in
https://codereview.chromium.org/2445323002/, patch ozone_extra.gni to
add caca and build & run that platform.
BUG=None

Committed: https://crrev.com/e62729a0624abe7b4bc892aba15457b119665735
Cr-Commit-Position: refs/heads/master@{#429271}

[fwang, 2016-10-28 10:19:48]

PTAL

I'll add a new commit to update the doc in
https://codereview.chromium.org/2457443003/

[tonikitoo, 2016-10-31 02:50:48]

Thanks for working on it, Fred. 

On 2016/10/28 10:19:48, fwang wrote:
> PTAL
> 
> I'll add a new commit to update the doc in
> https://codereview.chromium.org/2457443003/

In GYP, supplement.gypi was able to "override" GYP placeholder variables like
'ozone_external_platform_deps'.
Is this possible with GN?

Maybe I am asking this, because it is not clear too me how an external ozone
platform can be built making use of the GN variables in ozone_extra.gni.
For example, assume that one is willing to build 'caca' platform (just removed
from ToT), as an external ozone platform. Will
https://codereview.chromium.org/2457443003/ explain how to do it?

[tonikitoo, 2016-10-31 02:51:01]

tonikitoo@igalia.com changed reviewers:
+ tonikitoo@igalia.com

[tonikitoo, 2016-10-31 02:51:02]

https://codereview.chromium.org/2460853002/diff/1/ui/ozone/ozone_extra.gni
File ui/ozone/ozone_extra.gni (right):

https://codereview.chromium.org/2460853002/diff/1/ui/ozone/ozone_extra.gni#ne...
ui/ozone/ozone_extra.gni:16: # If your platform has unit tests, you can list the
corresponding source_set
I would write it in 3rd person:

"if a platform has unit tests, the corresponding source_set can be listed here
so that..."

[tonikitoo, 2016-10-31 02:55:33]

nit: TEST line in the commit message is too long (wrap at 80 chars?)

nit2: rather than a git SHA, maybe a codereview link is better?

[fwang, 2016-10-31 10:15:58]

On 2016/10/31 02:50:48, tonikitoo wrote:
> In GYP, supplement.gypi was able to "override" GYP placeholder variables like
> 'ozone_external_platform_deps'.
> Is this possible with GN?

I'm not sure whether I understand the question correctly, but per discussion on
ozone-dev "There is no replacement for supplement.gypi in GN" and the idea
instead is just to add a way to add more platforms py "patching" that separate
ozone_extra.gni, less likely to have merge conflicts
(https://groups.google.com/a/chromium.org/forum/#!topic/ozone-dev/F9zuGhn97W4).

> Maybe I am asking this, because it is not clear too me how an external ozone
> platform can be built making use of the GN variables in ozone_extra.gni.
> For example, assume that one is willing to build 'caca' platform (just removed
> from ToT), as an external ozone platform.

So that's what I did to test the directory, basically just copy back the caca/
directory into ui/ozone/platform and then patch ozone_extra.gni with:

ozone_external_platforms = [ "caca" ]
ozone_external_platform_deps = [ "platform/caca" ]

and build normally. This is essentially reverting
https://codereview.chromium.org/2445323002/ except that you don't have a
ozone_platform_caca build config to enable/disable the platform (if you add an
external platform, it is assumed that you do build it!) and that ozone_extra.gni
contains the change that used to be in ui/ozone/BUILD.gn (in order to avoid
merge conflicts).

>  Will https://codereview.chromium.org/2457443003/ explain how to do it?

For now, it explains how to revert the caca removal (as we are sure it works)
but I guess it will make sense to explain how to build it as an external
platform too.

[fwang, 2016-10-31 10:17:45]

Description was changed from

==========
Add Ozone configuration to facilitate management of external platforms

With the old GYP files, out-of-tree builds of external Ozone platforms
used to be possible. In practice, external Ozone platforms are really
handled as forks of chromium. In order to facilitate the management of
such forks, we introduce a separate ozone_extra.gni configuration file
where fork maintainers can list external platforms.

R=spang@chromium.org
TEST=Restore the ui/ozone/platform/caca directory removed in
e7dc921a4088b26b8108032968e1c11819895e3e, patch ozone_extra.gni to add caca and
build & run that platform.
BUG=None
==========

to

==========
Add Ozone configuration to facilitate management of external platforms

With the old GYP files, out-of-tree builds of external Ozone platforms
used to be possible. In practice, external Ozone platforms are really
handled as forks of chromium. In order to facilitate the management of
such forks, we introduce a separate ozone_extra.gni configuration file
where fork maintainers can list external platforms.

R=spang@chromium.org
TEST=Restore the ui/ozone/platform/caca directory removed in
https://codereview.chromium.org/2445323002/, patch ozone_extra.gni to
add caca and build & run that platform.
BUG=None
==========

[fwang, 2016-10-31 10:27:21]

fwang@igalia.com changed reviewers:
+ rjkroege@chromium.org

[fwang, 2016-10-31 10:27:21]

@tonikitoo: I adressed your review feedback.

For now the doc to explain building external platform is

https://codereview.chromium.org/2457443003/diff2/80001:100001/docs/ozone_over...

+ what is in ozone_extra.gni

[fwang, 2016-10-31 14:06:24]

On 2016/10/31 02:50:48, tonikitoo wrote:
> Maybe I am asking this, because it is not clear too me how an external ozone
> platform can be built making use of the GN variables in ozone_extra.gni.
> For example, assume that one is willing to build 'caca' platform (just removed
> from ToT), as an external ozone platform. Will
> https://codereview.chromium.org/2457443003/ explain how to do it?

People on ozone-dev suggested that git history is enough, but just in case it's
more convenient (especially to avoid going back to the commit before the caca
removal) I copied the copied the code to GitHub too with some explanations:
https://github.com/fred-wang/ozone-caca

[rjkroege, 2016-10-31 20:41:09]

On 2016/10/31 14:06:24, fwang wrote:
> On 2016/10/31 02:50:48, tonikitoo wrote:
> > Maybe I am asking this, because it is not clear too me how an external ozone
> > platform can be built making use of the GN variables in ozone_extra.gni.
> > For example, assume that one is willing to build 'caca' platform (just
removed
> > from ToT), as an external ozone platform. Will
> > https://codereview.chromium.org/2457443003/ explain how to do it?
> 
> People on ozone-dev suggested that git history is enough, but just in case
it's
> more convenient (especially to avoid going back to the commit before the caca
> removal) I copied the copied the code to GitHub too with some explanations:
> https://github.com/fred-wang/ozone-caca

You could add this link to the doc files? That would be nice.

And as long as this builds on CrOS, lgtm

[rjkroege, 2016-10-31 20:41:46]

https://codereview.chromium.org/2460853002/diff/20001/ui/ozone/BUILD.gn
File ui/ozone/BUILD.gn (right):

https://codereview.chromium.org/2460853002/diff/20001/ui/ozone/BUILD.gn#newco...
ui/ozone/BUILD.gn:15: ozone_platforms = ozone_external_platforms
Can you test this with a simple-chrome ChromeOS build before landing?

https://codereview.chromium.org/2460853002/diff/20001/ui/ozone/ozone_extra.gni
File ui/ozone/ozone_extra.gni (right):

https://codereview.chromium.org/2460853002/diff/20001/ui/ozone/ozone_extra.gn...
ui/ozone/ozone_extra.gni:7: # config. For example ozone_external_platforms = [
"foo1", "foo2", ... ]
Could you add a realistic example -- as in what happens with CrOS?

[fwang, 2016-10-31 20:56:05]

On 2016/10/31 20:41:09, rjkroege wrote:
> You could add this link to the doc files? That would be nice.

Yes, I think it will be a good idea to do so after this and other doc CL have
landed.

https://codereview.chromium.org/2460853002/diff/20001/ui/ozone/BUILD.gn
File ui/ozone/BUILD.gn (right):

https://codereview.chromium.org/2460853002/diff/20001/ui/ozone/BUILD.gn#newco...
ui/ozone/BUILD.gn:15: ozone_platforms = ozone_external_platforms
On 2016/10/31 20:41:46, rjkroege wrote:
> Can you test this with a simple-chrome ChromeOS build before landing?

OK, I have not tested caca on ChromeOS because software rendering is not allowed
but I can try it using
https://groups.google.com/a/chromium.org/group/ozone-dev/attach/4a886fa59b97b...

https://codereview.chromium.org/2460853002/diff/20001/ui/ozone/ozone_extra.gni
File ui/ozone/ozone_extra.gni (right):

https://codereview.chromium.org/2460853002/diff/20001/ui/ozone/ozone_extra.gn...
ui/ozone/ozone_extra.gni:7: # config. For example ozone_external_platforms = [
"foo1", "foo2", ... ]
On 2016/10/31 20:41:46, rjkroege wrote:
> Could you add a realistic example -- as in what happens with CrOS?

Not sure I follow the connection with CrOS... These are just a list of Ozone
platforms and should be independent of the target OS.

[fwang, 2016-11-02 12:50:08]

The CQ bit was checked by fwang@igalia.com

[commit-bot: I haz the power, 2016-11-02 12:50:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-02 14:11:44]

Description was changed from

==========
Add Ozone configuration to facilitate management of external platforms

With the old GYP files, out-of-tree builds of external Ozone platforms
used to be possible. In practice, external Ozone platforms are really
handled as forks of chromium. In order to facilitate the management of
such forks, we introduce a separate ozone_extra.gni configuration file
where fork maintainers can list external platforms.

R=spang@chromium.org
TEST=Restore the ui/ozone/platform/caca directory removed in
https://codereview.chromium.org/2445323002/, patch ozone_extra.gni to
add caca and build & run that platform.
BUG=None
==========

to

==========
Add Ozone configuration to facilitate management of external platforms

With the old GYP files, out-of-tree builds of external Ozone platforms
used to be possible. In practice, external Ozone platforms are really
handled as forks of chromium. In order to facilitate the management of
such forks, we introduce a separate ozone_extra.gni configuration file
where fork maintainers can list external platforms.

R=spang@chromium.org
TEST=Restore the ui/ozone/platform/caca directory removed in
https://codereview.chromium.org/2445323002/, patch ozone_extra.gni to
add caca and build & run that platform.
BUG=None
==========

[commit-bot: I haz the power, 2016-11-02 14:11:45]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-02 14:14:12]

Description was changed from

==========
Add Ozone configuration to facilitate management of external platforms

With the old GYP files, out-of-tree builds of external Ozone platforms
used to be possible. In practice, external Ozone platforms are really
handled as forks of chromium. In order to facilitate the management of
such forks, we introduce a separate ozone_extra.gni configuration file
where fork maintainers can list external platforms.

R=spang@chromium.org
TEST=Restore the ui/ozone/platform/caca directory removed in
https://codereview.chromium.org/2445323002/, patch ozone_extra.gni to
add caca and build & run that platform.
BUG=None
==========

to

==========
Add Ozone configuration to facilitate management of external platforms

With the old GYP files, out-of-tree builds of external Ozone platforms
used to be possible. In practice, external Ozone platforms are really
handled as forks of chromium. In order to facilitate the management of
such forks, we introduce a separate ozone_extra.gni configuration file
where fork maintainers can list external platforms.

R=spang@chromium.org
TEST=Restore the ui/ozone/platform/caca directory removed in
https://codereview.chromium.org/2445323002/, patch ozone_extra.gni to
add caca and build & run that platform.
BUG=None

Committed: https://crrev.com/e62729a0624abe7b4bc892aba15457b119665735
Cr-Commit-Position: refs/heads/master@{#429271}
==========

[commit-bot: I haz the power, 2016-11-02 14:14:13]

Patchset 2 (id:??) landed as
https://crrev.com/e62729a0624abe7b4bc892aba15457b119665735
Cr-Commit-Position: refs/heads/master@{#429271}

[fwang, 2016-11-02 14:31:05]

I opened https://codereview.chromium.org/2473713002/ for the caca doc. I'm still
not sure what was expected for chromeos=1 since the way you use ozone_extra.gni
is independent of the target os... The doc in ozone_extra.gni is supposed to
very generic and that file should not change too much if we want to avoid build
conflicts for out-of-tree maintainers.