gn: Support build directories outside the source tree.

tools/gn/source_dir.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "tools/gn/source_dir.h"
 
 #include "base/logging.h"
 #include "tools/gn/filesystem_utils.h"
 #include "tools/gn/source_file.h"
 
@@ skipping 23 matching lines @@
   if (!EndsWithSlash(value_))
     value_.push_back('/');
   AssertValueSourceDirString(value_);
 }
 
 SourceDir::~SourceDir() {
 }
 
 SourceFile SourceDir::ResolveRelativeFile(
     const base::StringPiece& p,
-    const base::StringPiece& source_root) const {
+    const base::FilePath& source_root) const {
   SourceFile ret;
 
   // It's an error to resolve an empty string or one that is a directory
   // (indicated by a trailing slash) because this is the function that expects
   // to return a file.
   if (p.empty() || (p.size() > 0 && p[p.size() - 1] == '/'))
     return SourceFile();
   if (p.size() >= 2 && p[0] == '/' && p[1] == '/') {
     // Source-relative.
     ret.value_.assign(p.data(), p.size());
     NormalizePath(&ret.value_);
     return ret;
   } else if (IsPathAbsolute(p)) {
     if (source_root.empty() ||
-        !MakeAbsolutePathRelativeIfPossible(source_root, p, &ret.value_)) {
+        !MakeAbsolutePathRelativeIfPossible(source_root.value(), p,
+                                            &ret.value_)) {
 #if defined(OS_WIN)
       // On Windows we'll accept "C:\foo" as an absolute path, which we want
       // to convert to "/C:..." here.
       if (p[0] != '/')
         ret.value_ = "/";
 #endif
       ret.value_.append(p.data(), p.size());
     }
     NormalizePath(&ret.value_);
     return ret;
   }
 
+  if (!source_root.empty()) {

[brettw, 2014/10/07 23:55:06]

Are these blocks necessary? Are you trying to handle the case where you're doing
a relative path to something outside the source root? Do we care about that
case? It seems like a separate issue than the one you are solving.

There are also a lot of string copies here (this function is called a lot) and
it won't compile on Windows.

Same for the one in the Dir case below.

[zeuthen, 2014/10/08 15:21:06]

I tried commenting this out and without it, the build fails with

 zeuthen@luna:/ssd/chromium/src/tools/gn$ ../../out/Debug/gn gen
/ssd/midlertidigt/chrome_build/out/Debug/
 ERROR at //tools/grit/grit_rule.gni:254:18: File is not inside output
directory.
   grit_outputs = get_path_info(
                  ^-------------
 The given file should be in the output directory. Normally you would specify
 "$target_out_dir/foo" or "$target_gen_dir/foo". I interpreted this as
 "//midlertidigt/chrome_build/out/Debug/gen/content/grit/content_resources.h".
 See //content/BUILD.gn:66:1: whence it was called.
 grit("resources") {

I think there's a way to optimize this so we don't have to call Resolve() -
basically, we're good until we call NormalizePath(): the culprit is that we're
trying to normalize a path of the form
//../../../ssd/midlertidigt/chrome_build/out/Debug which we'll erroneously
normalize to //midlertidigt/chrome_build/out/Debug/ ... so perhaps all we need
is to teach NormalizePath() about being smart and only do expensive stuff if we
end up going outside the source root.

+    std::string absolute = Resolve(source_root).Append(p.as_string()).value();
+    NormalizePath(&absolute);
+    if (!MakeAbsolutePathRelativeIfPossible(source_root.value(), absolute,
+                                            &ret.value_))
+      ret.value_ = absolute;
+    return ret;
+  }
+
+  // With no source_root_, there's nothing we can do about
+  // e.g. p=../../../path/to/file and value_=//source and we'll
+  // errornously return //file.
   ret.value_.reserve(value_.size() + p.size());
   ret.value_.assign(value_);
   ret.value_.append(p.data(), p.size());
-
   NormalizePath(&ret.value_);
   return ret;
 }
 
 SourceDir SourceDir::ResolveRelativeDir(
     const base::StringPiece& p,
-    const base::StringPiece& source_root) const {
+    const base::FilePath& source_root) const {
   SourceDir ret;
 
   if (p.empty())
     return ret;
   if (p.size() >= 2 && p[0] == '/' && p[1] == '/') {
     // Source-relative.
     ret.value_.assign(p.data(), p.size());
     if (!EndsWithSlash(ret.value_))
       ret.value_.push_back('/');
     NormalizePath(&ret.value_);
     return ret;
   } else if (IsPathAbsolute(p)) {
     if (source_root.empty() ||
-        !MakeAbsolutePathRelativeIfPossible(source_root, p, &ret.value_)) {
+        !MakeAbsolutePathRelativeIfPossible(source_root.value(), p,
+                                            &ret.value_)) {
 #if defined(OS_WIN)
       if (p[0] != '/')  // See the file case for why we do this check.
         ret.value_ = "/";
 #endif
       ret.value_.append(p.data(), p.size());
     }
     NormalizePath(&ret.value_);
     if (!EndsWithSlash(ret.value_))
       ret.value_.push_back('/');
     return ret;
   }
 
+  if (!source_root.empty()) {
+    std::string absolute = Resolve(source_root).Append(p.as_string()).value();
+    NormalizePath(&absolute);
+    if (!MakeAbsolutePathRelativeIfPossible(source_root.value(), absolute,
+                                            &ret.value_))
+      ret.value_ = absolute;
+    if (!EndsWithSlash(ret.value_))
+      ret.value_.push_back('/');
+    return ret;
+  }
+
   ret.value_.reserve(value_.size() + p.size());
   ret.value_.assign(value_);
   ret.value_.append(p.data(), p.size());
 
   NormalizePath(&ret.value_);
   if (!EndsWithSlash(ret.value_))
     ret.value_.push_back('/');
   AssertValueSourceDirString(ret.value_);
 
   return ret;
@@ skipping 17 matching lines @@
   // String the double-leading slash for source-relative paths.
   converted.assign(&value_[2], value_.size() - 2);
   return source_root.Append(UTF8ToFilePath(converted))
       .NormalizePathSeparatorsTo('/');
 }
 
 void SourceDir::SwapValue(std::string* v) {
   value_.swap(*v);
   AssertValueSourceDirString(value_);
 }