GN: Build libvpx on POSIX x86

GN: Build libvpx on POSIX x86

libvpx now builds on POSIX x86. But ARM NEON and Windows still need more
work.

Also fixed the helper script. It now generates .gni source lists and
outputs a correct vpx_config.asm for YASM.

R=brettw@chromium.org, tomfinegan@chromium.org

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

[Alpha Left Google, 2014-09-02 22:12:00]

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

[Alpha Left Google, 2014-09-03 18:27:46]

hclam@chromium.org changed reviewers:
+ fgalligan@google.com

[Alpha Left Google, 2014-09-03 18:27:46]

+fgalligan for libvpx scripts.

[brettw, 2014-09-03 20:44:43]

lgtm

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

https://codereview.chromium.org/530193002/diff/1/BUILD.gn#newcode86
BUILD.gn:86: args += [ "-f",  "rvds" ]
two spaces

https://codereview.chromium.org/530193002/diff/1/BUILD.gn#newcode208
BUILD.gn:208: ":gen_asm_offsets_vp8"
I put a comma after this if I'm using the multiline list style.

[Alpha Left Google, 2014-09-03 23:58:20]

The CQ bit was checked by hclam@chromium.org

[Alpha Left Google, 2014-09-03 23:58:32]

The CQ bit was unchecked by hclam@chromium.org

[Alpha Left Google, 2014-09-03 23:58:58]

hclam@chromium.org changed reviewers:
+ tomfinegan@chromium.org

[Alpha Left Google, 2014-09-03 23:58:59]

+tomfinegan

[Tom Finegan, 2014-09-04 01:59:05]

https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh
File generate_gypi.sh (right):

https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode19
generate_gypi.sh:19: BASE_DIR=`pwd`
Mind $()'ing this for me? It's the only backquoted command in the script.

https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode53
generate_gypi.sh:53: rm -rf $2
quote

Here and throughout (where appropriate).

https://google-styleguide.googlecode.com/svn/trunk/shell.xml#Quoting

https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode74
generate_gypi.sh:74: declare -a file_list=("${!1}")
I would just "local readonly" this and drop the declare -a. The () are taking
care of what declare is doing.

Also... Is this really what you want? You have an array with one element
(because of the quoting). The four loop is perfectly happy operating on the
strings in ${file_list[0]}... I don't think you even need the array.

Maybe I'm confused. Bash is hard. But:

# Quoted - 1 element
$ strings="one two three"
$ array_of_strings=("${strings}")
$ echo ${#array_of_strings[@]}
1
$ echo ${array_of_strings[@]}
one two three
$ echo ${array_of_strings[0]}
one two three

# Unquoted - N elements, 3 here.
$ strings="one two three"
$ array_of_strings=(${strings})
$ echo ${#array_of_strings[@]}
3
$ echo ${array_of_strings[@]}
one two three
$ echo ${array_of_strings[0]}
one

https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode77
generate_gypi.sh:77: for f in $file_list
If you remove the quoting above: s/$file_list/${file_list[@]}

https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode248
generate_gypi.sh:248: write_gypi source_list $BASE_DIR/$2.gypi
Looks like this ship sailed a looooong time ago, but:

Shouldn't $BASE_DIR be quoted, like, everywhere? It's just `pwd`, and there's a
chance that's going to have spaces. Is the reasoning here something along the
lines of, "This is FAR from the first thing that's going to break if someone
tries to build chromium in directory that produces a path string that contains
spaces."?

[fgalligan1, 2014-09-04 16:44:46]

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

[fgalligan1, 2014-09-04 16:44:46]

https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh
File generate_gypi.sh (right):

https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode27
generate_gypi.sh:27: echo "# Copyright (c) 2013 The Chromium Authors. All rights
reserved." >> $1
2014?

[Alpha Left Google, 2014-09-04 23:09:49]

https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh
File generate_gypi.sh (right):

https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode19
generate_gypi.sh:19: BASE_DIR=`pwd`
On 2014/09/04 01:59:05, Tom Finegan wrote:
> Mind $()'ing this for me? It's the only backquoted command in the script.

Done.

https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode27
generate_gypi.sh:27: echo "# Copyright (c) 2013 The Chromium Authors. All rights
reserved." >> $1
On 2014/09/04 16:44:46, fgalligan1 wrote:
> 2014?

Done.

https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode53
generate_gypi.sh:53: rm -rf $2
On 2014/09/04 01:59:05, Tom Finegan wrote:
> quote
> 
> Here and throughout (where appropriate).
> 
> https://google-styleguide.googlecode.com/svn/trunk/shell.xml#Quoting

Done.

https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode74
generate_gypi.sh:74: declare -a file_list=("${!1}")
On 2014/09/04 01:59:05, Tom Finegan wrote:
> I would just "local readonly" this and drop the declare -a. The () are taking
> care of what declare is doing.
> 
> Also... Is this really what you want? You have an array with one element
> (because of the quoting). The four loop is perfectly happy operating on the
> strings in ${file_list[0]}... I don't think you even need the array.
> 
> Maybe I'm confused. Bash is hard. But:
> 
> # Quoted - 1 element
> $ strings="one two three"
> $ array_of_strings=("${strings}")
> $ echo ${#array_of_strings[@]}
> 1
> $ echo ${array_of_strings[@]}
> one two three
> $ echo ${array_of_strings[0]}
> one two three
> 
> # Unquoted - N elements, 3 here.
> $ strings="one two three"
> $ array_of_strings=(${strings})
> $ echo ${#array_of_strings[@]}
> 3
> $ echo ${array_of_strings[@]}
> one two three
> $ echo ${array_of_strings[0]}
> one
> 

Done.

https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode77
generate_gypi.sh:77: for f in $file_list
On 2014/09/04 01:59:05, Tom Finegan wrote:
> If you remove the quoting above: s/$file_list/${file_list[@]}

Done.

https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode248
generate_gypi.sh:248: write_gypi source_list $BASE_DIR/$2.gypi
On 2014/09/04 01:59:05, Tom Finegan wrote:
> Looks like this ship sailed a looooong time ago, but:
> 
> Shouldn't $BASE_DIR be quoted, like, everywhere? It's just `pwd`, and there's
a
> chance that's going to have spaces. Is the reasoning here something along the
> lines of, "This is FAR from the first thing that's going to break if someone
> tries to build chromium in directory that produces a path string that contains
> spaces."?

The reasoning is: there's less than 5 people in the world who will actually roll
libvpx for libvpx and who is doing it is supposed to fix the script (if it
breaks). This is really just a helper script that makes life easier for rolling
libvpx. :)

[Tom Finegan, 2014-09-05 14:00:47]

On 2014/09/04 23:09:49, Alpha wrote:
> https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh
> File generate_gypi.sh (right):
> 
>
https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode19
> generate_gypi.sh:19: BASE_DIR=`pwd`
> On 2014/09/04 01:59:05, Tom Finegan wrote:
> > Mind $()'ing this for me? It's the only backquoted command in the script.
> 
> Done.
> 
>
https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode27
> generate_gypi.sh:27: echo "# Copyright (c) 2013 The Chromium Authors. All
rights
> reserved." >> $1
> On 2014/09/04 16:44:46, fgalligan1 wrote:
> > 2014?
> 
> Done.
> 
>
https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode53
> generate_gypi.sh:53: rm -rf $2
> On 2014/09/04 01:59:05, Tom Finegan wrote:
> > quote
> > 
> > Here and throughout (where appropriate).
> > 
> > https://google-styleguide.googlecode.com/svn/trunk/shell.xml#Quoting
> 
> Done.
> 
>
https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode74
> generate_gypi.sh:74: declare -a file_list=("${!1}")
> On 2014/09/04 01:59:05, Tom Finegan wrote:
> > I would just "local readonly" this and drop the declare -a. The () are
taking
> > care of what declare is doing.
> > 
> > Also... Is this really what you want? You have an array with one element
> > (because of the quoting). The four loop is perfectly happy operating on the
> > strings in ${file_list[0]}... I don't think you even need the array.
> > 
> > Maybe I'm confused. Bash is hard. But:
> > 
> > # Quoted - 1 element
> > $ strings="one two three"
> > $ array_of_strings=("${strings}")
> > $ echo ${#array_of_strings[@]}
> > 1
> > $ echo ${array_of_strings[@]}
> > one two three
> > $ echo ${array_of_strings[0]}
> > one two three
> > 
> > # Unquoted - N elements, 3 here.
> > $ strings="one two three"
> > $ array_of_strings=(${strings})
> > $ echo ${#array_of_strings[@]}
> > 3
> > $ echo ${array_of_strings[@]}
> > one two three
> > $ echo ${array_of_strings[0]}
> > one
> > 
> 
> Done.
> 
>
https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode77
> generate_gypi.sh:77: for f in $file_list
> On 2014/09/04 01:59:05, Tom Finegan wrote:
> > If you remove the quoting above: s/$file_list/${file_list[@]}
> 
> Done.
> 
>
https://codereview.chromium.org/530193002/diff/20001/generate_gypi.sh#newcode248
> generate_gypi.sh:248: write_gypi source_list $BASE_DIR/$2.gypi
> On 2014/09/04 01:59:05, Tom Finegan wrote:
> > Looks like this ship sailed a looooong time ago, but:
> > 
> > Shouldn't $BASE_DIR be quoted, like, everywhere? It's just `pwd`, and
there's
> a
> > chance that's going to have spaces. Is the reasoning here something along
the
> > lines of, "This is FAR from the first thing that's going to break if someone
> > tries to build chromium in directory that produces a path string that
contains
> > spaces."?
> 
> The reasoning is: there's less than 5 people in the world who will actually
roll
> libvpx for libvpx and who is doing it is supposed to fix the script (if it
> breaks). This is really just a helper script that makes life easier for
rolling
> libvpx. :)

lgtm / thanks ... maybe I'll make another clean up pass through here.

[Alpha Left Google, 2014-09-05 18:19:49]

The CQ bit was checked by hclam@chromium.org

[commit-bot: I haz the power, 2014-09-05 18:23:03]

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

[commit-bot: I haz the power, 2014-09-05 18:23:04]

Commit queue rejected this change because it did not recognize the base URL.
Please commit your change manually.

[Johann, 2014-09-05 18:25:17]

On 2014/09/05 18:23:04, I haz the power (commit-bot) wrote:
> Commit queue rejected this change because it did not recognize the base URL.
> Please commit your change manually.

This may need a rebase since some mips changes landed just before this. Should
be minor though.

After that, you need to use 'gcl commit <change>' directly as we don't have a
commit queue

[Alpha Left Google, 2014-09-05 21:22:38]

Committed patchset #4 (id:60001) manually as r291834 (presubmit successful).