tools/gn/exec_process.cc - Issue 646013003: GN: Capture and print stderr from exec_script

Unified Diff: tools/gn/exec_process.cc

Issue 646013003: GN: Capture and print stderr from exec_script (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Ha, even better for diffing... using really low --similarity Created 6 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: tools/gn/exec_process.cc

diff --git a/tools/gn/function_exec_script.cc b/tools/gn/exec_process.cc

similarity index 43%

copy from tools/gn/function_exec_script.cc

copy to tools/gn/exec_process.cc

index eb30a6ec399b64aac6ddad66441fdf03baf4aebf..d8354f9c69ce6cb7549101bb4c6057819255f31f 100644

--- a/tools/gn/function_exec_script.cc

+++ b/tools/gn/exec_process.cc

@@ -1,4 +1,4 @@

// Use of this source code is governed by a BSD-style license that can be

// found in the LICENSE file.

@@ -7,19 +7,6 @@

#include "base/logging.h"

#include "base/process/kill.h"

#include "base/process/launch.h"

-#include "base/strings/string_number_conversions.h"

-#include "base/strings/utf_string_conversions.h"

-#include "base/time/time.h"

-#include "build/build_config.h"

-#include "tools/gn/err.h"

-#include "tools/gn/filesystem_utils.h"

-#include "tools/gn/functions.h"

-#include "tools/gn/input_conversion.h"

-#include "tools/gn/input_file.h"

-#include "tools/gn/parse_tree.h"

-#include "tools/gn/scheduler.h"

-#include "tools/gn/trace.h"

-#include "tools/gn/value.h"

#if defined(OS_WIN)

#include <windows.h>

@@ -32,14 +19,11 @@

#include <fcntl.h>

#include <unistd.h>

+#include "base/posix/eintr_wrapper.h"

#include "base/posix/file_descriptor_shuffle.h"

#endif

-namespace functions {

-namespace {

-const char kNoExecSwitch[] = "no-exec";

+namespace internal {

#if defined(OS_WIN)

bool ExecProcess(const CommandLine& cmdline,

@@ -139,6 +123,22 @@ bool ExecProcess(const CommandLine& cmdline,

return true;

}

#else

+// Reads from the provided file descriptor and appends to output. Returns false

+// if the fd is closed or there is an unexpected error (not

+// EINTR/EAGAIN/EWOULDBLOCK).

+bool ReadFromPipe(int fd, std::string* output) {

+ char buffer[256];

+ int bytes_read = HANDLE_EINTR(read(fd, buffer, sizeof(buffer)));

+ if (bytes_read == -1) {

+ return errno == EAGAIN || errno == EWOULDBLOCK;

+ } else if (bytes_read <= 0) {

+ return false;

+ }

+ output->append(buffer, bytes_read);

+ return true;

brettw 2014/10/14 17:27:56 Nit: only one blank

cjhopman 2014/10/16 18:25:05 Done.

bool ExecProcess(const CommandLine& cmdline,

const base::FilePath& startup_dir,

std::string* std_out,

@@ -148,7 +148,7 @@ bool ExecProcess(const CommandLine& cmdline,

std::vector<std::string> argv = cmdline.argv();

- int pipe_fd[2];

+ int out_fd[2], err_fd[2];

pid_t pid;

base::InjectiveMultimap fd_shuffle1, fd_shuffle2;

scoped_ptr<char*[]> argv_cstr(new char*[argv.size() + 1]);

@@ -156,13 +156,20 @@ bool ExecProcess(const CommandLine& cmdline,

fd_shuffle1.reserve(3);

fd_shuffle2.reserve(3);

- if (pipe(pipe_fd) < 0)

+ if (pipe(out_fd) < 0)

return false;

+ base::ScopedFD out_read(out_fd[0]), out_write(out_fd[1]);

+ if (pipe(err_fd) < 0)

+ return false;

+ base::ScopedFD err_read(err_fd[0]), err_write(err_fd[1]);

+ if (out_read.get() > FD_SETSIZE || err_read.get() > FD_SETSIZE) {

+ return false;

brettw 2014/10/14 17:27:56 Indent (also remove {})

cjhopman 2014/10/16 18:25:05 Done.

+ }

switch (pid = fork()) {

case -1: // error

- close(pipe_fd[0]);

- close(pipe_fd[1]);

return false;

case 0: // child

{

@@ -179,9 +186,9 @@ bool ExecProcess(const CommandLine& cmdline,

_exit(127);

fd_shuffle1.push_back(

- base::InjectionArc(pipe_fd[1], STDOUT_FILENO, true));

+ base::InjectionArc(out_write.get(), STDOUT_FILENO, true));

fd_shuffle1.push_back(

- base::InjectionArc(dev_null, STDERR_FILENO, true));

+ base::InjectionArc(err_write.get(), STDERR_FILENO, true));

fd_shuffle1.push_back(

base::InjectionArc(dev_null, STDIN_FILENO, true));

// Adding another element here? Remeber to increase the argument to

@@ -209,18 +216,30 @@ bool ExecProcess(const CommandLine& cmdline,

// Close our writing end of pipe now. Otherwise later read would not

// be able to detect end of child's output (in theory we could still

// write to the pipe).

- close(pipe_fd[1]);

- char buffer[256];

- ssize_t bytes_read = 0;

- while (true) {

- bytes_read = HANDLE_EINTR(read(pipe_fd[0], buffer, sizeof(buffer)));

- if (bytes_read <= 0)

+ out_write.reset();

+ err_write.reset();

+ bool out_open = true, err_open = true;

+ while (out_open || err_open) {

+ fd_set read_fds;

+ FD_ZERO(&read_fds);

+ FD_SET(out_read.get(), &read_fds);

brettw 2014/10/14 17:27:56 select() sure is yucky!

+ FD_SET(err_read.get(), &read_fds);

+ int res =

+ HANDLE_EINTR(select(std::max(out_read.get(), err_read.get()) + 1,

+ &read_fds,

+ NULL,

+ NULL));

+ if (res <= 0)

break;

- std_out->append(buffer, bytes_read);

+ if (FD_ISSET(out_read.get(), &read_fds)) {

brettw 2014/10/14 17:27:56 No {} for consistency.

cjhopman 2014/10/16 18:25:05 Done.

+ out_open = ReadFromPipe(out_read.get(), std_out);

+ }

+ if (FD_ISSET(err_read.get(), &read_fds)) {

+ err_open = ReadFromPipe(err_read.get(), std_err);

+ }

}

- close(pipe_fd[0]);

return base::WaitForExitCode(pid, exit_code);

}

@@ -230,188 +249,5 @@ bool ExecProcess(const CommandLine& cmdline,

}

#endif

-} // namespace

-const char kExecScript[] = "exec_script";

-const char kExecScript_HelpShort[] =

- "exec_script: Synchronously run a script and return the output.";

-const char kExecScript_Help[] =

- "exec_script: Synchronously run a script and return the output.\n"

- "\n"

- " exec_script(filename,\n"

- " arguments = [],\n"

- " input_conversion = \"\",\n"

- " file_dependencies = [])\n"

- "\n"

- " Runs the given script, returning the stdout of the script. The build\n"

- " generation will fail if the script does not exist or returns a nonzero\n"

- " exit code.\n"

- "\n"

- " The current directory when executing the script will be the root\n"

- " build directory. If you are passing file names, you will want to use\n"

- " the rebase_path() function to make file names relative to this\n"

- " path (see \"gn help rebase_path\").\n"

- "\n"

- "Arguments:\n"

- "\n"

- " filename:\n"

- " File name of python script to execute. Non-absolute names will\n"

- " be treated as relative to the current build file.\n"

- "\n"

- " arguments:\n"

- " A list of strings to be passed to the script as arguments.\n"

- " May be unspecified or the empty list which means no arguments.\n"

- "\n"

- " input_conversion:\n"

- " Controls how the file is read and parsed.\n"

- " See \"gn help input_conversion\".\n"

- "\n"

- " If unspecified, defaults to the empty string which causes the\n"

- " script result to be discarded. exec script will return None.\n"

- "\n"

- " dependencies:\n"

- " (Optional) A list of files that this script reads or otherwise\n"

- " depends on. These dependencies will be added to the build result\n"

- " such that if any of them change, the build will be regenerated and\n"

- " the script will be re-run.\n"

- "\n"

- " The script itself will be an implicit dependency so you do not\n"

- " need to list it.\n"

- "\n"

- "Example:\n"

- "\n"

- " all_lines = exec_script(\n"

- " \"myscript.py\", [some_input], \"list lines\",\n"

- " [ rebase_path(\"data_file.txt\", root_build_dir) ])\n"

- "\n"

- " # This example just calls the script with no arguments and discards\n"

- " # the result.\n"

- " exec_script(\"//foo/bar/myscript.py\")\n";

-Value RunExecScript(Scope* scope,

- const FunctionCallNode* function,

- const std::vector<Value>& args,

- Err* err) {

- if (args.size() < 1 || args.size() > 4) {

- *err = Err(function->function(), "Wrong number of arguments to exec_script",

- "I expected between one and four arguments.");

- return Value();

- }

- const Settings* settings = scope->settings();

- const BuildSettings* build_settings = settings->build_settings();

- const SourceDir& cur_dir = scope->GetSourceDir();

- // Find the python script to run.

- if (!args[0].VerifyTypeIs(Value::STRING, err))

- return Value();

- SourceFile script_source =

- cur_dir.ResolveRelativeFile(args[0].string_value());

- base::FilePath script_path = build_settings->GetFullPath(script_source);

- if (!build_settings->secondary_source_path().empty() &&

- !base::PathExists(script_path)) {

- // Fall back to secondary source root when the file doesn't exist.

- script_path = build_settings->GetFullPathSecondary(script_source);

- }

- ScopedTrace trace(TraceItem::TRACE_SCRIPT_EXECUTE, script_source.value());

- trace.SetToolchain(settings->toolchain_label());

- // Add all dependencies of this script, including the script itself, to the

- // build deps.

- g_scheduler->AddGenDependency(script_path);

- if (args.size() == 4) {

- const Value& deps_value = args[3];

- if (!deps_value.VerifyTypeIs(Value::LIST, err))

- return Value();

- for (const auto& dep : deps_value.list_value()) {

- if (!dep.VerifyTypeIs(Value::STRING, err))

- return Value();

- g_scheduler->AddGenDependency(

- build_settings->GetFullPath(cur_dir.ResolveRelativeFile(

- dep.string_value())));

- }

- // Make the command line.

- const base::FilePath& python_path = build_settings->python_path();

- CommandLine cmdline(python_path);

- cmdline.AppendArgPath(script_path);

- if (args.size() >= 2) {

- // Optional command-line arguments to the script.

- const Value& script_args = args[1];

- if (!script_args.VerifyTypeIs(Value::LIST, err))

- return Value();

- for (const auto& arg : script_args.list_value()) {

- if (!arg.VerifyTypeIs(Value::STRING, err))

- return Value();

- cmdline.AppendArg(arg.string_value());

- }

- // Log command line for debugging help.

- trace.SetCommandLine(cmdline);

- base::TimeTicks begin_exec;

- if (g_scheduler->verbose_logging()) {

-#if defined(OS_WIN)

- g_scheduler->Log("Pythoning",

- base::UTF16ToUTF8(cmdline.GetCommandLineString()));

-#else

- g_scheduler->Log("Pythoning", cmdline.GetCommandLineString());

-#endif

- begin_exec = base::TimeTicks::Now();

- }

- base::FilePath startup_dir =

- build_settings->GetFullPath(build_settings->build_dir());

- // The first time a build is run, no targets will have been written so the

- // build output directory won't exist. We need to make sure it does before

- // running any scripts with this as its startup directory, although it will

- // be relatively rare that the directory won't exist by the time we get here.

- //

- // If this shows up on benchmarks, we can cache whether we've done this

- // or not and skip creating the directory.

- base::CreateDirectory(startup_dir);

- // Execute the process.

- // TODO(brettw) set the environment block.

- std::string output;

- std::string stderr_output; // TODO(brettw) not hooked up, see above.

- int exit_code = 0;

- if (!CommandLine::ForCurrentProcess()->HasSwitch(kNoExecSwitch)) {

- if (!ExecProcess(cmdline, startup_dir,

- &output, &stderr_output, &exit_code)) {

- *err = Err(function->function(), "Could not execute python.",

- "I was trying to execute \"" + FilePathToUTF8(python_path) + "\".");

- return Value();

- }

- if (g_scheduler->verbose_logging()) {

- g_scheduler->Log("Pythoning", script_source.value() + " took " +

- base::Int64ToString(

- (base::TimeTicks::Now() - begin_exec).InMilliseconds()) +

- "ms");

- }

- // TODO(brettw) maybe we need stderr also for reasonable stack dumps.

- if (exit_code != 0) {

- std::string msg = "Current dir: " + FilePathToUTF8(startup_dir) +

- "\nCommand: " + FilePathToUTF8(cmdline.GetCommandLineString()) +

- "\nReturned " + base::IntToString(exit_code);

- if (!output.empty())

- msg += " and printed out:\n\n" + output;

- else

- msg += ".";

- *err = Err(function->function(), "Script returned non-zero exit code.",

- msg);

- return Value();

- }

- // Default to None value for the input conversion if unspecified.

- return ConvertInputToValue(scope->settings(), output, function,

- args.size() >= 3 ? args[2] : Value(), err);

+} // namespace internal

-} // namespace functions

« no previous file with comments | « tools/gn/exec_process.h ('k') | tools/gn/exec_process_unittest.cc » ('j') | no next file with comments »