Fix solink and add a postsolink hook (use the hook for android strip)

Fix solink and add a postsolink hook (use the hook for android strip)

The solink command included the "real" link command twice and only used
the libs_section_prefix/postfix in one of the two. This splits the
construction of the link command up a bit to make it clearer.

This adds a simple post_solink arg to gcc_toolchain that allows the
caller to add more stuff to the solink command. This is used to
automatically strip all shared libraries in Android builds (in gyp, we
do this for a library once for each apk that it is in :/ ).

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

[cjhopman, 2014-07-07 23:00:50]

https://codereview.chromium.org/372813002/diff/1/build/toolchain/android/BUIL...
File build/toolchain/android/BUILD.gn (right):

https://codereview.chromium.org/372813002/diff/1/build/toolchain/android/BUIL...
build/toolchain/android/BUILD.gn:44: mkdir_command = "mkdir -p lib.stripped"
This mkdir isn't needed if ninja knows about the output file.

https://codereview.chromium.org/372813002/diff/1/build/toolchain/gcc_toolchai...
File build/toolchain/gcc_toolchain.gni (right):

https://codereview.chromium.org/372813002/diff/1/build/toolchain/gcc_toolchai...
build/toolchain/gcc_toolchain.gni:87: # the outputs of a solink command.
Currently, it looks like gn doesn't know about the .TOC output at all. I don't
know what the plan for that is. That is more complicated then just specifying it
as an output of these actions, though, since it should actually be used as the
inputs to other libraries (instead of the .so file).

Still, if there is no way for this "tool" to specify its outputs, then there's
no good way for the postsolink hook below to add outputs (or rather, there's no
way for ninja to know about such added outputs).

https://codereview.chromium.org/372813002/diff/1/build/toolchain/gcc_toolchai...
build/toolchain/gcc_toolchain.gni:92: command = "$link_command && $toc_command
&& $replace_command"
This is slightly different from the previous version. That one would be
something like:

$if_not_exists; then $link_command && $toc_command > \${lib}.TOC; else
$link_command && $toc_command > \${lib}.tmp && replace_command; fi

Afaict, `cmp` will fail if a file doesn't exist and so there isn't really a need
for the if/else.

It's possible that there is some platform where this doesn't do what I expect,
though (mac maybe?).

[brettw, 2014-07-08 22:06:54]

lgtm

https://codereview.chromium.org/372813002/diff/1/build/toolchain/android/BUIL...
File build/toolchain/android/BUILD.gn (right):

https://codereview.chromium.org/372813002/diff/1/build/toolchain/android/BUIL...
build/toolchain/android/BUILD.gn:44: mkdir_command = "mkdir -p lib.stripped"
On 2014/07/07 23:00:50, cjhopman wrote:
> This mkdir isn't needed if ninja knows about the output file.

Can you add a comment above this line about why it's needed?

https://codereview.chromium.org/372813002/diff/1/build/toolchain/gcc_toolchai...
File build/toolchain/gcc_toolchain.gni (right):

https://codereview.chromium.org/372813002/diff/1/build/toolchain/gcc_toolchai...
build/toolchain/gcc_toolchain.gni:87: # the outputs of a solink command.
I don't know the plan for this either, something we need to figure out.
Currently the outputs and such are hardcoded.

https://codereview.chromium.org/372813002/diff/1/build/toolchain/gcc_toolchai...
build/toolchain/gcc_toolchain.gni:94: if (defined(invoker.postsolink)) {
Can you add the extra stuff you added to the optional parameters documentation
above?

[Nico, 2014-07-08 23:02:29]

https://codereview.chromium.org/372813002/diff/1/build/toolchain/android/BUIL...
File build/toolchain/android/BUILD.gn (right):

https://codereview.chromium.org/372813002/diff/1/build/toolchain/android/BUIL...
build/toolchain/android/BUILD.gn:44: mkdir_command = "mkdir -p lib.stripped"
Can we not use mkdir? It's not available on Windows (well, it is, but not with a
'-p' flag I think), and I thought one of the points of gn was to make
chrome/android hostable on more platforms

[brettw, 2014-07-09 00:00:12]

I'm happy if we find a different way to do this, but I've never personally
considered hosting Android on more platforms.

[cjhopman, 2014-07-11 20:48:50]

https://codereview.chromium.org/372813002/diff/1/build/toolchain/android/BUIL...
File build/toolchain/android/BUILD.gn (right):

https://codereview.chromium.org/372813002/diff/1/build/toolchain/android/BUIL...
build/toolchain/android/BUILD.gn:44: mkdir_command = "mkdir -p lib.stripped"
On 2014/07/08 23:02:29, Nico (away) wrote:
> Can we not use mkdir? It's not available on Windows (well, it is, but not with
a
> '-p' flag I think), and I thought one of the points of gn was to make
> chrome/android hostable on more platforms
The right way to do this is to tell ninja about the output file in that
directory.

The next most "correct" way would probably be to make a separate target that
creates this directory (or actually just a file in it), and then have these
toolchains depend on that target. That's a bit more complicated than I would
like to do just to potentially make it easier to support other platforms...

I really only need the -p so that it doesn't fail if the directory already
exists, can i do (mkdir lib.stripped || true)? How about (if [ ! -e
lib.stripped]; then mkdir lib.stripped; fi)?

Really, my preference for this postsolink hook would be that I could write it as
a normal gn action (or whatever else) that just run as part of solink and had
access to the outputs. Then it would be easy to correctly handle other platforms
(or rather, it wouldn't be a special case and could be handled like all the
other gn stuff).

[cjhopman, 2014-07-29 17:15:27]

The CQ bit was checked by cjhopman@chromium.org

[commit-bot: I haz the power, 2014-07-29 17:16:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/cjhopman@chromium.org/372813002/60001

[commit-bot: I haz the power, 2014-07-29 20:15:34]

Change committed as 286282