Add fnl build target

Add fnl build target

Using gcc 4.9 and glibc.

R=viettrungluu@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/0e7c2c3064ddc1c7764076ffa8861ef751fb4438

[cdotstout, 2015-09-15 22:43:13]

cstout@chromium.org changed reviewers:
+ jamesr@chromium.org, ppi@chromium.org, toshik@chromium.org,
viettrungluu@chromium.org

[viettrungluu, 2015-09-15 23:48:23]

You also need to update some tests (all the bots are red).

https://codereview.chromium.org/1348493002/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1348493002/diff/1/BUILD.gn#newcode37
BUILD.gn:37: } else {
Presumably this should be under "is_linux" also.

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

https://codereview.chromium.org/1348493002/diff/1/build/config/BUILDCONFIG.gn...
build/config/BUILDCONFIG.gn:165: is_fnl = false
nit: I believe all these are in alphabetical order.

https://codereview.chromium.org/1348493002/diff/1/build/config/compiler/BUILD.gn
File build/config/compiler/BUILD.gn (right):

https://codereview.chromium.org/1348493002/diff/1/build/config/compiler/BUILD...
build/config/compiler/BUILD.gn:978: if (!is_debug && !using_sanitizer &&
Why !is_debug?

https://codereview.chromium.org/1348493002/diff/1/build/config/fnl/BUILD.gn
File build/config/fnl/BUILD.gn (right):

https://codereview.chromium.org/1348493002/diff/1/build/config/fnl/BUILD.gn#n...
build/config/fnl/BUILD.gn:1: # Copyright (c) 2013 The Chromium Authors. All
rights reserved.
nit: 2015 (and no "(c)" in the current copyright header)

https://codereview.chromium.org/1348493002/diff/1/build/toolchain/fnl/BUILD.gn
File build/toolchain/fnl/BUILD.gn (right):

https://codereview.chromium.org/1348493002/diff/1/build/toolchain/fnl/BUILD.g...
build/toolchain/fnl/BUILD.gn:10: # The CrOS build system supports many different
kinds of targets across
Update this comment for FNL?

https://codereview.chromium.org/1348493002/diff/1/build/toolchain/fnl/BUILD.g...
build/toolchain/fnl/BUILD.gn:17: compiler_prefix =
"${sysroot}/../../bin/x86_64-linux-"
It seems odd to be going outside the sysroot, but I don't know what's going
on....

https://codereview.chromium.org/1348493002/diff/1/build/toolchain/fnl/BUILD.g...
build/toolchain/fnl/BUILD.gn:18: cros_target_cc = "${compiler_prefix}gcc"
Can you rename these from cros_... to fnl_...?

https://codereview.chromium.org/1348493002/diff/1/mojo/public/cpp/system/test...
File mojo/public/cpp/system/tests/macros_unittest.cc (right):

https://codereview.chromium.org/1348493002/diff/1/mojo/public/cpp/system/test...
mojo/public/cpp/system/tests/macros_unittest.cc:88: // Prevent gcc
unneeded-internal-declaration warning
nit: period at the end of the comment.

https://codereview.chromium.org/1348493002/diff/1/mojo/public/cpp/system/test...
mojo/public/cpp/system/tests/macros_unittest.cc:89:
MOJO_ALLOW_UNUSED_LOCAL(kGlobalArray);
"LOCAL". Sigh. <shrug>

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mojob.py
File mojo/tools/mojob.py (right):

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mojob.py#newcode93
mojo/tools/mojob.py:93: if args.target_sysroot != None:
nit: |'target_sysroot' in args|

Or maybe |'target_sysroot' in args and args.target_sysroot|.

(I don't know if it'll store None or the empty string, but if you need to
distinguish between the two, it should be |... and args.target_sysroot is not
None|.)

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mojob.py#newcode94
mojo/tools/mojob.py:94: additional_args['target_sysroot'] =
os.path.join(Paths().src_root,
Hmmm, this implies that target_sysroot is relative to src_root.

Probably you should specify that in the description of the argument, though it
seems awkward that you then can't specify an absolute path.

Possibly more natural would be to resolve it against the current directory
(using os.path.abspath or whatever).

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mojob.py#newcode97
mojo/tools/mojob.py:97: if target_os == Config.OS_FNL:
I'm not a fan of having exits in random functions, so it'd probably be better to
have a check like the |if args.simulator and not args.ios:| check in main().

(You probably also don't want to allow target_sysroot for non-FNL.)

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/config.py
File mojo/tools/mopy/config.py (right):

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/config.py#n...
mojo/tools/mopy/config.py:19: OS_ANDROID = "android"
These are in alphabetical order.

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/config.py#n...
mojo/tools/mopy/config.py:49: Config.OS_WINDOWS, Config.OS_FNL)
"

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/gn.py
File mojo/tools/mopy/gn.py (right):

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/gn.py#newco...
mojo/tools/mopy/gn.py:29: elif config.target_os == Config.OS_FNL:
Do these checks in alphabetical order.

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/gn.py#newco...
mojo/tools/mopy/gn.py:45: def TargetSysrootForConfig(config, src_root):
Needs a docstring.

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/gn.py#newco...
mojo/tools/mopy/gn.py:67: Config.OS_FNL)
Alphabetical order.

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/gn.py#newco...
mojo/tools/mopy/gn.py:90: elif config.target_os == Config.OS_FNL:
"

https://codereview.chromium.org/1348493002/diff/1/third_party/mesa/BUILD.gn
File third_party/mesa/BUILD.gn (right):

https://codereview.chromium.org/1348493002/diff/1/third_party/mesa/BUILD.gn#n...
third_party/mesa/BUILD.gn:6: if (is_posix) {
&& !is_clang?

Or maybe even just |if (!is_clang)|?

[cdotstout, 2015-09-16 22:01:00]

Current trybot fails don't seem related to my change.

https://codereview.chromium.org/1348493002/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1348493002/diff/1/BUILD.gn#newcode37
BUILD.gn:37: } else {
On 2015/09/15 23:48:22, viettrungluu wrote:
> Presumably this should be under "is_linux" also.

Done.

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

https://codereview.chromium.org/1348493002/diff/1/build/config/BUILDCONFIG.gn...
build/config/BUILDCONFIG.gn:165: is_fnl = false
On 2015/09/15 23:48:22, viettrungluu wrote:
> nit: I believe all these are in alphabetical order.

Done.

https://codereview.chromium.org/1348493002/diff/1/build/config/compiler/BUILD.gn
File build/config/compiler/BUILD.gn (right):

https://codereview.chromium.org/1348493002/diff/1/build/config/compiler/BUILD...
build/config/compiler/BUILD.gn:978: if (!is_debug && !using_sanitizer &&
On 2015/09/15 23:48:22, viettrungluu wrote:
> Why !is_debug?

Because, as the comment below says, defining FORTIFY_SOURCE isn't compatible
with -O0.
The is_debug check was added in chromium.

https://codereview.chromium.org/1348493002/diff/1/build/config/fnl/BUILD.gn
File build/config/fnl/BUILD.gn (right):

https://codereview.chromium.org/1348493002/diff/1/build/config/fnl/BUILD.gn#n...
build/config/fnl/BUILD.gn:1: # Copyright (c) 2013 The Chromium Authors. All
rights reserved.
On 2015/09/15 23:48:22, viettrungluu wrote:
> nit: 2015 (and no "(c)" in the current copyright header)

Done.

https://codereview.chromium.org/1348493002/diff/1/build/toolchain/fnl/BUILD.gn
File build/toolchain/fnl/BUILD.gn (right):

https://codereview.chromium.org/1348493002/diff/1/build/toolchain/fnl/BUILD.g...
build/toolchain/fnl/BUILD.gn:10: # The CrOS build system supports many different
kinds of targets across
On 2015/09/15 23:48:22, viettrungluu wrote:
> Update this comment for FNL?

Done.

https://codereview.chromium.org/1348493002/diff/1/build/toolchain/fnl/BUILD.g...
build/toolchain/fnl/BUILD.gn:17: compiler_prefix =
"${sysroot}/../../bin/x86_64-linux-"
On 2015/09/15 23:48:22, viettrungluu wrote:
> It seems odd to be going outside the sysroot, but I don't know what's going
> on....

Right, this is a buildroot specific hack.  Reworked.

https://codereview.chromium.org/1348493002/diff/1/build/toolchain/fnl/BUILD.g...
build/toolchain/fnl/BUILD.gn:18: cros_target_cc = "${compiler_prefix}gcc"
On 2015/09/15 23:48:22, viettrungluu wrote:
> Can you rename these from cros_... to fnl_...?

Done.

https://codereview.chromium.org/1348493002/diff/1/mojo/public/cpp/system/test...
File mojo/public/cpp/system/tests/macros_unittest.cc (right):

https://codereview.chromium.org/1348493002/diff/1/mojo/public/cpp/system/test...
mojo/public/cpp/system/tests/macros_unittest.cc:88: // Prevent gcc
unneeded-internal-declaration warning
On 2015/09/15 23:48:22, viettrungluu wrote:
> nit: period at the end of the comment.

Done.

https://codereview.chromium.org/1348493002/diff/1/mojo/public/cpp/system/test...
mojo/public/cpp/system/tests/macros_unittest.cc:89:
MOJO_ALLOW_UNUSED_LOCAL(kGlobalArray);
On 2015/09/15 23:48:22, viettrungluu wrote:
> "LOCAL". Sigh. <shrug>

Acknowledged.

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mojob.py
File mojo/tools/mojob.py (right):

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mojob.py#newcode93
mojo/tools/mojob.py:93: if args.target_sysroot != None:
On 2015/09/15 23:48:22, viettrungluu wrote:
> nit: |'target_sysroot' in args|
> 
> Or maybe |'target_sysroot' in args and args.target_sysroot|.
> 
> (I don't know if it'll store None or the empty string, but if you need to
> distinguish between the two, it should be |... and args.target_sysroot is not
> None|.)

Done.

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mojob.py#newcode94
mojo/tools/mojob.py:94: additional_args['target_sysroot'] =
os.path.join(Paths().src_root,
On 2015/09/15 23:48:23, viettrungluu wrote:
> Hmmm, this implies that target_sysroot is relative to src_root.
> 
> Probably you should specify that in the description of the argument, though it
> seems awkward that you then can't specify an absolute path.
> 
> Possibly more natural would be to resolve it against the current directory
> (using os.path.abspath or whatever).

Done.

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mojob.py#newcode97
mojo/tools/mojob.py:97: if target_os == Config.OS_FNL:
On 2015/09/15 23:48:23, viettrungluu wrote:
> I'm not a fan of having exits in random functions, so it'd probably be better
to
> have a check like the |if args.simulator and not args.ios:| check in main().
> 
> (You probably also don't want to allow target_sysroot for non-FNL.)

Done.

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/config.py
File mojo/tools/mopy/config.py (right):

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/config.py#n...
mojo/tools/mopy/config.py:19: OS_ANDROID = "android"
On 2015/09/15 23:48:23, viettrungluu wrote:
> These are in alphabetical order.

Done.

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/config.py#n...
mojo/tools/mopy/config.py:49: Config.OS_WINDOWS, Config.OS_FNL)
On 2015/09/15 23:48:23, viettrungluu wrote:
> "

Done.

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/gn.py
File mojo/tools/mopy/gn.py (right):

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/gn.py#newco...
mojo/tools/mopy/gn.py:29: elif config.target_os == Config.OS_FNL:
On 2015/09/15 23:48:23, viettrungluu wrote:
> Do these checks in alphabetical order.

Done.

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/gn.py#newco...
mojo/tools/mopy/gn.py:45: def TargetSysrootForConfig(config, src_root):
On 2015/09/15 23:48:23, viettrungluu wrote:
> Needs a docstring.

Unused, removed.

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/gn.py#newco...
mojo/tools/mopy/gn.py:67: Config.OS_FNL)
On 2015/09/15 23:48:23, viettrungluu wrote:
> Alphabetical order.

Done.

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mopy/gn.py#newco...
mojo/tools/mopy/gn.py:90: elif config.target_os == Config.OS_FNL:
On 2015/09/15 23:48:23, viettrungluu wrote:
> "

Done.

https://codereview.chromium.org/1348493002/diff/1/third_party/mesa/BUILD.gn
File third_party/mesa/BUILD.gn (right):

https://codereview.chromium.org/1348493002/diff/1/third_party/mesa/BUILD.gn#n...
third_party/mesa/BUILD.gn:6: if (is_posix) {
On 2015/09/15 23:48:23, viettrungluu wrote:
> && !is_clang?
> 
> Or maybe even just |if (!is_clang)|?

Copied this from build/config/compiler/BUILD.gn so it should probably be changed
in both places.

[viettrungluu, 2015-09-16 22:19:46]

lgtm w/nit

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mojob.py
File mojo/tools/mojob.py (right):

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mojob.py#newcode97
mojo/tools/mojob.py:97: if target_os == Config.OS_FNL:
On 2015/09/16 22:01:00, cdotstout wrote:
> On 2015/09/15 23:48:23, viettrungluu wrote:
> > I'm not a fan of having exits in random functions, so it'd probably be
better
> to
> > have a check like the |if args.simulator and not args.ios:| check in main().
> > 
> > (You probably also don't want to allow target_sysroot for non-FNL.)
> 
> Done.

You didn't add a check in main(). You can if you want....

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mojob.py#newcode328
mojo/tools/mojob.py:328: if args.simulator and not args.ios:
Similar to this one.

https://codereview.chromium.org/1348493002/diff/40001/mojo/tools/mopy/gn.py
File mojo/tools/mopy/gn.py (left):

https://codereview.chromium.org/1348493002/diff/40001/mojo/tools/mopy/gn.py#o...
mojo/tools/mopy/gn.py:43: 
nit: Please undelete this line.

[cdotstout, 2015-09-16 22:27:10]

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mojob.py
File mojo/tools/mojob.py (right):

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mojob.py#newcode97
mojo/tools/mojob.py:97: if target_os == Config.OS_FNL:
On 2015/09/16 22:19:46, viettrungluu wrote:
> On 2015/09/16 22:01:00, cdotstout wrote:
> > On 2015/09/15 23:48:23, viettrungluu wrote:
> > > I'm not a fan of having exits in random functions, so it'd probably be
> better
> > to
> > > have a check like the |if args.simulator and not args.ios:| check in
main().
> > > 
> > > (You probably also don't want to allow target_sysroot for non-FNL.)
> > 
> > Done.
> 
> You didn't add a check in main(). You can if you want....

I didn't put it in main() because doing so forces you to provide
target_sysroot/toolchain_prefix when doing 'build' as well as 'gn', but it's
only needed at the 'gn' step.  (Maybe I can detect gn vs build in main(), I
spent 2 mins trying then gave up).

The build system complains if you don't provide what's needed, so I didn't
bother adding a check at the script level.

https://codereview.chromium.org/1348493002/diff/1/mojo/tools/mojob.py#newcode328
mojo/tools/mojob.py:328: if args.simulator and not args.ios:
On 2015/09/16 22:19:46, viettrungluu wrote:
> Similar to this one.

Acknowledged.

https://codereview.chromium.org/1348493002/diff/40001/mojo/tools/mopy/gn.py
File mojo/tools/mopy/gn.py (left):

https://codereview.chromium.org/1348493002/diff/40001/mojo/tools/mopy/gn.py#o...
mojo/tools/mopy/gn.py:43: 
On 2015/09/16 22:19:46, viettrungluu wrote:
> nit: Please undelete this line.

Done.

[cdotstout, 2015-09-17 22:14:18]

Committed patchset #5 (id:80001) manually as
0e7c2c3064ddc1c7764076ffa8861ef751fb4438.