Improved GN code related to resolving paths. Moved most of the logic related to resolving paths to ResolvePath and ResolveRelative in filesystem_utils.cc. Improved SourceDir class to return std::string with resolved path for cases when returning SourceDir or SourceFile wasn't really needed. R=brettw@chromium.org, dpranke@chromium.org Bug: 819720 Change-Id: Ia59da00e83de45431762ebb19a9153471f7e91e3 Reviewed-on: https://chromium-review.googlesource.com/953422 Reviewed-by: Brett Wilson <brettw@chromium.org> Commit-Queue: Dirk Pranke <dpranke@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#542692} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: 36773f30d94495bc81193a52b49504d2e137b9e3
diff --git a/tools/gn/build_settings.cc b/tools/gn/build_settings.cc index 38b830f..79b8365 100644 --- a/tools/gn/build_settings.cc +++ b/tools/gn/build_settings.cc
@@ -49,6 +49,11 @@ return dir.Resolve(root_path_).NormalizePathSeparatorsTo('/'); } +base::FilePath BuildSettings::GetFullPath(const std::string& path, + bool as_file) const { + return ResolvePath(path, as_file, root_path_).NormalizePathSeparatorsTo('/'); +} + base::FilePath BuildSettings::GetFullPathSecondary( const SourceFile& file) const { return file.Resolve(secondary_source_path_).NormalizePathSeparatorsTo('/'); @@ -59,6 +64,12 @@ return dir.Resolve(secondary_source_path_).NormalizePathSeparatorsTo('/'); } +base::FilePath BuildSettings::GetFullPathSecondary(const std::string& path, + bool as_file) const { + return ResolvePath(path, as_file, secondary_source_path_) + .NormalizePathSeparatorsTo('/'); +} + void BuildSettings::ItemDefined(std::unique_ptr<Item> item) const { DCHECK(item); if (!item_defined_callback_.is_null())
diff --git a/tools/gn/build_settings.h b/tools/gn/build_settings.h index 670f996..52f3625 100644 --- a/tools/gn/build_settings.h +++ b/tools/gn/build_settings.h
@@ -79,12 +79,21 @@ // root source tree. base::FilePath GetFullPath(const SourceFile& file) const; base::FilePath GetFullPath(const SourceDir& dir) const; + // Works the same way as other GetFullPath. + // Parameter as_file defines whether path should be treated as a + // SourceFile or SourceDir value. + base::FilePath GetFullPath(const std::string& path, bool as_file) const; // Returns the absolute OS path inside the secondary source path. Will return // an empty FilePath if the secondary source path is empty. When loading a // buildfile, the GetFullPath should always be consulted first. base::FilePath GetFullPathSecondary(const SourceFile& file) const; base::FilePath GetFullPathSecondary(const SourceDir& dir) const; + // Works the same way as other GetFullPathSecondary. + // Parameter as_file defines whether path should be treated as a + // SourceFile or SourceDir value. + base::FilePath GetFullPathSecondary(const std::string& path, + bool as_file) const; // Called when an item is defined from a background thread. void ItemDefined(std::unique_ptr<Item> item) const;
diff --git a/tools/gn/filesystem_utils.cc b/tools/gn/filesystem_utils.cc index 6cce895..f9f6794 100644 --- a/tools/gn/filesystem_utils.cc +++ b/tools/gn/filesystem_utils.cc
@@ -705,6 +705,109 @@ return ret; } +base::FilePath ResolvePath(const std::string& value, + bool as_file, + const base::FilePath& source_root) { + if (value.empty()) + return base::FilePath(); + + std::string converted; + if (!IsPathSourceAbsolute(value)) { + if (value.size() > 2 && value[2] == ':') { + // Windows path, strip the leading slash. + converted.assign(&value[1], value.size() - 1); + } else { + converted.assign(value); + } + return base::FilePath(UTF8ToFilePath(converted)); + } + + // String the double-leading slash for source-relative paths. + converted.assign(&value[2], value.size() - 2); + + if (as_file && source_root.empty()) + return UTF8ToFilePath(converted).NormalizePathSeparatorsTo('/'); + + return source_root.Append(UTF8ToFilePath(converted)) + .NormalizePathSeparatorsTo('/'); +} + +template <typename StringType> +std::string ResolveRelative(const StringType& input, + const std::string& value, + bool as_file, + const base::StringPiece& source_root) { + std::string result; + + if (input.size() >= 2 && input[0] == '/' && input[1] == '/') { + // Source-relative. + result.assign(input.data(), input.size()); + if (!as_file) { + if (!EndsWithSlash(result)) + result.push_back('/'); + } + NormalizePath(&result, source_root); + return result; + } else if (IsPathAbsolute(input)) { + if (source_root.empty() || + !MakeAbsolutePathRelativeIfPossible(source_root, input, &result)) { +#if defined(OS_WIN) + if (input[0] != '/') // See the file case for why we do this check. + result = "/"; +#endif + result.append(input.data(), input.size()); + } + NormalizePath(&result); + if (!as_file) { + if (!EndsWithSlash(result)) + result.push_back('/'); + } + return result; + } + + if (!source_root.empty()) { + std::string absolute = + FilePathToUTF8(ResolvePath(value, as_file, UTF8ToFilePath(source_root)) + .AppendASCII(input) + .value()); + NormalizePath(&absolute); + if (!MakeAbsolutePathRelativeIfPossible(source_root, absolute, &result)) { +#if defined(OS_WIN) + if (absolute[0] != '/') // See the file case for why we do this check. + result = "/"; +#endif + result.append(absolute.data(), absolute.size()); + } + if (!as_file && !EndsWithSlash(result)) + result.push_back('/'); + return result; + } + + // With no source_root, there's nothing we can do about + // e.g. input=../../../path/to/file and value=//source and we'll + // errornously return //file. + result.reserve(value.size() + input.size()); + result.assign(value); + result.append(input.data(), input.size()); + + NormalizePath(&result); + if (!as_file && !EndsWithSlash(result)) + result.push_back('/'); + + return result; +} + +// Explicit template instantiation +template std::string ResolveRelative(const base::StringPiece& input, + const std::string& value, + bool as_file, + const base::StringPiece& source_root); + +template std::string ResolveRelative(const std::string& input, + const std::string& value, + bool as_file, + const base::StringPiece& source_root); + std::string DirectoryWithNoLastSlash(const SourceDir& dir) { std::string ret;
diff --git a/tools/gn/filesystem_utils.h b/tools/gn/filesystem_utils.h index 0396283..1313279 100644 --- a/tools/gn/filesystem_utils.h +++ b/tools/gn/filesystem_utils.h
@@ -152,6 +152,31 @@ const SourceDir& dest_dir, const base::StringPiece& source_root = base::StringPiece()); +// Resolves a file or dir name (parameter input) relative to +// value directory. Will return an empty SourceDir/File on error +// and set the give *err pointer (required). Empty input is always an error. +// Returned value can be used to set value in either SourceFile or SourceDir +// (based on as_file parameter). +// +// Parameter as_file defines whether result path will look like a file path +// or it should be treated as a directory (contains "/" and the end +// of the string). +// +// If source_root is supplied, these functions will additionally handle the +// case where the input is a system-absolute but still inside the source +// tree. This is the case for some external tools. +template <typename StringType> +std::string ResolveRelative(const StringType& input, + const std::string& value, + bool as_file, + const base::StringPiece& source_root); + +// Resolves source file or directory relative to some given source root. Returns +// an empty file path on error. +base::FilePath ResolvePath(const std::string& value, + bool as_file, + const base::FilePath& source_root); + // Returns the given directory with no terminating slash at the end, such that // appending a slash and more stuff will produce a valid path. //
diff --git a/tools/gn/function_exec_script.cc b/tools/gn/function_exec_script.cc index 139b811..0f6397c 100644 --- a/tools/gn/function_exec_script.cc +++ b/tools/gn/function_exec_script.cc
@@ -137,19 +137,21 @@ return Value(); // Find the python script to run. - SourceFile script_source = - cur_dir.ResolveRelativeFile(args[0], err, - scope->settings()->build_settings()->root_path_utf8()); + std::string script_source_path = cur_dir.ResolveRelativeAs( + true, args[0], err, + scope->settings()->build_settings()->root_path_utf8()); if (err->has_error()) return Value(); - base::FilePath script_path = build_settings->GetFullPath(script_source); + base::FilePath script_path = + build_settings->GetFullPath(script_source_path, true); if (!build_settings->secondary_source_path().empty() && !base::PathExists(script_path)) { // Fall back to secondary source root when the file doesn't exist. - script_path = build_settings->GetFullPathSecondary(script_source); + script_path = + build_settings->GetFullPathSecondary(script_source_path, true); } - ScopedTrace trace(TraceItem::TRACE_SCRIPT_EXECUTE, script_source.value()); + ScopedTrace trace(TraceItem::TRACE_SCRIPT_EXECUTE, script_source_path); trace.SetToolchain(settings->toolchain_label()); // Add all dependencies of this script, including the script itself, to the @@ -163,10 +165,11 @@ for (const auto& dep : deps_value.list_value()) { if (!dep.VerifyTypeIs(Value::STRING, err)) return Value(); - g_scheduler->AddGenDependency( - build_settings->GetFullPath(cur_dir.ResolveRelativeFile( - dep, err, - scope->settings()->build_settings()->root_path_utf8()))); + g_scheduler->AddGenDependency(build_settings->GetFullPath( + cur_dir.ResolveRelativeAs( + true, dep, err, + scope->settings()->build_settings()->root_path_utf8()), + true)); if (err->has_error()) return Value(); } @@ -236,10 +239,12 @@ } } if (g_scheduler->verbose_logging()) { - g_scheduler->Log("Pythoning", script_source.value() + " took " + - base::Int64ToString( - (base::TimeTicks::Now() - begin_exec).InMilliseconds()) + - "ms"); + g_scheduler->Log( + "Pythoning", + script_source_path + " took " + + base::Int64ToString( + (base::TimeTicks::Now() - begin_exec).InMilliseconds()) + + "ms"); } if (exit_code != 0) {
diff --git a/tools/gn/function_get_path_info.cc b/tools/gn/function_get_path_info.cc index 1c542aa..e08eca1 100644 --- a/tools/gn/function_get_path_info.cc +++ b/tools/gn/function_get_path_info.cc
@@ -41,7 +41,7 @@ settings->build_settings()->root_path_utf8()); } - // Input is a directory. + // Input is a file. return current_dir.ResolveRelativeFile(input, err, settings->build_settings()->root_path_utf8()).GetDir(); } @@ -100,14 +100,12 @@ BuildDirType::OBJ)); } case WHAT_ABSPATH: { - if (!input_string.empty() && - input_string[input_string.size() - 1] == '/') { - return current_dir.ResolveRelativeDir(input, err, - settings->build_settings()->root_path_utf8()).value(); - } else { - return current_dir.ResolveRelativeFile(input, err, - settings->build_settings()->root_path_utf8()).value(); - } + bool as_dir = + !input_string.empty() && input_string[input_string.size() - 1] == '/'; + + return current_dir.ResolveRelativeAs( + !as_dir, input, err, settings->build_settings()->root_path_utf8(), + &input_string); } default: NOTREACHED();
diff --git a/tools/gn/source_dir.cc b/tools/gn/source_dir.cc index e8fdff0..17eeb61 100644 --- a/tools/gn/source_dir.cc +++ b/tools/gn/source_dir.cc
@@ -23,6 +23,38 @@ } } +// Validates input value (input_value) and sets proper error message. +// Note: Parameter blame_input is used only for generating error message. +template <typename StringType> +bool ValidateResolveInput(bool as_file, + const Value& blame_input_value, + const StringType& input_value, + Err* err) { + if (as_file) { + // 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 (input_value.empty()) { + *err = Err(blame_input_value, "Empty file path.", + "You can't use empty strings as file paths."); + return false; + } else if (input_value[input_value.size() - 1] == '/') { + std::string help = "You specified the path\n "; + help.append(std::string(input_value)); + help.append( + "\nand it ends in a slash, indicating you think it's a directory." + "\nBut here you're supposed to be listing a file."); + *err = Err(blame_input_value, "File path ends in a slash.", help); + return false; + } + } else if (input_value.empty()) { + *err = Err(blame_input_value, "Empty directory path.", + "You can't use empty strings as directories."); + return false; + } + return true; +} + } // namespace SourceDir::SourceDir() = default; @@ -43,176 +75,85 @@ SourceDir::~SourceDir() = default; +template <typename StringType> +std::string SourceDir::ResolveRelativeAs( + bool as_file, + const Value& blame_input_value, + const StringType& input_value, + Err* err, + const base::StringPiece& source_root) const { + if (!ValidateResolveInput<StringType>(as_file, blame_input_value, input_value, + err)) { + return std::string(); + } + return ResolveRelative(input_value, value_, as_file, source_root); +} + SourceFile SourceDir::ResolveRelativeFile( const Value& p, Err* err, const base::StringPiece& source_root) const { SourceFile ret; + if (!p.VerifyTypeIs(Value::STRING, err)) return 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. - const std::string& str = p.string_value(); - if (str.empty()) { - *err = Err(p, "Empty file path.", - "You can't use empty strings as file paths. That's just wrong."); - return ret; - } else if (str[str.size() - 1] == '/') { - *err = Err(p, "File path ends in a slash.", - "You specified the path\n " + str + "\n" - "and it ends in a slash, indicating you think it's a directory." - "\nBut here you're supposed to be listing a file."); + const std::string& input_string = p.string_value(); + if (!ValidateResolveInput<std::string>(true, p, input_string, err)) { return ret; } - - if (str.size() >= 2 && str[0] == '/' && str[1] == '/') { - // Source-relative. - ret.value_.assign(str.data(), str.size()); - NormalizePath(&ret.value_, source_root); - return ret; - } else if (IsPathAbsolute(str)) { - if (source_root.empty() || - !MakeAbsolutePathRelativeIfPossible(source_root, str, &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 (str[0] != '/') - ret.value_ = "/"; -#endif - ret.value_.append(str.data(), str.size()); - } - NormalizePath(&ret.value_); - return ret; - } - - if (!source_root.empty()) { - std::string absolute = - FilePathToUTF8(Resolve(UTF8ToFilePath(source_root)).AppendASCII( - str).value()); - NormalizePath(&absolute); - if (!MakeAbsolutePathRelativeIfPossible(source_root, absolute, - &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 (absolute[0] != '/') - ret.value_ = "/"; -#endif - ret.value_.append(absolute.data(), absolute.size()); - } - 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() + str.size()); - ret.value_.assign(value_); - ret.value_.append(str.data(), str.size()); - - NormalizePath(&ret.value_); + ret.value_ = ResolveRelative(input_string, value_, true, source_root); return ret; } +std::string SourceDir::ResolveRelativeAs(bool as_file, + const Value& v, + Err* err, + const base::StringPiece& source_root, + const std::string* v_value) const { + if (!v.VerifyTypeIs(Value::STRING, err)) + return std::string(); + + if (!v_value) { + v_value = &v.string_value(); + } + std::string result = + ResolveRelativeAs(as_file, v, *v_value, err, source_root); + if (!as_file) + AssertValueSourceDirString(result); + return result; +} + SourceDir SourceDir::ResolveRelativeDir( - const Value& p, + const Value& v, Err* err, const base::StringPiece& source_root) const { - if (!p.VerifyTypeIs(Value::STRING, err)) + if (!v.VerifyTypeIs(Value::STRING, err)) return SourceDir(); - return ResolveRelativeDir(p, p.string_value(), err, source_root); -} -SourceDir SourceDir::ResolveRelativeDir( - const Value& blame_but_dont_use, - const base::StringPiece& str, - Err* err, - const base::StringPiece& source_root) const { - SourceDir ret; - - if (str.empty()) { - *err = Err(blame_but_dont_use, "Empty directory path.", - "You can't use empty strings as directories. " - "That's just wrong."); - return ret; - } - - if (str.size() >= 2 && str[0] == '/' && str[1] == '/') { - // Source-relative. - ret.value_.assign(str.data(), str.size()); - if (!EndsWithSlash(ret.value_)) - ret.value_.push_back('/'); - NormalizePath(&ret.value_, source_root); - return ret; - } else if (IsPathAbsolute(str)) { - if (source_root.empty() || - !MakeAbsolutePathRelativeIfPossible(source_root, str, &ret.value_)) { -#if defined(OS_WIN) - if (str[0] != '/') // See the file case for why we do this check. - ret.value_ = "/"; -#endif - ret.value_.append(str.data(), str.size()); - } - NormalizePath(&ret.value_); - if (!EndsWithSlash(ret.value_)) - ret.value_.push_back('/'); - return ret; - } - - if (!source_root.empty()) { - std::string absolute = - FilePathToUTF8(Resolve(UTF8ToFilePath(source_root)).AppendASCII( - str.as_string()).value()); - NormalizePath(&absolute); - if (!MakeAbsolutePathRelativeIfPossible(source_root, absolute, - &ret.value_)) { -#if defined(OS_WIN) - if (absolute[0] != '/') // See the file case for why we do this check. - ret.value_ = "/"; -#endif - ret.value_.append(absolute.data(), absolute.size()); - } - if (!EndsWithSlash(ret.value_)) - ret.value_.push_back('/'); - return ret; - } - - ret.value_.reserve(value_.size() + str.size()); - ret.value_.assign(value_); - ret.value_.append(str.data(), str.size()); - - NormalizePath(&ret.value_); - if (!EndsWithSlash(ret.value_)) - ret.value_.push_back('/'); - AssertValueSourceDirString(ret.value_); - - return ret; + return ResolveRelativeDir<std::string>(v, v.string_value(), err, source_root); } base::FilePath SourceDir::Resolve(const base::FilePath& source_root) const { - if (is_null()) - return base::FilePath(); - - std::string converted; - if (is_system_absolute()) { - if (value_.size() > 2 && value_[2] == ':') { - // Windows path, strip the leading slash. - converted.assign(&value_[1], value_.size() - 1); - } else { - converted.assign(value_); - } - return base::FilePath(UTF8ToFilePath(converted)); - } - - // String the double-leading slash for source-relative paths. - converted.assign(&value_[2], value_.size() - 2); - return source_root.Append(UTF8ToFilePath(converted)) - .NormalizePathSeparatorsTo('/'); + return ResolvePath(value_, false, source_root); } void SourceDir::SwapValue(std::string* v) { value_.swap(*v); AssertValueSourceDirString(value_); } + +// Explicit template instantiation +template std::string SourceDir::ResolveRelativeAs( + bool as_file, + const Value& blame_input_value, + const std::string& input_value, + Err* err, + const base::StringPiece& source_root) const; + +template std::string SourceDir::ResolveRelativeAs( + bool as_file, + const Value& blame_input_value, + const base::StringPiece& input_value, + Err* err, + const base::StringPiece& source_root) const;
diff --git a/tools/gn/source_dir.h b/tools/gn/source_dir.h index 422fa89..142285c 100644 --- a/tools/gn/source_dir.h +++ b/tools/gn/source_dir.h
@@ -37,28 +37,57 @@ SourceDir(SwapIn, std::string* s); ~SourceDir(); - // Resolves a file or dir name relative to this source directory. Will return - // an empty SourceDir/File on error and set the give *err pointer (required). - // Empty input is always an error. + // Resolves a file or dir name (based on as_file parameter) relative + // to this source directory. Will return an empty string on error + // and set the give *err pointer (required). Empty input is always an error. + // + // Passed non null v_value will be used to resolve path (in cases where + // a substring has been extracted from the value, as with label resolution). + // In this use case parameter v is used to generate proper error. // // If source_root is supplied, these functions will additionally handle the // case where the input is a system-absolute but still inside the source // tree. This is the case for some external tools. + std::string ResolveRelativeAs( + bool as_file, + const Value& v, + Err* err, + const base::StringPiece& source_root = base::StringPiece(), + const std::string* v_value = nullptr) const; + + // Like ResolveRelativeAs above, but allows to produce result + // without overhead for string conversion (on input value). + template <typename StringType> + std::string ResolveRelativeAs( + bool as_file, + const Value& blame_input_value, + const StringType& input_value, + Err* err, + const base::StringPiece& source_root = base::StringPiece()) const; + + // Wrapper for ResolveRelativeAs. SourceFile ResolveRelativeFile( const Value& p, Err* err, const base::StringPiece& source_root = base::StringPiece()) const; - SourceDir ResolveRelativeDir( - const Value& p, - Err* err, - const base::StringPiece& source_root = base::StringPiece()) const; - // Like ResolveRelativeDir but takes a separate value (which gets blamed) - // and string to use (in cases where a substring has been extracted from the - // value, as with label resolution). + // Wrapper for ResolveRelativeAs. + template <typename StringType> SourceDir ResolveRelativeDir( - const Value& blame_but_dont_use, - const base::StringPiece& p, + const Value& blame_input_value, + const StringType& input_value, + Err* err, + const base::StringPiece& source_root = base::StringPiece()) const { + SourceDir ret; + ret.value_ = ResolveRelativeAs<StringType>(false, blame_input_value, + input_value, err, source_root); + return ret; + } + + // Wrapper for ResolveRelativeDir where input_value equals to + // v.string_value(). + SourceDir ResolveRelativeDir( + const Value& v, Err* err, const base::StringPiece& source_root = base::StringPiece()) const;
diff --git a/tools/gn/source_file.cc b/tools/gn/source_file.cc index b28c3ac..befc5a5 100644 --- a/tools/gn/source_file.cc +++ b/tools/gn/source_file.cc
@@ -61,23 +61,5 @@ } base::FilePath SourceFile::Resolve(const base::FilePath& source_root) const { - if (is_null()) - return base::FilePath(); - - std::string converted; - if (is_system_absolute()) { - if (value_.size() > 2 && value_[2] == ':') { - // Windows path, strip the leading slash. - converted.assign(&value_[1], value_.size() - 1); - } else { - converted.assign(value_); - } - return base::FilePath(UTF8ToFilePath(converted)); - } - - converted.assign(&value_[2], value_.size() - 2); - if (source_root.empty()) - return UTF8ToFilePath(converted).NormalizePathSeparatorsTo('/'); - return source_root.Append(UTF8ToFilePath(converted)) - .NormalizePathSeparatorsTo('/'); + return ResolvePath(value_, true, source_root); }
diff --git a/tools/gn/target_generator.cc b/tools/gn/target_generator.cc index bbc9793..ab85729 100644 --- a/tools/gn/target_generator.cc +++ b/tools/gn/target_generator.cc
@@ -222,24 +222,18 @@ const Value& input = input_list[i]; if (!input.VerifyTypeIs(Value::STRING, err_)) return false; - const std::string& input_str = input.string_value(); + const std::string input_str = input.string_value(); // Treat each input as either a file or a directory, depending on the // last character. - if (!input_str.empty() && input_str[input_str.size() - 1] == '/') { - // Resolve as directory. - SourceDir resolved = - dir.ResolveRelativeDir(input, input_str, err_, root_path); - if (err_->has_error()) - return false; - output_list.push_back(resolved.value()); - } else { - // Resolve as file. - SourceFile resolved = dir.ResolveRelativeFile(input, err_, root_path); - if (err_->has_error()) - return false; - output_list.push_back(resolved.value()); - } + bool as_dir = !input_str.empty() && input_str[input_str.size() - 1] == '/'; + + std::string resolved = + dir.ResolveRelativeAs(!as_dir, input, err_, root_path, &input_str); + if (err_->has_error()) + return false; + + output_list.push_back(resolved); } return true; }