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";
