Allow 'ninja -t compdb' accept one target #1546
Conversation
Test: ./ninja -t compdb -a wrong_target (fail) ./ninja -t compdb -a test_target (empty due to lack of rules) ./ninja -t compdb -a test_target cc cxx (ok) ./ninja -t compdb cxx (ok, regression test) ./ninja ninja_test && ./ninja_test (passed)
| @@ -710,26 +743,51 @@ int NinjaMain::ToolCompilationDatabase(const Options* options, int argc, | |||
|
|
|||
| optind = 1; | |||
| int opt; | |||
| while ((opt = getopt(argc, argv, const_cast<char*>("hx"))) != -1) { | |||
| string err; | |||
| Node* user_given_target = NULL; | |||
There was a problem hiding this comment.
Please create a scope in for case 'a' and move those two variable declarations there.
See http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-introduce
| case 'h': | ||
| default: | ||
| printf( | ||
| "usage: ninja -t compdb [options] [rules]\n" | ||
| "usage: ninja -t compdb [options] [target] [rules]\n" |
There was a problem hiding this comment.
Isn't [target] only useful when passing -a?
| ); | ||
| return 1; | ||
| } | ||
| } | ||
| argv += optind; | ||
| argc -= optind; | ||
|
|
||
| std::vector<Edge*> user_interested_edges; |
There was a problem hiding this comment.
Also move this variable into the if (user_given_target) {.
| } | ||
| if (visited_nodes->count(node)) { | ||
| return true; | ||
| } else { |
There was a problem hiding this comment.
No need for else after return.
| // Leaf node | ||
| if (!edge || visited_edges->count(edge)) { | ||
| return true; | ||
| } else { |
| } | ||
| if (node == NULL || depend_edges == NULL) { | ||
| Error("Internal error"); | ||
| return false; |
There was a problem hiding this comment.
I would use assert for this instead.
|
Before you continue working on this, I think we need to discuss if
|
|
|
ping? |
Please don't do this, I'm getting enough emails as it is.
Maybe separating them with
I see. Why does cquery load the whole file into memory though? |
|
I faced the same problem recently, and looking at the code I think it actually makes more sense to add a feature to 'targets' tool which would output all the rules that are used by given target. Then, you could use 'compdb' tool easily by giving it all those rules as arguments. In my case I could deduce the rules used by my targets from 'rules' tool with -d flag, so I didn't need the changes but here are my two cents: "-t targets rule X" gives you targets that use the rule X (existing feature, though not documented) and "-t targets rules X" could give you rules that are used by target X (possible new feature). Alternatively, to solve the problem with parameters ([options] [target] [rules] mess), and having in mind that most people would try to put target name instead of rule name in "-t compdb [rules]", I think it would be elegant to just specify a new flag like -t with semantics "treat rules parameter as targets", leading to "-t compdb -t [rules]" which would then interpret the rules arguments differently. Again, even without all these changes you may be able to discover rules used by your target from "-t rules -d" and then you could use existing compdb feature. |
|
closing in favor of #2319 |
Fix #1544
Test:
./ninja -t compdb -a wrong_target (fail)
./ninja -t compdb -a test_target (empty due to lack of rules)
./ninja -t compdb -a test_target cc cxx (ok)
./ninja -t compdb cxx (ok, regression test)
./ninja ninja_test && ./ninja_test (passed)