Commit 4dd3ef06 authored by brettw's avatar brettw Committed by Commit bot

Require build directories exist for some GN commands.

Some GN commands just query the state of the build. They generally take the
build directory as a first argument. But this is easy to forget and GN will
auto-create these directories if they don't exist. So I end up with directories
named "base/*" because I was trying to list all targets in base but forgot the
build argument.

This patch requires that the build directory contain a build.ninja file for
the "querying" commands to prevent this case.

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

Cr-Commit-Position: refs/heads/master@{#294071}
parent e39fad3a
...@@ -115,7 +115,7 @@ void PrintArgHelp(const base::StringPiece& name, const Value& value) { ...@@ -115,7 +115,7 @@ void PrintArgHelp(const base::StringPiece& name, const Value& value) {
int ListArgs(const std::string& build_dir) { int ListArgs(const std::string& build_dir) {
Setup* setup = new Setup; Setup* setup = new Setup;
setup->set_check_for_bad_items(false); setup->set_check_for_bad_items(false);
if (!setup->DoSetup(build_dir) || !setup->Run()) if (!setup->DoSetup(build_dir, false) || !setup->Run())
return 1; return 1;
Scope::KeyValueMap build_args; Scope::KeyValueMap build_args;
...@@ -233,7 +233,7 @@ int EditArgsFile(const std::string& build_dir) { ...@@ -233,7 +233,7 @@ int EditArgsFile(const std::string& build_dir) {
// Don't fill build arguments. We're about to edit the file which supplies // Don't fill build arguments. We're about to edit the file which supplies
// these in the first place. // these in the first place.
setup.set_fill_arguments(false); setup.set_fill_arguments(false);
if (!setup.DoSetup(build_dir)) if (!setup.DoSetup(build_dir, true))
return 1; return 1;
// Ensure the file exists. Need to normalize path separators since on // Ensure the file exists. Need to normalize path separators since on
......
...@@ -51,7 +51,7 @@ int RunCheck(const std::vector<std::string>& args) { ...@@ -51,7 +51,7 @@ int RunCheck(const std::vector<std::string>& args) {
// Deliberately leaked to avoid expensive process teardown. // Deliberately leaked to avoid expensive process teardown.
Setup* setup = new Setup(); Setup* setup = new Setup();
if (!setup->DoSetup(args[0])) if (!setup->DoSetup(args[0], false))
return 1; return 1;
if (!setup->Run()) if (!setup->Run())
return 1; return 1;
......
...@@ -579,7 +579,7 @@ int RunDesc(const std::vector<std::string>& args) { ...@@ -579,7 +579,7 @@ int RunDesc(const std::vector<std::string>& args) {
// Deliberately leaked to avoid expensive process teardown. // Deliberately leaked to avoid expensive process teardown.
Setup* setup = new Setup; Setup* setup = new Setup;
if (!setup->DoSetup(args[0])) if (!setup->DoSetup(args[0], false))
return 1; return 1;
if (!setup->Run()) if (!setup->Run())
return 1; return 1;
......
...@@ -77,7 +77,7 @@ int RunGen(const std::vector<std::string>& args) { ...@@ -77,7 +77,7 @@ int RunGen(const std::vector<std::string>& args) {
// Deliberately leaked to avoid expensive process teardown. // Deliberately leaked to avoid expensive process teardown.
Setup* setup = new Setup(); Setup* setup = new Setup();
if (!setup->DoSetup(args[0])) if (!setup->DoSetup(args[0], true))
return 1; return 1;
if (CommandLine::ForCurrentProcess()->HasSwitch(kSwitchCheck)) if (CommandLine::ForCurrentProcess()->HasSwitch(kSwitchCheck))
......
...@@ -67,7 +67,7 @@ int RunLs(const std::vector<std::string>& args) { ...@@ -67,7 +67,7 @@ int RunLs(const std::vector<std::string>& args) {
} }
Setup* setup = new Setup; Setup* setup = new Setup;
if (!setup->DoSetup(args[0]) || !setup->Run()) if (!setup->DoSetup(args[0], false) || !setup->Run())
return 1; return 1;
const CommandLine* cmdline = CommandLine::ForCurrentProcess(); const CommandLine* cmdline = CommandLine::ForCurrentProcess();
......
...@@ -230,7 +230,7 @@ int RunRefs(const std::vector<std::string>& args) { ...@@ -230,7 +230,7 @@ int RunRefs(const std::vector<std::string>& args) {
Setup* setup = new Setup; Setup* setup = new Setup;
setup->set_check_for_bad_items(false); setup->set_check_for_bad_items(false);
if (!setup->DoSetup(args[0]) || !setup->Run()) if (!setup->DoSetup(args[0], false) || !setup->Run())
return 1; return 1;
// Figure out the target or targets that the user is querying. // Figure out the target or targets that the user is querying.
......
...@@ -225,7 +225,7 @@ Setup::Setup() ...@@ -225,7 +225,7 @@ Setup::Setup()
Setup::~Setup() { Setup::~Setup() {
} }
bool Setup::DoSetup(const std::string& build_dir) { bool Setup::DoSetup(const std::string& build_dir, bool force_create) {
CommandLine* cmdline = CommandLine::ForCurrentProcess(); CommandLine* cmdline = CommandLine::ForCurrentProcess();
scheduler_.set_verbose_logging(cmdline->HasSwitch(kSwitchVerbose)); scheduler_.set_verbose_logging(cmdline->HasSwitch(kSwitchVerbose));
...@@ -241,8 +241,11 @@ bool Setup::DoSetup(const std::string& build_dir) { ...@@ -241,8 +241,11 @@ bool Setup::DoSetup(const std::string& build_dir) {
return false; return false;
if (!FillOtherConfig(*cmdline)) if (!FillOtherConfig(*cmdline))
return false; return false;
if (!FillBuildDir(build_dir)) // Must be after FillSourceDir to resolve.
// Must be after FillSourceDir to resolve.
if (!FillBuildDir(build_dir, !force_create))
return false; return false;
if (fill_arguments_) { if (fill_arguments_) {
if (!FillArguments(*cmdline)) if (!FillArguments(*cmdline))
return false; return false;
...@@ -438,7 +441,7 @@ bool Setup::FillSourceDir(const CommandLine& cmdline) { ...@@ -438,7 +441,7 @@ bool Setup::FillSourceDir(const CommandLine& cmdline) {
return true; return true;
} }
bool Setup::FillBuildDir(const std::string& build_dir) { bool Setup::FillBuildDir(const std::string& build_dir, bool require_exists) {
SourceDir resolved = SourceDir resolved =
SourceDirForCurrentDirectory(build_settings_.root_path()). SourceDirForCurrentDirectory(build_settings_.root_path()).
ResolveRelativeDir(build_dir); ResolveRelativeDir(build_dir);
...@@ -451,6 +454,21 @@ bool Setup::FillBuildDir(const std::string& build_dir) { ...@@ -451,6 +454,21 @@ bool Setup::FillBuildDir(const std::string& build_dir) {
if (scheduler_.verbose_logging()) if (scheduler_.verbose_logging())
scheduler_.Log("Using build dir", resolved.value()); scheduler_.Log("Using build dir", resolved.value());
if (require_exists) {
base::FilePath build_dir_path = build_settings_.GetFullPath(resolved);
if (!base::PathExists(build_dir_path.Append(
FILE_PATH_LITERAL("build.ninja")))) {
Err(Location(), "Not a build directory.",
"This command requires an existing build directory. I interpreted "
"your input\n\"" + build_dir + "\" as:\n " +
FilePathToUTF8(build_dir_path) +
"\nwhich doesn't seem to contain a previously-generated build.")
.PrintToStdout();
return false;
}
}
build_settings_.SetBuildDir(resolved); build_settings_.SetBuildDir(resolved);
return true; return true;
} }
......
...@@ -98,7 +98,14 @@ class Setup : public CommonSetup { ...@@ -98,7 +98,14 @@ class Setup : public CommonSetup {
// The parameter is the string the user specified for the build directory. We // The parameter is the string the user specified for the build directory. We
// will try to interpret this as a SourceDir if possible, and will fail if is // will try to interpret this as a SourceDir if possible, and will fail if is
// is malformed. // is malformed.
bool DoSetup(const std::string& build_dir); //
// With force_create = false, setup will fail if the build directory doesn't
// alreay exist with an args file in it. With force_create set to true, the
// directory will be created if necessary. Commands explicitly doing
// generation should set this to true to create it, but querying commands
// should set it to false to prevent creating oddly-named directories in case
// the user omits the build directory argument (which is easy to do).
bool DoSetup(const std::string& build_dir, bool force_create);
// Runs the load, returning true on success. On failure, prints the error // Runs the load, returning true on success. On failure, prints the error
// and returns false. This includes both RunPreMessageLoop() and // and returns false. This includes both RunPreMessageLoop() and
...@@ -139,8 +146,8 @@ class Setup : public CommonSetup { ...@@ -139,8 +146,8 @@ class Setup : public CommonSetup {
// Fills the build directory given the value the user has specified. // Fills the build directory given the value the user has specified.
// Must happen after FillSourceDir so we can resolve source-relative // Must happen after FillSourceDir so we can resolve source-relative
// paths. // paths. If require_exists is false, it will fail if the dir doesn't exist.
bool FillBuildDir(const std::string& build_dir); bool FillBuildDir(const std::string& build_dir, bool require_exists);
// Fills the python path portion of the command line. On failure, sets // Fills the python path portion of the command line. On failure, sets
// it to just "python". // it to just "python".
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment