Normalize paths in GN. Most SourceFile creation goes through SourceDir.Resolve which normalizes "." and "..". But there are a few cases that just go through the SourceFile constructor directly. Explicitly normalize those cases. This updates the PathOutput unit test since it was written to assume SourceFiles were not normalized. This is testing escaping which is orthogonal, so I changed the test accordingly. Some changed to OutputFiles which are already considered processed and not normalized, and I just deleted another backslash case which was redundantly testing the same thing. BUG=505816 Review URL: https://codereview.chromium.org/1214933007 Cr-Original-Commit-Position: refs/heads/master@{#336854} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: 8440203c77e972d5d5289f75ead381821e0756c5
diff --git a/tools/gn/BUILD.gn b/tools/gn/BUILD.gn index 8f9f012..3109f75 100644 --- a/tools/gn/BUILD.gn +++ b/tools/gn/BUILD.gn
@@ -267,6 +267,7 @@ "scope_per_file_provider_unittest.cc", "scope_unittest.cc", "source_dir_unittest.cc", + "source_file_unittest.cc", "string_utils_unittest.cc", "substitution_pattern_unittest.cc", "substitution_writer_unittest.cc",
diff --git a/tools/gn/gn.gyp b/tools/gn/gn.gyp index 7a74f50..639846b 100644 --- a/tools/gn/gn.gyp +++ b/tools/gn/gn.gyp
@@ -242,6 +242,7 @@ 'scope_per_file_provider_unittest.cc', 'scope_unittest.cc', 'source_dir_unittest.cc', + 'source_file_unittest.cc', 'string_utils_unittest.cc', 'substitution_pattern_unittest.cc', 'substitution_writer_unittest.cc',
diff --git a/tools/gn/path_output_unittest.cc b/tools/gn/path_output_unittest.cc index 131f55e..65dae8d 100644 --- a/tools/gn/path_output_unittest.cc +++ b/tools/gn/path_output_unittest.cc
@@ -84,8 +84,8 @@ { // Not other weird stuff std::ostringstream out; - writer.WriteFile(out, SourceFile("//foo/\"foo\\bar\".cc")); - EXPECT_EQ("../../foo/\"foo\\bar\".cc", out.str()); + writer.WriteFile(out, SourceFile("//foo/\"foo\".cc")); + EXPECT_EQ("../../foo/\"foo\".cc", out.str()); } } @@ -127,19 +127,18 @@ EXPECT_EQ("../../foo/\\\"foobar\\\".cc", out.str()); } - // Backslashes should get escaped on non-Windows and preserved on Windows. writer.set_escape_platform(ESCAPE_PLATFORM_WIN); { std::ostringstream out; - writer.WriteFile(out, SourceFile("//foo\\bar.cc")); - EXPECT_EQ("../../foo\\bar.cc", out.str()); + writer.WriteFile(out, OutputFile("foo\\bar.cc")); + EXPECT_EQ("foo\\bar.cc", out.str()); } writer.set_escape_platform(ESCAPE_PLATFORM_POSIX); { std::ostringstream out; - writer.WriteFile(out, SourceFile("//foo\\bar.cc")); - EXPECT_EQ("../../foo\\\\bar.cc", out.str()); + writer.WriteFile(out, OutputFile("foo\\bar.cc")); + EXPECT_EQ("foo\\\\bar.cc", out.str()); } }
diff --git a/tools/gn/source_file.cc b/tools/gn/source_file.cc index 8204295..41a602d 100644 --- a/tools/gn/source_file.cc +++ b/tools/gn/source_file.cc
@@ -30,12 +30,14 @@ : value_(p.data(), p.size()) { DCHECK(!value_.empty()); AssertValueSourceFileString(value_); + NormalizePath(&value_); } SourceFile::SourceFile(SwapIn, std::string* value) { value_.swap(*value); DCHECK(!value_.empty()); AssertValueSourceFileString(value_); + NormalizePath(&value_); } SourceFile::~SourceFile() {
diff --git a/tools/gn/source_file_unittest.cc b/tools/gn/source_file_unittest.cc new file mode 100644 index 0000000..8a9218f --- /dev/null +++ b/tools/gn/source_file_unittest.cc
@@ -0,0 +1,19 @@ +// Copyright 2015 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 "testing/gtest/include/gtest/gtest.h" +#include "tools/gn/source_file.h" + +// The SourceFile object should normalize the input passed to the constructor. +// The normalizer unit test checks for all the weird edge cases for normalizing +// so here just check that it gets called. +TEST(SourceFile, Normalize) { + SourceFile a("//foo/../bar.cc"); + EXPECT_EQ("//bar.cc", a.value()); + + std::string b_str("//foo/././../bar.cc"); + SourceFile b(SourceFile::SwapIn(), &b_str); + EXPECT_TRUE(b_str.empty()); // Should have been swapped in. + EXPECT_EQ("//bar.cc", b.value()); +}