Allow 'ninja -t compdb' accept targets

[csmoe]

Fix #1544

Cherry-picked #1546 and addressed the comments.
May I have your code review @jhasse ? Thanks

[csmoe]

@digit-google thanks for your comments, they're addressed.

[digit-google]

This looks good to me, but will require @jhasse 's approval.

Can I ask you to add some regression test in misc/output_test.py for this feature though?

[jhasse]

Thanks for having another go at this :)

[JamesWidman]

fwiw, i really like this feature and look forward to this PR being merged!

one obvious use case for C/C++ users is to produce the database consumed by clangd. It means i get more concise & relevant search hits when e.g. asking clangd for all references to some symbol x in some program foo.

it also means the CPU fans in my PC no longer go crazy as a consequence of clangd's attempt to background-index a large number of source files that i didn't even want to browse (because they don't participate in building the executable foo). And ofc this should also enable you to use your laptop's battery for significantly longer before you need to recharge. And you'll get useful LSP results faster, since clangd won't be wasting time on irrelevant source files.

thank you, @csmoe!

[JamesWidman]

side note (for anyone who, like me, only learned about the jq command recently): in addition to compile commands, -t compdb --target foo will produce entries for commands that don't translate C/C++ code to machine code (e.g. linker invocations, generation of header files, etc), so in case anyone wants to spare clangd from attempting to read object files, you could do something like:

ninja -t compdb --target foo | \
  jq '[.[] | select( .command | test("/bin/clang.* -c"))]' > compile_commands.json

(or test("/bin/your_compiler_here.* -c")

[JamesWidman]

ninja -t compdb --target foo | \
  jq '[.[] | select( .command | test("/bin/clang.* -c"))]' > compile_commands.json

In some future PR, it might be nice if the compilation database format were extended to allow a field like "command_type" as in:

  { "directory": "/home/user/llvm/build",
    "command": "clang-cl.exe [args...] /c /o t.o t.cc",
    "command_type": "compile_CXX",
    "file": "t.cc" },

...which generator programs might communicate to Ninja in the form of a rule variable:

 rule some_cxx_rule
   command = clang-cl.exe [args...] /o $out /c $in
   command_type = compile_CXX
   description = Building CXX object $out

...so that our jq query would be a little more portable:

ninja -t compdb --target foo | \
  jq '[.[] | select( .command_type | test("^compile_(CXX|C)$"))]' > compile_commands.json

(But again, this would be a separate PR, and afaict the current PR is complete (or, it looks like it will be, after #2479 is merged).)

[mcprat]

Would it be better to shift out arguments so that no flag is required for the targets list?

so instead of

-t compdb --target foo,bar

you would have

-t compdb foo bar

or if a flag is really necessary to break out of flag-only parsing, use syntax like in git

-t compdb -- foo bar

so they can be space separated instead of comma separated

[JamesWidman]

Would it be better to shift out arguments so that no flag is required for the targets list?

so instead of

-t compdb --target foo,bar

you would have

-t compdb foo bar

Prior to the commits in this PR, the output of ninja -t compdb -h is:

usage: ninja -t compdb [options] [rules]

options:
  -x     expand @rspfile style response file invocations

so ninja -t compdb foo bar already means "emit every compile command that uses rule foo or rule bar", and we probably don't want to break that.

[JamesWidman]

or if a flag is really necessary to break out of flag-only parsing, use syntax like in git

-t compdb -- foo bar

so they can be space separated instead of comma separated

i guess this would work:

ninja -t compdb [options] [rules] -- [targets]

if this is happening in a bash script, and if you've defined an array parameter where each element is the name of a target, then this alternative syntax might save users from learning how to join strings in bash:

 ninja -t compdb --target $(targets=( $(cat $WORKSPACE/active-executables.txt)); IFS=','; echo "${targets[*]}")

...though i guess it's not so bad if you use the paste command:

ninja -t compdb --target $(paste -s -d , $WORKSPACE/active-executables.txt)

Note, in the case where the generator program is cmake, you can get a list of all executable targets with a script like the following (zsh):

prepare_build_environment # a shell function that sets $BUILD_DIR
cmake_root=${$(realpath $(which cmake)):h:h}

cd -q $BUILD_DIR

cmake .  --trace-expand \
  |& rg 'add_executable\(' \
  |  rg -v -e "^(${BUILD_DIR}|${cmake_root})" \
  | sed -E -e 's/.*add_executable\([[:space:]]*([^[:space:]]+) .*/\1/'

if you redirect the output of this script to a file available-executables.txt, you can later use:

fzf --multi < available-executables.txt

to select the set of executables that are relevant to your current task, and redirect that to $WORKSPACE/active-executables.txt (which you can then use from a script when building, or when generating the compdb).

[JamesWidman]

so, boiling down the above, it seems like the only open question is whether we want:

• option 1 (same old command form, but new option for [options]):

ninja -t compdb --target TARGETS [rules]

...where TARGETS is a comma-separated list of names of targets;

or:

• option 2 (new command form):

ninja -t compdb [options] [rules] [-- targets]

i prefer option 1 (as already implemented), because option 2 requires me to remember that -t compdb foo -- bar means "rule foo and target bar", as opposed to "target foo and rule bar". (And also because it's not too hard to join an array of target names with commas.)

@mcprat, @csmoe, what do you think?

[JamesWidman]

minor nit: if we go with option 1 (--target TARGETS), can we change the spelling of the option to --targets (plural)? (I already accidentally used the plural form a couple times.)

[JamesWidman]

in case we need more motivation to merge this PR: i just wrote a small extension for Telescope (which is pretty straightforward) that reads a compilation database and hands the list of all primary source files from that database over to Telescope.

For example, if i cd to the build directory produced by cmake for building ninja itself and then run:

ninja -t compdb --target ninja > compile_commands.json

...here's what i see when i invoke my Telescope extension:

For anyone starting to read the Ninja source code for the first time, this seems like a good starting point. (And as soon as i hit <enter> to load any source file in that list, clangd indexes all translation units named by the same compile_commands.json, and none of the translation units not named therein. This means that e.g. the LSP command to list all references to a symbol will include only search hits from the correct set of translation units.)

By contrast, when i use an IDE to load project files generated by cmake, it is not at all obvious which source files are linked into the executable in which i'm currently interested.

To me, this means that i'll have an easier time browsing a C/C++ program foo with -t compdb --target foo and Telescope than i would with an IDE. And it's not because i've developed more muscle memory with these tools; it's because this set of tools is actually better!

[csmoe]

I like ninja -t compdb --targets TARGETS [rules], @JamesWidman Are you interested to take over this pr? I might not always in radar.

Feel free to modify anything or recreate one, but preserve lkchen's contribution record, he completed the main part.

[JamesWidman]

@JamesWidman Are you interested to take over this pr? I might not always in radar.

@csmoe sure! How should we proceed? (Does github's PR interface have something like a 'bequeath' button so that you can bequeath the PR to me, or should i open a new PR like you did, or...?)

[csmoe]

github seems not support that, so create your pr😃