Make regen with --dotfile and --root work. Also add a test for GetSelfInvocationCommandLine(). Also fix a bug in MakeAbsoluteFilePathRelativeIfPossible() found by the new test: It now works correctly if the two paths passed in have different slash types right after the drive letter. If handed "C:\foo" "C:/foo/f", the function used to return "..\foo\f", now it returns just "f". This also makes the exe_path in the gn regen command look nicer, and might fix a few other things. BUG=23 Change-Id: I683d6f0ef5d742686181ff11545a62c052f06605 Reviewed-on: https://gn-review.googlesource.com/c/3280 Commit-Queue: Brett Wilson <brettw@chromium.org> Reviewed-by: Brett Wilson <brettw@chromium.org>
diff --git a/tools/gn/build_settings.cc b/tools/gn/build_settings.cc index 84f2a3c..3cc211b 100644 --- a/tools/gn/build_settings.cc +++ b/tools/gn/build_settings.cc
@@ -12,7 +12,8 @@ BuildSettings::BuildSettings() = default; BuildSettings::BuildSettings(const BuildSettings& other) - : root_path_(other.root_path_), + : dotfile_name_(other.dotfile_name_), + root_path_(other.root_path_), root_path_utf8_(other.root_path_utf8_), secondary_source_path_(other.secondary_source_path_), python_path_(other.python_path_),
diff --git a/tools/gn/build_settings.h b/tools/gn/build_settings.h index 52f3625..51a7d6b 100644 --- a/tools/gn/build_settings.h +++ b/tools/gn/build_settings.h
@@ -39,8 +39,10 @@ // Absolute path of the source root on the local system. Everything is // relative to this. Does not end in a [back]slash. const base::FilePath& root_path() const { return root_path_; } + const base::FilePath& dotfile_name() const { return dotfile_name_; } const std::string& root_path_utf8() const { return root_path_utf8_; } void SetRootPath(const base::FilePath& r); + void set_dotfile_name(const base::FilePath& d) { dotfile_name_ = d; } // When nonempty, specifies a parallel directory higherarchy in which to // search for buildfiles if they're not found in the root higherarchy. This @@ -119,6 +121,7 @@ private: Label root_target_label_; + base::FilePath dotfile_name_; base::FilePath root_path_; std::string root_path_utf8_; base::FilePath secondary_source_path_;
diff --git a/tools/gn/filesystem_utils.cc b/tools/gn/filesystem_utils.cc index e8f0696..2fc4b24 100644 --- a/tools/gn/filesystem_utils.cc +++ b/tools/gn/filesystem_utils.cc
@@ -428,9 +428,14 @@ target.GetComponents(&target_components); #if defined(OS_WIN) // On Windows, it's impossible to have a relative path from C:\foo to D:\bar, - // so return the target as an aboslute path instead. + // so return the target as an absolute path instead. if (base_components[0] != target_components[0]) return target; + + // GetComponents() returns the first slash after the root. Set it to the + // same value in both component lists so that relative paths between + // "C:/foo/..." and "C:\foo\..." are computed correctly. + target_components[1] = base_components[1]; #endif size_t i; for (i = 0; i < base_components.size() && i < target_components.size(); i++) {
diff --git a/tools/gn/filesystem_utils_unittest.cc b/tools/gn/filesystem_utils_unittest.cc index 87e37f8..dcaa099 100644 --- a/tools/gn/filesystem_utils_unittest.cc +++ b/tools/gn/filesystem_utils_unittest.cc
@@ -188,6 +188,10 @@ base::FilePath(L"..\\.."), MakeAbsoluteFilePathRelativeIfPossible( base::FilePath(L"C:\\src\\out\\Debug"), base::FilePath(L"C:\\src"))); + EXPECT_EQ( + base::FilePath(L"..\\.."), + MakeAbsoluteFilePathRelativeIfPossible( + base::FilePath(L"C:\\src\\out\\Debug"), base::FilePath(L"C:/src"))); EXPECT_EQ(base::FilePath(L"."), MakeAbsoluteFilePathRelativeIfPossible(base::FilePath(L"C:\\src"), base::FilePath(L"C:\\src")));
diff --git a/tools/gn/ninja_build_writer.cc b/tools/gn/ninja_build_writer.cc index 56d6c5f..f13a8b9 100644 --- a/tools/gn/ninja_build_writer.cc +++ b/tools/gn/ninja_build_writer.cc
@@ -47,7 +47,10 @@ const Target* last_seen; }; -std::string GetSelfInvocationCommand(const BuildSettings* build_settings) { +} // namespace + +base::CommandLine GetSelfInvocationCommandLine( + const BuildSettings* build_settings) { const base::FilePath build_path = build_settings->build_dir().Resolve(build_settings->root_path()); @@ -81,6 +84,18 @@ escape_shell.inhibit_quoting = true; #endif + // If both --root and --dotfile are passed, make sure the --dotfile is + // made relative to the build dir here. + base::FilePath dotfile_path = build_settings->dotfile_name(); + if (!dotfile_path.empty()) { + if (build_path.IsAbsolute()) { + dotfile_path = + MakeAbsoluteFilePathRelativeIfPossible(build_path, dotfile_path); + } + cmdline.AppendSwitchPath(std::string("--") + switches::kDotfile, + dotfile_path.NormalizePathSeparatorsTo('/')); + } + const base::CommandLine& our_cmdline = *base::CommandLine::ForCurrentProcess(); const base::CommandLine::SwitchMap& switches = our_cmdline.GetSwitches(); @@ -91,13 +106,21 @@ // implicitly in the future. Keeping --args would mean changes to the file // would be ignored. if (i->first != switches::kQuiet && i->first != switches::kRoot && - i->first != switches::kArgs) { + i->first != switches::kDotfile && i->first != switches::kArgs) { std::string escaped_value = EscapeString(FilePathToUTF8(i->second), escape_shell, nullptr); cmdline.AppendSwitchASCII(i->first, escaped_value); } } + return cmdline; +} + +namespace { + +std::string GetSelfInvocationCommand(const BuildSettings* build_settings) { + base::CommandLine cmdline = GetSelfInvocationCommandLine( + build_settings); #if defined(OS_WIN) return base::WideToUTF8(cmdline.GetCommandLineString()); #else
diff --git a/tools/gn/ninja_build_writer.h b/tools/gn/ninja_build_writer.h index 9d2ac4c..f4351e4 100644 --- a/tools/gn/ninja_build_writer.h +++ b/tools/gn/ninja_build_writer.h
@@ -20,6 +20,10 @@ class Target; class Toolchain; +namespace base { +class CommandLine; +} // base + // Generates the toplevel "build.ninja" file. This references the individual // toolchain files and lists all input .gn files as dependencies of the // build itself. @@ -70,4 +74,8 @@ extern const char kNinjaRules_Help[]; +// Exposed for testing. +base::CommandLine GetSelfInvocationCommandLine( + const BuildSettings* build_settings); + #endif // TOOLS_GN_NINJA_BUILD_WRITER_H_
diff --git a/tools/gn/ninja_build_writer_unittest.cc b/tools/gn/ninja_build_writer_unittest.cc index 5e19df2..55ffcc9 100644 --- a/tools/gn/ninja_build_writer_unittest.cc +++ b/tools/gn/ninja_build_writer_unittest.cc
@@ -4,9 +4,12 @@ #include <sstream> +#include "base/command_line.h" +#include "base/files/file_util.h" #include "tools/gn/ninja_build_writer.h" #include "tools/gn/pool.h" #include "tools/gn/scheduler.h" +#include "tools/gn/switches.h" #include "tools/gn/target.h" #include "tools/gn/test_with_scheduler.h" #include "tools/gn/test_with_scope.h" @@ -14,6 +17,55 @@ using NinjaBuildWriterTest = TestWithScheduler; +class ScopedDotGNFile { + public: + ScopedDotGNFile(const base::FilePath& path) + : path_(path), + file_(path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE) { + EXPECT_TRUE(file_.IsValid()); + } + ~ScopedDotGNFile() { + file_.Close(); + base::DeleteFile(path_, false); + } + + private: + base::FilePath path_; + base::File file_; +}; + +TEST_F(NinjaBuildWriterTest, GetSelfInvocationCommandLine) { + // TestWithScope sets up a config with a build dir of //out/Debug. + TestWithScope setup; + base::CommandLine cmd_out(base::CommandLine::NO_PROGRAM); + + // Setup sets the default root dir to ".". + base::FilePath root(FILE_PATH_LITERAL(".")); + base::FilePath root_realpath = base::MakeAbsoluteFilePath(root); + + base::FilePath gn(FILE_PATH_LITERAL("testdot.gn")); + + // The file must exist on disk for MakeAbsoluteFilePath() to work. + ScopedDotGNFile dot_gn(gn); + base::FilePath gn_realpath = base::MakeAbsoluteFilePath(gn); + + // Without any parameters the self invocation should pass --root=../.. + // (from //out/Debug to //). + setup.build_settings()->SetRootPath(root_realpath); + cmd_out = GetSelfInvocationCommandLine(setup.build_settings()); + EXPECT_EQ("../..", cmd_out.GetSwitchValueASCII(switches::kRoot)); + EXPECT_FALSE(cmd_out.HasSwitch(switches::kDotfile)); + + // If --root is . and --dotfile is foo/.gn, then --dotfile also needs + // to to become ../../foo/.gn. + setup.build_settings()->SetRootPath(root_realpath); + setup.build_settings()->set_dotfile_name(gn_realpath); + cmd_out = GetSelfInvocationCommandLine(setup.build_settings()); + EXPECT_EQ("../..", cmd_out.GetSwitchValueASCII(switches::kRoot)); + EXPECT_EQ("../../testdot.gn", + cmd_out.GetSwitchValueASCII(switches::kDotfile)); +} + TEST_F(NinjaBuildWriterTest, TwoTargets) { TestWithScope setup; Err err;
diff --git a/tools/gn/setup.cc b/tools/gn/setup.cc index 9d2e7b1..27c6475 100644 --- a/tools/gn/setup.cc +++ b/tools/gn/setup.cc
@@ -561,19 +561,21 @@ // When --root is specified, an alternate --dotfile can also be set. // --dotfile should be a real file path and not a "//foo" source-relative // path. - base::FilePath dot_file_path = + base::FilePath dotfile_path = cmdline.GetSwitchValuePath(switches::kDotfile); - if (dot_file_path.empty()) { + if (dotfile_path.empty()) { dotfile_name_ = root_path.Append(kGnFile); } else { - dotfile_name_ = base::MakeAbsoluteFilePath(dot_file_path); + dotfile_name_ = base::MakeAbsoluteFilePath(dotfile_path); if (dotfile_name_.empty()) { Err(Location(), "Could not load dotfile.", - "The file \"" + FilePathToUTF8(dot_file_path) + + "The file \"" + FilePathToUTF8(dotfile_path) + "\" couldn't be loaded.") .PrintToStdout(); return false; } + // Only set dotfile_name if it was passed explicitly. + build_settings_.set_dotfile_name(dotfile_name_); } } else { // In the default case, look for a dotfile and that also tells us where the
diff --git a/tools/gn/switches.cc b/tools/gn/switches.cc index 3d77642..1e0082c 100644 --- a/tools/gn/switches.cc +++ b/tools/gn/switches.cc
@@ -63,9 +63,6 @@ Normally GN loads the ".gn"file from the source root for some basic configuration (see "gn help dotfile"). This flag allows you to use a different file. - - Note that this interacts with "--root" in a possibly incorrect way. - It would be nice to test the edge cases and document or fix. )"; const char kFailOnUnusedArgs[] = "fail-on-unused-args";