GN: Add symlink rules for dump_syms, symupload

build/config/symlink.gni

+# Copyright 2015 The Chromium Authors. All rights reserved.

[brettw, 2015/12/10 00:12:21]

This file should go with the script, so build/symlink.gni.

[agrieve, 2015/12/10 11:07:38]

Done.

+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+# Creates a symlink.
+# Args:
+#   to: Path to link to.
+#   from: Path to link from (default=root_out_dir/target_name)
+template("symlink") {

[brettw, 2015/12/10 00:12:21]

I think it would be nice if this mirrored the "copy" syntax as much as possible.
So use "source" and "output" (since you only take one file, lose the plural).

This would also help my confusion about what the from and to of a link are
(since the operation creates the thing in the opposite direction of the link).

[agrieve, 2015/12/10 11:07:38]

Done.

+  action(target_name) {
+    forward_variables_from(invoker,
+                           [
+                             "deps",
+                             "public_deps",
+                             "data_deps",
+                             "testonly",
+                             "visibility",
+                           ])
+    _to = invoker.to

[brettw, 2015/12/10 00:12:21]

I find the underscores weird to read and they don't do anything. The only time
to use underscores is for variables in the outer scope of a .gni file, since
they prevent exporting those symbols. Inside a template, it doesn't do anything.

[agrieve, 2015/12/10 11:07:38]

This pattern is actually fairly common, at least within Android-related files. 

I think they are a good idea because they make it clear that the variables are
not used as arguments to templates. So now if you see one without an _, you know
that it's data being passed to a template.

+
+    if (defined(invoker.from)) {
+      _from = invoker.from
+    } else {
+      _from = "$root_out_dir/${invoker.target_name}"
+    }
+
+    # TODO(agrieve): Declaring the output as _from triggers a

[brettw, 2015/12/10 00:12:21]

Fix: https://codereview.chromium.org/1506343003

We should really list the generated output here. If you don't want to wait for
that fix to roll in for this change to land, you can provide a different target
name as the output file which I believe will sidestep this issue.

[agrieve, 2015/12/10 11:07:38]

Great! I cherry-picked this change into a test branch in order to unblock
testing of the building-official-with-GN. So, I'm happy to wait for the fix to
roll. Changed to list the actual output.

+    # "multiple rules generate foo" warning in ninja when there is a target with
+    # the same name in a different toolchain
+    _stamp = "$target_gen_dir/$target_name.stamp"
+    outputs = [
+      _stamp,
+    ]
+    script = "//build/symlink.py"
+    args = [
+      "-f",
+      "--touch",
+      rebase_path(_stamp, root_build_dir),
+      rebase_path(_to, get_path_info(_from, "dir")),
+      rebase_path(_from, root_build_dir),
+    ]
+  }
+}
+
+# Creates a convenience symlink from root_build_dir/target_name to
+# host_tool_chain/target_name.
+# Note that for executables, using a copy (as opposed to a symlink) does not
+# work when is_component_build=true, since dependent libraries are found via
+# relative location.
+# Args:
+#   target_dep: Target that builds the file to symlink to
+#       (default=":${invoker.target_name}($host_toolchain)").
+#   to: File to symlink to (default="target_dep's out_dir/target_name")
+template("host_symlink") {

[brettw, 2015/12/10 00:12:21]

I find this:
  host_sym_link("foo") {
  }
really weird. It looks like an empty group that does nothing.

My first preference would be to just not do this template. But if we're going to
be doing this a nontrivial number of times, it will definitely get tedious to do
the get_label_info dance below.

In that case, I'd propose usage look more like:
  symlink_binary("foo") {
    binary_label = "//foo/bar($host_toolchain)"
    output = "$root_build_dir/foo"
  }
which makes it really clear what's going on, is more general, and is not
annoyingly repetitive or easy to mess up. The annoying get_label_info part is
still encapsulated.

[agrieve, 2015/12/10 11:07:38]

Made binary_label mandatory, but left output optional (feels similar to to
_binary, where output_name is optional).

+  symlink(target_name) {
+    _target_dep = ":${invoker.target_name}($host_toolchain)"
+    if (defined(invoker.target_dep)) {
+      _target_dep = invoker.target_dep
+    }
+
+    forward_variables_from(invoker,
+                           [
+                             "testonly",
+                             "visibility",
+                           ])
+
+    # The target doesn't need to be built before creating a symlink to it.
+    data_deps = [

[brettw, 2015/12/10 00:12:21]

I don't think you want to use data deps here. Data deps means the file is
required at runtime, and will be added to isolate files for executing tests. I
don't think you want that to happen with these binaries. Just use regular
"deps", I don't think this will unreasonable restrict the parallelism of the
build.

[agrieve, 2015/12/10 11:07:38]

Done.

+      _target_dep,
+    ]
+
+    if (defined(invoker.to)) {
+      to = invoker.to
+    } else {
+      _out_dir = get_label_info(_target_dep, "root_out_dir")
+      to = "$_out_dir/" + get_label_info(_target_dep, "name")
+    }
+  }
+}