From 4e4ec4c8ab8ea2e4340ae94f0f6c3f51653bccbe Mon Sep 17 00:00:00 2001 From: James Widman Date: Fri, 4 Oct 2024 16:45:44 -0700 Subject: [PATCH 1/2] Introduce new tool: `-t compdb-targets` Fixes #1544 Co-authored-by: Linkun Chen Co-authored-by: csmoe Co-authored-by: James Widman --- doc/manual.asciidoc | 5 ++ misc/output_test.py | 162 +++++++++++++++++++++++++++++----------- src/command_collector.h | 65 ++++++++++++++++ src/graph_test.cc | 49 ++++++++++++ src/ninja.cc | 148 +++++++++++++++++++++++++++++++----- src/util.cc | 16 ++++ src/util.h | 3 + 7 files changed, 386 insertions(+), 62 deletions(-) create mode 100644 src/command_collector.h diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc index e8b66807cf..bcd83a3f2f 100644 --- a/doc/manual.asciidoc +++ b/doc/manual.asciidoc @@ -289,6 +289,11 @@ http://clang.llvm.org/docs/JSONCompilationDatabase.html[JSON format] expected by the Clang tooling interface. _Available since Ninja 1.2._ +`compdb-targets`:: like `compdb`, but takes a list of targets instead of rules, +and expects at least one target. The resulting compilation database contains +all commands required to build the indicated targets, and _only_ those +commands. + `deps`:: show all dependencies stored in the `.ninja_deps` file. When given a target, show just the target's dependencies. _Available since Ninja 1.4._ diff --git a/misc/output_test.py b/misc/output_test.py index 81e49067c8..b9ded383f1 100755 --- a/misc/output_test.py +++ b/misc/output_test.py @@ -12,7 +12,7 @@ import tempfile import unittest from textwrap import dedent -from typing import Dict +import typing as T default_env = dict(os.environ) default_env.pop('NINJA_STATUS', None) @@ -20,6 +20,41 @@ default_env['TERM'] = '' NINJA_PATH = os.path.abspath('./ninja') +def remove_non_visible_lines(raw_output: bytes) -> str: + # When running in a smart terminal, Ninja uses CR (\r) to + # return the cursor to the start of the current line, prints + # something, then uses `\x1b[K` to clear everything until + # the end of the line. + # + # Thus printing 'FOO', 'BAR', 'ZOO' on the same line, then + # jumping to the next one results in the following output + # on Posix: + # + # '\rFOO\x1b[K\rBAR\x1b[K\rZOO\x1b[K\r\n' + # + # The following splits the output at both \r, \n and \r\n + # boundaries, which gives: + # + # [ '\r', 'FOO\x1b[K\r', 'BAR\x1b[K\r', 'ZOO\x1b[K\r\n' ] + # + decoded_lines = raw_output.decode('utf-8').splitlines(True) + + # Remove any item that ends with a '\r' as this means its + # content will be overwritten by the next item in the list. + # For the previous example, this gives: + # + # [ 'ZOO\x1b[K\r\n' ] + # + final_lines = [ l for l in decoded_lines if not l.endswith('\r') ] + + # Return a single string that concatenates all filtered lines + # while removing any remaining \r in it. Needed to transform + # \r\n into \n. + # + # "ZOO\x1b[K\n' + # + return ''.join(final_lines).replace('\r', '') + class BuildDir: def __init__(self, build_ninja: str): self.build_ninja = dedent(build_ninja) @@ -35,12 +70,18 @@ def __enter__(self): def __exit__(self, exc_type, exc_val, exc_tb): self.d.cleanup() + @property + def path(self) -> str: + return os.path.realpath(self.d.name) + + def run( self, - flags: str = '', + flags: T.Optional[str] = None, pipe: bool = False, raw_output: bool = False, - env: Dict[str, str] = default_env, + env: T.Dict[str, str] = default_env, + print_err_output = True, ) -> str: """Run Ninja command, and get filtered output. @@ -56,13 +97,17 @@ def run( env: Optional environment dictionary to run the command in. + print_err_output: set to False if the test expects ninja to print + something to stderr. (Otherwise, an error message from Ninja + probably represents a failed test.) + Returns: A UTF-8 string corresponding to the output (stdout only) of the Ninja command. By default, partial lines that were overwritten are removed according to the rules described in the comments below. """ - ninja_cmd = '{} {}'.format(NINJA_PATH, flags) + ninja_cmd = '{} {}'.format(NINJA_PATH, flags if flags else '') try: if pipe: output = subprocess.check_output( @@ -74,57 +119,27 @@ def run( output = subprocess.check_output(['script', '-qfec', ninja_cmd, '/dev/null'], cwd=self.d.name, env=env) except subprocess.CalledProcessError as err: - sys.stdout.buffer.write(err.output) + if print_err_output: + sys.stdout.buffer.write(err.output) + err.cooked_output = remove_non_visible_lines(err.output) raise err if raw_output: return output.decode('utf-8') - - # When running in a smart terminal, Ninja uses CR (\r) to - # return the cursor to the start of the current line, prints - # something, then uses `\x1b[K` to clear everything until - # the end of the line. - # - # Thus printing 'FOO', 'BAR', 'ZOO' on the same line, then - # jumping to the next one results in the following output - # on Posix: - # - # '\rFOO\x1b[K\rBAR\x1b[K\rZOO\x1b[K\r\n' - # - # The following splits the output at both \r, \n and \r\n - # boundaries, which gives: - # - # [ '\r', 'FOO\x1b[K\r', 'BAR\x1b[K\r', 'ZOO\x1b[K\r\n' ] - # - decoded_lines = output.decode('utf-8').splitlines(True) - - # Remove any item that ends with a '\r' as this means its - # content will be overwritten by the next item in the list. - # For the previous example, this gives: - # - # [ 'ZOO\x1b[K\r\n' ] - # - final_lines = [ l for l in decoded_lines if not l.endswith('\r') ] - - # Return a single string that concatenates all filtered lines - # while removing any remaining \r in it. Needed to transform - # \r\n into \n. - # - # "ZOO\x1b[K\n' - # - return ''.join(final_lines).replace('\r', '') + return remove_non_visible_lines(output) def run( build_ninja: str, - flags: str = '', + flags: T.Optional[str] = None, pipe: bool = False, raw_output: bool = False, - env: Dict[str, str] = default_env, + env: T.Dict[str, str] = default_env, + print_err_output = True, ) -> str: """Run Ninja with a given build plan in a temporary directory. """ with BuildDir(build_ninja) as b: - return b.run(flags, pipe, raw_output, env) + return b.run(flags, pipe, raw_output, env, print_err_output) @unittest.skipIf(platform.system() == 'Windows', 'These test methods do not work on Windows') class Output(unittest.TestCase): @@ -137,6 +152,16 @@ class Output(unittest.TestCase): '', )) + def _test_expected_error(self, plan: str, flags: T.Optional[str], expected: str): + """Run Ninja with a given plan and flags, and verify its cooked output against an expected content. + """ + actual = '' + try: + actual = run(plan, flags, print_err_output=False) + except subprocess.CalledProcessError as err: + actual = err.cooked_output + self.assertEqual(expected, actual) + def test_issue_1418(self) -> None: self.assertEqual(run( '''rule echo @@ -371,6 +396,59 @@ def test_tool_inputs(self) -> None: ) + def test_tool_compdb_targets(self) -> None: + plan = ''' +rule cat + command = cat $in $out +build out1 : cat in1 +build out2 : cat in2 out1 +build out3 : cat out2 out1 +build out4 : cat in4 +''' + + + self._test_expected_error(plan, '-t compdb-targets', +'''ninja: error: compdb-targets expects the name of at least one target +usage: ninja -t compdb [-hx] target [targets] + +options: + -h display this help message + -x expand @rspfile style response file invocations +''') + + self._test_expected_error(plan, '-t compdb-targets in1', + "ninja: fatal: 'in1' is not a target (i.e. it is not an output of any `build` statement)\n") + + self._test_expected_error(plan, '-t compdb-targets nonexistent_target', + "ninja: fatal: unknown target 'nonexistent_target'\n") + + + with BuildDir(plan) as b: + actual = b.run(flags='-t compdb-targets out3') + expected = f'''[ + {{ + "directory": "{b.path}", + "command": "cat in1 out1", + "file": "in1", + "output": "out1" + }}, + {{ + "directory": "{b.path}", + "command": "cat in2 out1 out2", + "file": "in2", + "output": "out2" + }}, + {{ + "directory": "{b.path}", + "command": "cat out2 out1 out3", + "file": "out2", + "output": "out3" + }} +] +''' + self.assertEqual(expected, actual) + + def test_explain_output(self): b = BuildDir('''\ build .FORCE: phony diff --git a/src/command_collector.h b/src/command_collector.h new file mode 100644 index 0000000000..003af9fbb7 --- /dev/null +++ b/src/command_collector.h @@ -0,0 +1,65 @@ +// Copyright 2024 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef NINJA_COMMAND_COLLECTOR_H_ +#define NINJA_COMMAND_COLLECTOR_H_ + +#include +#include +#include + +#include "graph.h" + +/// Collects the transitive set of edges that lead into a given set +/// of starting nodes. Used to implement the `compdb-targets` tool. +/// +/// When collecting inputs, the outputs of phony edges are always ignored +/// from the result, but are followed by the dependency walk. +/// +/// Usage is: +/// - Create instance. +/// - Call CollectFrom() for each root node to collect edges from. +/// - Call TakeResult() to retrieve the list of edges. +/// +struct CommandCollector { + void CollectFrom(const Node* node) { + assert(node); + + if (!visited_nodes_.insert(node).second) + return; + + Edge* edge = node->in_edge(); + if (!edge || !visited_edges_.insert(edge).second) + return; + + for (Node* input_node : edge->inputs_) + CollectFrom(input_node); + + if (!edge->is_phony()) + in_edges.push_back(edge); + } + + private: + std::unordered_set visited_nodes_; + std::unordered_set visited_edges_; + + /// we use a vector to preserve order from requisites to their dependents. + /// This may help LSP server performance in languages that support modules, + /// but it also ensures that the output of `-t compdb-targets foo` is + /// consistent, which is useful in regression tests. + public: + std::vector in_edges; +}; + +#endif // NINJA_COMMAND_COLLECTOR_H_ diff --git a/src/graph_test.cc b/src/graph_test.cc index 6c654eeb32..6483410e99 100644 --- a/src/graph_test.cc +++ b/src/graph_test.cc @@ -15,6 +15,7 @@ #include "graph.h" #include "build.h" +#include "command_collector.h" #include "test.h" using namespace std; @@ -310,6 +311,54 @@ TEST_F(GraphTest, InputsCollectorWithEscapes) { EXPECT_EQ("order_only", inputs[4]); } +TEST_F(GraphTest, CommandCollector) { + ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, + "build out1: cat in1\n" + "build mid1: cat in1\n" + "build out2: cat mid1\n" + "build out3 out4: cat mid1\n" + "build all: phony out1 out2 out3\n")); + { + CommandCollector collector; + auto& edges = collector.in_edges; + + // Start visit from out2; this should add `build mid1` and `build out2` to + // the edge list. + collector.CollectFrom(GetNode("out2")); + ASSERT_EQ(2u, edges.size()); + EXPECT_EQ("cat in1 > mid1", edges[0]->EvaluateCommand()); + EXPECT_EQ("cat mid1 > out2", edges[1]->EvaluateCommand()); + + // Add a visit from out1, this should append `build out1` + collector.CollectFrom(GetNode("out1")); + ASSERT_EQ(3u, edges.size()); + EXPECT_EQ("cat in1 > out1", edges[2]->EvaluateCommand()); + + // Another visit from all; this should add edges for out1, out2 and out3, + // but not all (because it's phony). + collector.CollectFrom(GetNode("all")); + ASSERT_EQ(4u, edges.size()); + EXPECT_EQ("cat in1 > mid1", edges[0]->EvaluateCommand()); + EXPECT_EQ("cat mid1 > out2", edges[1]->EvaluateCommand()); + EXPECT_EQ("cat in1 > out1", edges[2]->EvaluateCommand()); + EXPECT_EQ("cat mid1 > out3 out4", edges[3]->EvaluateCommand()); + } + + { + CommandCollector collector; + auto& edges = collector.in_edges; + + // Starting directly from all, will add `build out1` before `build mid1` + // compared to the previous example above. + collector.CollectFrom(GetNode("all")); + ASSERT_EQ(4u, edges.size()); + EXPECT_EQ("cat in1 > out1", edges[0]->EvaluateCommand()); + EXPECT_EQ("cat in1 > mid1", edges[1]->EvaluateCommand()); + EXPECT_EQ("cat mid1 > out2", edges[2]->EvaluateCommand()); + EXPECT_EQ("cat mid1 > out3 out4", edges[3]->EvaluateCommand()); + } +} + TEST_F(GraphTest, VarInOutPathEscaping) { ASSERT_NO_FATAL_FAILURE(AssertParse(&state_, "build a$ b: cat no'space with$ space$$ no\"space2\n")); diff --git a/src/ninja.cc b/src/ninja.cc index 7885bb3682..93c0ca6a2a 100644 --- a/src/ninja.cc +++ b/src/ninja.cc @@ -20,6 +20,8 @@ #include #include +#include +#include #ifdef _WIN32 #include "getopt.h" @@ -38,6 +40,7 @@ #include "build_log.h" #include "deps_log.h" #include "clean.h" +#include "command_collector.h" #include "debug_flags.h" #include "depfile_parser.h" #include "disk_interface.h" @@ -130,6 +133,8 @@ struct NinjaMain : public BuildLogUser { int ToolClean(const Options* options, int argc, char* argv[]); int ToolCleanDead(const Options* options, int argc, char* argv[]); int ToolCompilationDatabase(const Options* options, int argc, char* argv[]); + int ToolCompilationDatabaseForTargets(const Options* options, int argc, + char* argv[]); int ToolRecompact(const Options* options, int argc, char* argv[]); int ToolRestat(const Options* options, int argc, char* argv[]); int ToolUrtle(const Options* options, int argc, char** argv); @@ -932,8 +937,8 @@ std::string EvaluateCommandWithRspfile(const Edge* edge, return command; } -void printCompdb(const char* const directory, const Edge* const edge, - const EvaluateCommandMode eval_mode) { +void PrintOneCompdbObject(std::string const& directory, const Edge* const edge, + const EvaluateCommandMode eval_mode) { printf("\n {\n \"directory\": \""); PrintJSONString(directory); printf("\",\n \"command\": \""); @@ -977,37 +982,25 @@ int NinjaMain::ToolCompilationDatabase(const Options* options, int argc, argc -= optind; bool first = true; - vector cwd; - char* success = NULL; - - do { - cwd.resize(cwd.size() + 1024); - errno = 0; - success = getcwd(&cwd[0], cwd.size()); - } while (!success && errno == ERANGE); - if (!success) { - Error("cannot determine working directory: %s", strerror(errno)); - return 1; - } + std::string directory = GetWorkingDirectory(); putchar('['); - for (vector::iterator e = state_.edges_.begin(); - e != state_.edges_.end(); ++e) { - if ((*e)->inputs_.empty()) + for (const Edge* edge : state_.edges_) { + if (edge->inputs_.empty()) continue; if (argc == 0) { if (!first) { putchar(','); } - printCompdb(&cwd[0], *e, eval_mode); + PrintOneCompdbObject(directory, edge, eval_mode); first = false; } else { for (int i = 0; i != argc; ++i) { - if ((*e)->rule_->name() == argv[i]) { + if (edge->rule_->name() == argv[i]) { if (!first) { putchar(','); } - printCompdb(&cwd[0], *e, eval_mode); + PrintOneCompdbObject(directory, edge, eval_mode); first = false; } } @@ -1087,6 +1080,118 @@ int NinjaMain::ToolRestat(const Options* options, int argc, char* argv[]) { return EXIT_SUCCESS; } +struct CompdbTargets { + enum class Action { kDisplayHelpAndExit, kEmitCommands }; + + Action action; + EvaluateCommandMode eval_mode = ECM_NORMAL; + + std::vector targets; + + static CompdbTargets CreateFromArgs(int argc, char* argv[]) { + // + // grammar: + // ninja -t compdb-targets [-hx] target [targets] + // + CompdbTargets ret; + + // getopt_long() expects argv[0] to contain the name of + // the tool, i.e. "compdb-targets". + argc++; + argv--; + + // Phase 1: parse options: + optind = 1; // see `man 3 getopt` for documentation on optind + int opt; + while ((opt = getopt(argc, argv, const_cast("hx"))) != -1) { + switch (opt) { + case 'x': + ret.eval_mode = ECM_EXPAND_RSPFILE; + break; + case 'h': + default: + ret.action = CompdbTargets::Action::kDisplayHelpAndExit; + return ret; + } + } + + // Phase 2: parse operands: + int const targets_begin = optind; + int const targets_end = argc; + + if (targets_begin == targets_end) { + Error("compdb-targets expects the name of at least one target"); + ret.action = CompdbTargets::Action::kDisplayHelpAndExit; + } else { + ret.action = CompdbTargets::Action::kEmitCommands; + for (int i = targets_begin; i < targets_end; ++i) { + ret.targets.push_back(argv[i]); + } + } + + return ret; + } +}; + +void PrintCompdb(std::string const& directory, std::vector const& edges, + const EvaluateCommandMode eval_mode) { + putchar('['); + + bool first = true; + for (const Edge* edge : edges) { + if (edge->is_phony() || edge->inputs_.empty()) + continue; + if (!first) + putchar(','); + PrintOneCompdbObject(directory, edge, eval_mode); + first = false; + } + + puts("\n]"); +} + +int NinjaMain::ToolCompilationDatabaseForTargets(const Options* options, + int argc, char* argv[]) { + auto compdb = CompdbTargets::CreateFromArgs(argc, argv); + + switch (compdb.action) { + case CompdbTargets::Action::kDisplayHelpAndExit: { + printf( + "usage: ninja -t compdb [-hx] target [targets]\n" + "\n" + "options:\n" + " -h display this help message\n" + " -x expand @rspfile style response file invocations\n"); + return 1; + } + + case CompdbTargets::Action::kEmitCommands: { + CommandCollector collector; + + for (const std::string& target_arg : compdb.targets) { + std::string err; + Node* node = CollectTarget(target_arg.c_str(), &err); + if (!node) { + Fatal("%s", err.c_str()); + return 1; + } + if (!node->in_edge()) { + Fatal( + "'%s' is not a target " + "(i.e. it is not an output of any `build` statement)", + node->path().c_str()); + } + collector.CollectFrom(node); + } + + std::string directory = GetWorkingDirectory(); + PrintCompdb(directory, collector.in_edges, compdb.eval_mode); + } break; + } + + return 0; +} + int NinjaMain::ToolUrtle(const Options* options, int argc, char** argv) { // RLE encoded. const char* urtle = @@ -1141,6 +1246,9 @@ const Tool* ChooseTool(const string& tool_name) { Tool::RUN_AFTER_LOAD, &NinjaMain::ToolTargets }, { "compdb", "dump JSON compilation database to stdout", Tool::RUN_AFTER_LOAD, &NinjaMain::ToolCompilationDatabase }, + { "compdb-targets", + "dump JSON compilation database for a given list of targets to stdout", + Tool::RUN_AFTER_LOAD, &NinjaMain::ToolCompilationDatabaseForTargets }, { "recompact", "recompacts ninja-internal data structures", Tool::RUN_AFTER_LOAD, &NinjaMain::ToolRecompact }, { "restat", "restats all outputs in the build log", diff --git a/src/util.cc b/src/util.cc index ac1b14e55f..70421bf119 100644 --- a/src/util.cc +++ b/src/util.cc @@ -21,6 +21,7 @@ #include #include #include +#include #endif #include @@ -917,6 +918,21 @@ double GetLoadAverage() { } #endif // _WIN32 +std::string GetWorkingDirectory() { + std::string ret; + char* success = NULL; + do { + ret.resize(ret.size() + 1024); + errno = 0; + success = getcwd(&ret[0], ret.size()); + } while (!success && errno == ERANGE); + if (!success) { + Fatal("cannot determine working directory: %s", strerror(errno)); + } + ret.resize(strlen(&ret[0])); + return ret; +} + bool Truncate(const string& path, size_t size, string* err) { #ifdef _WIN32 int fh = _sopen(path.c_str(), _O_RDWR | _O_CREAT, _SH_DENYNO, diff --git a/src/util.h b/src/util.h index 211a43d348..b38578c326 100644 --- a/src/util.h +++ b/src/util.h @@ -102,6 +102,9 @@ int GetProcessorCount(); /// on error. double GetLoadAverage(); +/// a wrapper for getcwd() +std::string GetWorkingDirectory(); + /// Truncates a file to the given size. bool Truncate(const std::string& path, size_t size, std::string* err); From b785947b1f3e986cdc17b3cf0744ca79ad6ea88a Mon Sep 17 00:00:00 2001 From: James Widman Date: Sun, 13 Oct 2024 01:31:01 -0700 Subject: [PATCH 2/2] Fix display of labels in a `[horizontal]` list The introduction of the entry for `compdb-targets` in the `[horizontal]` labeled list in doc/manual.asciidoc revealed some display issues in the left column: First, the web browser would insert a line break in the middle of the label `compdb-targets`, so that it looked like this: compdb- targets We fix this by applying the `white-space: nowrap` attribute to the left column. After this is fixed, we see practically no space between the end of the longest label and the beginning of the text in the second column; we fix this with the `padding-right` attribute. Finally, we align all labels to the right side of the column so that there is a consistent amount of horizontal space between the end of each label and the beginning of the text in the second column. --- doc/style.css | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/doc/style.css b/doc/style.css index 363e272b24..2be09de8b1 100644 --- a/doc/style.css +++ b/doc/style.css @@ -53,3 +53,16 @@ div.chapter { p { margin-top: 0; } + +/* The following applies to the left column of a [horizontal] labeled list: */ +table.horizontal > tbody > tr > td:nth-child(1) { + + /* prevent the insertion of a line-break in the middle of a label: */ + white-space: nowrap; + + /* insert a little horizontal padding between the two columns: */ + padding-right: 1.5em; + + /* right-justify labels: */ + text-align: end; +}