commit 2691ec86d4edf1b746f72ef159cda55df8d66446
author Haowei Wu <haowei@google.com> Fri Apr 19 16:52:55 2019 -0700
committer Commit Bot <commit-bot@chromium.org> Mon Apr 22 18:27:25 2019 +0000
tree 925939f46db29285d791b860c9f861e3a18d644a
parent 78db9de5aed44f03763176ac6067fb1a73798684

Canonicalize metadata walk_keys list before comaparing.

This change patches Target::GetMetadata function to canonicalize
walk_keys before comparing with deps.

Bug: crbug.com/gn/68
Test: Local and CQ
Change-Id: I15152dfa01f76538fdfb95a839b62134006d3195
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/4680
Reviewed-by: Julie Hockett <juliehockett@google.com>
Commit-Queue: Haowei Wu <haowei@google.com>

tools/gn/metadata_walk_unittest.cc

diff --git a/tools/gn/metadata_walk_unittest.cc b/tools/gn/metadata_walk_unittest.cc
index 634a2c5..a18a07e 100644
--- a/tools/gn/metadata_walk_unittest.cc
+++ b/tools/gn/metadata_walk_unittest.cc
@@ -203,8 +203,8 @@
   EXPECT_TRUE(result.empty());
   EXPECT_TRUE(err.has_error());
   EXPECT_EQ(err.message(),
-            "I was expecting //foo:missing to be a dependency of "
-            "//foo:one(//toolchain:default). "
+            "I was expecting //foo:missing(//toolchain:default) to be a "
+            "dependency of //foo:one(//toolchain:default). "
             "Make sure it's included in the deps or data_deps, and that you've "
             "specified the appropriate toolchain.")
       << err.message();

tools/gn/target.cc

diff --git a/tools/gn/target.cc b/tools/gn/target.cc
index 94fb994..376e09f 100644
--- a/tools/gn/target.cc
+++ b/tools/gn/target.cc
@@ -917,6 +917,7 @@
   // Gather walk keys and find the appropriate target. Targets identified in
   // the walk key set must be deps or data_deps of the declaring target.
   const DepsIteratorRange& all_deps = GetDeps(Target::DEPS_ALL);
+  const SourceDir current_dir("//");
   for (const auto& next : next_walk_keys) {
     DCHECK(next.type() == Value::STRING);
 
@@ -941,10 +942,19 @@
     }
 
     // Otherwise, look through the target's deps for the specified one.
+    // Canonicalize the label if possible.
+    Label next_label =
+        Label::Resolve(current_dir, settings()->toolchain_label(), next, err);
+    if (next_label.is_null()) {
+      *err = Err(next.origin(), std::string("Failed to canonicalize ") +
+                                    next.string_value() + std::string("."));
+    }
+    std::string canonicalize_next_label = next_label.GetUserVisibleName(true);
+
     bool found_next = false;
     for (const auto& dep : all_deps) {
       // Match against the label with the toolchain.
-      if (dep.label.GetUserVisibleName(true) == next.string_value()) {
+      if (dep.label.GetUserVisibleName(true) == canonicalize_next_label) {
         // If we haven't walked this dep yet, go down into it.
         auto pair = targets_walked->insert(dep.ptr);
         if (pair.second) {
@@ -961,7 +971,7 @@
     // Propagate it back to the user.
     if (!found_next) {
       *err = Err(next.origin(),
-                 std::string("I was expecting ") + next.string_value() +
+                 std::string("I was expecting ") + canonicalize_next_label +
                      std::string(" to be a dependency of ") +
                      label().GetUserVisibleName(true) +
                      ". Make sure it's included in the deps or data_deps, and "

tools/gn/target_unittest.cc

diff --git a/tools/gn/target_unittest.cc b/tools/gn/target_unittest.cc
index 673422e..b6ad0ff 100644
--- a/tools/gn/target_unittest.cc
+++ b/tools/gn/target_unittest.cc
@@ -1252,8 +1252,8 @@
                   &err);
   EXPECT_TRUE(err.has_error());
   EXPECT_EQ(err.message(),
-            "I was expecting //foo:missing to be a dependency of "
-            "//foo:one(//toolchain:default). "
+            "I was expecting //foo:missing(//toolchain:default) to be a "
+            "dependency of //foo:one(//toolchain:default). "
             "Make sure it's included in the deps or data_deps, and that you've "
             "specified the appropriate toolchain.")
       << err.message();