Add fusl BUILD.gn rules

Add fusl BUILD.gn rules

Part of #619

R=viettrungluu@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/4446eb0e2d7a3e9bca844dc343aa4094eaae9a97

[kulakowski, 2016-01-11 23:29:03]

This builds you .a. Moving headers or crt into place isn't yet done.

[kulakowski, 2016-01-11 23:33:52]

Missed the symlink as it is globally .gitignored.

[viettrungluu, 2016-01-11 23:49:10]

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn
File fusl/BUILD.gn (right):

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn#newcode1
fusl/BUILD.gn:1: static_library("fusl") {
nit: should have a copyright header (remember that it's 2016)

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn#newcode19
fusl/BUILD.gn:19: "-Werror=implicit-function-declaration",
I wonder if these -Werror=... are really needed (or if they can be implied by
something stronger, e.g., making all warnings errors). <shrug>

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn#newcode24
fusl/BUILD.gn:24: # Flags we add to get build working
We should probably fix these. :)

(Not in this change.)

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn#newcode30
fusl/BUILD.gn:30: include_dirs = [ "arch/x86_64" ]
I think eventually we'll want include_dirs (and also -ffreestanding, -nostdinc,
and maybe some others) as part of a config, until we have a toolchain hardwired
to use this. But we can worry about this later.

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn#newcode32
fusl/BUILD.gn:32: include_dirs += [
Can't you just add these to the include_dirs = ... above?

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn#newcode38
fusl/BUILD.gn:38: "src/aio/aio.c",
Probably eventually we should have separate BUILD.gn files for each of these
subdirectories (each defining a source_set), but we can worry about that later.

Note that *usually* we prefer full paths (i.e., "//fusl/src/aio/aio_suspend.c")
*or* no path (for files in the current directory), and mostly try to avoid
relative paths. But I'm OK with this for now, given the above.

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn#newcode1327
fusl/BUILD.gn:1327: "./src/fenv/x86_64/fenv.s",
Is there a reason for the "./"? And for doing |sources +=|, rather than having
it all in the above list?

I guess this is the x64-specific stuff, which is a good reason for "+=" ... but
I don't think you need the "./".

[kulakowski, 2016-01-11 23:59:16]

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn
File fusl/BUILD.gn (right):

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn#newcode1
fusl/BUILD.gn:1: static_library("fusl") {
On 2016/01/11 23:49:09, viettrungluu wrote:
> nit: should have a copyright header (remember that it's 2016)

Done.

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn#newcode19
fusl/BUILD.gn:19: "-Werror=implicit-function-declaration",
On 2016/01/11 23:49:09, viettrungluu wrote:
> I wonder if these -Werror=... are really needed (or if they can be implied by
> something stronger, e.g., making all warnings errors). <shrug>

Yeah, I'm happy addressing these over time as we rationalize this with normal
build configs. They are grouped more or less in order of weirdness, e.g. the
freestanding/nostdinc at the top.

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn#newcode24
fusl/BUILD.gn:24: # Flags we add to get build working
On 2016/01/11 23:49:09, viettrungluu wrote:
> We should probably fix these. :)
> 
> (Not in this change.)

Acknowledged.

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn#newcode30
fusl/BUILD.gn:30: include_dirs = [ "arch/x86_64" ]
On 2016/01/11 23:49:10, viettrungluu wrote:
> I think eventually we'll want include_dirs (and also -ffreestanding,
-nostdinc,
> and maybe some others) as part of a config, until we have a toolchain
hardwired
> to use this. But we can worry about this later.

sgtm

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn#newcode32
fusl/BUILD.gn:32: include_dirs += [
On 2016/01/11 23:49:09, viettrungluu wrote:
> Can't you just add these to the include_dirs = ... above?

Marked as arch specific vs not.

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn#newcode38
fusl/BUILD.gn:38: "src/aio/aio.c",
On 2016/01/11 23:49:09, viettrungluu wrote:
> Probably eventually we should have separate BUILD.gn files for each of these
> subdirectories (each defining a source_set), but we can worry about that
later.
> 
> Note that *usually* we prefer full paths (i.e.,
"//fusl/src/aio/aio_suspend.c")
> *or* no path (for files in the current directory), and mostly try to avoid
> relative paths. But I'm OK with this for now, given the above.

Made full paths. Agreed that we want to split this up over time, but how
probably depends on how we want to structure things.

https://codereview.chromium.org/1573403002/diff/20001/fusl/BUILD.gn#newcode1327
fusl/BUILD.gn:1327: "./src/fenv/x86_64/fenv.s",
On 2016/01/11 23:49:09, viettrungluu wrote:
> Is there a reason for the "./"? And for doing |sources +=|, rather than having
> it all in the above list?
> 
> I guess this is the x64-specific stuff, which is a good reason for "+=" ...
but
> I don't think you need the "./".

Fixed the ./, and mentioned that they are arch specific in a comment.

[viettrungluu, 2016-01-12 00:04:52]

lgtm, assuming green trybots

[kulakowski, 2016-01-12 00:14:03]

Description was changed from

==========
Add fusl BUILD.gn rules

Part of #619

R=viettrungluu@chromium.org
==========

to

==========
Add fusl BUILD.gn rules

Part of #619

R=viettrungluu@chromium.org

Committed:
https://chromium.googlesource.com/external/mojo/+/4446eb0e2d7a3e9bca844dc343a...
==========

[kulakowski, 2016-01-12 00:14:04]

Committed patchset #5 (id:80001) manually as
4446eb0e2d7a3e9bca844dc343aa4094eaae9a97.