build CrOS chrome with gn

build CrOS chrome with gn

Augment the CrOS gn build to successfully build CrOS Chrome with gn.

BUG=558623
Committed: https://crrev.com/294d703324a8914ca636209b32fca94034ce9e16
Cr-Commit-Position: refs/heads/master@{#363616}

[rjkroege, 2015-11-30 21:57:46]

The CQ bit was checked by rjkroege@chromium.org to run a CQ dry run

[rjkroege, 2015-11-30 21:58:36]

rjkroege@chromium.org changed reviewers:
+ dpranke@chromium.org

[rjkroege, 2015-11-30 21:58:36]

ptal

[commit-bot: I haz the power, 2015-11-30 22:00:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1491463002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1491463002/1

[Peter Mayo, 2015-11-30 22:11:24]

petermayo@chromium.org changed reviewers:
+ petermayo@chromium.org

[Peter Mayo, 2015-11-30 22:11:24]

This looks like it is replacing the build to run the chromeos UI on Linux with
the cross-compiled variety.

https://codereview.chromium.org/1491463002/diff/1/build/config/BUILDCONFIG.gn
File build/config/BUILDCONFIG.gn (right):

https://codereview.chromium.org/1491463002/diff/1/build/config/BUILDCONFIG.gn...
build/config/BUILDCONFIG.gn:187: _default_toolchain =
"//build/toolchain/cros:clang_$target_cpu"
I don't think this is the intent.

[rjkroege, 2015-11-30 22:34:57]

On 2015/11/30 22:11:24, Peter Mayo wrote:
> This looks like it is replacing the build to run the chromeos UI on Linux with
> the cross-compiled variety.
> 
> https://codereview.chromium.org/1491463002/diff/1/build/config/BUILDCONFIG.gn
> File build/config/BUILDCONFIG.gn (right):
> 
>
https://codereview.chromium.org/1491463002/diff/1/build/config/BUILDCONFIG.gn...
> build/config/BUILDCONFIG.gn:187: _default_toolchain =
> "//build/toolchain/cros:clang_$target_cpu"
> I don't think this is the intent.

I don't understand this comment. What is the intent that you calling out? 

I note in passing that the code as written did not work: board-specific gn
builds would not succeed. When changed in this fashion, it becomes possible to
use a board-specific args.gn to get a working Chrome for ChromeOS board build
and chromeos builds would appear to stay successful. However, it's entirely
possible that this change is the wrong way to accomplish my objective. So can
you suggest what I should do instead?

[commit-bot: I haz the power, 2015-11-30 23:57:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-30 23:57:25]

Dry run: This issue passed the CQ dry run.

[Peter Mayo, 2015-12-01 00:51:21]

I withdraw my previous comments.

I don't understand what's going on, so I'm just going to jump off the review.

https://codereview.chromium.org/1491463002/diff/1/build/config/BUILDCONFIG.gn
File build/config/BUILDCONFIG.gn (right):

https://codereview.chromium.org/1491463002/diff/1/build/config/BUILDCONFIG.gn...
build/config/BUILDCONFIG.gn:187: _default_toolchain =
"//build/toolchain/cros:clang_$target_cpu"
On 2015/11/30 22:11:24, Peter Mayo wrote:
> I don't think this is the intent.

I see that without this the other code the other code is all dead, and so this
may be the new intent.

[Peter Mayo, 2015-12-01 00:51:24]

I withdraw my previous comments.

I don't understand what's going on, so I'm just going to jump off the review.

[Peter Mayo, 2015-12-01 00:51:39]

Description was changed from

==========
build CrOS chrome with gn

Augment the CrOS gn build to successfully build CrOS Chrome with gn.

BUG=558623
==========

to

==========
build CrOS chrome with gn

Augment the CrOS gn build to successfully build CrOS Chrome with gn.

BUG=558623
==========

[Peter Mayo, 2015-12-01 00:51:39]

petermayo@chromium.org changed reviewers:
- petermayo@chromium.org

[rjkroege, 2015-12-01 01:26:52]

On 2015/12/01 00:51:24, Peter Mayo wrote:
> I withdraw my previous comments.
> 
> I don't understand what's going on, so I'm just going to jump off the review.

Per my previous comment: with this patch and the following args.gn, I got a
running Chrome build with gn for an auron_paine device.

# Build arguments go here. Examples:
#   is_component_build = true
#   is_debug = false
# See "gn args <out_dir> --list" for available build arguments.
is_debug = false
target_os = "chromeos"
target_cpu = "x64"
use_goma = true

# need to use the specific version of goma for CrOS.
goma_dir = "/usr/local/google/home/rjkroege/g/.cros_cache/common/goma+2"

# Board config wanted gcc.
is_clang = false

# Board uses ozone
use_ozone = true
ozone_platform_gbm = true

# Specify the cros compilers. Mandatory for a board build.
cros_target_cc = "x86_64-cros-linux-gnu-gcc
-B/usr/local/google/home/rjkroege/g/.cros_cache/chrome-sdk/tarballs/auron_paine+7657.0.0+target_toolchain/usr/x86_64-pc-linux-gnu/x86_64-cros-linux-gnu/binutils-bin/2.25.51-gold"
cros_target_cxx = "x86_64-cros-linux-gnu-g++
-B/usr/local/google/home/rjkroege/g/.cros_cache/chrome-sdk/tarballs/auron_paine+7657.0.0+target_toolchain/usr/x86_64-pc-linux-gnu/x86_64-cros-linux-gnu/binutils-bin/2.25.51-gold"
cros_target_ar = "x86_64-cros-linux-gnu-gcc-ar"


use_libjpeg_turbo = true
treat_warnings_as_errors = false
# Probably doesn't matter.
# use_brlapi= true

linux_use_gold_flags=true
linux_use_debug_fission= false
use_vtable_verify=false
host_clang=true
disable_nacl=false
use_evdev_gestures=true
target_arch= "x64"

# Do I need this?
# target_cpu = "x64"
clang_use_chrome_plugins=true
system_libdir= "lib64"

# audio...
use_cras= true
# The above should make this one not necessary. But it seems to be.
use_pulseaudio = false

# Need this to find the libs and stuff.
target_sysroot=
"/usr/local/google/home/rjkroege/g/.cros_cache/chrome-sdk/tarballs/auron_paine+7644.0.0+sysroot_chromeos-base_chromeos-chrome.tar.xz"

linux_link_libbrlapi=true
use_v4l2_codec= true 
use_xkbcommon= true
clang = false
linux_use_bundled_binutils=false
use_v4lplugin= false
linux_use_bundled_gold=false
icu_use_data_file_flag=true
asan=false
use_mesa_platform_null= true
use_system_minigbm=true
ozone_auto_platforms=false
use_vgem_map= true
internal_khronos_glcts_tests=true

ozone_platform= "gbm"
use_mesa_platform_null = true

release_extra_cflags = "-g"

[Dirk Pranke, 2015-12-01 02:01:44]

Okay, after having looked at this a bit and talked to Robert, I can see how some
of the confusion arises.

By default, what we want to have happen is for target_os="chromeos" to not do a
real cross-compile, so devs can build and test things on desktop as they do
today. 

That means that we want to use something that uses the same executables and same
compiler flags as the linux clang_x64 toolchain, which is why setting the
default_toolchain to linux:clang_x64 before made sense. 

In order for the cros_target_* args to take effect, you had to use the
custom_toolchain as well.

However, I can also see how it makes sense to just always set the toolchain to
cros:clang_x64 and then have the custom logic inside
//build/toolchain/cros/BUILD.gn (which is the approach taken in this CL).

I'm not sure it matters too much which approach we take. 

I've asked Robert to add some more comments explaining the difference between
the non-cross-compile build and the cross-compile (real) builds to the file.

I suspect we can probably kill off the "custom_toolchain" arg completely with
this patch, as I think the only reason we had it was for CrOS, but we might want
to make a separate change and see if anyone else (opera, imgtec, etc.) needs it.

[rjkroege, 2015-12-01 19:31:46]

On 2015/12/01 02:01:44, Dirk Pranke wrote:
> Okay, after having looked at this a bit and talked to Robert, I can see how
some
> of the confusion arises.
> 
> By default, what we want to have happen is for target_os="chromeos" to not do
a
> real cross-compile, so devs can build and test things on desktop as they do
> today. 
> 
> That means that we want to use something that uses the same executables and
same
> compiler flags as the linux clang_x64 toolchain, which is why setting the
> default_toolchain to linux:clang_x64 before made sense. 
> 
> In order for the cros_target_* args to take effect, you had to use the
> custom_toolchain as well.
> 
> However, I can also see how it makes sense to just always set the toolchain to
> cros:clang_x64 and then have the custom logic inside
> //build/toolchain/cros/BUILD.gn (which is the approach taken in this CL).
> 
> I'm not sure it matters too much which approach we take. 
> 
> I've asked Robert to add some more comments explaining the difference between
> the non-cross-compile build and the cross-compile (real) builds to the file.

done. PTAL

> 
> I suspect we can probably kill off the "custom_toolchain" arg completely with
> this patch, as I think the only reason we had it was for CrOS, but we might
want
> to make a separate change and see if anyone else (opera, imgtec, etc.) needs
it.

[Dirk Pranke, 2015-12-01 20:13:48]

the comments at the top look good.

https://codereview.chromium.org/1491463002/diff/20001/build/toolchain/cros/BU...
File build/toolchain/cros/BUILD.gn (right):

https://codereview.chromium.org/1491463002/diff/20001/build/toolchain/cros/BU...
build/toolchain/cros/BUILD.gn:44: clang_toolchain("clang_x64") {
is there really a reason to support both the _x64 and _x86 toolchains? It seems
like a board-specific build would need to set target_cpu explicitly, no? Is
there much advantage to supporting a 32-bit linux compilation target if it isn't
also board-specific ?

[rjkroege, 2015-12-02 00:22:39]

ptal

https://codereview.chromium.org/1491463002/diff/20001/build/toolchain/cros/BU...
File build/toolchain/cros/BUILD.gn (right):

https://codereview.chromium.org/1491463002/diff/20001/build/toolchain/cros/BU...
build/toolchain/cros/BUILD.gn:44: clang_toolchain("clang_x64") {
On 2015/12/01 20:13:48, Dirk Pranke wrote:
> is there really a reason to support both the _x64 and _x86 toolchains? 

Probably not? I have simplified the file.

> It seems
> like a board-specific build would need to set target_cpu explicitly, no? Is
> there much advantage to supporting a 32-bit linux compilation target if it
isn't
> also board-specific ?

[Dirk Pranke, 2015-12-02 00:25:17]

https://codereview.chromium.org/1491463002/diff/40001/build/config/BUILDCONFI...
File build/config/BUILDCONFIG.gn (right):

https://codereview.chromium.org/1491463002/diff/40001/build/config/BUILDCONFI...
build/config/BUILDCONFIG.gn:188: _default_toolchain =
"//build/toolchain/cros:clang_$target_cpu"
If someone tries to do a build with target_os="chromeos" target_cpu="arm",
this'll break.

I suggest we either get rid of the $target_cpu part, or at least assert that
it's == host_cpu .

[rjkroege, 2015-12-03 18:48:58]

rjkroege@chromium.org changed reviewers:
+ jam@chromium.org, sky@chromium.org

[rjkroege, 2015-12-03 18:48:59]

dpranke@: ptal. 

sky@: for the linux_sandbox.cc change. This is necessary to cross-compile for
ARM CrOS.

jam@: for the change to content/common/BUILD.gn. ARM CrOS builds set
use_v4l2_codec and so require the third_party/libyuv dependency if webrtc is
off.

https://codereview.chromium.org/1491463002/diff/40001/build/config/BUILDCONFI...
File build/config/BUILDCONFIG.gn (right):

https://codereview.chromium.org/1491463002/diff/40001/build/config/BUILDCONFI...
build/config/BUILDCONFIG.gn:188: _default_toolchain =
"//build/toolchain/cros:clang_$target_cpu"
On 2015/12/02 00:25:17, Dirk Pranke wrote:
> If someone tries to do a build with target_os="chromeos" target_cpu="arm",
> this'll break.
> 
> I suggest we either get rid of the $target_cpu part, or at least assert that
> it's == host_cpu .

Done. And verified that (with the right args.gn) that I can build at least
content shell for ARM

[Dirk Pranke, 2015-12-03 18:52:08]

closer ... one question, because if your change actually works for arm, I'm not
sure I understand why :).

https://codereview.chromium.org/1491463002/diff/60001/build/toolchain/cros/BU...
File build/toolchain/cros/BUILD.gn (right):

https://codereview.chromium.org/1491463002/diff/60001/build/toolchain/cros/BU...
build/toolchain/cros/BUILD.gn:45: toolchain_cpu = "x64"
is this right? wouldn't you need this to be 

  toolchain_cpu = target_cpu 

?

(same question below)

[rjkroege, 2015-12-03 19:52:31]

ptal

https://codereview.chromium.org/1491463002/diff/60001/build/toolchain/cros/BU...
File build/toolchain/cros/BUILD.gn (right):

https://codereview.chromium.org/1491463002/diff/60001/build/toolchain/cros/BU...
build/toolchain/cros/BUILD.gn:45: toolchain_cpu = "x64"
On 2015/12/03 18:52:08, Dirk Pranke wrote:
> is this right? wouldn't you need this to be 
> 
>   toolchain_cpu = target_cpu 
> 
> ?
> 
> (same question below)


Done.

[Dirk Pranke, 2015-12-03 19:54:09]

lgtm

[rjkroege, 2015-12-03 20:05:29]

rjkroege@chromium.org changed reviewers:
+ brettw@chromium.org

[rjkroege, 2015-12-03 20:05:30]

brettw@chromium.org: Please review changes in build/config/BUILDCONFIG.gn

[rjkroege, 2015-12-03 20:05:56]

The CQ bit was checked by rjkroege@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-03 20:07:12]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1491463002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1491463002/100001

[commit-bot: I haz the power, 2015-12-03 20:42:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-03 20:42:23]

Dry run: This issue passed the CQ dry run.

[sky, 2015-12-03 20:54:23]

sky@chromium.org changed reviewers:
+ erg@chromium.org

[sky, 2015-12-03 20:54:23]

LGTM
+erg

[Elliot Glaysher, 2015-12-03 20:55:55]

lgtm

[jam, 2015-12-04 05:13:40]

lgtm

[brettw, 2015-12-07 20:22:01]

lgtm

[rjkroege, 2015-12-07 20:24:10]

The CQ bit was checked by rjkroege@chromium.org

[rjkroege, 2015-12-07 20:24:11]

The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org
Link to the patchset: https://codereview.chromium.org/1491463002/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2015-12-07 20:26:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1491463002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1491463002/100001

[commit-bot: I haz the power, 2015-12-07 22:56:37]

Description was changed from

==========
build CrOS chrome with gn

Augment the CrOS gn build to successfully build CrOS Chrome with gn.

BUG=558623
==========

to

==========
build CrOS chrome with gn

Augment the CrOS gn build to successfully build CrOS Chrome with gn.

BUG=558623
==========

[commit-bot: I haz the power, 2015-12-07 22:56:38]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-12-07 22:57:40]

Description was changed from

==========
build CrOS chrome with gn

Augment the CrOS gn build to successfully build CrOS Chrome with gn.

BUG=558623
==========

to

==========
build CrOS chrome with gn

Augment the CrOS gn build to successfully build CrOS Chrome with gn.

BUG=558623

Committed: https://crrev.com/294d703324a8914ca636209b32fca94034ce9e16
Cr-Commit-Position: refs/heads/master@{#363616}
==========

[commit-bot: I haz the power, 2015-12-07 22:57:42]

Patchset 6 (id:??) landed as
https://crrev.com/294d703324a8914ca636209b32fca94034ce9e16
Cr-Commit-Position: refs/heads/master@{#363616}