Redo GN "args" command

Previously "gn args" printed build arguments. That has now moved to "gn args <outdir> --list" along with a new --short mode that's a bit more convenient for some uses.

Now "gn args" runs your editor on the build arguments for the given output directory, and re-generates the build given those arguments. This is an easier way to manage changing build arguments for a build (previously you would have to re-type everything or edit build.ninja manually).

"gn gen" now always uses the existing arguments for the given output dir, unless "--args" is manually specified (giving the old behavior of just using those). This also allows a more convenient way for a user to recover from a borked build (sincetimes I ran into a state where something was missing that prevent ninja from even starting enough to rebuild the build).

I removed the "show" and "refresh" ninja phony rules since the new commands cover those cases.

This patch adds some additional tracing to build startup since I noticed it was missing when trying to figure out why the args command was so slow (I fixed the main reason, it was with new code I added).

Added proper escaping for printing string values and unit tests for these.

Two minor build file fixes.

R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/265693003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@268042 0039d316-1c4b-4281-b951-d872f2087c98

build/config/clang/BUILD.gn

@@ -32,7 +32,7 @@ config("find_bad_constructs") {
 # compile with these so may want to remove this config.
 config("extra_warnings") {
   cflags = [
-    "-Wheader-hygiene"
+    "-Wheader-hygiene",
 
     # Warns when a const char[] is converted to bool.
     "-Wstring-conversion",

third_party/wtl/BUILD.gn

@@ -10,5 +10,5 @@ config("wtl_includes") {
 # actually generate anything linkable, and inject the required config for
 # making the include directories work.
 group("wtl") {
-  all_dependent_configs = ":wtl_includes"
+  all_dependent_configs = [ ":wtl_includes" ]
 }

tools/gn/BUILD.gn

@@ -203,6 +203,7 @@ test("gn_unittests") {
     "test_with_scope.cc",
     "test_with_scope.h",
     "tokenizer_unittest.cc",
+    "value_unittest.cc",
     "visibility_unittest.cc",
   ]
   deps = [

tools/gn/args.cc

@@ -26,20 +26,33 @@ const char kBuildArgs_Help[] =
     "   - default_os\n"
     "   - os (by default this is the same as \"default_os\")\n"
     "\n"
-    "  Second, arguments specified on the command-line via \"--args\" are\n"
-    "  applied. These can override the system default ones, and add new ones.\n"
-    "  These are whitespace-separated. For example:\n"
+    "  If specified, arguments from the --args command line flag are used. If\n"
+    "  that flag is not specified, args from previous builds in the build\n"
+    "  directory will be used (this is in the file args.gn in the build\n"
+    "  directory).\n"
     "\n"
-    "    gn --args=\"enable_doom_melon=false\" os=\\\"beos\\\"\n"
-    "\n"
-    "  Third, toolchain overrides are applied. These are specified in the\n"
+    "  Last, for targets being compiled with a non-default toolchain, the\n"
+    "  toolchain overrides are applied. These are specified in the\n"
     "  toolchain_args section of a toolchain definition. The use-case for\n"
     "  this is that a toolchain may be building code for a different\n"
     "  platform, and that it may want to always specify Posix, for example.\n"
     "  See \"gn help toolchain_args\" for more.\n"
     "\n"
-    "  It is an error to specify an override for a build argument that never\n"
-    "  appears in a \"declare_args\" call.\n"
+    "  If you specify an override for a build argument that never appears in\n"
+    "  a \"declare_args\" call, a nonfatal error will be displayed.\n"
+    "\n"
+    "Examples\n"
+    "\n"
+    "  gn args out/FooBar\n"
+    "      Create the directory out/FooBar and open an editor. You would type\n"
+    "      something like this into that file:\n"
+    "          enable_doom_melon=false\n"
+    "          os=\"android\"\n"
+    "\n"
+    "  gn gen out/FooBar --args=\"enable_doom_melon=true os=\\\"android\\\"\"\n"
+    "      This will overwrite the build directory with the given arguments.\n"
+    "      (Note that the quotes inside the args command will usually need to\n"
+    "      be escaped for your shell to pass through strings values.)\n"
     "\n"
     "How build arguments are used\n"
     "\n"

tools/gn/command_args.cc

@@ -2,21 +2,38 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <stdio.h>
+#include <stdlib.h>
+
 #include <map>
 
+#include "base/command_line.h"
+#include "base/environment.h"
+#include "base/file_util.h"
+#include "base/process/launch.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "tools/gn/commands.h"
+#include "tools/gn/filesystem_utils.h"
 #include "tools/gn/input_file.h"
 #include "tools/gn/parse_tree.h"
 #include "tools/gn/setup.h"
 #include "tools/gn/standard_out.h"
 #include "tools/gn/tokenizer.h"
+#include "tools/gn/trace.h"
+
+#if defined(OS_WIN)
+#include <windows.h>
+#include <shellapi.h>
+#endif
 
 namespace commands {
 
 namespace {
 
+const char kSwitchList[] = "list";
+const char kSwitchShort[] = "short";
+
 bool DoesLineBeginWithComment(const base::StringPiece& line) {
   // Skip whitespace.
   size_t i = 0;
@@ -95,67 +112,50 @@ void PrintArgHelp(const base::StringPiece& name, const Value& value) {
   }
 }
 
-}  // namespace
-
-extern const char kArgs[] = "args";
-extern const char kArgs_HelpShort[] =
-    "args: Display configurable arguments declared by the build.";
-extern const char kArgs_Help[] =
-    "gn args [arg name]\n"
-    "  Displays all arguments declared by buildfiles along with their\n"
-    "  description. Build arguments are anything in a declare_args() block\n"
-    "  in any buildfile. The comment preceding the declaration will be\n"
-    "  displayed here (so comment well!).\n"
-    "\n"
-    "  These arguments can be overridden on the command-line:\n"
-    "    --args=\"doom_melon_setting=5 component_build=1\"\n"
-    "  or in a toolchain definition (see \"gn help buildargs\" for more on\n"
-    "  how this all works).\n"
-    "\n"
-    "  If \"arg name\" is specified, only the information for that argument\n"
-    "  will be displayed. Otherwise all arguments will be displayed.\n";
-
-int RunArgs(const std::vector<std::string>& args) {
+int ListArgs(const std::string& build_dir) {
   Setup* setup = new Setup;
   setup->set_check_for_bad_items(false);
-  // TODO(brettw) bug 343726: Use a temporary directory instead of this
-  // default one to avoid messing up any build that's in there.
-  if (!setup->DoSetup("//out/Default/") || !setup->Run())
+  if (!setup->DoSetup(build_dir) || !setup->Run())
     return 1;
 
   Scope::KeyValueMap build_args;
   setup->build_settings().build_args().MergeDeclaredArguments(&build_args);
 
-  if (args.size() == 1) {
-    // Get help on a specific command.
-    Scope::KeyValueMap::const_iterator found_arg = build_args.find(args[0]);
+  // Find all of the arguments we care about. Use a regular map so they're
+  // sorted nicely when we write them out.
+  std::map<base::StringPiece, Value> sorted_args;
+  std::string list_value =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(kSwitchList);
+  if (list_value.empty()) {
+    // List all values.
+    for (Scope::KeyValueMap::const_iterator i = build_args.begin();
+         i != build_args.end(); ++i)
+      sorted_args.insert(*i);
+  } else {
+    // List just the one specified as the parameter to --list.
+    Scope::KeyValueMap::const_iterator found_arg = build_args.find(list_value);
     if (found_arg == build_args.end()) {
       Err(Location(), "Unknown build argument.",
-          "You asked for \"" + args[0] + "\" which I didn't find in any "
-          "buildfile\nassociated with this build.");
+          "You asked for \"" + list_value + "\" which I didn't find in any "
+          "build file\nassociated with this build.").PrintToStdout();
       return 1;
     }
-    PrintArgHelp(args[0], found_arg->second);
-    return 0;
-  } else if (args.size() > 1) {
-    // Too many arguments.
-    Err(Location(), "You're holding it wrong.",
-        "Usage: \"gn args [arg name]\"").PrintToStdout();
-    return 1;
+    sorted_args.insert(*found_arg);
   }
 
-  // List all arguments. First put them in a regular map so they're sorted.
-  std::map<base::StringPiece, Value> sorted_args;
-  for (Scope::KeyValueMap::const_iterator i = build_args.begin();
-       i != build_args.end(); ++i)
-    sorted_args.insert(*i);
-
-  OutputString(
-      "Available build arguments. Note that the which arguments are declared\n"
-      "and their default values may depend on other arguments or the current\n"
-      "platform and architecture. So setting some values may add, remove, or\n"
-      "change the default value of other values.\n\n");
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(kSwitchShort)) {
+    // Short key=value output.
+    for (std::map<base::StringPiece, Value>::iterator i = sorted_args.begin();
+         i != sorted_args.end(); ++i) {
+      OutputString(i->first.as_string());
+      OutputString(" = ");
+      OutputString(i->second.ToString(true));
+      OutputString("\n");
+    }
+    return 0;
+  }
 
+  // Long output.
   for (std::map<base::StringPiece, Value>::iterator i = sorted_args.begin();
        i != sorted_args.end(); ++i) {
     PrintArgHelp(i->first, i->second);
@@ -165,4 +165,183 @@ int RunArgs(const std::vector<std::string>& args) {
   return 0;
 }
 
+#if defined(OS_WIN)
+
+bool RunEditor(const base::FilePath& file_to_edit) {
+  SHELLEXECUTEINFO info;
+  memset(&info, 0, sizeof(info));
+  info.cbSize = sizeof(info);
+  info.fMask = SEE_MASK_NOCLOSEPROCESS | SEE_MASK_CLASSNAME;
+  info.lpFile = file_to_edit.value().c_str();
+  info.nShow = SW_SHOW;
+  info.lpClass = L".txt";
+  if (!::ShellExecuteEx(&info)) {
+    Err(Location(), "Couldn't run editor.",
+        "Just edit \"" + FilePathToUTF8(file_to_edit) +
+        "\" manually instead.").PrintToStdout();
+    return false;
+  }
+
+  if (!info.hProcess) {
+    // Windows re-used an existing process.
+    OutputString("\"" + FilePathToUTF8(file_to_edit) +
+                 "\" opened in editor, save it and press <Enter> when done.\n");
+    getchar();
+  } else {
+    OutputString("Waiting for editor on \"" + FilePathToUTF8(file_to_edit) +
+                 "\"...\n");
+    ::WaitForSingleObject(info.hProcess, INFINITE);
+    ::CloseHandle(info.hProcess);
+  }
+  return true;
+}
+
+#else  // POSIX
+
+void RunEditor(const base::FilePath& file_to_edit) {
+  // Prefer $VISUAL, then $EDITOR, then vi.
+  const char* editor_ptr = getenv("VISUAL");
+  if (!editor_ptr)
+    editor_ptr = getenv("EDITOR");
+  if (!editor_ptr)
+    editor_ptr = "vi";
+
+  std::string cmd(editor_ptr);
+  cmd.append(" \"");
+
+  // Its impossible to do this properly since we don't know the user's shell,
+  // but quoting and escaping internal quotes should handle 99.999% of all
+  // cases.
+  std::string escaped_name = file_to_edit.value();
+  ReplaceSubstringsAfterOffset(&escaped_name, 0, "\"", "\\\"");
+  cmd.append(escaped_name);
+  cmd.push_back('"');
+
+  OutputString("Waiting for editor on \"" + file_to_edit.value() +
+               "\"...\n");
+  return system(cmd.c_str()) == 0;
+}
+
+#endif
+
+int EditArgsFile(const std::string& build_dir) {
+  {
+    // Scope the setup. We only use it for some basic state. We'll do the
+    // "real" build below in the gen command.
+    Setup setup;
+    setup.set_check_for_bad_items(false);
+    // Don't fill build arguments. We're about to edit the file which supplies
+    // these in the first place.
+    setup.set_fill_arguments(false);
+    if (!setup.DoSetup(build_dir))
+      return 1;
+
+    // Ensure the file exists. Need to normalize path separators since on
+    // Windows they can come out as forward slashes here, and that confuses some
+    // of the commands.
+    base::FilePath arg_file =
+        setup.build_settings().GetFullPath(setup.GetBuildArgFile())
+        .NormalizePathSeparators();
+    if (!base::PathExists(arg_file)) {
+      std::string argfile_default_contents =
+          "# Build arguments go here. Examples:\n"
+          "#   enable_doom_melon = true\n"
+          "#   crazy_something = \"absolutely\"\n";
+#if defined(OS_WIN)
+      // Use Windows lineendings for this file since it will often open in
+      // Notepad which can't handle Unix ones.
+      ReplaceSubstringsAfterOffset(&argfile_default_contents, 0, "\n", "\r\n");
+#endif
+      base::CreateDirectory(arg_file.DirName());
+      base::WriteFile(arg_file, argfile_default_contents.c_str(),
+                      argfile_default_contents.size());
+    }
+
+    ScopedTrace editor_trace(TraceItem::TRACE_SETUP, "Waiting for editor");
+    if (!RunEditor(arg_file))
+      return 1;
+  }
+
+  // Now do a normal "gen" command.
+  OutputString("Generating files...\n");
+  std::vector<std::string> gen_commands;
+  gen_commands.push_back(build_dir);
+  return RunGen(gen_commands);
+}
+
+}  // namespace
+
+extern const char kArgs[] = "args";
+extern const char kArgs_HelpShort[] =
+    "args: Display or configure arguments declared by the build.";
+extern const char kArgs_Help[] =
+    "gn args [arg name]\n"
+    "\n"
+    "  See also \"gn help buildargs\" for a more high-level overview of how\n"
+    "  build arguments work.\n"
+    "\n"
+    "Usage\n"
+    "  gn args <dir_name>\n"
+    "      Open the arguments for the given build directory in an editor\n"
+    "      (as specified by the EDITOR environment variable). If the given\n"
+    "      build directory doesn't exist, it will be created and an empty\n"
+    "      args file will be opened in the editor. You would type something\n"
+    "      like this into that file:\n"
+    "          enable_doom_melon=false\n"
+    "          os=\"android\"\n"
+    "\n"
+    "      Note: you can edit the build args manually by editing the file\n"
+    "      \"args.gn\" in the build directory and then running\n"
+    "      \"gn gen <build_dir>\".\n"
+    "\n"
+    "  gn args <dir_name> --list[=<exact_arg>] [--short]\n"
+    "      Lists all build arguments available in the current configuration,\n"
+    "      or, if an exact_arg is specified for the list flag, just that one\n"
+    "      build argument.\n"
+    "\n"
+    "      The output will list the declaration location, default value, and\n"
+    "      comment preceeding the declaration. If --short is specified,\n"
+    "      only the names and values will be printed.\n"
+    "\n"
+    "      If the dir_name is specified, the build configuration will be\n"
+    "      taken from that build directory. The reason this is needed is that\n"
+    "      the definition of some arguments is dependent on the build\n"
+    "      configuration, so setting some values might add, remove, or change\n"
+    "      the default values for other arguments. Specifying your exact\n"
+    "      configuration allows the proper arguments to be displayed.\n"
+    "\n"
+    "      Instead of specifying the dir_name, you can also use the\n"
+    "      command-line flag to specify the build configuration:\n"
+    "        --args=<exact list of args to use>\n"
+    "\n"
+    "Examples\n"
+    "  gn args out/Debug\n"
+    "    Opens an editor with the args for out/Debug.\n"
+    "\n"
+    "  gn args out/Debug --list --short\n"
+    "    Prints all arguments with their default values for the out/Debug\n"
+    "    build.\n"
+    "\n"
+    "  gn args out/Debug --list=cpu_arch\n"
+    "    Prints information about the \"cpu_arch\" argument for the out/Debug\n"
+    "    build.\n"
+    "\n"
+    "  gn args --list --args=\"os=\\\"android\\\" enable_doom_melon=true\"\n"
+    "    Prints all arguments with the default values for a build with the\n"
+    "    given arguments set (which may affect the values of other\n"
+    "    arguments).\n";
+
+int RunArgs(const std::vector<std::string>& args) {
+  if (args.size() != 1) {
+    Err(Location(), "Exactly one build dir needed.",
+        "Usage: \"gn args <build_dir>\"\n"
+        "Or see \"gn help args\" for more variants.").PrintToStdout();
+    return 1;
+  }
+
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(kSwitchList))
+    return ListArgs(args[0]);
+  return EditArgsFile(args[0]);
+}
+
 }  // namespace commands

tools/gn/escape_unittest.cc

@@ -17,8 +17,9 @@ TEST(Escape, Shell) {
   opts.mode = ESCAPE_SHELL;
   std::string result = EscapeString("asdf: \"$\\bar", opts, NULL);
 #if defined(OS_WIN)
-  // Windows shell doesn't escape backslashes.
-  EXPECT_EQ("\"asdf: \"$\\bar\"", result);
+  // Windows shell doesn't escape backslashes, but it does backslash-escape
+  // quotes.
+  EXPECT_EQ("\"asdf: \\\"$\\bar\"", result);
 #else
   EXPECT_EQ("\"asdf: \\\"$\\\\bar\"", result);
 #endif